From 135f14e22fb60a03577e195ae3385a113aa494c3 Mon Sep 17 00:00:00 2001 From: Gunnar Kreitz Date: Tue, 29 Apr 2025 16:44:15 +0200 Subject: [PATCH 1/8] Fix crash with pip==25.1, which changed caching for find_all_candidates --- piptools/repositories/pypi.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/piptools/repositories/pypi.py b/piptools/repositories/pypi.py index 5a4448e3f..f585dfc34 100644 --- a/piptools/repositories/pypi.py +++ b/piptools/repositories/pypi.py @@ -447,9 +447,13 @@ def _wheel_support_index_min(self: Wheel, tags: list[Tag]) -> int: Wheel.support_index_min = _wheel_support_index_min self._available_candidates_cache = {} - # If we don't clear this cache then it can contain results from an - # earlier call when allow_all_wheels wasn't active. See GH-1532 - self.finder.find_all_candidates.cache_clear() + # Finder internally caches results, and there is no public method to + # clear the cache, so we re-create the object here. If we don't clear + # this cache then it can contain results from an earlier call when + # allow_all_wheels wasn't active. See GH-1532 + self._finder = self.command._build_package_finder( + options=self.options, session=self.session + ) try: yield From 809239a59762de9170e26e5d3af85c795f70561a Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Tue, 24 Jun 2025 16:04:12 -0500 Subject: [PATCH 2/8] Conditionally rewrite abspath 'comes_from' data MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In order to support newer `pip` versions, `pip-compile` needs to handle the fact that the `comes_from` field on a parsed requirement may be rewritten from a relative path to an absolute one. The change in `pip` occurred in version 24.3. `comes_from` is populated with absolute paths. `pip-compile` targets usage in which a relative path is supplied as input and ought to be preserved in the annotations for its outputs. As such, even though the relative path has been translated to an absolute one, the original (relative) path should be rendered. Because the data coming back from `pip` are ambiguous to some degree, as a heuristic, this change will use whether or not the original path was absolute to determine behavior. Therefore, the following files will result in correct behavior: # reqs.in -r other-reqs.in # other-reqs.in some-pkg `some-pkg` came from `-r other-reqs.in`. By contrast, if the CWD is `/opt`, and `-r reqs.in` is used, these data will be *incorrectly* rendered with relative paths: # reqs.in -r /opt/other-reqs.in # /opt/other-reqs.in some-pkg `some-pkg` came from `-r /opt/other-reqs.in`, but will be annotated with `-r other-reqs.in` Note that the relative path which is used is, in this scenario, correct, but not reflective of the inputs to the program. However, it is possible, if the absolute paths are really wanted in such a case, to ask for them using `pip-compile -r /opt/reqs.in`. Co-authored-by: 🇺🇦 Sviatoslav Sydorenko (Святослав Сидоренко) --- piptools/_compat/pip_compat.py | 62 ++++++++++++++++++++++++++++++++-- piptools/scripts/compile.py | 4 +-- 2 files changed, 61 insertions(+), 5 deletions(-) diff --git a/piptools/_compat/pip_compat.py b/piptools/_compat/pip_compat.py index 31c45f9a1..a28ffd1c4 100644 --- a/piptools/_compat/pip_compat.py +++ b/piptools/_compat/pip_compat.py @@ -1,8 +1,9 @@ from __future__ import annotations import optparse +import os from dataclasses import dataclass -from typing import TYPE_CHECKING, Iterable, Iterator, Set, cast +from typing import TYPE_CHECKING, Callable, Iterable, Iterator, Set, cast from pip._internal.cache import WheelCache from pip._internal.index.package_finder import PackageFinder @@ -83,7 +84,25 @@ def parse_requirements( options: optparse.Values | None = None, constraint: bool = False, isolated: bool = False, + comes_from_stdin: bool = False, ) -> Iterator[InstallRequirement]: + # the `comes_from` data will be rewritten based on a number of conditions + # + # None do not rewrite + # callable programmatic rewrite + # str fixed rewrite + rewrite_comes_from: str | Callable[[str], str] | None + + if comes_from_stdin: + # if data is coming from stdin, then `comes_from="-r -"` + rewrite_comes_from = "-r -" + elif _filename_is_abspath(filename): + rewrite_comes_from = None + else: + # if the input was a relative path, set the rewrite rule to rewrite + # absolute paths to be relative + rewrite_comes_from = _rewrite_absolute_comes_from_location + for parsed_req in _parse_requirements( filename, session, finder=finder, options=options, constraint=constraint ): @@ -94,7 +113,46 @@ def parse_requirements( file_link = FileLink(install_req.link.url) file_link._url = parsed_req.requirement install_req.link = file_link - yield copy_install_requirement(install_req) + install_req = copy_install_requirement(install_req) + if rewrite_comes_from is None: + pass + elif isinstance(rewrite_comes_from, str): + install_req.comes_from = rewrite_comes_from + else: + install_req.comes_from = rewrite_comes_from(install_req.comes_from) + yield install_req + + +def _filename_is_abspath(filename: str) -> bool: + """ + Check if a path is an absolute path, using exactly the normalization + used in ``pip>=24.3`` in order to ensure consistent results. + """ + return os.path.abspath(filename) == filename + + +def _rewrite_absolute_comes_from_location(original_comes_from: str, /) -> str: + """ + This is the rewrite rule used when ``-r`` or ``-c`` appears in + ``comes_from`` data with an absolute path. + + The ``-r`` or ``-c`` qualifier is retained, and the path is relativized + with respect to the CWD. + """ + # require `-r` or `-c` as the source + if not original_comes_from.startswith(("-r ", "-c ")): + return original_comes_from + + # split on the space + prefix, _, suffix = original_comes_from.partition(" ") + + # if the path was not absolute, bail out + if not _filename_is_abspath(suffix): + return original_comes_from + + # make it relative to the current working dir + suffix = os.path.relpath(suffix) + return f"{prefix} {suffix}" def create_wheel_cache(cache_dir: str, format_control: str | None = None) -> WheelCache: diff --git a/piptools/scripts/compile.py b/piptools/scripts/compile.py index 7c53435c2..a597fe602 100755 --- a/piptools/scripts/compile.py +++ b/piptools/scripts/compile.py @@ -354,7 +354,6 @@ def cli( # reading requirements from install_requires in setup.py. tmpfile = tempfile.NamedTemporaryFile(mode="wt", delete=False) tmpfile.write(sys.stdin.read()) - comes_from = "-r -" tmpfile.flush() reqs = list( parse_requirements( @@ -362,10 +361,9 @@ def cli( finder=repository.finder, session=repository.session, options=repository.options, + comes_from_stdin=True, ) ) - for req in reqs: - req.comes_from = comes_from constraints.extend(reqs) elif is_setup_file: setup_file_found = True From 723660ae1209619e54905d85825d803726436010 Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Tue, 24 Jun 2025 16:52:23 -0500 Subject: [PATCH 3/8] Update the supported pip version to 25.1.1 Update `pipsupported` to pick the latest versions of `pip` and `setuptools`.[^1] For Python 3.8, a lower `pip` version is chosen, as 25.1.1 dropped support for 3.8 . [^1]: The `setuptools` bound might not be needed anymore on newer pip versions, but this needs separate assessment. --- tox.ini | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tox.ini b/tox.ini index dba0b4bb2..fbb9f2ec5 100644 --- a/tox.ini +++ b/tox.ini @@ -14,8 +14,9 @@ extras = testing coverage: coverage deps = - pipsupported: pip==24.2 - pipsupported: setuptools <= 75.8.2 + pipsupported: pip == 24.3.1 ; python_version < "3.9" + pipsupported: pip == 25.1.1 ; python_version >= "3.9" + pipsupported: setuptools <= 80.0 piplowest: pip == 22.2.* ; python_version < "3.12" piplowest: pip == 23.2.* ; python_version >= "3.12" From 103d92e1ad0621c1bbfdeb40d8e160909d40bf34 Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Sat, 28 Jun 2025 21:29:12 -0500 Subject: [PATCH 4/8] Switch to pathlib for pip_compat module Use pathlib for relativizing paths, checking absolute paths. This replaces some os.path usage. --- piptools/_compat/pip_compat.py | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/piptools/_compat/pip_compat.py b/piptools/_compat/pip_compat.py index a28ffd1c4..6bed0da0b 100644 --- a/piptools/_compat/pip_compat.py +++ b/piptools/_compat/pip_compat.py @@ -1,7 +1,7 @@ from __future__ import annotations import optparse -import os +import pathlib from dataclasses import dataclass from typing import TYPE_CHECKING, Callable, Iterable, Iterator, Set, cast @@ -96,7 +96,7 @@ def parse_requirements( if comes_from_stdin: # if data is coming from stdin, then `comes_from="-r -"` rewrite_comes_from = "-r -" - elif _filename_is_abspath(filename): + elif pathlib.Path(filename).is_absolute(): rewrite_comes_from = None else: # if the input was a relative path, set the rewrite rule to rewrite @@ -123,14 +123,6 @@ def parse_requirements( yield install_req -def _filename_is_abspath(filename: str) -> bool: - """ - Check if a path is an absolute path, using exactly the normalization - used in ``pip>=24.3`` in order to ensure consistent results. - """ - return os.path.abspath(filename) == filename - - def _rewrite_absolute_comes_from_location(original_comes_from: str, /) -> str: """ This is the rewrite rule used when ``-r`` or ``-c`` appears in @@ -146,12 +138,14 @@ def _rewrite_absolute_comes_from_location(original_comes_from: str, /) -> str: # split on the space prefix, _, suffix = original_comes_from.partition(" ") + file_path = pathlib.Path(suffix) + # if the path was not absolute, bail out - if not _filename_is_abspath(suffix): + if not file_path.is_absolute(): return original_comes_from # make it relative to the current working dir - suffix = os.path.relpath(suffix) + suffix = str(file_path.relative_to(pathlib.Path.cwd())) return f"{prefix} {suffix}" From 6c2c53a354b9b08fdf51ada9be438ad4967e4e7e Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Mon, 30 Jun 2025 10:15:09 -0500 Subject: [PATCH 5/8] Add tests for new pip-compile path behavior MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The new path normalization step now has explicit tests which verify and describe the behavior rigorously. The logic for absolute paths applies in newer pip versions, and therefore is entirely conditional on the pip version which is in use. These changes include some new test tooling: - The current pip version is available as a fixture, as is `pip_produces_absolute_paths`, which is just a version-comparison on that version to indicate the cut-line for the path normalization behavior. - `TestFilesCollection` is a dataclass for writing parameters. It's a stub filesystem layout + methods for putting those files into a temp dir. Co-authored-by: 🇺🇦 Sviatoslav Sydorenko (Святослав Сидоренко) --- piptools/_compat/pip_compat.py | 6 +- tests/test_cli_compile.py | 212 +++++++++++++++++++++++++++++++++ 2 files changed, 214 insertions(+), 4 deletions(-) diff --git a/piptools/_compat/pip_compat.py b/piptools/_compat/pip_compat.py index 6bed0da0b..bc4fdc125 100644 --- a/piptools/_compat/pip_compat.py +++ b/piptools/_compat/pip_compat.py @@ -114,11 +114,9 @@ def parse_requirements( file_link._url = parsed_req.requirement install_req.link = file_link install_req = copy_install_requirement(install_req) - if rewrite_comes_from is None: - pass - elif isinstance(rewrite_comes_from, str): + if isinstance(rewrite_comes_from, str): install_req.comes_from = rewrite_comes_from - else: + elif rewrite_comes_from is not None: install_req.comes_from = rewrite_comes_from(install_req.comes_from) yield install_req diff --git a/tests/test_cli_compile.py b/tests/test_cli_compile.py index afc9c8776..043558f0f 100644 --- a/tests/test_cli_compile.py +++ b/tests/test_cli_compile.py @@ -1,5 +1,6 @@ from __future__ import annotations +import dataclasses import hashlib import os import pathlib @@ -7,6 +8,7 @@ import shutil import subprocess import sys +import typing from textwrap import dedent from unittest import mock from unittest.mock import MagicMock @@ -39,6 +41,55 @@ ) +@pytest.fixture(scope="session") +def installed_pip_version(): + return get_pip_version_for_python_executable(sys.executable) + + +@pytest.fixture(scope="session") +def pip_produces_absolute_paths(installed_pip_version): + # in pip v24.3, new normalization will occur because `comes_from` started + # to be normalized to abspaths + return installed_pip_version >= Version("24.3") + + +@dataclasses.dataclass +class TestFilesCollection: + """ + A small data-builder for setting up files in a tmp dir. + + Contains a name for use as the ID in parametrized tests and contents. + 'contents' maps from subpaths in the tmp dir to file content or callables + which produce file content given the tmp dir. + """ + + # the name for the collection of files + name: str + # static or computed contents + contents: dict[str, str | typing.Callable[[pathlib.Path], str]] + + def __str__(self) -> str: + return self.name + + def populate(self, tmp_path: pathlib.Path) -> None: + """Populate the tmp dir with file contents.""" + for path_str, content in self.contents.items(): + path = tmp_path / path_str + path.parent.mkdir(exist_ok=True, parents=True) + if isinstance(content, str): + path.write_text(content) + else: + path.write_text(content(tmp_path)) + + def get_path_to(self, filename: str) -> str: + """Given a filename, find the (first) path to that filename in the contents.""" + return next( + k + for k in self.contents.keys() + if (k == filename) or k.endswith(f"/{filename}") + ) + + @pytest.fixture( autouse=True, params=[ @@ -3838,3 +3889,164 @@ def test_stdout_should_not_be_read_when_stdin_is_not_a_plain_file( out = runner.invoke(cli, [req_in.as_posix(), "--output-file", fifo.as_posix()]) assert out.exit_code == 0, out + + +@pytest.mark.parametrize("input_path_absolute", (True, False)) +@pytest.mark.parametrize( + "test_files_collection", + ( + TestFilesCollection( + "relative_include", + { + "requirements2.in": "small-fake-a\n", + "requirements.in": "-r requirements2.in\n", + }, + ), + TestFilesCollection( + "absolute_include", + { + "requirements2.in": "small-fake-a\n", + "requirements.in": lambda tmpdir: f"-r {(tmpdir / 'requirements2.in').as_posix()}", + }, + ), + ), + ids=str, +) +def test_second_order_requirements_path_handling( + pip_conf, + runner, + tmp_path, + monkeypatch, + input_path_absolute, + test_files_collection, +): + """ + Given nested requirements files, the internal requirements will be written + in the output, and it will be absolute or relative depending only on + whether or not the initial path was absolute or relative. + + This is only accurate on pip>=24.3 , as earlier versions do not normalize + the `comes_from` data to absolute paths. + On lower pip versions, assert the previous output is produced, in which + `-r` inputs are preserved. + """ + test_files_collection.populate(tmp_path) + + # the input path is given on the CLI as absolute or relative + # and this determines the expected output path as well + if input_path_absolute: + input_path = (tmp_path / "requirements.in").as_posix() + output_path = (tmp_path / "requirements2.in").as_posix() + else: + input_path = "requirements.in" + output_path = "requirements2.in" + + with monkeypatch.context() as m: + m.chdir(tmp_path) + + out = runner.invoke( + cli, + [ + "--output-file", + "-", + "--quiet", + "--no-header", + "--no-emit-options", + "-r", + input_path, + ], + ) + + assert out.exit_code == 0 + assert out.stdout == dedent( + f"""\ + small-fake-a==0.2 + # via -r {output_path} + """ + ) + + +@pytest.mark.parametrize( + "test_files_collection", + ( + TestFilesCollection( + "parent_dir", + { + "requirements2.in": "small-fake-a\n", + "subdir/requirements.in": "-r ../requirements2.in\n", + }, + ), + TestFilesCollection( + "subdir", + { + "requirements.in": "-r ./subdir/requirements2.in", + "subdir/requirements2.in": "small-fake-a\n", + }, + ), + TestFilesCollection( + "sibling_dir", + { + "subdir1/requirements.in": "-r ../subdir2/requirements2.in", + "subdir2/requirements2.in": "small-fake-a\n", + }, + ), + ), + ids=str, +) +def test_second_order_requirements_relative_path_in_separate_dir( + pip_conf, + runner, + tmp_path, + monkeypatch, + test_files_collection, + pip_produces_absolute_paths, +): + """ + As in the above test, nested requirements file path handling. + But in this case, the primary variable is the relative location of the two + requirements. + + The expectation is that the output path will be relative to the current + working directory, *not* relative to the dir containing the initial + requirements file. + """ + test_files_collection.populate(tmp_path) + # the input is the path to 'requirements.in' relative to the starting dir + input_path = test_files_collection.get_path_to("requirements.in") + # the output should also be relative to the starting dir, the path to 'requirements2.in' + output_path = test_files_collection.get_path_to("requirements2.in") + + # for older pip versions, recompute the output path to be relative to the input path + if not pip_produces_absolute_paths: + # traverse upwards to the root tmp dir, and append the output path to that + # similar to pathlib.Path.relative_to(..., walk_up=True) + relative_segments = len(pathlib.Path(input_path).parents) - 1 + output_path = str( + pathlib.Path(input_path).parent / ("../" * relative_segments) / output_path + ) + # pathlib paths normalize `./foo` to `foo` automatically; de-norm that case + if str(pathlib.Path(input_path).parent) == ".": + output_path = "./" + output_path + + with monkeypatch.context() as m: + m.chdir(tmp_path) + out = runner.invoke( + cli, + [ + "--output-file", + "-", + "--quiet", + "--no-header", + "--no-emit-options", + "-r", + input_path, + ], + ) + + assert out.exit_code == 0 + assert out.stdout == dedent( + f"""\ + small-fake-a==0.2 + # via -r {output_path} + """ + ) From 694b9a1d9e6e6ba3245dba7cb0ff3c72d93ec052 Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Wed, 2 Jul 2025 09:58:05 -0500 Subject: [PATCH 6/8] Fix behavior and testing of Windows '-r' includes 1. As a small tweak in `pip_compat`, output relativized paths using `as_posix()` formatting. This makes output more consistent in this particular case, and therefore easier to test. 2. Update the path normalization tests for `-r`/`-c` usage to expressly handle the various cases around Windows paths correctly. These changes are aimed at getting to a state where tests are passing in CI, with no extra changes in the source tree. (Other than (1), above.) --- piptools/_compat/pip_compat.py | 2 +- tests/test_cli_compile.py | 38 ++++++++++++++++++++++------------ 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/piptools/_compat/pip_compat.py b/piptools/_compat/pip_compat.py index bc4fdc125..c2f77f65b 100644 --- a/piptools/_compat/pip_compat.py +++ b/piptools/_compat/pip_compat.py @@ -143,7 +143,7 @@ def _rewrite_absolute_comes_from_location(original_comes_from: str, /) -> str: return original_comes_from # make it relative to the current working dir - suffix = str(file_path.relative_to(pathlib.Path.cwd())) + suffix = file_path.relative_to(pathlib.Path.cwd()).as_posix() return f"{prefix} {suffix}" diff --git a/tests/test_cli_compile.py b/tests/test_cli_compile.py index 043558f0f..cd51a8a6e 100644 --- a/tests/test_cli_compile.py +++ b/tests/test_cli_compile.py @@ -3917,6 +3917,7 @@ def test_second_order_requirements_path_handling( runner, tmp_path, monkeypatch, + pip_produces_absolute_paths, input_path_absolute, test_files_collection, ): @@ -3924,23 +3925,29 @@ def test_second_order_requirements_path_handling( Given nested requirements files, the internal requirements will be written in the output, and it will be absolute or relative depending only on whether or not the initial path was absolute or relative. - - This is only accurate on pip>=24.3 , as earlier versions do not normalize - the `comes_from` data to absolute paths. - On lower pip versions, assert the previous output is produced, in which - `-r` inputs are preserved. """ test_files_collection.populate(tmp_path) # the input path is given on the CLI as absolute or relative # and this determines the expected output path as well if input_path_absolute: - input_path = (tmp_path / "requirements.in").as_posix() - output_path = (tmp_path / "requirements2.in").as_posix() + input_path = str(tmp_path / "requirements.in") + output_path = str(tmp_path / "requirements2.in") else: input_path = "requirements.in" output_path = "requirements2.in" + # on older pip versions, in which pip `comes_from` data is not normalized, + # override the expected output_path + # + # the second-order requirement will be preserved verbatim if the both paths + # are absolute on these pip versions + both_paths_are_absolute = ( + input_path_absolute and test_files_collection.name == "absolute_include" + ) + if not pip_produces_absolute_paths and both_paths_are_absolute: + output_path = (tmp_path / "requirements.in").read_text().partition(" ")[-1] + with monkeypatch.context() as m: m.chdir(tmp_path) @@ -4020,13 +4027,18 @@ def test_second_order_requirements_relative_path_in_separate_dir( if not pip_produces_absolute_paths: # traverse upwards to the root tmp dir, and append the output path to that # similar to pathlib.Path.relative_to(..., walk_up=True) + # + # the join must be done while preserving the original path (which may be posix-style) + # and the relative path part will be made in platform-specific path syntax + # but then the two parts will be joined with a forward-slash... + # + # on Windows, this can result in mixed slashes, e.g. + # `D:\tempstorage\abcdefg\sbudir/requirements2.in` + # + # this is the pip behavior, and we inherit it relative_segments = len(pathlib.Path(input_path).parents) - 1 - output_path = str( - pathlib.Path(input_path).parent / ("../" * relative_segments) / output_path - ) - # pathlib paths normalize `./foo` to `foo` automatically; de-norm that case - if str(pathlib.Path(input_path).parent) == ".": - output_path = "./" + output_path + relpath = str(pathlib.Path(input_path).parent / ("../" * relative_segments)) + output_path = f"{relpath}/{output_path}" with monkeypatch.context() as m: m.chdir(tmp_path) From 8f3cba7db08b5eb90330fd4ab161d2bb26881316 Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Thu, 3 Jul 2025 17:43:37 -0500 Subject: [PATCH 7/8] Always convert paths in 'comes_from' to posix In order to guarantee more consistent outputs, especially in the case of mixed paths with forward- and back-slashes, this changeset updates the `comes_from` handling to *always* apply a level of normalization. Paths passed back through the `parse_requirements()` wrapper will now always be formatted as posix paths. --- piptools/_compat/pip_compat.py | 43 ++++++++++++++++++++++++---------- tests/test_cli_compile.py | 31 +++++------------------- 2 files changed, 37 insertions(+), 37 deletions(-) diff --git a/piptools/_compat/pip_compat.py b/piptools/_compat/pip_compat.py index c2f77f65b..750bad470 100644 --- a/piptools/_compat/pip_compat.py +++ b/piptools/_compat/pip_compat.py @@ -86,22 +86,20 @@ def parse_requirements( isolated: bool = False, comes_from_stdin: bool = False, ) -> Iterator[InstallRequirement]: - # the `comes_from` data will be rewritten based on a number of conditions - # - # None do not rewrite - # callable programmatic rewrite - # str fixed rewrite - rewrite_comes_from: str | Callable[[str], str] | None + # the `comes_from` data will be rewritten in different ways in different conditions + # each rewrite rule is expressible as a str->str function or as a static replacement + rewrite_comes_from: Callable[[str], str] | str if comes_from_stdin: # if data is coming from stdin, then `comes_from="-r -"` rewrite_comes_from = "-r -" elif pathlib.Path(filename).is_absolute(): - rewrite_comes_from = None + # if the input path is absolute, just normalize paths to posix-style + rewrite_comes_from = _normalize_comes_from_location else: # if the input was a relative path, set the rewrite rule to rewrite # absolute paths to be relative - rewrite_comes_from = _rewrite_absolute_comes_from_location + rewrite_comes_from = _relativize_comes_from_location for parsed_req in _parse_requirements( filename, session, finder=finder, options=options, constraint=constraint @@ -114,14 +112,16 @@ def parse_requirements( file_link._url = parsed_req.requirement install_req.link = file_link install_req = copy_install_requirement(install_req) + if isinstance(rewrite_comes_from, str): install_req.comes_from = rewrite_comes_from - elif rewrite_comes_from is not None: + else: install_req.comes_from = rewrite_comes_from(install_req.comes_from) + yield install_req -def _rewrite_absolute_comes_from_location(original_comes_from: str, /) -> str: +def _relativize_comes_from_location(original_comes_from: str, /) -> str: """ This is the rewrite rule used when ``-r`` or ``-c`` appears in ``comes_from`` data with an absolute path. @@ -138,15 +138,34 @@ def _rewrite_absolute_comes_from_location(original_comes_from: str, /) -> str: file_path = pathlib.Path(suffix) - # if the path was not absolute, bail out + # if the path was not absolute, normalize to posix-style and finish processing if not file_path.is_absolute(): - return original_comes_from + return f"{prefix} {file_path.as_posix()}" # make it relative to the current working dir suffix = file_path.relative_to(pathlib.Path.cwd()).as_posix() return f"{prefix} {suffix}" +def _normalize_comes_from_location(original_comes_from: str, /) -> str: + """ + This is the rewrite rule when ``-r`` or ``-c`` appears in ``comes_from`` + data and the input path was absolute, meaning we should not relativize the locations. + + Instead, we apply minimal normalization, ensuring that posix-style paths are used. + """ + # require `-r` or `-c` as the source + if not original_comes_from.startswith(("-r ", "-c ")): + return original_comes_from + + # split on the space + prefix, _, suffix = original_comes_from.partition(" ") + + # convert to a posix-style path + suffix = pathlib.Path(suffix).as_posix() + return f"{prefix} {suffix}" + + def create_wheel_cache(cache_dir: str, format_control: str | None = None) -> WheelCache: kwargs: dict[str, str | None] = {"cache_dir": cache_dir} if PIP_VERSION[:2] <= (23, 0): diff --git a/tests/test_cli_compile.py b/tests/test_cli_compile.py index cd51a8a6e..fbccac0e7 100644 --- a/tests/test_cli_compile.py +++ b/tests/test_cli_compile.py @@ -1967,7 +1967,7 @@ def test_many_inputs_includes_all_annotations(pip_conf, runner, tmp_path, num_in "small-fake-a==0.1", " # via", ] - + [f" # -r {req_in}" for req_in in req_ins] + + [f" # -r {req_in.as_posix()}" for req_in in req_ins] ) + "\n" ) @@ -3931,23 +3931,12 @@ def test_second_order_requirements_path_handling( # the input path is given on the CLI as absolute or relative # and this determines the expected output path as well if input_path_absolute: - input_path = str(tmp_path / "requirements.in") - output_path = str(tmp_path / "requirements2.in") + input_path = (tmp_path / "requirements.in").as_posix() + output_path = (tmp_path / "requirements2.in").as_posix() else: input_path = "requirements.in" output_path = "requirements2.in" - # on older pip versions, in which pip `comes_from` data is not normalized, - # override the expected output_path - # - # the second-order requirement will be preserved verbatim if the both paths - # are absolute on these pip versions - both_paths_are_absolute = ( - input_path_absolute and test_files_collection.name == "absolute_include" - ) - if not pip_produces_absolute_paths and both_paths_are_absolute: - output_path = (tmp_path / "requirements.in").read_text().partition(" ")[-1] - with monkeypatch.context() as m: m.chdir(tmp_path) @@ -4027,18 +4016,10 @@ def test_second_order_requirements_relative_path_in_separate_dir( if not pip_produces_absolute_paths: # traverse upwards to the root tmp dir, and append the output path to that # similar to pathlib.Path.relative_to(..., walk_up=True) - # - # the join must be done while preserving the original path (which may be posix-style) - # and the relative path part will be made in platform-specific path syntax - # but then the two parts will be joined with a forward-slash... - # - # on Windows, this can result in mixed slashes, e.g. - # `D:\tempstorage\abcdefg\sbudir/requirements2.in` - # - # this is the pip behavior, and we inherit it relative_segments = len(pathlib.Path(input_path).parents) - 1 - relpath = str(pathlib.Path(input_path).parent / ("../" * relative_segments)) - output_path = f"{relpath}/{output_path}" + output_path = ( + pathlib.Path(input_path).parent / ("../" * relative_segments) / output_path + ).as_posix() with monkeypatch.context() as m: m.chdir(tmp_path) From 19c6294e8e41cd4720d1f330dace484729670825 Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Tue, 8 Jul 2025 11:36:55 -0500 Subject: [PATCH 8/8] Apply various suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Minor fixups and adjustments - Update several docstrings to PEP 257 style, with an imperative description as the first line - Simplify `rewrite_comes_from` by making it always a callable Co-authored-by: 🇺🇦 Sviatoslav Sydorenko (Святослав Сидоренко) --- piptools/_compat/pip_compat.py | 38 ++++++++++++++++------------ tests/test_cli_compile.py | 45 +++++++++++++++++----------------- 2 files changed, 45 insertions(+), 38 deletions(-) diff --git a/piptools/_compat/pip_compat.py b/piptools/_compat/pip_compat.py index 750bad470..5caf0056a 100644 --- a/piptools/_compat/pip_compat.py +++ b/piptools/_compat/pip_compat.py @@ -87,12 +87,12 @@ def parse_requirements( comes_from_stdin: bool = False, ) -> Iterator[InstallRequirement]: # the `comes_from` data will be rewritten in different ways in different conditions - # each rewrite rule is expressible as a str->str function or as a static replacement - rewrite_comes_from: Callable[[str], str] | str + # each rewrite rule is expressible as a str->str function + rewrite_comes_from: Callable[[str], str] if comes_from_stdin: # if data is coming from stdin, then `comes_from="-r -"` - rewrite_comes_from = "-r -" + rewrite_comes_from = _rewrite_comes_from_to_hardcoded_stdin_value elif pathlib.Path(filename).is_absolute(): # if the input path is absolute, just normalize paths to posix-style rewrite_comes_from = _normalize_comes_from_location @@ -113,28 +113,32 @@ def parse_requirements( install_req.link = file_link install_req = copy_install_requirement(install_req) - if isinstance(rewrite_comes_from, str): - install_req.comes_from = rewrite_comes_from - else: - install_req.comes_from = rewrite_comes_from(install_req.comes_from) + install_req.comes_from = rewrite_comes_from(install_req.comes_from) yield install_req +def _rewrite_comes_from_to_hardcoded_stdin_value(_: str, /) -> str: + """Produce the hardcoded ``comes_from`` value for stdin.""" + return "-r -" + + def _relativize_comes_from_location(original_comes_from: str, /) -> str: """ + Convert a ``comes_from`` path to a relative posix path. + This is the rewrite rule used when ``-r`` or ``-c`` appears in ``comes_from`` data with an absolute path. - The ``-r`` or ``-c`` qualifier is retained, and the path is relativized - with respect to the CWD. + The ``-r`` or ``-c`` qualifier is retained, the path is relativized + with respect to the CWD, and the path is converted to posix style. """ # require `-r` or `-c` as the source if not original_comes_from.startswith(("-r ", "-c ")): return original_comes_from # split on the space - prefix, _, suffix = original_comes_from.partition(" ") + prefix, space_sep, suffix = original_comes_from.partition(" ") file_path = pathlib.Path(suffix) @@ -144,26 +148,30 @@ def _relativize_comes_from_location(original_comes_from: str, /) -> str: # make it relative to the current working dir suffix = file_path.relative_to(pathlib.Path.cwd()).as_posix() - return f"{prefix} {suffix}" + return f"{prefix}{space_sep}{suffix}" def _normalize_comes_from_location(original_comes_from: str, /) -> str: """ + Convert a ``comes_from`` path to a posix-style path. + This is the rewrite rule when ``-r`` or ``-c`` appears in ``comes_from`` - data and the input path was absolute, meaning we should not relativize the locations. + data and the input path was absolute, meaning we should not relativize the + locations. - Instead, we apply minimal normalization, ensuring that posix-style paths are used. + The ``-r`` or ``-c`` qualifier is retained, and the path is converted to + posix style. """ # require `-r` or `-c` as the source if not original_comes_from.startswith(("-r ", "-c ")): return original_comes_from # split on the space - prefix, _, suffix = original_comes_from.partition(" ") + prefix, space_sep, suffix = original_comes_from.partition(" ") # convert to a posix-style path suffix = pathlib.Path(suffix).as_posix() - return f"{prefix} {suffix}" + return f"{prefix}{space_sep}{suffix}" def create_wheel_cache(cache_dir: str, format_control: str | None = None) -> WheelCache: diff --git a/tests/test_cli_compile.py b/tests/test_cli_compile.py index fbccac0e7..7f862b5b3 100644 --- a/tests/test_cli_compile.py +++ b/tests/test_cli_compile.py @@ -84,9 +84,9 @@ def populate(self, tmp_path: pathlib.Path) -> None: def get_path_to(self, filename: str) -> str: """Given a filename, find the (first) path to that filename in the contents.""" return next( - k - for k in self.contents.keys() - if (k == filename) or k.endswith(f"/{filename}") + stub_file_path + for stub_file_path in self.contents + if (stub_file_path == filename) or stub_file_path.endswith(f"/{filename}") ) @@ -3891,7 +3891,9 @@ def test_stdout_should_not_be_read_when_stdin_is_not_a_plain_file( assert out.exit_code == 0, out -@pytest.mark.parametrize("input_path_absolute", (True, False)) +@pytest.mark.parametrize( + "input_path_absolute", (True, False), ids=("absolute-input", "relative-input") +) @pytest.mark.parametrize( "test_files_collection", ( @@ -3922,23 +3924,22 @@ def test_second_order_requirements_path_handling( test_files_collection, ): """ - Given nested requirements files, the internal requirements will be written - in the output, and it will be absolute or relative depending only on - whether or not the initial path was absolute or relative. + Test normalization of ``-r`` includes in output. + + Given nested requirements files, the internal requirements file path will + be written in the output, and it will be absolute or relative depending + only on whether or not the initial path was absolute or relative. """ test_files_collection.populate(tmp_path) # the input path is given on the CLI as absolute or relative # and this determines the expected output path as well - if input_path_absolute: - input_path = (tmp_path / "requirements.in").as_posix() - output_path = (tmp_path / "requirements2.in").as_posix() - else: - input_path = "requirements.in" - output_path = "requirements2.in" + input_dir_path = tmp_path if input_path_absolute else pathlib.Path(".") + input_path = (input_dir_path / "requirements.in").as_posix() + output_path = (input_dir_path / "requirements2.in").as_posix() - with monkeypatch.context() as m: - m.chdir(tmp_path) + with monkeypatch.context() as revertable_ctx: + revertable_ctx.chdir(tmp_path) out = runner.invoke( cli, @@ -3998,13 +3999,11 @@ def test_second_order_requirements_relative_path_in_separate_dir( pip_produces_absolute_paths, ): """ - As in the above test, nested requirements file path handling. - But in this case, the primary variable is the relative location of the two - requirements. + Test normalization of ``-r`` includes when the requirements files are in + distinct directories. - The expectation is that the output path will be relative to the current - working directory, *not* relative to the dir containing the initial - requirements file. + Confirm that the output path will be relative to the current working + directory. """ test_files_collection.populate(tmp_path) # the input is the path to 'requirements.in' relative to the starting dir @@ -4021,8 +4020,8 @@ def test_second_order_requirements_relative_path_in_separate_dir( pathlib.Path(input_path).parent / ("../" * relative_segments) / output_path ).as_posix() - with monkeypatch.context() as m: - m.chdir(tmp_path) + with monkeypatch.context() as revertable_ctx: + revertable_ctx.chdir(tmp_path) out = runner.invoke( cli, [