Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpo-38632: respect SOURCE_DATE_EPOCH when building .tar sdists #20331

Closed
wants to merge 1 commit into from

Conversation

Carreau
Copy link
Contributor

@Carreau Carreau commented May 23, 2020

This currently only affects .tar as compression may also add some
variability, Gzip for example adds current timestamp.

With this I am able without further modification to do reproducible
build bytes of bytes of IPython with no source code changes in IPython
itself.

Adding reproducibility to other format will need to be on a per-format
basis, which currently is tough as distutils seem to shell out to do the
compression. I can do some refactor and do in process tar and
compressing – which should be faster/more robust, but will be another
pull request.

https://bugs.python.org/issue38632

This currently only affects .tar as compression may also add some
variability, Gzip for example adds current timestamp.

With this I am able without further modification to do reproducible
build bytes of bytes of IPython with no source code changes in IPython
itself.

Adding reproducibility to other format will need to be on a per-format
basis, which currently is tough as distutils seem to shell out to do the
compression. I can do some refactor and do in process tar and
compressing – which should be faster/more robust, but will be another
pull requests.
has been added in distutils. When the ``SOURCE_DATE_EPOCH`` environment
variable is set, the ``mtime`` of the files in an sdist tar archive will not
be later than ``SOURCE_DATE_EPOCH``. This is a firs step to simplify getting
byte identical reproducibility of source dists.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Can you explain why this PR is a first step / partial support?

  2. distutils is mostly invisible these days, importated and extended by setuptools. Do the changes impact setuptools sdists?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There a bunch of other things that add variability to the end artifact. For example ZIP using current datetime in the header, which also needs to be fixed, making sure directory/files are traveres in the same order... etc.

I try to keep things minimal as PRs here tends to sits for years, and the bigger the longer.

2. distutils is mostly invisible these days, imported and extended by setuptools. Do the changes impact setuptools sdists?

Yes, it extends and call it, so if the result of this is not reproducible then it needs to be monkeypatch in setuptools (see pypa/setuptools#2136 which I need to finish) Basically 1/2 of the upstream patch is copy-pasting code from distutils to tweak the internal behavior.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds OK then!

# https://reproducible-builds.org/specs/source-date-epoch/
# we are at least sure that when it is set no timestamp can be later than
# this.
if (sde:= os.environ.get('SOURCE_DATE_EPOCH')):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (sde:= os.environ.get('SOURCE_DATE_EPOCH')):
if (sde := os.environ.get('SOURCE_DATE_EPOCH')):

if ORIGINAL_SDE is None:
del os.environ['SOURCE_DATE_EPOCH']
else:
os.environ['SOURCE_DATE_EPOCH'] = ORIGINAL_SDE
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could make all this setup/teardown simpler by using one of the helpers in test.support or maybe unittest.mock.patch?

@raboof
Copy link

raboof commented Dec 1, 2020

@Carreau thanks for looking into this! It seems this PR only needs a couple of smallish tweaks. Do you prefer to make those yourself or would you appreciate others building upon this PR?

@Carreau
Copy link
Contributor Author

Carreau commented Dec 1, 2020

Feel free to take over, I might get back to it at some point; but my understanding is that most of that should anyway go into setuptools which now ship its own copy of distutils.

I might get back to that at some point, but miss the time to so those days.

@raboof
Copy link

raboof commented Dec 2, 2020

my understanding is that most of that should anyway go into setuptools

Gotcha, with https://www.python.org/dev/peps/pep-0632/ incoming this might not so urgent to solve at the cpython side anymore.

@merwok
Copy link
Member

merwok commented May 11, 2021

Code has been moved, please submit this change to https://github.com/pypa/distutils/

@merwok merwok closed this May 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants