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

mesonpy: include uncommited changes in sdist #58

Merged
merged 1 commit into from
May 26, 2022
Merged

Conversation

FFY00
Copy link
Member

@FFY00 FFY00 commented May 20, 2022

Fixes #53

Signed-off-by: Filipe Laíns [email protected]

@FFY00
Copy link
Member Author

FFY00 commented May 20, 2022

@rgommers can you have a look?

Copy link
Contributor

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

This doesn't seem to work. It now builds an sdist rather than error out, and shows this warning:

WARNING: Repository has uncommitted changes that will not be included in the dist tarball

However, the uncommitted changes are not actually included in the sdist - and including desired changes was the goal here.

@FFY00
Copy link
Member Author

FFY00 commented May 20, 2022

Ah, sorry, I misread the Meson commit. Then it is non-trivial to implement this, I will have a look later.

@rgommers
Copy link
Contributor

scipy/scipy#16187 (comment) points to the Flit code for this. Flit does seem to have the behavior we want: do not include files that are not tracked, but do include uncommitted changes to tracked files. The code in Flit to do this is quite concise. Seems like a good option to just copy that.

@eli-schwartz
Copy link
Member

Ah, sorry, I misread the Meson commit. Then it is non-trivial to implement this, I will have a look later.

Yeah, Meson doesn't include any prepackaged support for doing this. :( Any way you slice it, meson dist will want to use git archive as a transfer mechanism and I don't think that can handle uncommitted changes; it fundamentally utilizes a git tree ref.

Previous versions of Meson actually warned you on uncommitted changes, because those changes would not be disted. The most recent release outright errors, and offers a flag to restore the 0.61.0 behavior of merely warning you that those changes won't be included. The logic behind the change is that it was easy to not notice the warning, so we changed the emphasis of the warning itself.

The flit approach is pretty good if your goal is to include uncommitted changes. It does mean that you'll no longer get some things that git-archive does, such as gitattributes export-ignore / export-subst, but SciPy isn't using either one so that probably does not matter.

@FFY00 FFY00 force-pushed the uncomitted-changes branch from 4342a0c to dc880f6 Compare May 25, 2022 19:49
@FFY00
Copy link
Member Author

FFY00 commented May 25, 2022

@rgommers can you give this a try before merging just to confirm it indeed works as expected for SciPy? Thanks!

@FFY00 FFY00 force-pushed the uncomitted-changes branch 2 times, most recently from 92518cd to 1f4e0d7 Compare May 25, 2022 19:58
@FFY00 FFY00 force-pushed the uncomitted-changes branch from 1f4e0d7 to c564059 Compare May 25, 2022 20:05
@rgommers
Copy link
Contributor

Thanks @FFY00, this appears to work. At least I tested editing a .py file, running $ python -m build --sdist --skip-dependency-check --no-isolation to pick up meson-python from this branch, and then getting a built sdist that indeed includes that change to the .py file.

One small thing to improve perhaps, this warning may be confusing:

+ meson dist --allow-dirty --no-tests --formats gztar                                               
WARNING: Repository has uncommitted changes that will not be included in the dist tarball
Created /home/rgommers/code/scipy/.mesonpy-dqpt326g/build/meson-dist/SciPy-1.9.0.dev0.tar.gz
Successfully built scipy-1.9.0.dev0.tar.gz

How about logging right above it something like "meson-python detected changes to tracked files and will include those in the sdist (please ignore the warning 'Repository has uncommitted changes ...' below)" ?

Whether pip install . with build isolation works, I can only test after 0.5.0 is up on PyPI I think. Will reserve some time for that tomorrow morning.

@FFY00
Copy link
Member Author

FFY00 commented May 26, 2022

How about logging right above it something like "meson-python detected changes to tracked files and will include those in the sdist (please ignore the warning 'Repository has uncommitted changes ...' below)" ?

That makes sense, but we are currently not able to detect this, and it would probably require some work to do so. I will push a 0.5.0 with this PR as-is for now and then revisit this.

@FFY00 FFY00 merged commit e4a9c3d into main May 26, 2022
@FFY00 FFY00 deleted the uncomitted-changes branch May 26, 2022 00:31
rgommers added a commit to rgommers/meson-python that referenced this pull request Jul 26, 2022
This dependency was bumped from 0.60.0 to 0.62.0 in mesonbuildgh-58,
but only in one of the two places where it was needed. These
two places should probably always be kept consistent with each other,
even though technically it's not _required_.
FFY00 pushed a commit that referenced this pull request Jul 26, 2022
This dependency was bumped from 0.60.0 to 0.62.0 in gh-58,
but only in one of the two places where it was needed. These
two places should probably always be kept consistent with each other,
even though technically it's not _required_.
FFY00 pushed a commit that referenced this pull request Jul 28, 2022
This dependency was bumped from 0.60.0 to 0.62.0 in gh-58,
but only in one of the two places where it was needed. These
two places should probably always be kept consistent with each other,
even though technically it's not _required_.

Signed-off-by: Filipe Laíns <[email protected]>
@rgommers rgommers added the enhancement New feature or request label Aug 8, 2023
dnicolodi added a commit to dnicolodi/meson-python that referenced this pull request Feb 29, 2024
Including uncommitted changes in the sdist was implemented in mesonbuild#58
after the discussion in mesonbuild#53. However, the current behavior for which
uncommitted changes to files under version control are included but
not other files, is an hard to justify surprising half measure.

After careful analysis, it has been determined that none of the use
cases for this feature is still valid. Removing it makes the
implementation easier, and the behavior less surprising and easier to
document an explain.
dnicolodi added a commit to dnicolodi/meson-python that referenced this pull request Feb 29, 2024
Including uncommitted changes in the sdist was implemented in mesonbuild#58
after the discussion in mesonbuild#53. However, the current behavior for which
uncommitted changes to files under version control are included but
not other files, is an hard to justify surprising half measure.

After careful analysis, it has been determined that none of the use
cases for this feature is still valid. Removing it makes the
implementation easier, and the behavior less surprising and easier to
document an explain.

While at it, modernize the associated test.
dnicolodi added a commit to dnicolodi/meson-python that referenced this pull request Apr 18, 2024
Including uncommitted changes in the sdist was implemented in mesonbuild#58
after the discussion in mesonbuild#53. However, the current behavior for which
uncommitted changes to files under version control are included but
not other files, is a hard to justify surprising half measure.

After careful analysis, it has been determined that none of the use
cases for this feature is still valid. Removing it makes the
implementation easier, and the behavior less surprising and easier to
document and explain.

While at it, modernize the associated test.
dnicolodi added a commit to dnicolodi/meson-python that referenced this pull request Apr 23, 2024
Including uncommitted changes in the sdist was implemented in mesonbuild#58
after the discussion in mesonbuild#53. However, the current behavior for which
uncommitted changes to files under version control are included but
not other files, is a hard to justify surprising half measure.

After careful analysis, it has been determined that none of the use
cases for this feature is still valid. Removing it makes the
implementation easier, and the behavior less surprising and easier to
document and explain.

While at it, modernize the associated test.
dnicolodi added a commit to dnicolodi/meson-python that referenced this pull request Apr 23, 2024
Including uncommitted changes in the sdist was implemented in mesonbuild#58
after the discussion in mesonbuild#53. However, the current behavior for which
uncommitted changes to files under version control are included but
not other files, is a hard to justify surprising half measure.

After careful analysis, it has been determined that none of the use
cases for this feature is still valid. Removing it makes the
implementation easier, and the behavior less surprising and easier to
document and explain.

While at it, modernize the associated test.
dnicolodi added a commit to dnicolodi/meson-python that referenced this pull request May 1, 2024
Including uncommitted changes in the sdist was implemented in mesonbuild#58
after the discussion in mesonbuild#53. However, the current behavior for which
uncommitted changes to files under version control are included but
not other files, is a hard to justify surprising half measure.

After careful analysis, it has been determined that none of the use
cases for this feature is still valid. Removing it makes the
implementation easier, and the behavior less surprising and easier to
document and explain.

While at it, modernize the associated test.
dnicolodi added a commit to dnicolodi/meson-python that referenced this pull request May 2, 2024
Including uncommitted changes in the sdist was implemented in mesonbuild#58
after the discussion in mesonbuild#53. However, the current behavior for which
uncommitted changes to files under version control are included but
not other files, is a hard to justify surprising half measure.

After careful analysis, it has been determined that none of the use
cases for this feature is still valid. Removing it makes the
implementation easier, and the behavior less surprising and easier to
document and explain.

While at it, modernize the associated test.
dnicolodi added a commit that referenced this pull request May 2, 2024
Including uncommitted changes in the sdist was implemented in #58
after the discussion in #53. However, the current behavior for which
uncommitted changes to files under version control are included but
not other files, is a hard to justify surprising half measure.

After careful analysis, it has been determined that none of the use
cases for this feature is still valid. Removing it makes the
implementation easier, and the behavior less surprising and easier to
document and explain.

While at it, modernize the associated test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Do not ignore uncommitted changes in repo
3 participants