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.

Sign up for GitHub

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

Make sdist determinisitic by setting gzip mtime to 0 #870

Closed
wants to merge 1 commit into from
Closed

Make sdist determinisitic by setting gzip mtime to 0 #870

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion poetry/masonry/builders/sdist.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ def build(self, target_dir=None): # type: (Path) -> Path
target = target_dir / "{}-{}.tar.gz".format(
self._package.pretty_name, self._meta.version
)
gz = GzipFile(target.as_posix(), mode="wb")
gz = GzipFile(target.as_posix(), mode="wb", mtime=0)
Copy link
Member

Choose a reason for hiding this comment

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

Hello @achow101,

thanks a lot for your contribution. Fixing this looks necessary to me. Are there any reasons for settings the mtime to 0 and not to the current time via time.time()?

fin swimmer

Copy link
Author

Choose a reason for hiding this comment

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

It's literally the whole point of this PR. A fixed time is needed otherwise the resulting archive is non-deterministic. Setting to time.time() means that the timestamp will continue to be variable which is what I'm trying to fix.

Copy link
Member

Choose a reason for hiding this comment

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

Ah , sorry it was obviously to late yesterday. The current timestamp is already used if the parameter is omitted and I thought it is something similar to the already merged #1541.

So let's change my question :) Why do you want the file to be deterministic?

Copy link
Author

Choose a reason for hiding this comment

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

I subscribe to the idea that the releases that you publish should be built in such a way that other people can repeat the build process and produce the exact same files, down to the bit. This makes it much easier to audit the published binaries/archives. https://reproducible-builds.org/ basically explains why.

There's also a bunch of things already in poetry which make the build results more reproducible, so reproducibility is clearly something that is intended in this project.

tar = tarfile.TarFile(
target.as_posix(), mode="w", fileobj=gz, format=tarfile.PAX_FORMAT
)
Expand Down
16 changes: 16 additions & 0 deletions tests/masonry/builders/test_sdist.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# -*- coding: utf-8 -*-
import ast
import gzip
import shutil
import tarfile

Expand Down Expand Up @@ -473,3 +474,18 @@ def test_proper_python_requires_if_three_digits_precision_version_specified():
parsed = p.parsestr(to_str(pkg_info))

assert parsed["Requires-Python"] == "==2.7.15"


def test_sdist_mtime_zero():
poetry = Poetry.create(project("module1"))

builder = SdistBuilder(poetry, NullEnv(), NullIO())
builder.build()

sdist = fixtures_dir / "module1" / "dist" / "module1-0.1.tar.gz"

assert sdist.exists()

with gzip.open(str(sdist), "rb") as gz:
gz.read(100)
assert gz.mtime == 0