Deprecate tools.setuptools.license-files #4837
Conversation
cdce8p
force-pushed
the
639-deprecated-setuptools.license-files
branch
from
6dee35f to
ab277d3
Compare
docs/userguide/pyproject_config.rst
Outdated
| @@ -99,7 +99,7 @@ Key Value Type (TOML) Notes | |||
| See :doc:`/userguide/datafiles`. | |||
| ``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.
| @@ -89,6 +89,21 @@ def _apply_tool_table(dist: Distribution, config: dict, filename: StrPath): | |||
| if not tool_table: | |||
| return # short-circuit | |||
|
|
|||
| 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.
Deprecate
tools.setuptools.license-filesin favor ofproject.license-files.Ref #4829
/CC @abravalheri