-
-
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: feature/pep639
Are you sure you want to change the base?
Changes from all commits
fc14fdf
7dfe073
f8a036f
7247772
c825f3e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Added initial support for license expression (`PEP 639 <https://peps.python.org/pep-0639/#add-license-expression-field>`_). -- by :user:`cdce8p` |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -88,6 +88,7 @@ def read_pkg_file(self, file): | |
self.url = _read_field_from_msg(msg, 'home-page') | ||
self.download_url = _read_field_from_msg(msg, 'download-url') | ||
self.license = _read_field_unescaped_from_msg(msg, 'license') | ||
self.license_expression = _read_field_unescaped_from_msg(msg, 'license_expression') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this use an |
||
|
||
self.long_description = _read_field_unescaped_from_msg(msg, 'description') | ||
if self.long_description is None and self.metadata_version >= Version('2.1'): | ||
|
@@ -175,7 +176,7 @@ def write_field(key, value): | |
if attr_val is not None: | ||
write_field(field, attr_val) | ||
|
||
license = self.get_license() | ||
license = self.license_expression or self.get_license() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So this mean that we need a follow-up PR for the |
||
if license: | ||
write_field('License', rfc822_escape(license)) | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -58,6 +58,7 @@ def apply(dist: Distribution, config: dict, filename: StrPath) -> Distribution: | |
os.chdir(root_dir) | ||
try: | ||
dist._finalize_requires() | ||
dist._finalize_license_expression() | ||
dist._finalize_license_files() | ||
finally: | ||
os.chdir(current_directory) | ||
|
@@ -181,16 +182,19 @@ def _long_description( | |
dist._referenced_files.add(file) | ||
|
||
|
||
def _license(dist: Distribution, val: dict, root_dir: StrPath | None): | ||
def _license(dist: Distribution, val: str | dict, root_dir: StrPath | None): | ||
from setuptools.config import expand | ||
|
||
if "file" in val: | ||
# XXX: Is it completely safe to assume static? | ||
value = expand.read_files([val["file"]], root_dir) | ||
_set_config(dist, "license", _static.Str(value)) | ||
dist._referenced_files.add(val["file"]) | ||
if isinstance(val, str): | ||
_set_config(dist, "license_expression", _static.Str(val)) | ||
else: | ||
_set_config(dist, "license", _static.Str(val["text"])) | ||
if "file" in val: | ||
# XXX: Is it completely safe to assume static? | ||
value = expand.read_files([val["file"]], root_dir) | ||
_set_config(dist, "license", _static.Str(value)) | ||
dist._referenced_files.add(val["file"]) | ||
else: | ||
_set_config(dist, "license", _static.Str(val["text"])) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would prefer to flatten this nested |
||
|
||
|
||
def _people(dist: Distribution, val: list[dict], _root_dir: StrPath | None, kind: str): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -24,6 +24,13 @@ class RedefiningStaticFieldAsDynamic(ValidationError): | |
) | ||
|
||
|
||
class IncludedDependencyGroupMustExist(ValidationError): | ||
_DESC = """An included dependency group must exist and must not be cyclic. | ||
""" | ||
__doc__ = _DESC | ||
_URL = "https://peps.python.org/pep-0735/" | ||
|
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am supposing the reason for these files to be changed is that this PR is cascaded on top of #4734. So ideally we should merge that other one first and possibly rebase. I have changed the PR to target the |
||
def validate_project_dynamic(pyproject: T) -> T: | ||
project_table = pyproject.get("project", {}) | ||
dynamic = project_table.get("dynamic", []) | ||
|
@@ -49,4 +56,27 @@ def validate_project_dynamic(pyproject: T) -> T: | |
return pyproject | ||
|
||
|
||
EXTRA_VALIDATIONS = (validate_project_dynamic,) | ||
def validate_include_depenency(pyproject: T) -> T: | ||
dependency_groups = pyproject.get("dependency-groups", {}) | ||
for key, value in dependency_groups.items(): | ||
for each in value: | ||
if ( | ||
isinstance(each, dict) | ||
and (include_group := each.get("include-group")) | ||
and include_group not in dependency_groups | ||
): | ||
raise IncludedDependencyGroupMustExist( | ||
message=f"The included dependency group {include_group} doesn't exist", | ||
value=each, | ||
name=f"data.dependency_groups.{key}", | ||
definition={ | ||
"description": cleandoc(IncludedDependencyGroupMustExist._DESC), | ||
"see": IncludedDependencyGroupMustExist._URL, | ||
}, | ||
rule="PEP 735", | ||
) | ||
# TODO: check for `include-group` cycles (can be conditional to graphlib) | ||
return pyproject | ||
|
||
|
||
EXTRA_VALIDATIONS = (validate_project_dynamic, validate_include_depenency) |
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,7 @@ | |
from typing import TYPE_CHECKING, Any, Union | ||
|
||
from more_itertools import partition, unique_everseen | ||
from packaging.licenses import canonicalize_license_expression | ||
from packaging.markers import InvalidMarker, Marker | ||
from packaging.specifiers import InvalidSpecifier, SpecifierSet | ||
from packaging.version import Version | ||
|
@@ -27,6 +28,7 @@ | |
from ._reqs import _StrOrIter | ||
from .config import pyprojecttoml, setupcfg | ||
from .discovery import ConfigDiscovery | ||
from .errors import InvalidConfigError | ||
from .monkey import get_unpatched | ||
from .warnings import InformationOnly, SetuptoolsDeprecationWarning | ||
|
||
|
@@ -288,6 +290,7 @@ class Distribution(_Distribution): | |
'long_description_content_type': lambda: None, | ||
'project_urls': dict, | ||
'provides_extras': dict, # behaves like an ordered set | ||
'license_expression': lambda: None, | ||
'license_file': lambda: None, | ||
'license_files': lambda: None, | ||
'install_requires': list, | ||
|
@@ -402,6 +405,23 @@ def _normalize_requires(self): | |
(k, list(map(str, _reqs.parse(v or [])))) for k, v in extras_require.items() | ||
) | ||
|
||
def _finalize_license_expression(self) -> None: | ||
"""Normalize license and license_expression.""" | ||
license_expr = self.metadata.license_expression | ||
if license_expr: | ||
normalized = canonicalize_license_expression(license_expr) | ||
if license_expr != normalized: | ||
InformationOnly.emit(f"Normalizing '{license_expr}' to '{normalized}'") | ||
self.metadata.license_expression = normalized | ||
|
||
for cl in self.metadata.get_classifiers(): | ||
if not cl.startswith("License :: "): | ||
continue | ||
raise InvalidConfigError( | ||
"License classifier are deprecated in favor of the license expression. " | ||
f"Remove the '{cl}' classifier." | ||
) | ||
Comment on lines
+420
to
+423
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because it is a deprecation, should we give a |
||
|
||
def _finalize_license_files(self) -> None: | ||
"""Compute names of all license files which should be included.""" | ||
license_files: list[str] | None = self.metadata.license_files | ||
|
@@ -652,6 +672,7 @@ def parse_config_files( | |
pyprojecttoml.apply_configuration(self, filename, ignore_option_errors) | ||
|
||
self._finalize_requires() | ||
self._finalize_license_expression() | ||
self._finalize_license_files() | ||
|
||
def fetch_build_eggs( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,7 +23,7 @@ | |
from setuptools.config import expand, pyprojecttoml, setupcfg | ||
from setuptools.config._apply_pyprojecttoml import _MissingDynamic, _some_attrgetter | ||
from setuptools.dist import Distribution | ||
from setuptools.errors import RemovedConfigError | ||
from setuptools.errors import InvalidConfigError, RemovedConfigError | ||
|
||
from .downloads import retrieve_file, urls_from_file | ||
|
||
|
@@ -156,6 +156,31 @@ def main_gui(): pass | |
def main_tomatoes(): pass | ||
""" | ||
|
||
PEP639_LICENSE_TEXT = """\ | ||
[project] | ||
name = "spam" | ||
version = "2020.0.0" | ||
authors = [ | ||
{email = "[email protected]"}, | ||
{name = "Tzu-Ping Chung"} | ||
] | ||
license = {text = "MIT"} | ||
""" | ||
|
||
PEP639_LICENSE_EXPRESSION = """\ | ||
[project] | ||
name = "spam" | ||
version = "2020.0.0" | ||
authors = [ | ||
{email = "[email protected]"}, | ||
{name = "Tzu-Ping Chung"} | ||
] | ||
license = "mit or apache-2.0" # should be normalized in metadata | ||
classifiers = [ | ||
"Development Status :: 5 - Production/Stable", | ||
] | ||
""" | ||
|
||
|
||
def _pep621_example_project( | ||
tmp_path, | ||
|
@@ -251,10 +276,60 @@ def test_utf8_maintainer_in_metadata( # issue-3663 | |
assert f"Maintainer-email: {expected_maintainers_meta_value}" in content | ||
|
||
|
||
class TestLicenseFiles: | ||
# TODO: After PEP 639 is accepted, we have to move the license-files | ||
# to the `project` table instead of `tool.setuptools` | ||
@pytest.mark.parametrize( | ||
('pyproject_text', 'license', 'license_expression', 'content_str'), | ||
( | ||
pytest.param( | ||
PEP639_LICENSE_TEXT, | ||
'MIT', | ||
None, | ||
'License: MIT', | ||
id='license-text', | ||
), | ||
pytest.param( | ||
PEP639_LICENSE_EXPRESSION, | ||
None, | ||
'MIT OR Apache-2.0', | ||
'License: MIT OR Apache-2.0', # TODO Metadata version '2.4' | ||
id='license-expression', | ||
), | ||
), | ||
) | ||
def test_license_in_metadata( | ||
license, | ||
license_expression, | ||
content_str, | ||
pyproject_text, | ||
tmp_path, | ||
): | ||
pyproject = _pep621_example_project( | ||
tmp_path, | ||
"README", | ||
pyproject_text=pyproject_text, | ||
) | ||
dist = pyprojecttoml.apply_configuration(makedist(tmp_path), pyproject) | ||
assert dist.metadata.license == license | ||
assert dist.metadata.license_expression == license_expression | ||
pkg_file = tmp_path / "PKG-FILE" | ||
with open(pkg_file, "w", encoding="utf-8") as fh: | ||
dist.metadata.write_pkg_file(fh) | ||
content = pkg_file.read_text(encoding="utf-8") | ||
assert content_str in content | ||
|
||
|
||
def test_license_expression_with_bad_classifier(tmp_path): | ||
text = PEP639_LICENSE_EXPRESSION.rsplit("\n", 2)[0] | ||
pyproject = _pep621_example_project( | ||
tmp_path, | ||
"README", | ||
f"{text}\n \"License :: OSI Approved :: MIT License\"\n]", | ||
) | ||
msg = "License classifier are deprecated.*'License :: OSI Approved :: MIT License'" | ||
with pytest.raises(InvalidConfigError, match=msg): | ||
pyprojecttoml.apply_configuration(makedist(tmp_path), pyproject) | ||
|
||
|
||
class TestLicenseFiles: | ||
def base_pyproject(self, tmp_path, additional_text): | ||
pyproject = _pep621_example_project(tmp_path, "README") | ||
text = pyproject.read_text(encoding="utf-8") | ||
|
@@ -267,6 +342,24 @@ def base_pyproject(self, tmp_path, additional_text): | |
pyproject.write_text(text, encoding="utf-8") | ||
return pyproject | ||
|
||
def base_pyproject_license_pep639(self, tmp_path): | ||
pyproject = _pep621_example_project(tmp_path, "README") | ||
text = pyproject.read_text(encoding="utf-8") | ||
|
||
# Sanity-check | ||
assert 'license = {file = "LICENSE.txt"}' in text | ||
assert 'license-files' not in text | ||
assert "[tool.setuptools]" not in text | ||
|
||
text = re.sub( | ||
r"(license = {file = \"LICENSE.txt\"})\n", | ||
("license = \"licenseref-Proprietary\"\nlicense-files = [\"_FILE*\"]\n"), | ||
text, | ||
count=1, | ||
) | ||
pyproject.write_text(text, encoding="utf-8") | ||
return pyproject | ||
|
||
def test_both_license_and_license_files_defined(self, tmp_path): | ||
setuptools_config = '[tool.setuptools]\nlicense-files = ["_FILE*"]' | ||
pyproject = self.base_pyproject(tmp_path, setuptools_config) | ||
|
@@ -283,6 +376,18 @@ def test_both_license_and_license_files_defined(self, tmp_path): | |
assert set(dist.metadata.license_files) == {"_FILE.rst", "_FILE.txt"} | ||
assert dist.metadata.license == "LicenseRef-Proprietary\n" | ||
|
||
def test_both_license_and_license_files_defined_pep639(self, tmp_path): | ||
# Set license and license-files | ||
pyproject = self.base_pyproject_license_pep639(tmp_path) | ||
|
||
(tmp_path / "_FILE.txt").touch() | ||
(tmp_path / "_FILE.rst").touch() | ||
|
||
dist = pyprojecttoml.apply_configuration(makedist(tmp_path), pyproject) | ||
assert set(dist.metadata.license_files) == {"_FILE.rst", "_FILE.txt"} | ||
assert dist.metadata.license is None | ||
assert dist.metadata.license_expression == "LicenseRef-Proprietary" | ||
|
||
def test_default_patterns(self, tmp_path): | ||
setuptools_config = '[tool.setuptools]\nzip-safe = false' | ||
# ^ used just to trigger section validation | ||
|
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.
This is a bit complicated.
Setuptools has many vendored dependencies (which may vary with time), so the correct license expression is not that simple. Additionally, I don't know if it is going to be trivial to derive a correct expression automatically from the code base at each release (e.g. what happens if a maintainer change a vendored dependency? how can we guarantee the license expression is up-to-date?)...
Would it make more sense to simply omit the license expression and rely on
license-files
or an automatic scan of the relevant directories?