-
Notifications
You must be signed in to change notification settings - Fork 70
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
ENH: do not include uncommitted changes in the sdist #587
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
17b9cac
ENH: do not include uncommitted changes in the sdist
dnicolodi e0f4385
ENH: make sdist archives reproducible
dnicolodi 6d59055
DOC: remove release process description from published documentation
dnicolodi 2936b4d
DOC: document how to create source distributions
dnicolodi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
.. SPDX-FileCopyrightText: 2024 The meson-python developers | ||
.. | ||
.. SPDX-License-Identifier: MIT | ||
|
||
.. _sdist: | ||
|
||
****************************** | ||
Creating a source distribution | ||
****************************** | ||
|
||
A source distribution for the project can be created executing | ||
|
||
.. code-block:: console | ||
|
||
$ python -m build --sdist . | ||
|
||
in the project root folder. This will create a ``.tar.gz`` archive in the | ||
``dist`` folder in the project root folder. This archive contains the full | ||
contents of the latest commit in revision control with all revision control | ||
metadata removed. Uncommitted modifications and files unknown to the revision | ||
control system are not included. | ||
|
||
The source distribution archive is created by adding the required metadata | ||
files to the archive obtained by executing the ``meson dist --no-tests | ||
--allow-dirty`` command. To generate a source distribution, ``meson-python`` | ||
must successfully configure the Meson project by running the ``meson setup`` | ||
command. Additional arguments can be passed to ``meson dist`` to alter its | ||
behavior. Refer to the relevant `Meson documentation`__ and to the | ||
:ref:`how-to-guides-meson-args` guide for details. | ||
|
||
The ``meson dist`` command uses the archival tool of the underlying revision | ||
control system for creating the archive. This implies that a source | ||
distribution can only be created for a project versioned in a revision control | ||
system. Meson supports the Git and Mercurial revision control systems. | ||
|
||
Files can be excluded from the source distribution via the relevant mechanism | ||
provided by the revision control system. When using Git as a revision control | ||
system, it is possible to exclude files from the source distribution setting | ||
the ``export-ignore`` attribute. For example, adding a ``.gitattributes`` | ||
files containing | ||
|
||
.. code-block:: none | ||
|
||
dev/** export-ignore | ||
|
||
would result in the ``dev`` folder to be excluded from the source | ||
distribution. Refer to the ``git archive`` documentation__ for | ||
details. Another mechanism to alter the content of the source distribution is | ||
offered by dist scripts. Refer to the relevant `Meson documentation`__ for | ||
details. | ||
|
||
__ https://mesonbuild.com/Creating-releases.html | ||
__ https://git-scm.com/docs/git-archive#ATTRIBUTES | ||
__ https://mesonbuild.com/Reference-manual_builtin_meson.html#mesonadd_dist_script |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
# SPDX-FileCopyrightText: 2024 The meson-python developers | ||
# | ||
# SPDX-License-Identifier: MIT | ||
|
||
project('very-long-project-name-that-makes-the-paths-within-the-sdist-exceed-100-characters-xxxxxxxxxxxxxxxxx', version: '1.0.0') |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
# SPDX-FileCopyrightText: 2021 The meson-python developers | ||
# | ||
# SPDX-License-Identifier: MIT | ||
|
||
[build-system] | ||
build-backend = 'mesonpy' | ||
requires = ['meson-python'] | ||
|
||
[project] | ||
name = 'long-path' | ||
dynamic = ['version'] |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes are meaningful and not strictly related to uncommitted changes to files.
Example with SciPy:
Compared to what I see in the sdist (screenshots because extracting files changes ownership):
with 0.16.0:
with this PR:
Note that the file permissions changed compared to what's in the git repo, and ownership now includes my own username. The former I'm not sure about either way, that is behavior inherited from
meson dist
it looks like. The latter seems undesirable.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change is unintended, in the sense that I trusted
meson dist
(and thus in turngit archive
) to be doing the right thing. I'm also not sure whether the previous behavior was deliberate: it seems that files generated via dist scripts do not go through the same metadata mangling, and the metadata mangling seems more an accidental omission than a normalization.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For file ownership, I checked
git archive --format=tar.gz HEAD > tmp.tar.gz
and it sets the ownership toroot
. So I guess the repacking changes it to local-username. Either way, the current behavior seems preferred, since it gives the same result when run on multiple machines.Re file permissions: there are config settings for it, e.g.
tar.umask
in https://git-scm.com/docs/git-archive#_configuration. Whatgit-archive
does by default is probably good for reproducibility, and seems to be by design.Having a peek in
meson/mdist.py
, it doesn't pass any options like--owner=0
/--group=0
,--no-same-owner
,--no-same-permissions
totar
. Not sure if that just never came up, or was rejected before. I can't find anything related in the Meson issue tracker so quickly.I wouldn't read too much into that, since this code was written pretty early in the project's history, and I don't think I knew about
add_dist_script
at the time; use ofadd_dist_script
probably hadn't come up at all yet.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only spent a few minutes investigating the thing, and it seems that the archive metadata changes happen in Meson. I tend to think that it is a Meson bug. Unless there are subtleties I am missing, meson-python just copies over the archive members as is. The Meson behavior needs to be fixed, but in the meantime we can fix the metadata in meson-python.
What does
git archive
do, other than setting the owner to root:root?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One funny thing, is that
git archive
, Meson, and meson-python seem to use three different tar formats.It would be nice if there were a way to avoid packing the files up three times, but I'm not sure there is one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
git archive
andmeson dist
are trying to do fundamentally different things as a result of dist scripts existing. It will never be possible to reuse git's output directly (at least, impossible except as a micro-optimization for a subset of cases that always excludes the creation of python dist tarballs with sdist metadata.It would be possible to uplift the meson-python specific changes into
meson dist
, of course. At the most basic level, it's "just" a kind of domain specific dist script.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice for
meson dist
to be reproducible if the dist scripts are reproducible, following a bit more closely whatgit archive
does. Using the correct file metadata in the archive does not seem difficult to achieve, and it is at least a step in the right direction.shutil.make_archive()
hasowner
andgroup
argument, that seem to fit at least part of the requirement.For avoiding packing the files three times, I was more thinking about not having the lower layer pack them at all and have the upper layer do the packing in the way it likes, more than reusing the tarball generated by the lower layer. meson-python just needs to add a file to the archive, and this is possible without unpacking and repacking. The reason why meson-python nedds to unpack and repack the tarball is to fix the rood directory in the arvhive: the project name and version seen by Meson and meson-python are not necessarily the same.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Part", eh. :D
Granted this is something that could be properly changed by giving up shutil.make_archive as a bad idea and rolling tarfile/zipfile manually.
... although they probably should be, and for dynamic version it will be required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure they should. For project that only want to produce a Python packages, specifying the version only in
pyproject.toml
seems a perfectly valid thing to do, and for project that are more than a Python package, it seems reasonable to have the Python part have a different name than the whole. Introducing a restriction for them to be the same does not seem worth just to make it a bit easier to generate the sdist and it would be easier only if we fixmeson dist
to emit the right archive member metadata.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not saying it should be restricted, just that in a very large number of common cases they will be the same and won't need tweaking.
It is, at least, a useful micro optimization to check for (assuming no other issues).
As for versions, you can provide it in either or. But if you provide it in meson.build you can handle git versioning as well as using the version as a replacement string to insert into e.g. _version.py, so it is my (entirely personal) opinion that people should want to declare it as dynamic, because it is simply superior.