From f190b5b225a2c11b7c2df136347c0c093fa3aa75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miro=20Hron=C4=8Dok?= Date: Fri, 23 Jul 2021 08:00:01 +0000 Subject: [PATCH] %pyproject_buildrequires now fails when it encounters an invalid requirement Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1983053 Related: rhbz#1950291 --- README.md | 34 ++++++++++++++ pyproject-rpm-macros.spec | 5 +- pyproject_buildrequires.py | 65 +++++++++++++++++++------- pyproject_buildrequires_testcases.yaml | 38 +++++++++++++-- tests/python-markupsafe.spec | 3 ++ 5 files changed, 123 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index c36bbba..18f1871 100644 --- a/README.md +++ b/README.md @@ -268,8 +268,42 @@ undefine `%{py3_shbang_opt}` to turn it off. Some valid Python version specifiers are not supported. +When a dependency is specified via an URL or local path, for example as: + + https://github.com/ActiveState/appdirs/archive/8eacfa312d77aba28d483fbfb6f6fc54099622be.zip + /some/path/foo-1.2.3.tar.gz + git+https://github.com/sphinx-doc/sphinx.git@96dbe5e3 + +The `%pyproject_buildrequires` macro is unable to convert it to an appropriate RPM requirement and will fail. +If the URL contains the `packageName @` prefix as specified in [PEP 508], +the requirement will be generated without a version constraint: + + appdirs@https://github.com/ActiveState/appdirs/archive/8eacfa312d77aba28d483fbfb6f6fc54099622be.zip + foo@file:///some/path/foo-1.2.3.tar.gz + +Will be converted to: + + python3dist(appdirs) + python3dist(foo) + +Alternatively, when an URL requirement parsed from a text file +given as positional argument to `%pyproject_buildrequires` +contains the `#egg=packageName` fragment, +as documented in [pip's documentation]: + + git+https://github.com/sphinx-doc/sphinx.git@96dbe5e3#egg=sphinx + +The requirements will be converted to package names without versions, e.g.: + + python3dist(sphinx) + +However upstreams usually only use direct URLs for their requirements as workarounds, +so be prepared for problems. + +[PEP 508]: https://www.python.org/dev/peps/pep-0508/ [PEP 517]: https://www.python.org/dev/peps/pep-0517/ [PEP 518]: https://www.python.org/dev/peps/pep-0518/ +[pip's documentation]: https://pip.pypa.io/en/stable/cli/pip_install/#vcs-support Testing the macros diff --git a/pyproject-rpm-macros.spec b/pyproject-rpm-macros.spec index 760c43f..dcda597 100644 --- a/pyproject-rpm-macros.spec +++ b/pyproject-rpm-macros.spec @@ -12,7 +12,7 @@ License: MIT # In other cases, such as backports, increment the point # release. Version: 0 -Release: 45%{?dist} +Release: 46%{?dist} # Macro files Source001: macros.pyproject @@ -115,6 +115,9 @@ export HOSTNAME="rpmbuild" # to speedup tox in network-less mock, see rhbz#1856 %license LICENSE %changelog +* Fri Jul 23 2021 Miro Hrončok - 0-46 +- %%pyproject_buildrequires now fails when it encounters an invalid requirement + * Fri Jul 23 2021 Fedora Release Engineering - 0-45 - Rebuilt for https://fedoraproject.org/wiki/Fedora_35_Mass_Rebuild diff --git a/pyproject_buildrequires.py b/pyproject_buildrequires.py index f507748..50c63f6 100644 --- a/pyproject_buildrequires.py +++ b/pyproject_buildrequires.py @@ -11,12 +11,16 @@ import re import tempfile import email.parser import pathlib +import urllib # Some valid Python version specifiers are not supported. # Allow only the forms we know we can handle. VERSION_RE = re.compile(r'[a-zA-Z0-9.-]+(\.\*)?') +# We treat this as comment in requirements files, as does pip +COMMENT_RE = re.compile(r'(^|\s+)#.*$') + class EndPass(Exception): """End current pass of generating requirements""" @@ -50,6 +54,31 @@ def hook_call(): print_err('HOOK STDOUT:', line) +def pkgname_from_egg_fragment(requirement_str): + parsed_url = urllib.parse.urlparse(requirement_str) + parsed_fragment = urllib.parse.parse_qs(parsed_url.fragment) + if 'egg' in parsed_fragment: + return parsed_fragment['egg'][0] + return None + + +def guess_reason_for_invalid_requirement(requirement_str): + if ':' in requirement_str: + return ( + 'It might be an URL. ' + '%pyproject_buildrequires cannot handle all URL-based requirements. ' + 'Add PackageName@ (see PEP 508) to the URL to at least require any version of PackageName.' + ) + if '/' in requirement_str: + return ( + 'It might be a local path. ' + '%pyproject_buildrequires cannot handle local paths as requirements. ' + 'Use an URL with PackageName@ (see PEP 508) to at least require any version of PackageName.' + ) + # No more ideas + return None + + class Requirements: """Requirement printer""" def __init__(self, get_installed_version, extras=None, @@ -81,18 +110,27 @@ class Requirements: return True return False - def add(self, requirement_str, *, source=None): + def add(self, requirement_str, *, source=None, allow_egg_pkgname=False): """Output a Python-style requirement string as RPM dep""" print_err(f'Handling {requirement_str} from {source}') try: requirement = Requirement(requirement_str) - except InvalidRequirement as e: + except InvalidRequirement: + if allow_egg_pkgname and (egg_name := pkgname_from_egg_fragment(requirement_str)): + requirement = Requirement(egg_name) + requirement.url = requirement_str + else: + hint = guess_reason_for_invalid_requirement(requirement_str) + message = f'Requirement {requirement_str!r} from {source} is invalid.' + if hint: + message += f' Hint: {hint}' + raise ValueError(message) + + if requirement.url: print_err( - f'WARNING: Skipping invalid requirement: {requirement_str}\n' - + f' {e}', + f'WARNING: Simplifying {requirement_str!r} to {requirement.name!r}.' ) - return name = canonicalize_name(requirement.name) if (requirement.marker is not None and @@ -128,7 +166,7 @@ class Requirements: if not VERSION_RE.fullmatch(str(specifier.version)): raise ValueError( f'Unknown character in version: {specifier.version}. ' - + '(This is probably a bug in pyproject-rpm-macros.)', + + '(This might be a bug in pyproject-rpm-macros.)', ) together.append(convert(python3dist(name, python3_pkgversion=self.python3_pkgversion), specifier.operator, specifier.version)) @@ -146,10 +184,10 @@ class Requirements: print_err(f'Exiting dependency generation pass: {source}') raise EndPass(source) - def extend(self, requirement_strs, *, source=None): + def extend(self, requirement_strs, **kwargs): """add() several requirements""" for req_str in requirement_strs: - self.add(req_str, source=source) + self.add(req_str, **kwargs) def get_backend(requirements): @@ -241,13 +279,7 @@ def generate_run_requirements(backend, requirements): def parse_requirements_lines(lines, path=None): packages = [] for line in lines: - line, _, comment = line.partition('#') - if comment.startswith('egg='): - # not a real comment - # e.g. git+https://github.com/monty/spam.git@master#egg=spam&... - egg, *_ = comment.strip().partition(' ') - egg, *_ = egg.strip().partition('&') - line = egg[4:] + line = COMMENT_RE.sub('', line) line = line.strip() if line.startswith('-r'): recursed_path = line[2:].strip() @@ -343,7 +375,8 @@ def generate_requires( lines = req_file.read().splitlines() packages = parse_requirements_lines(lines, pathlib.Path(req_file.name)) requirements.extend(packages, - source=f'requirements file {req_file.name}') + source=f'requirements file {req_file.name}', + allow_egg_pkgname=True) requirements.check(source='all requirement files') if use_build_system: backend = get_backend(requirements) diff --git a/pyproject_buildrequires_testcases.yaml b/pyproject_buildrequires_testcases.yaml index b9c7fe7..f5d9876 100644 --- a/pyproject_buildrequires_testcases.yaml +++ b/pyproject_buildrequires_testcases.yaml @@ -92,8 +92,7 @@ Single value version with unsupported compatible operator: [build-system] requires = ["pkg ~= 42", "foo"] build-backend = "foo.build" - stderr_contains: "WARNING: Skipping invalid requirement: pkg ~= 42" - result: 0 + except: ValueError Asterisk in version with unsupported compatible operator: installed: @@ -102,8 +101,34 @@ Asterisk in version with unsupported compatible operator: [build-system] requires = ["pkg ~= 0.1.*", "foo"] build-backend = "foo.build" - stderr_contains: "WARNING: Skipping invalid requirement: pkg ~= 0.1.*" - result: 0 + except: ValueError + +Local path as requirement: + installed: + toml: 1 + pyproject.toml: | + [build-system] + requires = ["./pkg-1.2.3.tar.gz", "foo"] + build-backend = "foo.build" + except: ValueError + +Pip's egg=pkgName requirement not in requirements file: + installed: + toml: 1 + pyproject.toml: | + [build-system] + requires = ["git+https://github.com/monty/spam.git@master#egg=spam", "foo"] + build-backend = "foo.build" + except: ValueError + +URL without egg fragment as requirement: + installed: + toml: 1 + pyproject.toml: | + [build-system] + requires = ["git+https://github.com/pkg-dev/pkg.git@96dbe5e3", "foo"] + build-backend = "foo.build" + except: ValueError Build system dependencies in pyproject.toml with extras: generate_extras: true @@ -125,9 +150,9 @@ Build system dependencies in pyproject.toml with extras: "equal == 0.5.0", "arbitrary_equal === 0.6.0", "asterisk_equal == 0.6.*", + "appdirs@https://github.com/ActiveState/appdirs/archive/8eacfa312d77aba28d483fbfb6f6fc54099622be.zip", "multi[Extras1,Extras2] == 6.0", "combo >2, <5, != 3.0.0", - "invalid!!ignored", "py2 ; python_version < '2.7'", "py3 ; python_version > '3.0'", ] @@ -145,11 +170,13 @@ Build system dependencies in pyproject.toml with extras: python3dist(equal) = 0.5 python3dist(arbitrary-equal) = 0.6 (python3dist(asterisk-equal) >= 0.6 with python3dist(asterisk-equal) < 0.7) + python3dist(appdirs) python3dist(multi) = 6 python3dist(multi[extras1]) = 6 python3dist(multi[extras2]) = 6 ((python3dist(combo) < 3 or python3dist(combo) > 3) with python3dist(combo) < 5 with python3dist(combo) > 2) python3dist(py3) + stderr_contains: "WARNING: Simplifying 'appdirs@https://github.com/ActiveState/appdirs/archive/8eacfa312d77aba28d483fbfb6f6fc54099622be.zip' to 'appdirs'." result: 0 Build system dependencies in pyproject.toml without extras: @@ -576,6 +603,7 @@ With pyproject.toml, requirements file and with -N option: python3dist(paramiko) python3dist(sqlalchemy) python3dist(spam) + stderr_contains: "WARNING: Simplifying 'git+https://github.com/monty/spam.git@master#egg=spam' to 'spam'." result: 0 With pyproject.toml, requirements file and without -N option: diff --git a/tests/python-markupsafe.spec b/tests/python-markupsafe.spec index b67d59d..3f4e3bd 100644 --- a/tests/python-markupsafe.spec +++ b/tests/python-markupsafe.spec @@ -28,6 +28,9 @@ Summary: %{summary} # we don't have pip-tools packaged in Fedora yet sed -i /pip-tools/d requirements/dev.in +# help the macros understand the URL in requirements/docs.in +sed -Ei 's/sphinx\.git@([0-9a-f]+)/sphinx.git@\1#egg=sphinx/' requirements/docs.in + %generate_buildrequires # requirements/dev.in recursively includes tests.in and docs.in