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

RFE: option to ignore pep517 and force the use of setup.py sdist (was: Problem with v0.41 and poetry) #111

Open
Cito opened this issue Feb 26, 2020 · 5 comments

Comments

@Cito
Copy link

Cito commented Feb 26, 2020

Thanks for providing check-manifest. This is really useful.

However, I ran into a problem with v0.41. Since this version, when a pyproject.tom file is detected which has build-backend specified, then the sdist is built with pep517.build --source instead of the usual setup.py sdist.

However, this does not seem to work with poetry. In my pyproject.tom I have build-backend = "poetry.masonry.api", so check-manifest uses pep517.build --source. However, that command does not seem to care about the MANIFEST.in file, and does not put the files listed there in the sdist.

I'm not sure what's going wrong here. Should build backends generally consider MANIFEST.in and poetry just fails to do this? Or am I supposed to specify my sdist files differently from MANIFEST.in, maybe somewhere in pyproject.toml?

@mgedmin
Copy link
Owner

mgedmin commented Feb 26, 2020

AFAIU Poetry is a setuptools replacement that doesn't use MANIFEST.in. I don't think check-manifest is a useful tool for Poetry users. (It's been brought up before in #103.)

I haven't used Poetry myself. I'm not sure if its include/exclude settings are used for source distributions, or just binary distributions. In any case, MANIFEST.in is a setuptools feature and I wouldn't expect all other build backends to consider it.

I'm curious that you mentioned the check-manifest version number. Are you saying that check-manifest 0.40 worked better for you on a project that uses poetry? But I thought poetry didn't use setup.py at all, so how could have check-manifest even work? Was your project actually using two alternative build systems (setuptools and poetry) at the same time?

@Cito
Copy link
Author

Cito commented Feb 26, 2020

@mgedmin thanks for the quick response. Yes, I am using both pyproject.toml and setup.py for backward compatibility.

Regarding the include/exclude settings in tool.poetry, unfortunately they affect both wheels and source distributions, so they can't replace MANIFEST.in. It is possible to set format = "sdist", but only in the the packages setting in tool.poetry. I will create an issue for that in the poetry project.

What concerns check-manifest: If you say MANIFEST.in is a setuptools feature only, then shouldn't you build the source distribution with setuptools, or use pep517 only in case that the build-backend is known to support MANIFEST.in?

@mgedmin
Copy link
Owner

mgedmin commented Feb 26, 2020

Yes, I am using both pyproject.toml and setup.py for backward compatibility.

That is a use-case I hadn't considered before. It makes sense to support it. Perhaps a command-line option (--ignore-pep517) as well as a config file option in setup.py ([check-manifest] ignore_pep517 = true)?

If you say MANIFEST.in is a setuptools feature only, then shouldn't you build the source distribution with setuptools, or use pep517 only in case that the build-backend is known to support MANIFEST.in?

Well, people requested PEP 517 build support (#105), so I added it. I think it makes sense to check if source distributions contain all the files you expect them to contain, no matter what build system you use to produce the source distributions. And "manifest" can be used in the general sense of list of things inside a package, so even check-manifest's name still makes sense. (I'll have to change the documentation to de-emphasize MANIFEST.in, to reduce confusion.)

@Cito
Copy link
Author

Cito commented Feb 26, 2020

Yes, it boils down to the question how to build the sdist when both setup.py and pyproject.toml exist. The suggested option would be a solution. Or just build both ways and check whether in both cases the sdist contains the necessary files. If both are incomplete, count this as an error. If both are complete, count it as no error. If only one of them is complete, the igrore-pep517 option could then determine whether this case should be considered an error or just a warning.

@mgedmin mgedmin changed the title Problem with v0.41 and poetry RFE: option to ignore pep517 and force the use of setup.py sdist (was: Problem with v0.41 and poetry) Feb 27, 2020
@bhearsum
Copy link

bhearsum commented Mar 3, 2020

This appears to cause an issue with pip-compile as well. It does not include pep517 as a dependency when compiling a requirements file. And if you've included hashes in your compiled requirements, you end up with errors such as:
pep517 from https://files.pythonhosted.org/packages/f4/9b/82910c0f01f29c7bdd8fc4306ed03e1742256612e2cfca8f05ebb21958ab/pep517-0.8.1-py2.py3-none-any.whl#sha256=882e2eeeffe39ccd6be6122d98300df18d80950cb5f449766d64149c94c5614a (from check-manifest==0.41->-r /balrog/agent/requirements/test.txt (line 23))

Edit: I think this is pebkac, please ignore!

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

No branches or pull requests

3 participants