Skip to content

[BUG] PEP 631 TOMLs can conflict with deprecated configuration files #3300

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

Closed
nullableVoidPtr opened this issue May 1, 2022 · 5 comments · Fixed by #3306 or #4696
Closed

[BUG] PEP 631 TOMLs can conflict with deprecated configuration files #3300

nullableVoidPtr opened this issue May 1, 2022 · 5 comments · Fixed by #3306 or #4696
Labels
bug Needs Triage Issues that need to be evaluated for severity and status.

Comments

@nullableVoidPtr
Copy link
Contributor

nullableVoidPtr commented May 1, 2022

setuptools version

setuptools==62.1.0

Python version

Python 3.10.4

OS

Arch Linux

Additional environment information

No response

Description

A package with PEP 621/631 compliant pyproject.toml and the build_meta backend may have its metadata and other configuration information overlaid with other information from setup.py or setup.cfg (which may be retained for fallback with --no-use-pep517 while setuptools transitions to a stable PEP 517 implementation by default).

One cause is this helper function which guarantees that the "fallback" setup.py would be called even if pyproject.toml is fully defined.

Expected behavior

I expected setuptools to identify that pyproject.toml has the project and dependencies table, and would further ignore other configuration files (though I may be disregarding the value/use of dynamic fields with this expectation).

How to Reproduce

  1. pip install setuptools wheel build
  2. git clone https://gist.github.com/nullableVoidPtr/d3f528136eff5c0e827ee98fd75da9dd
  3. python -m build
  4. cat project.egg-info/requires.txt

Output

click
wheel
click

There is a duplication of the dependency click and the inclusion of wheel which was only included in setup.cfg.
(Side note: the discrepancy in dependencies between pyproject.toml and setup.py is highly unlikely in actual packages, but is used to illustrate the issue here).
There was no warnings relating to the conflict observed in the output of python -m build.

Note that this also works with setup.cfg (replacing setup.py) with the following:

[options]
install_requires =
    click
    wheel
@nullableVoidPtr nullableVoidPtr added bug Needs Triage Issues that need to be evaluated for severity and status. labels May 1, 2022
@nullableVoidPtr
Copy link
Contributor Author

CC @abravalheri, thank you for your hard work!

@abravalheri
Copy link
Contributor

abravalheri commented May 3, 2022

Hi @nullableVoidPtr, thank you for reporting this.

Is the silent overwrite the best action for this scenario?
Or should we first introduce a warning?

@nullableVoidPtr
Copy link
Contributor Author

The main concern here is PEP 517 builds silently using something which should have been a non-default fallback (at least in my case), so a warning would be good. There is a similar issue remediated at #3195, but the fix doesn't appear to apply to this case; not sure what's going on there but I'll need to debug.

One fix I've thought about was something in setup.py which allows it to be effectively disabled or behave differently under non-PEP517 builds (e.g. specifically invoked by pip --no-use-pep517). This could either be another setup keyword like legacy_use_only=True or trust_toml=True.
Not sure if there's a case for something like from setuptools import is_pep517 for more involved setup.py scripts (personally, I think it could cause discrepancies in legacy-system builds and 'modern' builds).

@abravalheri
Copy link
Contributor

abravalheri commented May 4, 2022

Hi @nullableVoidPtr.

With #3306, pyproject.toml will take precedence over setup.py. I also added a warning to inform the users. This should be enough to handle the issue, right?

One fix I've thought about was something in setup.py which allows it to be effectively disabled or behave differently under non-PEP517 builds (e.g. specifically invoked by pip --no-use-pep517).

This is a nice suggestion, however, for the time being I think we should avoid growing the number of "levers and knobs" in setuptools (they are already too many...).

It might be just a personal impression, but using both pip --no-use-pep517 and pyproject.toml metadata at the same time sounds very "edge case"-y... I don't know if going out of our way to support this is worth the effort and the maintenance cost.

@nullableVoidPtr
Copy link
Contributor Author

This is a nice suggestion, however, for the time being I think we should avoid growing the number of "levers and knobs" in setuptools (they are already too many...).
It might be just a personal impression, but using both pip --no-use-pep517 and pyproject.toml metadata at the same time sounds very "edge case"-y... I don't know if going out of our way to support this is worth the effort and the maintenance cost.

Absolutely fair call to make, and even moreso once the backend is stable with project metadata.
Happy to leave this issue as-is for #3306 to close.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Needs Triage Issues that need to be evaluated for severity and status.
Projects
None yet
2 participants