Code review comment for lp:~richard-wilbur/bzr/1537319-reproducible-builds

Revision history for this message
Richard Wilbur (richard-wilbur) wrote :

I'm happy to accommodate multiple merge proposals but am in need of counsel on how best to accomplish that. Should I shelve everything except one changeset, polish, push to launchpad, propose the merge, review, merge, and repeat? Using the same branch? Should I break the changes into separate branches?

I see the main changes here as:
revision 6614: Add explicit support to bzr for creating SOURCE_DATE_EPOCH from a bzr branch. (If no version information is available, presently returns 0 => *nix epoch. Would a different value be more suitable?)

revision 6615:
./Makefile: Change make rules to sort the file lists for constant file order in tar file creation. Also use numeric user and group ids, root/root => 0/0 and clamp modification time to SOURCE_DATE_EPOCH when creating tar files. (Set SOURCE_DATE_EPOCH with `bzr sde` if not already set.)

./doc/en/Makefile: Use (or set with `bzr sde`) SOURCE_DATE_EPOCH to create BUILD_DATE for sphinx documentation build.

./bzrlib/doc_generate/autodoc_bash_completion.py: Try to get SOURCE_DATE_EPOCH and, if found, use it to create timestamp and include 'Last commit: %' in output file. Retitled from 'Generation time' to 'Last commit' as it seemed more descriptive.

./bzrlib/doc_generate/autodoc_man.py: Try to get SOURCE_DATE_EPOCH and, if found, use it to create date/time stamps. Split preamble and head so that datestamp and timestamp are only output when SOURCE_DATE_EPOCH is available.

./bzrlib/doc_generate/autodoc_rstx.py: Try to get SOURCE_DATE_EPOCH and, if found, use it to create date/time stamps. Split preamble so that timestamp is output when SOURCE_DATE_EPOCH is available.

revision 6616: Fixed syntax of date/time stamp creation in bzrlib/doc_generate/autodoc_*.py. Changed man head to always use datestamp but in the absence of SOURCE_DATE_EPOCH, use 1 second before the *nix epoch (valid, impossible date).

1. I see the utility of factoring common code out of the bzrlib/doc_generate/autodoc_*.py into bzrlib/doc_generate/__init__.py
If we decide to substitute a flag date/time for missing SOURCE_DATE_EPOCH information, this will greatly simplify the process of factoring out the common code (more will be common), and it will also simplify the code which creates the documentation as there won't be as many conditionally included pieces.

If so, what would be a good, fixed value to use as a flag date/time? I notice that I already return "0" from `bzr sde` if there is no revision information on the branch and use "-1" in the event we are missing SOURCE_DATE_EPOCH in the man documentation generator. I think a case could be made for these being materially similar situations: no revision information available. So I think it would benefit us to use a consistent flag for this situation. I like "-1" as it should never occur as a valid source date. The only issue would possibly occur if SOURCE_DATE_EPOCH were to be treated as an unsigned integer, in which case "0" would be preferrable. I expect "0" is pretty much equally unlikely to be a valid source date. (I suppose in Python we are relatively unlikely to ever find SOURCE_DATE_EPOCH treated as an unsigned integer. If we are interested in recommending this flag date to others we should be sensitive to the possibility of issues with other languages.)

2. All the doc/??/Makefile are copies of the doc/en/Makefile, so that is already factored out. (Maybe I misunderstood which Makefiles you were referring to?)

3. I'm certainly in favor of tests and a (documented) setup where we can build all docs and see the results as a baseline. How would you recommend proceeding here? What would you suggest we need to do or create?

« Back to merge proposal