-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add initial support for License-Expression (PEP 639) #4706
base: main
Are you sure you want to change the base?
Conversation
4c8ccfb
to
1237a7d
Compare
1237a7d
to
93d0c67
Compare
Hi @cdce8p thank you very much for having a look on this. We probably have to organise so that PEP 639 lands after PEP 685 implementation (ideally we want a couple of days/weeks between the two so that users have time to report potential bugs). |
setuptools/_distutils/dist.py
Outdated
return self.license_expression | ||
|
||
get_license_expression = get_license_expression | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be possible to implement this feature without changing the _distutils
folder?
In general, we don't modify the contents to that folder directly. Instead, the code is maintained in the pypa/distutils
repository and merged back in setuptools periodically, (see also our development guidelines). So changes in the folder have to be submitted first in pypa/distutils
and later merged here.
Additionally, there are some people that still use the deprecated SETUPTOOLS_USE_DISTUTILS=stdlib
in Python3.9 ... 3.11, so we need to ensure it is also backwards compatible with the stdlib version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be possible to implement this feature without changing the
_distutils
folder?
Yes. Removed those changes as they weren't necessary to begin with. I added them initially but the better solution was to add license_expression
to _DISTUTILS_UNSUPPORTED_METADATA
in setuptools/dist.py
. This is also fully compatible with SETUPTOOLS_USE_DISTUTILS=stdlib
. 390b95f
93d0c67
to
390b95f
Compare
Maybe I'm missing something obvious but I'm not quite sure how PEP 685 is related. Would you mind explaining it a bit more? The way I've designed the PR, it shouldn't depend on anything. It just allows users to specify |
Opened #4734 with just the |
The problem is that the metadata version of the core metadata is incremental:
So we cannot implement PEP 639 before 685, otherwise metadata validation may fail. Even if the PR is independent we cannot merge it now because of the nature of the releases of setuptools (we cannot risk PEP 639 support being released before PEP 685 support). |
The validation will only happen once we update the metadata version. If not, a lot of wheel uploads would probably fail as we currently include What I'm proposing here is simply to add the basic support to setuptools. It won't get validated by PyPI until we're ready to move to |
For sure the existence of the By accepting only some aspects of PEP 639, we add other kinds of spec violations that some tools may not be prepared to deal with. So for the sake of reducing the unknown risks, I propose we pipeline the changes you kindly contributed after the implementation of 685. Unfortunately, it means that people cannot start modifying their |
Sorry, I have been using the wrong PEP number, sorry for the confusion. PEP 685 is important, but the really big one for us to implement before 639 is PEP 643 (the one with |
I understand your reasoning. One last suggestion. The last days I've been looking into how My suggestion
What do you think? |
Summary of changes
validate_pyproject
to0.22
https://github.com/abravalheri/validate-pyproject/blob/v0.22/CHANGELOG.rst
License-Expression
core metadataMissing: The license expression isn't yet validated. The next version of
validate_pyproject
will likely add that once a new version of packaging has been released. From the PEP:It should be fine to omit that part for the moment.
Refs: #4629