-
-
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
License_files improvements #2620
Conversation
4f3fc2e
to
7432981
Compare
docs/references/keywords.rst
Outdated
A list of glob patterns for license related files that should be included. | ||
If neither ``license_file`` nor ``license_files`` is specified, this option | ||
defaults to ``LICEN[CS]E*``, ``COPYING*``, ``NOTICE*``, and ``AUTHORS*``. | ||
Any ``exclude`` specified in ``MANIFEST.in`` will overwrite it. |
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.
In my experience, MANIFEST.in
only affects the sdist. Can you confirm that exclude
in MANIFEST.in with LICENSE
present has the same effect as not specifying a license on older Setuptools?
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.
I'm not exactly sure what you mean. AFAIK the change itself will only the sdist
command. bdist_wheel
will use the license_file
and license_files
fields or the default patterns. Therefore it's probably not wise to exclude a license file in the MANIFEST.in
to begin with. But in case you only care about the source dist, any exclude
does have president over a matched license file. There are two existing test cases that cover it.
I'm going to remove the last sentence, since it's probably more confusing than helpful especially to beginners.
setuptools/setuptools/tests/test_egg_info.py
Lines 637 to 655 in 7432981
({ | |
'setup.cfg': DALS(""" | |
[metadata] | |
license_files = LICENSE | |
"""), | |
'MANIFEST.in': "exclude LICENSE", | |
'LICENSE': "Test license" | |
}, [], ['LICENSE']), # license file is manually excluded | |
({ | |
'setup.cfg': DALS(""" | |
[metadata] | |
license_files = | |
LICENSE-ABC | |
LICENSE-XYZ | |
"""), | |
'MANIFEST.in': "exclude LICENSE-XYZ", | |
'LICENSE-ABC': "ABC license", | |
'LICENSE-XYZ': "XYZ license" | |
}, ['LICENSE-ABC'], ['LICENSE-XYZ']), # subset is manually excluded |
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.
Nice work. Thorough and clean. Just a couple of nitpicks or requests for additional information, but it's basically ready to go.
7432981
to
608c376
Compare
Thanks for the review! I did a rebase to get rid of the test fix and pushed a small commit to address the comments: 608c376 As a side note here, I was thinking about if it would be possible to do these steps even earlier. PEP 639, currently still a draft but quite far along, would add the |
I'm in favor of exposing the license text(s) in metadata. I don't have any active context on such an effort. I don't even know where license files are presented currently. I'd start out by filing a bug and describe the problem and start exploring possible solutions. I'd use that forum to examine the code and current behavior and devise a transition to a solution that meets the need. I'd identify projects and people that would be affected by such a change and gather their feedback. |
As of setuptools v56.0.0, the `license_file` entry is deprecated, raising `SetuptoolsDeprecationWarning` during the `packaging` step. Refs: pypa/setuptools#2620 Signed-off-by: Mike Fiedler <[email protected]>
As of setuptools v56.0.0, the `license_file` entry is deprecated, raising `SetuptoolsDeprecationWarning` during the `packaging` step. Refs: pypa/setuptools#2620 Signed-off-by: Mike Fiedler <[email protected]>
* chore: update codebase for modern python We're on Python 3.7 and higher, so we can clean up any artifacts left from previously-supported Python eras. Most of the heavy lifting was done with `pyupgrade` followed by unused improts removal (thanks to `flake8` check) Refs: https://pypi.org/project/pyupgrade/ Closes #203 Signed-off-by: Mike Fiedler <[email protected]> * refactor: replace `os.path`/`glob` with `pathlib` Since Python 3.4, `pathlib` provides a nicer interface to manipulate paths, removing the need to split and reassemble strings. The addition of `pytest.param(..., id=fn.name)` is to set the test name output to a human-friendly string in the event of failure or `--verbose` Signed-off-by: Mike Fiedler <[email protected]> * chore: replace deprecated setuptools entry As of setuptools v56.0.0, the `license_file` entry is deprecated, raising `SetuptoolsDeprecationWarning` during the `packaging` step. Refs: pypa/setuptools#2620 Signed-off-by: Mike Fiedler <[email protected]>
Fix the following warning: ``` /usr/lib/python3.10/site-packages/setuptools/config/setupcfg.py:459: SetuptoolsDeprecationWarning: The license_file parameter is deprecated, use license_files instead. warnings.warn(msg, warning_class) ``` Ref: pypa/setuptools#2620
In setuptools-56.0.0¹ license_file was changed to license_files. The former causes a deprecation warning. ¹ pypa/setuptools#2620 https://setuptools.pypa.io/en/latest/history.html?highlight=license_files#v56-0-0
In setuptools-56.0.0¹ license_file was changed to license_files. The former causes a deprecation warning. ¹ pypa/setuptools#2620 https://setuptools.pypa.io/en/latest/history.html?highlight=license_files#v56-0-0
Summary of changes
license_file
andlicense_files
options.bdist_wheel
.https://wheel.readthedocs.io/en/stable/user_guide.html#including-license-files-in-the-generated-wheel-file
license_file
file is used.license_files
is a full replacement.license_files
option.The implementation is similar to the one used by
wheel
: wheel/bdist_wheel.pyCloses #2616
Pull Request Checklist
changelog.d/
.(See documentation for details)