:thumbsup:

I do think we should do this but is there any potential fallout from this change?

I can't really think of any major breakage from this the only effect it has is to export that environment variable. So there would have to be a build system which would change behavior because of that specific environment variable being set, which I think is unlikely.

Also, wouldn't also setting %clamp_mtime_to_source_date_epoch fix the reported issues without the need for other projects to pay attention to SOURCE_DATE_EPOCH? (I understand that would have a much larger chance of disrupting something.)

We could do both, but I strongly suspect we probably don't want to do that unless we're actually ready this time to run Debian and openSUSE-style reproducible builds.

It didn't go so well the first go around with Fedora 23...

I do think we should do this but is there any potential fallout from this change?
I can't really think of any major breakage from this the only effect it has is to export that environment variable. So there would have to be a build system which would change behavior because of that specific environment variable being set, which I think is unlikely.

I can't imagine what could go wrong. The builds are either not reproducible and they are not using this env variable or this env variable can help. Of course there might used different code paths based on this env variable, so it might break something, but this is not very likely.

I can start discussion about this PR on devel ML or create change proposal if needed, but I am interested just in my niche use case, not in the reproducible builds as whole.

Also, wouldn't also setting %clamp_mtime_to_source_date_epoch fix the reported issues without the need for other projects to pay attention to SOURCE_DATE_EPOCH? (I understand that would have a much larger chance of disrupting something.)

I am not sure what it does. Will it fail the build if the mtime of some file in package is greater then the value of the SOURCE_DATE_EPOCH? Will it just change some timestamps? Dunno, therefore I have not proposed to change this.

We could do both, but I strongly suspect we probably don't want to do that unless we're actually ready this time to run Debian and openSUSE-style reproducible builds.
It didn't go so well the first go around with Fedora 23...

I don't understand. What didn't go so well? Were there additional build failures or the builds were not reproducible? I don't propose this change to fully achieve the latter. It just helps with my niche use case and it might help with others.

I can't really think of any major breakage from this the only effect it has is to export that environment variable. So there would have to be a build system which would change behavior because of that specific environment variable being set, which I think is unlikely.

The change you're proposing isn't really going to break anything. There are three settings related to reproducible builds:

#   If true, set the SOURCE_DATE_EPOCH environment variable
#   to the timestamp of the topmost changelog entry
%source_date_epoch_from_changelog 0

#   If true, make sure that timestamps in built rpms
#   are not later than the value of SOURCE_DATE_EPOCH.
#   Is ignored when SOURCE_DATE_EPOCH is not set.
%clamp_mtime_to_source_date_epoch 0

#   If set, the value of the build hostname in built rpms
#   uses the value of this macro instead of the actual
#   hostname of the machine that built the package.
#%_buildhost koji.fedoraproject.org

If we set all three, we would be set up for reproducible builds. However, this doesn't mean our packages are reproducibly built.

Also, we should never set the build hostname override in redhat-rpm-config. That should only exist within Koji... I'm waiting on this Koji PR to make that happen.

I don't understand. What didn't go so well? Were there additional build failures or the builds were not reproducible?

There were many non-reproducible builds, but more importantly, no one tried to fix them. The project stalled out.

I don't want to give people the impression we're doing reproducibility testing by turning on all three options. Exporting the variable from the changelog is useful for a number of things, even outside of "reproducible builds".

@vondruch can you rebase please and also bump version in spec?

@vondruch can you rebase please and also bump version in spec?

Done

I'm confused. What exactly does "RPM can set the SOURCE_DATE_EPOCH environment variable" mean? Does it set the variable or not? Does it only set if told to? How do I do that?

Or should it read: RPM now sets the SOURCE_DATE_EPOCH environment variable?

@churchyard With this change, RPM now sets the variable. For additional reproducibility, setting %clamp_mtime_to_source_date_epoch 1 can be set so that the RPMs don't have dates later than the changelog datestamp.

The spec says: When making changes, update version by +1, leave release alone.

We are using %source_date_epoch_from_changelog in official openSUSE builds for over a year and I found it helps a lot in making packages build reproducibly. No trouble there so far. Maybe some y2038 bugs.
Not sure if some people were confused by the rpm Build Date header using it.

%clamp_mtime_to_source_date_epoch is still off by default, because it had some negative effects on python .pyc files that embed the (original) timestamp of the associated .py file. https://bugzilla.opensuse.org/show_bug.cgi?id=1133809

Not sure if some people were confused by the rpm Build Date header using it.

@bmwiedemann Wait, this freezes the build time as well? That's not particularly helpful... Why would that be included in this flag? If anything, that should be a separate flag...

The final goal is to have bit-identical binary rpms and there is no other way I found to override the build date, so it made sense to have it this way. There is still the signature date telling when the package was signed and rpm --delsign allows to drop that for comparison purposes.

https://en.opensuse.org/openSUSE:Reproducible_Builds documents %use_source_date_epoch_as_buildtime as a separate flag but that seems to be from a non-upstream rpm patch.

@bmwiedemann Yeah, seems like that's a non-upstream change... :weary:

I've submitted a variation of that patch for upstream RPM just now: https://github.com/rpm-software-management/rpm/pull/785

This likely changes the default python .pyc file Invalidation Mode to CHECKED_HASH: https://github.com/python/cpython/pull/5200/files -- it might make sense to swap that over to UNCHECKED_HASH for performance reasons.

@churchyard @pviktori @cstratak @torsava please see comment above which might affect Python.

That behavior is part of Python 3.7 and 3.8.

At least for Python 3.7 (python3 package for now), we have disabled this because of test failures: https://src.fedoraproject.org/rpms/python3/c/9f5808cf53efafc3f76c1ed425d5b0bb7818e871?branch=master

Yet various Python packages might compile their bytecode in the CHECKED_HASH mode. Will check.

@cyberpear Thanks for the report.

This is my Fedora 30:

$ hexdump /usr/lib/python3.7/site-packages/__pycache__/distro.cpython-37.pyc | head -n1
0000000 0d42 0a0d 0000 0000 fd3a 5c57 a8f3 0000

This is a file from a freshly rawhide mockbuilt Python package:

$ hexdump usr/lib/python3.7/site-packages/__pycache__/distro.cpython-37.pyc | head -n1
0000000 0d42 0a0d 0003 0000 e9dc 9efd 61dc 5368

Now a quote from PEP 552:

The pyc header currently consists of 3 32-bit words. We will expand it to 4. The first word will continue to be the magic number, versioning the bytecode and pyc format. The second word, conceptually the new word, will be a bit field. The interpretation of the rest of the header and invalidation behavior of the pyc depends on the contents of the bit field.

If the bit field is 0, the pyc is a traditional timestamp-based pyc. I.e., the third and forth words will be the timestamp and file size respectively, and invalidation will be done by comparing the metadata of the source file with that in the header.

If the lowest bit of the bit field is set, the pyc is a hash-based pyc. We call the second lowest bit the check_source flag...

See the second world used to be 0000 0000, yet now it is 0003 0000. Both the lowest bit and the second lowest bit are set, the pyc was built in the CHECKED_HASH mode.

I confirm this has changed the behavior. I believe it might have a negative impact on performance.

Quickly brainstorming here, I see several options:

1) modify %py3_build, %py_byte_compile and brp-python-bytecompile to force the TIMESTAMP mode

This obviously has problems, such as:

• workarounds that actually want the (UN)CHECKED_HASH mode would fail - however a new workaround might be fabricated (such a a %pyc_invalidation_mode macro)
• packages that bytecompile by their own custom mechanism (autotools, cmake, whatnot) will not respect this

2) explicitly aim for UNCHECKED_HASH for performance

This is the most convenient mode when it comes to performance. However powerusers are used to edit installed .py files to debug problems. That would stop working. Also, not quite sure ATM how to achieve that (again, custom build systems might not respect that).

3) revert this change

Any other ideas? @pviktori @cstratak @torsava are all on EuroPython this week, not sure they will be able to pitch in. I can try to bring this to python-devel.

See the (very short) discussion on python-devel about the deterministic pycs when Python 3.7 introduced this.

https://lists.fedoraproject.org/archives/list/python-devel@lists.fedoraproject.org/thread/Y2QRYFOWXUZWEUOHX7JKPFO2A3N23ATK/

This is a periodic reminder that Fedora has a process for changes like this (what could possibly go wrong).

One nice option could be to have UNCHECKED_HASH compare the mtime of .py and .pyc file and if they dont match up, behave like CHECKED_HASH. This would make it fast in the default case of packaged .pyc files and make it safer/easier/unsurprisinger for power-users to debug.

Otherwise: what is the point of having a hash in there, if you never check it?

One nice option could be to have UNCHECKED_HASH compare the mtime of .py and .pyc file and if they dont match up, behave like CHECKED_HASH. This would make it fast in the default case of packaged .pyc files and make it safer/easier/unsurprisinger for power-users to debug.

Otherwise: what is the point of having a hash in there, if you never check it?

If you compare mtimes, you basically have the TIMESTAMP mode. I don't see the benefit of inventing a new "check timestamp and only check hash if the previous check failed" invalidation mode.

Nay, the TIMESTAMP mode compares 1 mtime to bytes from a .pyc header.
Comparing 2 mtimes has the advantage that both or none are modified by rpmbuild/SOURCE_DATE_EPOCH so it remains consistent.

Oh. Yes, comparing just the mtimes of pyc and py files and falling back to hash comparison if pyc is older would probably work (unless I'm missing something). However that is still a new invalidation mode and I would like to avoid carrying it downstream only. If done upstream, this needs to be proposed for Python 3.9 (Fedora 34 or later) - we might be able to backport it sooner or even pioneer the change like we did with C.UTF-8 locale coercion, however definitively not for Fedora 31.

From my point of view:

• checked_hash means performance tradeoff (I've done some really quick test and import of a large Python file is much slower with this invalidation mode)
• unchecked_hash means usability tradeoff (even it's a bad idea to edit files shipped by RPM, it might be useful in some cases)
• timestamp invalidation mode would mean to sacrifice reproducible builds for now but from my point of view, this is the best thing to do now.

Question is whether we should update Python macros or revert this change and do it again as a Fedora change and prepare everything in advance.

We could probably adapt our macros , but we cannot fix every package that bytecompiles without them.

I see one more option: patch CPython to make py_compile ignore SOURCE_DATE_EPOCH in RPM build only.
That would revert the behavior for Fedora's builds for now, and we can go raise the issue upstream that forcing CHECKED_HASH is probably not a good thing to do in all cases.
I'm not against a new invalidation mode, but that's for a longer discussion.

s/all cases/all cases where SOURCE_DATE_EPOCH is set/

Note that this would still break workarounds like this: https://src.fedoraproject.org/rpms/glib2/c/de2e4aad989fff9bdd232b7f68b5e619198f5ef6?branch=master

Also note that it's not in all cases where SOURCE_DATE_EPOCH is set, se also https://github.com/python/cpython/pull/9607

Working on a workaround in https://github.com/fedora-python/cpython/pull/3

Quick grep over the spec files revels no Fedora package that would explicitly set SOURCE_DATE_EPOCH in spec to get a different bytecode.

https://src.fedoraproject.org/rpms/glib2/c/de2e4aad989fff9bdd232b7f68b5e619198f5ef6?branch=master has been committed over.

Note that this would still break workarounds like this: https://src.fedoraproject.org/rpms/glib2/c/de2e4aad989fff9bdd232b7f68b5e619198f5ef6?branch=master

This change was reverted in glib2, instead it now uses PYTHONHASHSEED=0 environment variable (SOURCE_DATE_EPOCH is no longer set).