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

BUG: add generated files to sdist #177

Merged
merged 7 commits into from
Nov 1, 2022

Conversation

lithomas1
Copy link
Member

@lithomas1 lithomas1 commented Oct 15, 2022

Generated files were not being added to the sdist.
This is useful for pandas, since we run a versioneer script to generate the sdist.

I think scipy avoids this, since they don't add generate a version file for their sdist?

I'll add a test, sometime in the future, but I'm a little busy now, so feel free to push to this branch in the meantime.

@lithomas1
Copy link
Member Author

cc @rgommers @FFY00

@rgommers
Copy link
Contributor

Thanks @lithomas1. I don't think this specific change is a good idea - it's going to pull in all sorts of files that were not intended to be included. I think what you want instead to include specific files that are not under version control (like version.py generated by versioneer) is either meson.add_dist_script() or the if fs.exists... code from https://github.com/FFY00/meson-python/issues/159#issuecomment-1265962676.

I think scipy avoids this, since they don't add generate a version file for their sdist?

We had a similar version-related file with the git hash, but got rid of it when moving to Meson. We may want to bring it back, but it hasn't been a priority so far.

@lithomas1
Copy link
Member Author

lithomas1 commented Oct 17, 2022

I'm a little confused. I'm pretty sure the only way to get generated files into the sdist is by adding them with meson.add_dist_script()(which I'm using with pandas), so there wouldn't be any unwanted generated files.

One thing that meson-python might consider doing, is parsing a MANIFEST file like setuptools. The .tar.gz meson generates includes a bunch of extra things, and its probably much easier to do this filtering in meson.

@FFY00
Copy link
Member

FFY00 commented Oct 17, 2022

it's going to pull in all sorts of files that were not intended to be included

Ralf, can you elaborate on that? AFAICT it will only pull files that are present in the Meson dist but not on the git checkout.

@rgommers
Copy link
Contributor

it's going to pull in all sorts of files that were not intended to be included

Ralf, can you elaborate on that? AFAICT it will only pull files that are present in the Meson dist but not on the git checkout.

I think I misunderstood what's going on here. A quick try with a breakpoint under path.is_file() showed files that we didn't want to include on SciPy, and the PR title/description seemed to say "all generated files". I did some more testing with the actual change in this PR, and it seems fine indeed.

@rgommers rgommers added the enhancement New feature or request label Oct 18, 2022
@rgommers rgommers added this to the v0.11.0 milestone Oct 18, 2022
@lithomas1
Copy link
Member Author

Cool, I'll add tests in the future then.

@FFY00
Copy link
Member

FFY00 commented Oct 19, 2022

I pushed a small refactoring to make the code a bit easier to maintain, I hope you don't mind. Let me know if you want me to avoid that in the future.

@lithomas1
Copy link
Member Author

I pushed a small refactoring to make the code a bit easier to maintain, I hope you don't mind. Let me know if you want me to avoid that in the future.

No worries. Thanks for fixing the pre-commit as well.
I'm going to be busy since midterms are rolling around again, so feel free to finish up anything I have open in case I don't circle back to it in time.

@lithomas1
Copy link
Member Author

pre-commit.ci autofix

@lithomas1
Copy link
Member Author

@rgommers @FFY00 This is as green as it gets.

Copy link
Member

@FFY00 FFY00 left a comment

Choose a reason for hiding this comment

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

Thank you @lithomas1!

mesonpy/__init__.py Outdated Show resolved Hide resolved
@FFY00 FFY00 changed the title Add generated files to sdist BUG: add generated files to sdist Oct 31, 2022
@FFY00 FFY00 enabled auto-merge (squash) October 31, 2022 00:51
@FFY00 FFY00 disabled auto-merge October 31, 2022 14:59
@FFY00 FFY00 merged commit 71c6a3a into mesonbuild:main Nov 1, 2022
@lithomas1 lithomas1 deleted the fix-generated-file-sdist branch November 1, 2022 10:37
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.

3 participants