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

Add support for `license_files` option in metadata #1767

Merged
merged 4 commits into from Nov 16, 2019
Merged

Conversation

@kchmck
Copy link
Contributor

kchmck commented May 20, 2019

Summary of changes

Support for license_file was added to bdist_wheel in pypa/wheel#47 / pypa/wheel@0acfb4c and then to setuptools in #1536. Last year, wheel also added support for a license_files option in pypa/wheel#138 / pypa/wheel@59976ab to enable including more than one license. At the same time, license_file was deprecated in that project, and using it leads to a DeprecationWarning if warnings are enabled. The docs for wheel also mention that license_files is the favored option.

Currently, setuptools only supports license_file, which leads to the DeprecationWarning code path if wheel is also used when building distributions.

This PR adds the license_files functionality to setuptools, which enables the more versatile form to work across both tools and allows users to take advantage of the functionality without hitting deprecated code.

A few notes about the implementation:

  • If license_file is specified, the file is also added to the set (same behavior as wheel), which preserves backwards compatibility. I wasn't sure if any sort of deprecation was appropriate here, so I left that part out

  • I added license_files to Distribution and ConfigMetadataHandler because it seemed to be the best way to reuse the list option parser, but please advise if it should be done a different way

  • Leaving the option empty in wheel disables license auto-including, which AFAICT matches the default behavior of setuptools if either option is empty (and MANIFEST.in doesn't include them)

Pull Request Checklist

  • Changes have tests
  • News fragment added in changelog.d. See documentation for details
@jakirkham

This comment has been minimized.

Copy link
Contributor

jakirkham commented Jul 2, 2019

Would it be possible to get a review?

@techdragon

This comment has been minimized.

Copy link

techdragon commented Sep 9, 2019

@jakirkham I cant put proper review comments as I'm not on the setuptools team, but it looks like @kchmck has a pretty good PR here. 👍

Edit: I'd like to second the call for a review, since I'm eager to use this in packages/places that are reliant on setuptools' behaviour. It would be especially useful to have this reviewed in light of current discussions about package license metadata, https://discuss.python.org/t/improving-license-clarity-with-better-package-metadata/2154

techdragon pushed a commit to techdragon/poetry that referenced this pull request Sep 9, 2019
- Added support for specifying a set of license files as part of the project config.
- Moved the license file config into the package configuration. (`Poetry.create()`) this matches how similar configurations such as “readme” are configured.
- Updated the builders to use the project level license file list instead of searching for their own (previously different) lists of license files.
- Moved the existing auto-discovery behaviours to become the default case during `create()` if nothing specific was configured.
- Fix sdispater#866 by modifing the base template used by `SdistBuilder`. It now uses `setuptools`, since as documented by @seifertm in issue sdispater#866 `pip` uses `setuptools` anyway and this clears the way to immediately supporting pypa/setuptools#1767 once this is merged and released from `setuptools`.
techdragon pushed a commit to techdragon/poetry that referenced this pull request Sep 10, 2019
- Added support for specifying a set of license files as part of the project config.
- Moved the license file config into the package configuration. (`Poetry.create()`) this matches how similar configurations such as “readme” are configured.
- Updated the builders to use the project level license file list instead of searching for their own (previously different) lists of license files.
- Moved the existing auto-discovery behaviours to become the default case during `create()` if nothing specific was configured.
- Fix sdispater#866 by modifing the base template used by `SdistBuilder`. It now uses `setuptools`, since as documented by @seifertm in issue sdispater#866 `pip` uses `setuptools` anyway and this clears the way to immediately supporting pypa/setuptools#1767 once this is merged and released from `setuptools`. Until `setuptools` updates to support multiple license files, this will result in a single log line in the debug output from `setuptools` so this is ok to merge before the change to `setuptools` is merged.
techdragon pushed a commit to techdragon/poetry that referenced this pull request Sep 10, 2019
- Added support for specifying a set of license files as part of the project config.
- Moved the license file config into the package configuration. (`Poetry.create()`) this matches how similar configurations such as “readme” are configured.
- Updated the builders to use the project level license file list instead of searching for their own (previously different) lists of license files.
- Moved the existing auto-discovery behaviours to become the default case during `create()` if nothing specific was configured.
- Added tests for the new features
- Added the “license-files” field to the poetry-schema.json
- Fix sdispater#866 by modifing the base template used by `SdistBuilder`. It now uses `setuptools`, since as documented by @seifertm in issue sdispater#866 `pip` uses `setuptools` anyway and this clears the way to immediately supporting pypa/setuptools#1767 once this is merged and released from `setuptools`. Until `setuptools` updates to support multiple license files, this will result in a single log line in the debug output from `setuptools` so this is ok to merge before the change to `setuptools` is merged.
techdragon pushed a commit to techdragon/poetry that referenced this pull request Sep 10, 2019
- Added support for specifying a set of license files as part of the project config.
- Moved the license file config into the package configuration. (`Poetry.create()`) this matches how similar configurations such as “readme” are configured.
- Updated the builders to use the project level license file list instead of searching for their own (previously different) lists of license files.
- Moved the existing auto-discovery behaviours to become the default case during `create()` if nothing specific was configured.
- Added tests for the new features
- Documented the new configuration field.
- Added the “license-files” field to the poetry-schema.json
- Fix sdispater#866 by modifing the base template used by `SdistBuilder`. It now uses `setuptools`, since as documented by @seifertm in issue sdispater#866 `pip` uses `setuptools` anyway and this clears the way to immediately supporting pypa/setuptools#1767 once this is merged and released from `setuptools`. Until `setuptools` updates to support multiple license files, this will result in a single log line in the debug output from `setuptools` so this is ok to merge before the change to `setuptools` is merged.
techdragon pushed a commit to techdragon/poetry that referenced this pull request Sep 10, 2019
- Added support for specifying a set of license files as part of the project config.
- Moved the license file config into the package configuration. (`Poetry.create()`) this matches how similar configurations such as “readme” are configured.
- Updated the builders to use the project level license file list instead of searching for their own (previously different) lists of license files.
- Moved the existing auto-discovery behaviours to become the default case during `create()` if nothing specific was configured.
- Added tests for the new features
- Documented the new configuration field.
- Added the “license-files” field to the poetry-schema.json
- Fix sdispater#866 by modifing the base template used by `SdistBuilder`. It now uses `setuptools`, since as documented by @seifertm in issue sdispater#866 `pip` uses `setuptools` anyway and this clears the way to immediately supporting pypa/setuptools#1767 once this is merged and released from `setuptools`. Until `setuptools` updates to support multiple license files, this will result in a single log line in the debug output from `setuptools` so this is ok to merge before the change to `setuptools` is merged.
techdragon pushed a commit to techdragon/poetry that referenced this pull request Sep 12, 2019
- Added support for specifying a set of license files as part of the project config.
- Moved the license file config into the package configuration. (`Poetry.create()`) this matches how similar configurations such as “readme” are configured.
- Updated the builders to use the project level license file list instead of searching for their own (previously different) lists of license files.
- Moved the existing auto-discovery behaviours to become the default case during `create()` if nothing specific was configured.
- Added tests for the new features
- Documented the new configuration field.
- Added the “license-files” field to the poetry-schema.json
- Fix sdispater#866 by modifing the base template used by `SdistBuilder`. It now uses `setuptools`, since as documented by @seifertm in issue sdispater#866 `pip` uses `setuptools` anyway and this clears the way to immediately supporting pypa/setuptools#1767 once this is merged and released from `setuptools`. Until `setuptools` updates to support multiple license files, this will result in a single log line in the debug output from `setuptools` so this is ok to merge before the change to `setuptools` is merged.
techdragon pushed a commit to techdragon/poetry that referenced this pull request Sep 12, 2019
- Added support for specifying a set of license files as part of the project config.
- Moved the license file config into the package configuration. (`Poetry.create()`) this matches how similar configurations such as “readme” are configured.
- Updated the builders to use the project level license file list instead of searching for their own (previously different) lists of license files.
- Moved the existing auto-discovery behaviours to become the default case during `create()` if nothing specific was configured.
- Added tests for the new features
- Documented the new configuration field.
- Added the “license-files” field to the poetry-schema.json
- Updated pre-commit config to use the correct black source repo.
- Fix sdispater#866 by modifing the base template used by `SdistBuilder`. It now uses `setuptools`, since as documented by @seifertm in issue sdispater#866 `pip` uses `setuptools` anyway and this clears the way to immediately supporting pypa/setuptools#1767 once this is merged and released from `setuptools`. Until `setuptools` updates to support multiple license files, this will result in a single log line in the debug output from `setuptools` so this is ok to merge before the change to `setuptools` is merged.
techdragon pushed a commit to techdragon/poetry that referenced this pull request Sep 12, 2019
- Fixes sdispater#1350
- Added support for specifying a set of license files as part of the project config.
- Moved the license file config into the package configuration. (`Poetry.create()`) this matches how similar configurations such as “readme” are configured.
- Updated the builders to use the project level license file list instead of searching for their own (previously different) lists of license files.
- Moved the existing auto-discovery behaviours to become the default case during `create()` if nothing specific was configured.
- Added tests for the new features
- Documented the new configuration field.
- Added the “license-files” field to the poetry-schema.json
- Updated pre-commit config to use the correct black source repo.
- Fix sdispater#866 by modifing the base template used by `SdistBuilder`. It now uses `setuptools`, since as documented by @seifertm in issue sdispater#866 `pip` uses `setuptools` anyway and this clears the way to immediately supporting pypa/setuptools#1767 once this is merged and released from `setuptools`. Until `setuptools` updates to support multiple license files, this will result in a single log line in the debug output from `setuptools` so this is ok to merge before the change to `setuptools` is merged.
techdragon pushed a commit to techdragon/poetry that referenced this pull request Sep 12, 2019
- Fixes sdispater#1350
- Added support for specifying a set of license files as part of the project config.
- Moved the license file config into the package configuration. (`Poetry.create()`) this matches how similar configurations such as “readme” are configured.
- Updated the builders to use the project level license file list instead of searching for their own (previously different) lists of license files.
- Moved the existing auto-discovery behaviours to become the default case during `create()` if nothing specific was configured.
- Added tests for the new features
- Documented the new configuration field.
- Added the “license-files” field to the poetry-schema.json
- Updated pre-commit config to use the correct black source repo.
- Fix sdispater#866 by modifing the base template used by `SdistBuilder`. It now uses `setuptools`, since as documented by @seifertm in issue sdispater#866 `pip` uses `setuptools` anyway and this clears the way to immediately supporting pypa/setuptools#1767 once this is merged and released from `setuptools`. Until `setuptools` updates to support multiple license files, this will result in a single log line in the debug output from `setuptools` so this is ok to merge before the change to `setuptools` is merged.
techdragon pushed a commit to techdragon/poetry that referenced this pull request Sep 12, 2019
- Fixes sdispater#1350
- Added support for specifying a set of license files as part of the project config.
- Moved the license file config into the package configuration. (`Poetry.create()`) this matches how similar configurations such as “readme” are configured.
- Updated the builders to use the project level license file list instead of searching for their own (previously different) lists of license files.
- Moved the existing auto-discovery behaviours to become the default case during `create()` if nothing specific was configured.
- Added tests for the new features
- Documented the new configuration field.
- Added the “license-files” field to the poetry-schema.json
- Updated pre-commit config to use the correct black source repo.
- Fix sdispater#866 by modifing the base template used by `SdistBuilder`. It now uses `setuptools`, since as documented by @seifertm in issue sdispater#866 `pip` uses `setuptools` anyway and this clears the way to immediately supporting pypa/setuptools#1767 once this is merged and released from `setuptools`. Until `setuptools` updates to support multiple license files, this will result in a single log line in the debug output from `setuptools` so this is ok to merge before the change to `setuptools` is merged.
techdragon pushed a commit to techdragon/poetry that referenced this pull request Sep 12, 2019
- Fixes sdispater#1350
- Added support for specifying a set of license files as part of the project config.
- Moved the license file config into the package configuration. (`Poetry.create()`) this matches how similar configurations such as “readme” are configured.
- Updated the builders to use the project level license file list instead of searching for their own (previously different) lists of license files.
- Moved the existing auto-discovery behaviours to become the default case during `create()` if nothing specific was configured.
- Added tests for the new features
- Documented the new configuration field.
- Added the “license-files” field to the poetry-schema.json
- Updated pre-commit config to use the correct black source repo.
- Fix sdispater#866 by modifing the base template used by `SdistBuilder`. It now uses `setuptools`, since as documented by @seifertm in issue sdispater#866 `pip` uses `setuptools` anyway and this clears the way to immediately supporting pypa/setuptools#1767 once this is merged and released from `setuptools`. Until `setuptools` updates to support multiple license files, this will result in a single log line in the debug output from `setuptools` so this is ok to merge before the change to `setuptools` is merged.
techdragon pushed a commit to techdragon/poetry that referenced this pull request Sep 16, 2019
- Fixes sdispater#1350
- Added support for specifying a set of license files as part of the project config.
- Moved the license file config into the package configuration. (`Poetry.create()`) this matches how similar configurations such as “readme” are configured.
- Updated the builders to use the project level license file list instead of searching for their own (previously different) lists of license files.
- Moved the existing auto-discovery behaviours to become the default case during `create()` if nothing specific was configured.
- Added tests for the new features
- Documented the new configuration field.
- Added the “license-files” field to the poetry-schema.json
- Updated pre-commit config to use the correct black source repo.
- Fix sdispater#866 by modifing the base template used by `SdistBuilder`. It now uses `setuptools`, since as documented by @seifertm in issue sdispater#866 `pip` uses `setuptools` anyway and this clears the way to immediately supporting pypa/setuptools#1767 once this is merged and released from `setuptools`. Until `setuptools` updates to support multiple license files, this will result in a single log line in the debug output from `setuptools` so this is ok to merge before the change to `setuptools` is merged.
techdragon pushed a commit to techdragon/poetry that referenced this pull request Sep 17, 2019
- Fixes sdispater#1350
- Added support for specifying a set of license files as part of the project config.
- Moved the license file config into the package configuration. (`Poetry.create()`) this matches how similar configurations such as “readme” are configured.
- Updated the builders to use the project level license file list instead of searching for their own (previously different) lists of license files.
- Moved the existing auto-discovery behaviours to become the default case during `create()` if nothing specific was configured.
- Added tests for the new features
- Documented the new configuration field.
- Added the “license-files” field to the poetry-schema.json
- Updated pre-commit config to use the correct black source repo.
- Fix sdispater#866 by modifing the base template used by `SdistBuilder`. It now uses `setuptools`, since as documented by @seifertm in issue sdispater#866 `pip` uses `setuptools` anyway and this clears the way to immediately supporting pypa/setuptools#1767 once this is merged and released from `setuptools`. Until `setuptools` updates to support multiple license files, this will result in a single log line in the debug output from `setuptools` so this is ok to merge before the change to `setuptools` is merged.
techdragon pushed a commit to techdragon/poetry that referenced this pull request Sep 17, 2019
- Fixes sdispater#1350
- Added support for specifying a set of license files as part of the project config.
- Moved the license file config into the package configuration. (`Factory.create_poetry()`) this matches how similar configurations such as “readme” are configured.
- Updated the builders to use the project level license file list instead of searching for their own (previously different) lists of license files.
- Moved the existing auto-discovery behaviours to become the default case during `create_poetry()` if nothing specific was configured.
- Added tests for the new features
- Documented the new configuration field.
- Added the “license-files” field to the poetry-schema.json
- Updated pre-commit config to use the correct black source repo, and set the default language for better consistency with any other pre-commit checks that will be added in the future.
- Fix sdispater#866 by modifing the base template used by `SdistBuilder`. It now uses `setuptools`, since as documented by @seifertm in issue sdispater#866 `pip` uses `setuptools` anyway and this clears the way to immediately supporting pypa/setuptools#1767 once this is merged and released from `setuptools`. Until `setuptools` updates to support multiple license files, this will result in a single log line in the debug output from `setuptools` so this is ok to merge before the change to `setuptools` is merged.
techdragon pushed a commit to techdragon/poetry that referenced this pull request Sep 26, 2019
- Fixes sdispater#1350
- Added support for specifying a set of license files as part of the project config.
- Moved the license file config into the package configuration. (`Factory.create_poetry()`) this matches how similar configurations such as “readme” are configured.
- Updated the builders to use the project level license file list instead of searching for their own (previously different) lists of license files.
- Moved the existing auto-discovery behaviours to become the default case during `create_poetry()` if nothing specific was configured.
- Added tests for the new features
- Documented the new configuration field.
- Added the “license-files” field to the poetry-schema.json
- Updated pre-commit config to use the correct black source repo, and set the default language for better consistency with any other pre-commit checks that will be added in the future.
- Fix sdispater#866 by modifing the base template used by `SdistBuilder`. It now uses `setuptools`, since as documented by @seifertm in issue sdispater#866 `pip` uses `setuptools` anyway and this clears the way to immediately supporting pypa/setuptools#1767 once this is merged and released from `setuptools`. Until `setuptools` updates to support multiple license files, this will result in a single log line in the debug output from `setuptools` so this is ok to merge before the change to `setuptools` is merged.
@jaraco

This comment has been minimized.

Copy link
Member

jaraco commented Oct 27, 2019

An important reference to this effort is this thread, where many in the PyPA settle on supporting license files.

@jaraco
jaraco approved these changes Oct 27, 2019
Copy link
Member

jaraco left a comment

This generally looks good. Just a few small issues and this can be accepted.

setuptools/command/sdist.py Outdated Show resolved Hide resolved
setuptools/command/sdist.py Outdated Show resolved Hide resolved
setuptools/tests/test_egg_info.py Outdated Show resolved Hide resolved
setuptools/dist.py Outdated Show resolved Hide resolved
@jakirkham

This comment has been minimized.

Copy link
Contributor

jakirkham commented Nov 1, 2019

Looks like this is ready for another review! 😄

@jaraco jaraco merged commit 68dbb70 into pypa:master Nov 16, 2019
6 checks passed
6 checks passed
Summary 1 potential rule
Details
codecov/patch 94.44% of diff hit (target 84.69%)
Details
codecov/project 84.71% (+0.01%) compared to 7748921
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details
@jakirkham

This comment has been minimized.

Copy link
Contributor

jakirkham commented Nov 17, 2019

Thanks all for getting this in! This is a really great feature 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.