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

Allow different options for checking the sdist, e.g. flit build or build #15

Closed
fwitte opened this issue Aug 21, 2023 · 3 comments
Closed

Comments

@fwitte
Copy link

fwitte commented Aug 21, 2023

Hi,

thanks for your practical tool, I discovered it through mgedmin/check-manifest#163 while wondering how I can fix my pipeline after having migrated away from a setup.py invocated build. I am using flit as build system now and want to use flit build to build my package, since flit build includes my tests, some examples and the docs in the sdist (whether it belongs there or not seems to be a controversial discussion... https://discuss.python.org/t/should-sdists-include-docs-and-tests/14578/23 I decided to go with it as before, until someone tells me not do to it for good reasons :D).

You tool runs build which only includes the src directory, pyproject.toml, readme and metadata in the sdist, therefore the check fails because all kinds of source files from the vcs are missing in the sdist. I could use the git-only list to prevent the tool from throwing an error, but since I actually want to check, whether all the files are shipped inside the sdist, I think an option to replace the build command

cmd = [sys.executable, "-m", "build", "--sdist", "--outdir", outdir]

in some way could be very helpful. What do you think? I have opened a respective PR :).

Best!

@fwitte fwitte changed the title Test the sdist for running flit build instead of build Allow different options for checking the sdist, e.g. flit build or build Aug 21, 2023
@henryiii
Copy link
Owner

One of the main reasons this exists is due to this horrible bug/design of flit, where standardized tools (like build, build-and-inspect-python-package, etc.) can't build a valid flit-core sdist due to the fact that flit doesn't run flit-core like standardized tools do. What I'd highly recommend doing is fixing flit-core to always include these important files via flit's config. Here's the config in pypa/packaging, which was part of why this tool was written:

[tool.flit.sdist]
include = ["LICENSE*", "tests/", "docs/", "CHANGELOG.rst"]
exclude = ["docs/_build", "tests/manylinux/build-hello-world.sh", "tests/musllinux/build.sh", "tests/hello-world.c", "tests/__pycache__", "build/__pycache__"]

https://github.com/pypa/packaging/blob/7e68d828f265ef05cf4cd3b5def9baffef8c2968/pyproject.toml#L38-L40

(I'm not totally against this option as long as it was opt-in, but want to point out why I think it's backwards - the point of the tool was to make sure you configure your repo correctly to support any standardized backend following PEP 517 and to catch exactly what you are seeing).

@fwitte
Copy link
Author

fwitte commented Aug 21, 2023

Ah, got it! Thank you for your help, have a good week!

@fwitte fwitte closed this as completed Aug 21, 2023
@henryiii
Copy link
Owner

Ahh, one more note, was looking at flit-core's changelog for something else:

New options flit build --use-vcs and flit build --no-use-vcs to enable & disable including all committed files in the sdist. For now --use-vcs is the default, but this is likely to change in a future version, to bring flit build in line with standard build frontends like python -m build (PR #625).

So it looks like it's really, really a good idea to fix the file inclusion, unless you switch to flit build --use-vcs. I like this much better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants