Skip to content

Commit 1df8bd3

Browse files
cdce8pcdce8p
committed
Code review
1 parent f28fd3e commit 1df8bd3

File tree

4 files changed

+55
-51
lines changed

4 files changed

+55
-51
lines changed

‎flit_core/flit_core/config.py‎

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ class ConfigError(ValueError):
7575
}
7676

7777
default_license_files_globs = ['COPYING*', 'LICEN[CS]E*']
78+
license_files_allowed_chars = re.compile(r'^[\w\-\.\/\*\?\[\]]+$')
7879

7980

8081
def read_flit_config(path):
@@ -450,9 +451,9 @@ def _expand_requires_extra(re):
450451

451452

452453
def _license_files_from_globs(project_dir: Path, globs, use_defaults = False):
453-
license_files = []
454+
license_files = set()
454455
for pattern in globs:
455-
if pattern.startswith("/"):
456+
if isabs_ish(pattern):
456457
raise ConfigError(
457458
"Invalid glob pattern for [project.license-files]: '{}'. "
458459
"Pattern must not start with '/'.".format(pattern)
@@ -462,6 +463,12 @@ def _license_files_from_globs(project_dir: Path, globs, use_defaults = False):
462463
"Invalid glob pattern for [project.license-files]: '{}'. "
463464
"Pattern must not contain '..'".format(pattern)
464465
)
466+
if license_files_allowed_chars.match(pattern) is None:
467+
raise ConfigError(
468+
"Invalid glob pattern for [project.license-files]: '{}'. "
469+
"Pattern contains invalid characters. "
470+
"https://packaging.python.org/en/latest/specifications/pyproject-toml/#license-files"
471+
)
465472
try:
466473
files = [
467474
str(file.relative_to(project_dir)).replace(osp.sep, "/")
@@ -477,8 +484,8 @@ def _license_files_from_globs(project_dir: Path, globs, use_defaults = False):
477484
raise ConfigError(
478485
"No files found for [project.license-files]: '{}' pattern".format(pattern)
479486
)
480-
license_files.extend(files)
481-
return sorted(license_files)
487+
license_files.update(files)
488+
return license_files
482489

483490
def _check_type(d, field_name, cls):
484491
if not isinstance(d[field_name], cls):
@@ -566,6 +573,7 @@ def read_pep621_metadata(proj, path) -> LoadedConfig:
566573
if 'requires-python' in proj:
567574
md_dict['requires_python'] = proj['requires-python']
568575

576+
license_files = set()
569577
if 'license' in proj:
570578
_check_type(proj, 'license', dict)
571579
license_tbl = proj['license']
@@ -584,25 +592,32 @@ def read_pep621_metadata(proj, path) -> LoadedConfig:
584592
raise ConfigError(
585593
"[project.license] should specify file or text, not both"
586594
)
587-
lc.referenced_files.append(license_tbl['file'])
595+
license_files.add(license_tbl['file'])
588596
elif 'text' in license_tbl:
589597
pass
590598
else:
591599
raise ConfigError(
592600
"file or text field required in [project.license] table"
593601
)
594602

595-
license_files = []
596603
if 'license-files' in proj:
597604
_check_type(proj, 'license-files', list)
598605
globs = proj['license-files']
599606
license_files = _license_files_from_globs(path.parent, globs)
607+
if isinstance(proj.get('license'), dict):
608+
raise ConfigError(
609+
"license-files cannot be used with a license table, "
610+
"use 'project.license' with a license expression instead"
611+
)
600612
else:
601-
license_files = _license_files_from_globs(
602-
path.parent, default_license_files_globs, use_defaults=True
613+
license_files.update(
614+
_license_files_from_globs(
615+
path.parent, default_license_files_globs, use_defaults=True
616+
)
603617
)
604-
lc.referenced_files.extend(license_files)
605-
md_dict['license_files'] = license_files
618+
license_files_sorted = sorted(license_files)
619+
lc.referenced_files.extend(license_files_sorted)
620+
md_dict['license_files'] = license_files_sorted
606621

607622
if 'authors' in proj:
608623
_check_type(proj, 'authors', list)

‎flit_core/pyproject.toml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ description = "Distribution-building parts of Flit. See flit package for more in
1212
dependencies = []
1313
requires-python = '>=3.6'
1414
readme = "README.rst"
15-
license = {file = "LICENSE"}
1615
license-files = ["LICENSE*", "flit_core/vendor/**/LICENSE*"]
1716
classifiers = [
1817
"License :: OSI Approved :: BSD License",

‎flit_core/tests_core/test_config.py‎

Lines changed: 25 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,6 @@
11
import logging
2-
import re
3-
import os
42
import sys
53
from pathlib import Path
6-
from unittest.mock import patch
74
import pytest
85

96
from flit_core import config
@@ -147,6 +144,7 @@ def test_bad_include_paths(path, err_match):
147144
({'license-files': ["/LICENSE"]}, r"'/LICENSE'.+must not start with '/'"),
148145
({'license-files': ["../LICENSE"]}, r"'../LICENSE'.+must not contain '..'"),
149146
({'license-files': ["NOT_FOUND"]}, r"No files found.+'NOT_FOUND'"),
147+
({'license-files': ["(LICENSE | LICENCE)"]}, "Pattern contains invalid characters"),
150148
pytest.param(
151149
{'license-files': ["**LICENSE"]}, r"'\*\*LICENSE'.+Invalid pattern",
152150
marks=[pytest.mark.skipif(
@@ -159,6 +157,10 @@ def test_bad_include_paths(path, err_match):
159157
sys.version_info < (3, 13), reason="Pattern started to raise ValueError in 3.13"
160158
)]
161159
),
160+
(
161+
{'license': {'file': 'LICENSE'}, 'license-files': ["LICENSE"]},
162+
"license-files cannot be used with a license table",
163+
),
162164
({'keywords': 'foo'}, 'list'),
163165
({'keywords': ['foo', 7]}, 'strings'),
164166
({'entry-points': {'foo': 'module1:main'}}, 'entry-point.*tables'),
@@ -181,7 +183,7 @@ def test_bad_pep621_info(proj_bad, err_match):
181183
proj = {'name': 'module1', 'version': '1.0', 'description': 'x'}
182184
proj.update(proj_bad)
183185
with pytest.raises(config.ConfigError, match=err_match):
184-
config.read_pep621_metadata(proj, samples_dir / 'pep621')
186+
config.read_pep621_metadata(proj, samples_dir / 'pep621' / 'pyproject.toml')
185187

186188
@pytest.mark.parametrize(('readme', 'err_match'), [
187189
({'file': 'README.rst'}, 'required'),
@@ -197,29 +199,23 @@ def test_bad_pep621_readme(readme, err_match):
197199
'name': 'module1', 'version': '1.0', 'description': 'x', 'readme': readme
198200
}
199201
with pytest.raises(config.ConfigError, match=err_match):
200-
config.read_pep621_metadata(proj, samples_dir / 'pep621')
201-
202-
203-
@pytest.mark.parametrize(('value', 'files'), [
204-
('[]', []),
205-
('["LICENSE"]', ["LICENSE"]),
206-
('["LICENSE*"]', ["LICENSE"]),
207-
('["**/LICENSE*"]', ["LICENSE", "module/vendor/LICENSE_VENDOR"]),
208-
('["module/vendor/LICENSE*"]', ["module/vendor/LICENSE_VENDOR"]),
209-
('["LICENSE", "module/**/LICENSE*"]', ["LICENSE", "module/vendor/LICENSE_VENDOR"]),
202+
config.read_pep621_metadata(proj, samples_dir / 'pep621' / 'pyproject.toml')
203+
204+
205+
@pytest.mark.parametrize(('proj_license_files', 'files'), [
206+
({}, ["LICENSE"]), # Only match default patterns
207+
({'license-files': []}, []),
208+
({'license-files': ["LICENSE"]}, ["LICENSE"]),
209+
({'license-files': ["LICENSE*"]}, ["LICENSE"]),
210+
({'license-files': ["LICEN[CS]E*"]}, ["LICENSE"]),
211+
({'license-files': ["**/LICENSE*"]}, ["LICENSE", "module/vendor/LICENSE_VENDOR"]),
212+
({'license-files': ["module/vendor/LICENSE*"]}, ["module/vendor/LICENSE_VENDOR"]),
213+
({'license-files': ["LICENSE", "module/**/LICENSE*"]}, ["LICENSE", "module/vendor/LICENSE_VENDOR"]),
214+
# Add project.license.file + match default patterns
215+
({'license': {'file': 'module/vendor/LICENSE_VENDOR'}}, ["LICENSE", "module/vendor/LICENSE_VENDOR"]),
210216
])
211-
def test_pep621_license_files(value, files):
212-
path = samples_dir / 'pep621_license_files' / 'pyproject.toml'
213-
data = path.read_text()
214-
data = re.sub(
215-
r"(^license-files = )(?:\[.*\])", r"\g<1>{}".format(value),
216-
data, count=1, flags=re.M
217-
)
218-
dir = os.getcwd()
219-
try:
220-
os.chdir(samples_dir / 'pep621_license_files')
221-
with patch("pathlib.Path.read_text", return_value=data):
222-
info = config.read_flit_config(path)
223-
assert info.metadata['license_files'] == files
224-
finally:
225-
os.chdir(dir)
217+
def test_pep621_license_files(proj_license_files, files):
218+
proj = {'name': 'module1', 'version': '1.0', 'description': 'x'}
219+
proj.update(proj_license_files)
220+
info = config.read_pep621_metadata(proj, samples_dir / 'pep621_license_files' / 'pyproject.toml')
221+
assert info.metadata['license_files'] == files

‎flit_core/tests_core/test_wheel.py‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import os
21
from pathlib import Path
32
from zipfile import ZipFile
43

@@ -49,13 +48,8 @@ def test_data_dir(tmp_path):
4948

5049

5150
def test_license_files(tmp_path):
52-
dir = os.getcwd()
53-
try:
54-
os.chdir(samples_dir / 'pep621_license_files')
55-
info = make_wheel_in(samples_dir / 'pep621_license_files' / 'pyproject.toml', tmp_path)
56-
assert_isfile(info.file)
57-
with ZipFile(info.file, 'r') as zf:
58-
assert 'module1-0.1.dist-info/licenses/LICENSE' in zf.namelist()
59-
assert 'module1-0.1.dist-info/licenses/module/vendor/LICENSE_VENDOR' in zf.namelist()
60-
finally:
61-
os.chdir(dir)
51+
info = make_wheel_in(samples_dir / 'pep621_license_files' / 'pyproject.toml', tmp_path)
52+
assert_isfile(info.file)
53+
with ZipFile(info.file, 'r') as zf:
54+
assert 'module1-0.1.dist-info/licenses/LICENSE' in zf.namelist()
55+
assert 'module1-0.1.dist-info/licenses/module/vendor/LICENSE_VENDOR' in zf.namelist()

0 commit comments

Comments
 (0)