Opened 4 years ago

Closed 4 years ago

#21500 closed enhancement (fixed)

Add instructions for using ccache

Reported by: paulmasson Owned by:
Priority: minor Milestone: sage-7.4
Component: documentation Keywords:
Cc: Merged in:
Authors: Paul Masson Reviewers: Jeroen Demeyer, Matthias Koeppe
Report Upstream: N/A Work issues:
Branch: 0161c5c (Commits) Commit: 0161c5c3742d52ff4e9af8c5092a16051f1e5557
Dependencies: Stopgaps:

Description


Change History (21)

comment:1 Changed 4 years ago by paulmasson

  • Authors set to Paul Masson
  • Component changed from PLEASE CHANGE to documentation
  • Priority changed from major to minor
  • Type changed from PLEASE CHANGE to enhancement

comment:2 Changed 4 years ago by paulmasson

  • Branch set to u/paulmasson/add_instructions_for_using_ccache

comment:3 Changed 4 years ago by paulmasson

  • Commit set to e22bbb45383e761795b12a4c2239416afa37d2d8
  • Status changed from new to needs_review

comment:4 Changed 4 years ago by git

  • Commit changed from e22bbb45383e761795b12a4c2239416afa37d2d8 to 1cb5405e38b595872977ef5e84c88daeb42b4039

Branch pushed to git repo; I updated commit sha1. New commits:

1cb5405Add instruction for ccache

comment:5 Changed 4 years ago by git

  • Commit changed from 1cb5405e38b595872977ef5e84c88daeb42b4039 to b1f502e639b1ac232d5fe04d75f4a0b4dd395f9a

Branch pushed to git repo; I updated commit sha1. New commits:

cfd681dMinor cleanup
b1f502eMinor cleanup

comment:6 Changed 4 years ago by jdemeyer

  • Status changed from needs_review to needs_work

Can we please take this opportunity to clarify better that people really should use make instead of ./sage -b unless they know what they are doing. For example, your added paragraph seems to imply that it's fine to run just ./sage -b when switching branches. However, in many cases that will not work because of changes to packages.

The text should make it clear that you need to run make to compile Sage (or make build to skip the documentation). If you made changes only to Sage library files, you can use ./sage -b. When switching branches, usually you change more than that.

comment:7 Changed 4 years ago by jdemeyer

To clarify a bit better what I mean:

  1. The stuff you added about ccache really belongs with the part about make and not with sage -b.
  1. Moreover, I would suggest to mention make first (so people become less drilled to use sage -b all the time) and sage -b after that.

For me, 1. must be fixed (hence needs_work), while 2. is an optional improvement that you may want to do.

comment:8 Changed 4 years ago by jhpalmieri

Should these instructions go in the developer's guide at all? It might make more sense to put them in the installation guide, with a pointer from the developer's guide ("if you switch branches a lot, you might consider installing ccache: see ..."). The installation guide already has a very brief mention of ccache, for what that's worth. That could be expanded, of course.

Last edited 4 years ago by jhpalmieri (previous) (diff)

comment:9 follow-up: Changed 4 years ago by paulmasson

Jeroen, the documentation as written is very clear on when to run sage -b and make. Since I'd assume it is mostly read by developers just getting started, the current order of instructions seems preferable to me. How many developers start out altering packages before Sage Python code?

I wanted my warning to be close to the statement "This should be quite fast", since that's what triggered the whole discussion for me. I can see now that inserting the warning in the middle of the section is confusing, so I'll move it to the end. That way it will apply to all of the rebuilding cases described above it.

John, the installation guide doesn't make clear why one would want to install ccache. The only reason I did is because I regularly switch branches when working on tickets. That's a development issue, not an installation issue, so I think it belongs here.

comment:10 Changed 4 years ago by git

  • Commit changed from b1f502e639b1ac232d5fe04d75f4a0b4dd395f9a to 82e686ef9cf7867089bf144d0fe50456f0f7f897

Branch pushed to git repo; I updated commit sha1. New commits:

82e686eAdd instructions for ccache

comment:11 Changed 4 years ago by git

  • Commit changed from 82e686ef9cf7867089bf144d0fe50456f0f7f897 to afdd55944a072a18ad99610d9d8021014ee7c12b

Branch pushed to git repo; I updated commit sha1. New commits:

afdd559Minor cleanup

comment:12 Changed 4 years ago by paulmasson

  • Status changed from needs_work to needs_review

comment:13 Changed 4 years ago by mkoeppe

typo Recynthonization -> Recythonization

comment:14 in reply to: ↑ 9 Changed 4 years ago by jdemeyer

Replying to paulmasson:

How many developers start out altering packages before Sage Python code?

Almost everybody does! But, just like you, they do not realize that changing branches often means changing packages. That is exactly the problem with the current documentation (although I agree that this is orthogonal to this ticket).

comment:15 Changed 4 years ago by jdemeyer

Nitpick: I do not think that WARNING is correct here. Can you make it a NOTE instead?

comment:16 Changed 4 years ago by jdemeyer

  • Reviewers set to Jeroen Demeyer
  • Status changed from needs_review to needs_work

Apart from this last nitpick, this is good for me.

comment:17 Changed 4 years ago by git

  • Commit changed from afdd55944a072a18ad99610d9d8021014ee7c12b to 0161c5c3742d52ff4e9af8c5092a16051f1e5557

Branch pushed to git repo; I updated commit sha1. New commits:

0161c5cCorrections

comment:18 Changed 4 years ago by paulmasson

  • Status changed from needs_work to needs_review

Done.

comment:19 Changed 4 years ago by jdemeyer

  • Status changed from needs_review to positive_review

comment:20 Changed 4 years ago by mkoeppe

  • Reviewers changed from Jeroen Demeyer to Jeroen Demeyer, Matthias Koeppe

comment:21 Changed 4 years ago by vbraun

  • Branch changed from u/paulmasson/add_instructions_for_using_ccache to 0161c5c3742d52ff4e9af8c5092a16051f1e5557
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.