Deprecate tools.setuptools.license-files#4837
Deprecate tools.setuptools.license-files#4837abravalheri merged 3 commits intopypa:feature/pep639from
Conversation
cdce8p
force-pushed
the
639-deprecated-setuptools.license-files
branch
from
6dee35f to
ab277d3
Compare
abravalheri
left a comment
abravalheri
left a comment
There was a problem hiding this comment.
Thank you very much @cdce8p!
docs/userguide/pyproject_config.rst
Outdated
| ``exclude-package-data`` table/inline-table Empty by default. See :doc:`/userguide/datafiles`. | ||
| ------------------------- --------------------------- ------------------------- | ||
| ``license-files`` array of glob patterns **Provisional** - likely to change with :pep:`639` | ||
| ``license-files`` array of glob patterns **Deprecated** - use ``project.license-files`` instead. See :doc:`PyPUG:guides/writing-pyproject-toml/#license-files` |
There was a problem hiding this comment.
I notice that sphinx is issuing the following warning:
/home/runner/work/setuptools/setuptools/docs/userguide/pyproject_config.rst:103: WARNING: unknown document: 'PyPUG:guides/writing-pyproject-toml/#license-files' [ref.doc]
There was a problem hiding this comment.
Seems the anchor link doesn't exist. Opened pypa/packaging.python.org#1816 upstream and used the generic link here so it doesn't block the PR.
|
|
||
| if "license-files" in tool_table: | ||
| if dist.metadata.license_files: | ||
| raise InvalidConfigError( |
There was a problem hiding this comment.
Why are you throwing an exception when the intention is to deprecate something? Emitting a DeprecationWarning should be surely sufficient? This exception breaks tons of workloads on our side.
There was a problem hiding this comment.
This configuration has been documented as provisional since its introduction, so it was never stable to require a deprecation period.
The breaking change is documented in the changelogs for the major version bump to inform users adaptation may be required.
There was a problem hiding this comment.
That said, I had a look on how this exception is being triggered in the wild and I can see pylint as a typical example: pylint-dev/pylint#10289
They did not had project.license-files defined but ratter in setup.cfg.
@cdce8p, was the intention just to double definition inside pyproject.toml or more general to avoid license-files being defined in multiple places?
If it was the first, should we change the condition it to:
if `license-files` in config.get("project", {}):(Untested)
There was a problem hiding this comment.
That said, I had a look on how this exception is being triggered in the wild and I can see
pylintas a typical example: pylint-dev/pylint#10289
Seems for pylint in particular, I was actually the one who introduce the issue almost three years ago 😅
pylint-dev/pylint#7076
The config as I had written it was problematic though. metadata.license-files (from setup.cfg) would always be overwritten by tool.setuptools.license-files. So in a sense, raising an error here was kind of a good thing as it's fixed now.
They did not had
project.license-filesdefined but ratter insetup.cfg.@cdce8p, was the intention just to double definition inside
pyproject.tomlor more general to avoid license-files being defined in multiple places?
Left a comment on the PR already (see #4899 (review)) but in a sense the intention was to prohibit using project.license-files and tool.setuptools.license-files together.
3 participants
Deprecate
tools.setuptools.license-filesin favor ofproject.license-files.Ref #4829
/CC @abravalheri