Allow file: for requires statements in setup.cfg by akx · Pull Request #3253 · pypa/setuptools

akx · 2022-04-06T13:10:47Z

Summary of changes

As requested and discussed in #1951, this PR enables using file: in ~~the various requires statements~~ install_requires and extras_require statements in setup.cfg.

As @jaraco mentioned in #1951 (comment), this adds a (small) admonition advising against library packagers pinning their dependencies too tightly with this feature.

Closes #1951

Pull Request Checklist

Changes have tests
Changes are documented
News fragment added in changelog.d/.

abravalheri

Hi @akx, thank you very much for analysing the previous discussion and providing the PR.

I will leave the review to the maintainers that were participating in the original discussion, but I would like to note that this implementation does not seem to be forward compatible, because it only tackles setup.cfg.

According to #1688 (comment), the long term view is to compile setup.cfg into pyproject.toml, therefore it would be necessary to support this feature in pyproject.toml as well.

akx · 2022-04-06T14:25:26Z

[...] the long term view is to compile setup.cfg into pyproject.toml, therefore it would be necessary to support this feature in pyproject.toml as well.

Ah, sure thing! I can also take a look at getting that in here, if the approach seems otherwise sound by maintainers :)

abravalheri · 2022-04-06T14:33:23Z

Thank you very much for considering that @akx. Yes, it is definitely good to wait for the feedback before making any change.

jaraco · 2022-04-07T00:50:55Z

I’m okay to give it a try.

akx · 2022-04-07T07:15:16Z

@abravalheri Hmm, by the looks of it enabling something like

[tool.setuptools.dynamic.optional-dependencies.bonus]
file = ["requirements-bonus.txt"]

(along the lines of

setuptools/setuptools/tests/config/test_pyprojecttoml.py

Lines 66 to 71 in e5552d3

    
           [tool.setuptools.dynamic.version] 
        
           attr = "pkg.__version__.VERSION" 
        
           [tool.setuptools.dynamic.readme] 
        
           file = ["README.md"] 
        
           content-type = "text/markdown"

)

would require changes to the JSON Schema for pyproject.toml, which... ~~apparently is maintained in validate_pyproject? Am I on the right track there?~~ apparently is maintained in validate_pyproject, and I took the plunge, adding akx/validate-pyproject@9019d53 ...
However, we presumably do want for a pyproject.toml to be able to contain statically defined optional dependency groups and file-based dependency groups, so setuptools.config._validate_pyproject.extra_validations.RedefiningStaticFieldAsDynamic would need to be augmented to allow that... so as not to muddy the waters here, maybe implementing this for pyproject.tomls should be done in a separate constellation of PRs?

abravalheri · 2022-04-07T07:34:06Z

Hi @akx, If you change the code in setuptools, I am more than happy to follow up and tackle the schemas myself.

For the time being, you can create a separated test case that uses you the ignore_option_errors parameter to bypass the validation. Once I modify the schemas, I will also modify that.

akx · 2022-04-07T10:24:04Z

@abravalheri Please see #3255 for the TOML case :)

abravalheri · 2022-04-08T12:45:48Z

docs/userguide/declarative_config.rst

-setup_requires           list-semi                            36.7.0
-install_requires         list-semi
-extras_require           section                                              [#opt-2]_
+setup_requires           file:, list-semi                     36.7.0          [#opt-5]_


Question: would it make more sense to restrict this change to install_requires and extras_requires?

tests_require is deprecated, so I feel like we should not touch it.

setup_requires is largely obsoleted by PEP 518.
In very specific use cases people might still reach for it inside setup.py, but not in the context of setup.cfg. I am afraid that adding support for file: here would encourage people from using this (mostly obsolete) field instead of relying on PEP 518.

I understand that adding support for the file: directive for setup_requires and tests_require basically come for free, but I still think we should refrain from doing that.

I get where you're coming from here, but various comments in #1951 say file: support is the last thing preventing them from moving from an imperative setup.py to a PEP 518 setup.cfg build. So, if those folks had been using files for their setup_requires and tests_require, not allowing file: for them would make that porting work less straightforward.

I have no other strong opinion on this, just that thought :)

Thank you very much @akx for pointing this out. I re-read the discussion in issue 1951 and I did not see any special request for setup_requires and tests_requires.

My view is that tests_requires, already does not work, so I don't think it would be helping anyone interested in the migration.

On the other hand, setup_requires only have value (right now) when computed dynamically via setup.py.

However I can see people starting to use it once it gets file: support just to workaround limitations in whichever GitHub bots they might be using for automatic dependency updates. In my opinion that would be an unfortunate side effect, since PEP 518 should be preferred. Any problems with these automation bots should be addressed by the bots themselves, not by undermining PEP 518.

On a side note, considering the forward compatibility with pyproject.toml:

Currently setup_requires can be automatically converted to [build-system] requires by an automatic generator.

If setup_requires gains the capability of being dynamically defined via file:, the translation will no longer be straight forward and we will have to update Allow file: for dependencies in TOML #3255 to handle this new use case.

akx · 2022-06-08T05:21:21Z

@abravalheri @jaraco I rebased this to get rid of the merge conflicts. What are the next steps to get this merged..?

abravalheri · 2022-06-08T08:38:04Z

Thank you very much @akx for rebasing the PR.

Personally, I have some strong reservations against handling dynamic tests_require and setup_requires (the main reasons are described in my previous comments #3253 (comment) and #3253 (comment)).

Can we restrict the change to install_requires and extras_require?

Refs pypa#1951

akx · 2022-06-14T11:55:39Z

@abravalheri Okiedoke :) Removed the file: stuff related to tests_require and setup_requires (the code is still gently touched in setupcfg.py since parse_list_semicolon became a classmethod).

abravalheri · 2022-06-14T11:59:16Z

Thank you very much @akx and sorry for the trouble.
Since Jason seems to be OK with the change, I will try to merge the PRs yet this week.
(I am currently focusing on organising changes in the documentation and I had in the backlog the PEP 660, but I will try to make my best to not delay even more this merge).

docs/userguide/declarative_config.rst

abravalheri · 2022-06-19T18:42:09Z

Sorry for the delay @akx.

I merged the changes and published a new version of setuptools.
For the time being, I have added the status of beta, then we can see the feedback from the community.

nedbat · 2022-06-19T20:33:05Z

Thanks for this! :)

pganssle · 2022-06-19T20:56:53Z

I'm still pretty negative on this "feature".

There really was no missing functionality, and this will almost certainly make the ecosystem as a whole worse off, as it encourages an anti-pattern. 🙁 It is basically a Norman Door.

I don't think we'll get feedback from the people affected by this because they are exactly the people who think it's a great feature because they don't know that they are doing anything wrong.

henryiii · 2022-11-03T17:40:01Z

Just saw the first usage of this I've seen in the wild: https://github.com/Flexget/Flexget/blob/f55a297fae18e9779d9df9e545974727ad39b07a/requirements.txt :)

akx force-pushed the file-requirements branch from 2dbbabb to 5e79441 Compare April 6, 2022 13:12

abravalheri reviewed Apr 6, 2022

View reviewed changes

akx force-pushed the file-requirements branch from 5e79441 to c4d5727 Compare April 7, 2022 10:08

akx mentioned this pull request Apr 7, 2022

Allow file: for dependencies in TOML #3255

Merged

4 tasks

abravalheri reviewed Apr 8, 2022

View reviewed changes

akx force-pushed the file-requirements branch from c4d5727 to ea57d9b Compare June 8, 2022 05:20

Allow file: for requires statements in setup.cfg

40fbbf2

Refs pypa#1951

akx force-pushed the file-requirements branch from ea57d9b to 40fbbf2 Compare June 14, 2022 11:53

akx requested a review from abravalheri June 14, 2022 11:55

abravalheri approved these changes Jun 14, 2022

View reviewed changes

docs/userguide/declarative_config.rst Outdated Show resolved Hide resolved

Update docs/userguide/declarative_config.rst

916ed27

abravalheri force-pushed the file-requirements branch from 6e0b885 to 916ed27 Compare June 14, 2022 12:10

abravalheri merged commit 21d5b57 into pypa:main Jun 19, 2022

henryiii mentioned this pull request Nov 3, 2022

Wheel contains no dependency metadata when built via sdist pypa/build#530

Closed

antoninbas mentioned this pull request Jun 16, 2023

Fix setup.cfg format issue. Remove empty requirements.txt. p4lang/ptf#193

Closed

fperrin mentioned this pull request Jul 24, 2023

Fix binary type representation robshakir/pyangbind#297

Merged

peterkaminski mentioned this pull request Oct 4, 2023

Dynamically load requirements.txt in pyproject.toml jepler/chap#15

Merged

Uh oh!

Conversation

akx commented Apr 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary of changes

Pull Request Checklist

Uh oh!

abravalheri left a comment

Choose a reason for hiding this comment

Uh oh!

akx commented Apr 6, 2022

Uh oh!

abravalheri commented Apr 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jaraco commented Apr 7, 2022

Uh oh!

akx commented Apr 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

abravalheri commented Apr 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

akx commented Apr 7, 2022

Uh oh!

abravalheri Apr 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akx Apr 8, 2022

Choose a reason for hiding this comment

Uh oh!

abravalheri Apr 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

akx commented Jun 8, 2022

Uh oh!

abravalheri commented Jun 8, 2022

Uh oh!

akx commented Jun 14, 2022

Uh oh!

abravalheri commented Jun 14, 2022

Uh oh!

Uh oh!

abravalheri commented Jun 19, 2022

Uh oh!

nedbat commented Jun 19, 2022

Uh oh!

pganssle commented Jun 19, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

henryiii commented Nov 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

akx commented Apr 6, 2022 •

edited

Loading

abravalheri commented Apr 6, 2022 •

edited

Loading

akx commented Apr 7, 2022 •

edited

Loading

abravalheri commented Apr 7, 2022 •

edited

Loading

abravalheri Apr 8, 2022 •

edited

Loading

abravalheri Apr 8, 2022 •

edited

Loading

pganssle commented Jun 19, 2022 •

edited

Loading

henryiii commented Nov 3, 2022 •

edited

Loading