From 42f4ecf4955faa3d4732ead571914258fc21d5e7 Mon Sep 17 00:00:00 2001 From: sputt Date: Sat, 22 Jun 2024 10:55:53 -0700 Subject: [PATCH 1/2] Update the format of the solution file to allow the source of extras to be preserved. A new format in the explanation clause provides a [] delimited extra that supplied the requirement. For example, if cryptography was provided by the "security" extra from the requests library and your project required requests[security], the lines would now appear as: myproject==1.0 requests==2.0 # myproject (>=2 [security]) cryptography==1.0.0 # requests[security] (>=1) The extra marker is required to build the dependency tree back exactly. Additionally, a new flag --remove-constraints will allow easy refreshing of solution files, by removing the pins they provide when passed via --constraints, producing a solution with the same versions but refreshed metadata. This fixes #79 --- MODULE.bazel | 2 +- req_compile/cmdline.py | 18 ++-- req_compile/compile.py | 14 +++- req_compile/dists.py | 97 +++++++--------------- req_compile/repos/solution.py | 22 ++++- tests/repos/requests_kerberos_solution.txt | 13 +++ tests/repos/test_solution.py | 12 ++- tests/test_compile.py | 12 +-- tests/test_dists.py | 15 ++-- tests/test_integration.py | 4 +- version.bzl | 2 +- 11 files changed, 118 insertions(+), 93 deletions(-) create mode 100644 tests/repos/requests_kerberos_solution.txt diff --git a/MODULE.bazel b/MODULE.bazel index c690a9a..cdd6154 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -2,5 +2,5 @@ module( name = "rules_req_compile", - version = "1.0.0rc21", + version = "1.0.0rc22", ) diff --git a/req_compile/cmdline.py b/req_compile/cmdline.py index 5b71840..bb198d4 100644 --- a/req_compile/cmdline.py +++ b/req_compile/cmdline.py @@ -346,12 +346,12 @@ def __init__(self, node: DependencyNode, multiline): def __str__(self): write_to = StringIO() - constraints = list(req_compile.dists.build_constraints(self.node)) + constraints = req_compile.dists.build_explanation(self.node) if len(constraints) == 1: if self.multiline: write_to.write("via ") - write_to.write(f"{constraints[0]}") + write_to.write(f"{next(iter(constraints))}") else: if self.multiline: write_to.write("via\n") @@ -763,14 +763,15 @@ def compile_main(raw_args: Optional[Sequence[str]] = None) -> None: "from stdin.", ) group.add_argument( - "-c,--constraints", + "-c", + "--constraints", action="append", dest="constraints", metavar="constraints_file", help="Constraints file or project directory to use as constraints.", ) group.add_argument( - "-e,--extra", + "-e", "--extra", action="append", dest="extras", default=[], @@ -778,7 +779,7 @@ def compile_main(raw_args: Optional[Sequence[str]] = None) -> None: help="Extras to apply automatically to source packages.", ) group.add_argument( - "-P,--upgrade-package", + "-P", "--upgrade-package", action="append", dest="upgrade_packages", metavar="package_name", @@ -796,6 +797,12 @@ def compile_main(raw_args: Optional[Sequence[str]] = None) -> None: action="store_true", help="Remove distributions not satisfied via --source from the output.", ) + group.add_argument( + "--remove-constraints", + default=False, + action="store_true", + help="Remove constraints pins from the output.", + ) group.add_argument( "-p", "--pre", @@ -979,6 +986,7 @@ def compile_main(raw_args: Optional[Sequence[str]] = None) -> None: repo, extras=args.extras, constraint_reqs=constraint_reqs, + remove_constraints=args.remove_constraints, only_binary=args.only_binary, ) except RepositoryInitializationError as ex: diff --git a/req_compile/compile.py b/req_compile/compile.py index 95bfa50..0ea1f62 100644 --- a/req_compile/compile.py +++ b/req_compile/compile.py @@ -267,6 +267,7 @@ def perform_compile( input_reqs: Iterable[RequirementContainer], repo: Repository, constraint_reqs: Optional[Iterable[RequirementContainer]] = None, + remove_constraints: bool = False, extras: Optional[Iterable[str]] = None, allow_circular_dependencies: bool = True, only_binary: Optional[Set[NormName]] = None, @@ -282,6 +283,9 @@ def perform_compile( repo: Repository to use as a source of Python packages. extras: Extras to apply automatically to source projects constraint_reqs: Constraints to use when compiling + remove_constraints: Whether to remove the constraints from the solution. By default, + constraints are added, so you can see why a requirement was pinned to a particular + version. allow_circular_dependencies: Whether to allow circular dependencies only_binary: Set of projects that should only consider binary distributions. max_downgrade: The maximum number of version downgrades that will be allowed for conflicts. @@ -333,13 +337,15 @@ def perform_compile( node, None, repo, results, options, max_downgrade=max_downgrade ) except (NoCandidateException, MetadataError) as ex: - _add_constraints(all_pinned, constraint_reqs, results) + if not remove_constraints: + _add_constraints(all_pinned, constraint_reqs, results) ex.results = results raise - # Add the constraints in, so it will show up as a contributor in the results. - # The same is done in the exception block above - _add_constraints(all_pinned, constraint_reqs, results) + if not remove_constraints: + # Add the constraints in, so it will show up as a contributor in the results. + # The same is done in the exception block above + _add_constraints(all_pinned, constraint_reqs, results) return results, roots diff --git a/req_compile/dists.py b/req_compile/dists.py index af84b54..de25fb3 100644 --- a/req_compile/dists.py +++ b/req_compile/dists.py @@ -4,21 +4,8 @@ import itertools import logging import sys -from typing import ( - Any, - Callable, - Dict, - Iterable, - Iterator, - List, - Optional, - Set, - Tuple, - Union, -) +from typing import Any, Dict, Iterable, Iterator, List, Optional, Set, Union -import packaging.requirements -import packaging.version import pkg_resources from req_compile.containers import RequirementContainer @@ -68,6 +55,7 @@ def __lt__(self, other: Any) -> bool: @property def extras(self) -> Set[str]: + """Extras for this node that its reverse dependencies have requested.""" extras = set() for rdep in self.reverse_deps: assert ( @@ -112,7 +100,12 @@ def build_constraints(self) -> pkg_resources.Requirement: return result -def build_constraints(root_node: DependencyNode) -> Iterable[str]: +def build_explanation(root_node: DependencyNode) -> collections.abc.Collection[str]: + """Build an explanation for why a node was included in the solution. + + The explanation provides the version constraints supplied by the reverse + dependencies for this node. + """ constraints: List[str] = [] for node in root_node.reverse_deps: assert ( @@ -123,15 +116,16 @@ def build_constraints(root_node: DependencyNode) -> Iterable[str]: all_reqs |= set(node.metadata.requires(extra=extra)) for req in all_reqs: if normalize_project_name(req.project_name) == root_node.key: - _process_constraint_req(req, node, constraints) + constraints.append(_process_constraint_req(req, node)) return constraints def _process_constraint_req( - req: pkg_resources.Requirement, node: DependencyNode, constraints: List[str] -) -> None: + req: pkg_resources.Requirement, node: DependencyNode +) -> str: assert node.metadata is not None, "Node {} must be solved".format(node) - extra = None + extras: Set[str] = set() + # Determine which extras, if any, were the reason this req was included. if req.marker: for marker in req.marker._markers: # pylint: disable=protected-access if ( @@ -139,10 +133,24 @@ def _process_constraint_req( and marker[0].value == "extra" and marker[1].value == "==" ): - extra = marker[2].value - source = node.metadata.name + (("[" + extra + "]") if extra else "") - specifics = " (" + str(req.specifier) + ")" if req.specifier else "" # type: ignore[attr-defined] - constraints.extend([source + specifics]) + extras.add(marker[2].value.strip().lower()) + source = node.metadata.name + ( + ("[" + ",".join(sorted(extras)) + "]") if extras else "" + ) + + specifics = "" + if req.specifier: # type: ignore[attr-defined] + specifics = str(req.specifier) # type: ignore[attr-defined] + + # Determine which extras this req was itself requesting. + if req.extras: + specifics += ( + f" [{','.join(sorted(extra.strip().lower() for extra in req.extras))}]" + ) + + if specifics: + specifics = f" ({specifics.strip()})" + return source + specifics class DistributionCollection: @@ -273,15 +281,6 @@ def remove_dists( node.metadata = None node.complete = False - def build( - self, roots: Iterable[DependencyNode] - ) -> Iterable[pkg_resources.Requirement]: - results = self.generate_lines(roots) - return [ - parse_requirement("==".join([result[0][0], str(result[0][1])])) - for result in results - ] - def visit_nodes( self, roots: Iterable[DependencyNode], @@ -320,40 +319,6 @@ def visit_nodes( ) return _visited - def generate_lines( - self, - roots: Iterable[DependencyNode], - req_filter: Optional[Callable[[DependencyNode], bool]] = None, - strip_extras: bool = False, - ) -> Iterable[ - Tuple[Tuple[str, Optional[packaging.version.Version], Optional[str]], str] - ]: - """ - Generate the lines of a results file from this collection - Args: - roots (iterable[DependencyNode]): List of roots to generate lines from - req_filter (Callable): Filter to apply to each element of the collection. - Return True to keep a node, False to exclude it - Returns: - (list[str]) List of rendered node entries in the form of - reqname==version # reasons - """ - req_filter = req_filter or (lambda _: True) - results: List[ - Tuple[Tuple[str, Optional[packaging.version.Version], Optional[str]], str] - ] = [] - for node in self.visit_nodes(roots): - if node.metadata is None: - continue - if not node.metadata.meta and req_filter(node): - constraints = build_constraints(node) - name, version = node.metadata.to_definition(node.extras) - if strip_extras: - name = name.split("[", 1)[0] - constraint_text = ", ".join(sorted(constraints)) - results.append(((name, version, node.metadata.hash), constraint_text)) - return results - def __contains__(self, project_name: str) -> bool: req_name = project_name.split("[")[0] return normalize_project_name(req_name) in self.nodes diff --git a/req_compile/repos/solution.py b/req_compile/repos/solution.py index 1db6a57..b5f2304 100644 --- a/req_compile/repos/solution.py +++ b/req_compile/repos/solution.py @@ -249,10 +249,12 @@ def _add_sources( url: Optional[str] = None, dist_hash: Optional[str] = None, ) -> None: - pkg_names = map(lambda x: x.split(" ")[0], sources) + pkg_names = map(lambda x: x.split(" ", 1)[0], sources) constraints = map( lambda x: ( - x.split(" ")[1].replace("(", "").replace(")", "") if "(" in x else None + x.split(" ", 1)[1].replace("(", "").replace(")", "") + if "(" in x + else None ), sources, ) @@ -330,10 +332,24 @@ def _create_metadata_req( extra = next(iter(req_compile.utils.parse_requirement(name).extras)) marker = ' ; extra == "{}"'.format(extra) + # req will only have extras if the solution file had them in the left-hand + # side of == expression, e.g. req[extra]==1.0. Since pip doesn't support having + # extras on the left-hand side for constraints files, we don't emit this + # any longer. + extras = req.extras + if constraints and ("[" in constraints and "]" in constraints): + # Parse out the extras that brought in this requirement. It will look like + # (>1.0 [extra1,extra2]). Usually it would just be one unless the distribution + # includes a requirement under multiple extras. + constraints, extra_string = constraints.split("[", 1) + constraints = constraints.strip() + extra_string = extra_string.replace("]", "") + extras = {extra.strip() for extra in extra_string.split(",")} + return req_compile.utils.parse_requirement( "{}{}{}{}".format( metadata.name, - ("[" + ",".join(req.extras) + "]") if req.extras else "", + ("[" + ",".join(sorted(extras)) + "]") if extras else "", constraints if constraints else "", marker, ) diff --git a/tests/repos/requests_kerberos_solution.txt b/tests/repos/requests_kerberos_solution.txt new file mode 100644 index 0000000..f8f9fed --- /dev/null +++ b/tests/repos/requests_kerberos_solution.txt @@ -0,0 +1,13 @@ +certifi==2024.6.2 # requests (>=2017.4.17) +cffi==1.15.1 # cryptography (>=1.12) +charset-normalizer==3.3.2 # requests (<4,>=2) +cryptography==42.0.8 # pyspnego, requests-kerberos (>=1.3) +decorator==5.1.1 # gssapi +gssapi==1.8.2 # pyspnego[kerberos] (>=1.6.0) +idna==3.7 # requests (<4,>=2.5) +krb5==0.5.1 # pyspnego[kerberos] (>=0.3.0) +pycparser==2.21 # cffi +pyspnego==0.9.2 # requests-kerberos (>=0.9.2 [kerberos]) +requests==2.31.0 # requests-kerberos (>=1.1.0) +requests-kerberos==0.14.0 # - +urllib3==2.0.7 # requests (<3,>=1.21.1) diff --git a/tests/repos/test_solution.py b/tests/repos/test_solution.py index e3b2594..ab3ae63 100644 --- a/tests/repos/test_solution.py +++ b/tests/repos/test_solution.py @@ -177,7 +177,6 @@ def test_round_trip( def test_writing_repo_sources(mock_metadata, mock_pypi, tmp_path): - mock_pypi.load_scenario("normal") results, nodes = req_compile.compile.perform_compile( @@ -230,6 +229,17 @@ def test_load_additive_constraints(): assert constraints == pkg_resources.Requirement.parse("idna<2.9,>=2.5") +def test_load_extras() -> None: + """Test that if the correct extras are associated with the correct requirements.""" + solution_repo = SolutionRepository( + os.path.join(os.path.dirname(__file__), "requests_kerberos_solution.txt") + ) + assert solution_repo.solution["pyspnego"].extras == {"kerberos"} + assert [req for req in solution_repo.solution["requests-kerberos"].metadata.requires( + None + ) if req.project_name=="pyspnego"][0] == parse_requirement("pyspnego[kerberos]>=0.9.2") + + def test_load_extra_first(): """Test that solutions that refer to a requirement with an extra before it is defined correctly add the requirement with the extra""" diff --git a/tests/test_compile.py b/tests/test_compile.py index 671de1c..2dd3253 100644 --- a/tests/test_compile.py +++ b/tests/test_compile.py @@ -1,6 +1,6 @@ # pylint: disable=redefined-outer-name import os -from typing import Iterable, Optional, Tuple +from typing import Iterable, Optional, Set, Tuple from unittest import mock import pkg_resources @@ -14,6 +14,7 @@ import req_compile.utils from req_compile.compile import AllOnlyBinarySet from req_compile.containers import DistInfo, RequirementContainer +from req_compile.dists import DependencyNode, DistributionCollection from req_compile.repos.multi import MultiRepository from req_compile.repos.repository import Candidate, Repository, filename_to_candidate from req_compile.repos.source import SourceRepository @@ -29,10 +30,11 @@ def test_mock_pypi(mock_pypi): assert metadata.version == req_compile.utils.parse_version("1.0.0") -def _real_outputs(results): - outputs = results[0].build(results[1]) - outputs = sorted(outputs, key=lambda x: x.name) - return set(str(req) for req in outputs) +def _real_outputs(results: Tuple[DistributionCollection, Set[DependencyNode]]): + return set( + "{}=={}".format(*node.metadata.to_definition(node.extras)) + for node in results[0].visit_nodes(results[1]) + ) @fixture diff --git a/tests/test_dists.py b/tests/test_dists.py index 0d55a28..34b6b62 100644 --- a/tests/test_dists.py +++ b/tests/test_dists.py @@ -2,7 +2,7 @@ from pkg_resources import Requirement from req_compile.containers import DistInfo -from req_compile.dists import DistributionCollection +from req_compile.dists import DistributionCollection, build_explanation def test_unconstrained(): @@ -153,11 +153,14 @@ def test_repo_with_extra(): ) dists.add_dist(metadata_c, root_a, pkg_resources.Requirement.parse("a")) - lines = dists.generate_lines({root}) - assert sorted(lines) == [ - (("a[test]", "1.0.0", None), "root"), - (("b", "2.0.0", None), "a[test]"), - (("c", "2.0.0", None), "a"), + results = [ + ("==".join(node.metadata.to_definition(node.extras)), build_explanation(node)) + for node in dists.visit_nodes({root}) + ] + assert sorted(results) == [ + ("a[test]==1.0.0", ["root ([test])"]), + ("b==2.0.0", ["a[test]"]), + ("c==2.0.0", ["a"]), ] diff --git a/tests/test_integration.py b/tests/test_integration.py index a4097e5..c9a7782 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -3,6 +3,8 @@ import subprocess import sys +from req_compile.config import read_pip_default_index + ROOT_DIR = os.path.abspath(os.path.join(os.path.dirname(__file__), os.path.pardir)) @@ -45,7 +47,7 @@ def test_compile_req_compile(tmp_path): "-m", "req_compile", "-i", - "https://pypi.org/simple", + read_pip_default_index() or "https://pypi.org/simple", ".", "--wheel-dir", str(tmp_path), diff --git a/version.bzl b/version.bzl index 4aa027a..54227b1 100644 --- a/version.bzl +++ b/version.bzl @@ -1,3 +1,3 @@ """req-compile version""" -VERSION = "1.0.0rc21" +VERSION = "1.0.0rc22" From 8a22cd07360e8def0a13001fe36c56d2de2426d6 Mon Sep 17 00:00:00 2001 From: Andre Brisco Date: Mon, 24 Jun 2024 07:42:30 -0700 Subject: [PATCH 2/2] Fixed formatting --- req_compile/cmdline.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/req_compile/cmdline.py b/req_compile/cmdline.py index bb198d4..4c72aa4 100644 --- a/req_compile/cmdline.py +++ b/req_compile/cmdline.py @@ -771,7 +771,8 @@ def compile_main(raw_args: Optional[Sequence[str]] = None) -> None: help="Constraints file or project directory to use as constraints.", ) group.add_argument( - "-e", "--extra", + "-e", + "--extra", action="append", dest="extras", default=[], @@ -779,7 +780,8 @@ def compile_main(raw_args: Optional[Sequence[str]] = None) -> None: help="Extras to apply automatically to source packages.", ) group.add_argument( - "-P", "--upgrade-package", + "-P", + "--upgrade-package", action="append", dest="upgrade_packages", metavar="package_name",