From 7ebcb500148cc8c774b96b69032bdb01daaba870 Mon Sep 17 00:00:00 2001 From: IceTDrinker <49040125+IceTDrinker@users.noreply.github.com> Date: Sat, 13 Feb 2021 16:04:12 +0100 Subject: [PATCH 01/11] Consider specifiers for equality operator to pin a dependency in make_install_requirement - Changed function signature to take ireq directly rather than passing its members one by one - Changed type hint for version as I was getting a pip Version rather than a string, maybe there are some parts of the code calling with string though - Can confirm it generates a pinned torch with === in my use case as expected --- piptools/repositories/local.py | 2 +- piptools/repositories/pypi.py | 3 +-- piptools/utils.py | 13 +++++++++++-- tests/conftest.py | 2 +- 4 files changed, 14 insertions(+), 6 deletions(-) diff --git a/piptools/repositories/local.py b/piptools/repositories/local.py index e2a96be4d..bc9851656 100644 --- a/piptools/repositories/local.py +++ b/piptools/repositories/local.py @@ -64,7 +64,7 @@ def find_best_match(self, ireq, prereleases=None): if existing_pin and ireq_satisfied_by_existing_pin(ireq, existing_pin): project, version, _ = as_tuple(existing_pin) return make_install_requirement( - project, version, ireq.extras, constraint=ireq.constraint + project, version, ireq ) else: return self.repository.find_best_match(ireq, prereleases) diff --git a/piptools/repositories/pypi.py b/piptools/repositories/pypi.py index 433187dda..d64f48027 100644 --- a/piptools/repositories/pypi.py +++ b/piptools/repositories/pypi.py @@ -159,8 +159,7 @@ def find_best_match(self, ireq, prereleases=None): return make_install_requirement( best_candidate.name, best_candidate.version, - ireq.extras, - constraint=ireq.constraint, + ireq, ) def resolve_reqs(self, download_dir, ireq, wheel_cache): diff --git a/piptools/utils.py b/piptools/utils.py index 5ed2c38b9..3bea0be1e 100644 --- a/piptools/utils.py +++ b/piptools/utils.py @@ -21,6 +21,7 @@ from pip._internal.vcs import is_url from pip._vendor.packaging.markers import Marker from pip._vendor.packaging.specifiers import SpecifierSet +from pip._vendor.packaging.version import Version _KT = TypeVar("_KT") _VT = TypeVar("_VT") @@ -66,16 +67,24 @@ def comment(text: str) -> str: def make_install_requirement( - name: str, version: str, extras: Iterable[str], constraint: bool = False + name: str, version: Version, ireq: InstallRequirement ) -> InstallRequirement: # If no extras are specified, the extras string is blank extras_string = "" + extras = ireq.extras if extras: # Sort extras for stability extras_string = f"[{','.join(sorted(extras))}]" + version_specifier = "==" + for specifier in ireq.specifier: + specifier_version = Version(specifier.version) + if specifier.operator == "===" and specifier_version == version: + version_specifier = "===" + break + return install_req_from_line( - str(f"{name}{extras_string}=={version}"), constraint=constraint + str(f"{name}{extras_string}{version_specifier}{version}"), constraint=ireq.constraint ) diff --git a/tests/conftest.py b/tests/conftest.py index 0936845ed..4a7d2b36d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -69,7 +69,7 @@ def find_best_match(self, ireq, prereleases=False): raise NoCandidateFound(ireq, tried_versions, ["https://fake.url.foo"]) best_version = max(versions, key=Version) return make_install_requirement( - key_from_ireq(ireq), best_version, ireq.extras, constraint=ireq.constraint + key_from_ireq(ireq), best_version, ireq ) def get_dependencies(self, ireq): From 37c84a7550c5ba41c5591c009b95768f2f88cd73 Mon Sep 17 00:00:00 2001 From: IceTDrinker <49040125+IceTDrinker@users.noreply.github.com> Date: Sat, 13 Feb 2021 16:29:54 +0100 Subject: [PATCH 02/11] Apply tox -e checkqa --- piptools/repositories/local.py | 4 +--- piptools/utils.py | 3 ++- tests/conftest.py | 4 +--- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/piptools/repositories/local.py b/piptools/repositories/local.py index bc9851656..4cb04178b 100644 --- a/piptools/repositories/local.py +++ b/piptools/repositories/local.py @@ -63,9 +63,7 @@ def find_best_match(self, ireq, prereleases=None): existing_pin = self.existing_pins.get(key) if existing_pin and ireq_satisfied_by_existing_pin(ireq, existing_pin): project, version, _ = as_tuple(existing_pin) - return make_install_requirement( - project, version, ireq - ) + return make_install_requirement(project, version, ireq) else: return self.repository.find_best_match(ireq, prereleases) diff --git a/piptools/utils.py b/piptools/utils.py index 3bea0be1e..c3af5de7c 100644 --- a/piptools/utils.py +++ b/piptools/utils.py @@ -84,7 +84,8 @@ def make_install_requirement( break return install_req_from_line( - str(f"{name}{extras_string}{version_specifier}{version}"), constraint=ireq.constraint + str(f"{name}{extras_string}{version_specifier}{version}"), + constraint=ireq.constraint, ) diff --git a/tests/conftest.py b/tests/conftest.py index 4a7d2b36d..c7021951c 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -68,9 +68,7 @@ def find_best_match(self, ireq, prereleases=False): ] raise NoCandidateFound(ireq, tried_versions, ["https://fake.url.foo"]) best_version = max(versions, key=Version) - return make_install_requirement( - key_from_ireq(ireq), best_version, ireq - ) + return make_install_requirement(key_from_ireq(ireq), best_version, ireq) def get_dependencies(self, ireq): if ireq.editable or is_url_requirement(ireq): From 6a3b232ee34bd251456261ed6f8e1af3b29a1fc0 Mon Sep 17 00:00:00 2001 From: IceTDrinker <49040125+IceTDrinker@users.noreply.github.com> Date: Sat, 13 Feb 2021 16:47:29 +0100 Subject: [PATCH 03/11] Prefer forcing version to string rather than building a Version object from a specifier version as it made tests fail --- piptools/utils.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/piptools/utils.py b/piptools/utils.py index c3af5de7c..af011ced3 100644 --- a/piptools/utils.py +++ b/piptools/utils.py @@ -67,7 +67,7 @@ def comment(text: str) -> str: def make_install_requirement( - name: str, version: Version, ireq: InstallRequirement + name: str, version: Union[str, Version], ireq: InstallRequirement ) -> InstallRequirement: # If no extras are specified, the extras string is blank extras_string = "" @@ -78,8 +78,7 @@ def make_install_requirement( version_specifier = "==" for specifier in ireq.specifier: - specifier_version = Version(specifier.version) - if specifier.operator == "===" and specifier_version == version: + if specifier.operator == "===" and specifier.version == str(version): version_specifier = "===" break From 12dace5f7f4af2272ef1faa68dd61a3d6831a88e Mon Sep 17 00:00:00 2001 From: IceTDrinker <49040125+IceTDrinker@users.noreply.github.com> Date: Sun, 14 Feb 2021 10:18:56 +0100 Subject: [PATCH 04/11] Rename variables to have clearer names --- piptools/utils.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/piptools/utils.py b/piptools/utils.py index af011ced3..cde26a5e1 100644 --- a/piptools/utils.py +++ b/piptools/utils.py @@ -76,14 +76,15 @@ def make_install_requirement( # Sort extras for stability extras_string = f"[{','.join(sorted(extras))}]" - version_specifier = "==" + version_pin_operator = "==" + version_as_str = str(version) for specifier in ireq.specifier: - if specifier.operator == "===" and specifier.version == str(version): - version_specifier = "===" + if specifier.operator == "===" and specifier.version == version_as_str: + version_pin_operator = "===" break return install_req_from_line( - str(f"{name}{extras_string}{version_specifier}{version}"), + str(f"{name}{extras_string}{version_pin_operator}{version}"), constraint=ireq.constraint, ) From eface673f0a85412d1a5b7f9d169928279ade875 Mon Sep 17 00:00:00 2001 From: IceTDrinker <49040125+IceTDrinker@users.noreply.github.com> Date: Sun, 14 Feb 2021 10:21:06 +0100 Subject: [PATCH 05/11] Write test for requirements pinned with === and multiple pin sources - Use case example where torch has multiple pin sources --- tests/test_cli_compile.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/test_cli_compile.py b/tests/test_cli_compile.py index 603d3f83f..158898950 100644 --- a/tests/test_cli_compile.py +++ b/tests/test_cli_compile.py @@ -1555,3 +1555,27 @@ def test_duplicate_reqs_combined( assert out.exit_code == 0, out assert str(test_package_2) in out.stderr assert "test-package-1==0.1" in out.stderr + + +@pytest.mark.network +def test_triple_equal_pinned_depency_is_used(runner): + """ + Test that pip-compile properly emits the pinned requirement with === + torchvision 0.8.2 requires torch==1.7.1 which can resolve to versions with + patches (e.g. torch 1.7.1+cu110), we want torch===1.7.1 without patches + """ + with open("requirements.in", "w") as reqs_in: + reqs_in.write("torch===1.7.1\n") + reqs_in.write("torchvision===0.8.2\n") + + out = runner.invoke( + cli, + [ + "--find-links", + "https://download.pytorch.org/whl/torch_stable.html", + ], + ) + + assert out.exit_code == 0, out + assert "torch===1.7.1" in out.stderr + assert "torchvision===0.8.2" in out.stderr From 37029fd955f3c873d342526f00abd25e891fbe0b Mon Sep 17 00:00:00 2001 From: IceTDrinker <49040125+IceTDrinker@users.noreply.github.com> Date: Sun, 14 Feb 2021 11:16:46 +0100 Subject: [PATCH 06/11] Use fake test packages which demonstrates the torch use-case --- tests/test_cli_compile.py | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/tests/test_cli_compile.py b/tests/test_cli_compile.py index 158898950..25d8fa4a3 100644 --- a/tests/test_cli_compile.py +++ b/tests/test_cli_compile.py @@ -1558,24 +1558,29 @@ def test_duplicate_reqs_combined( @pytest.mark.network -def test_triple_equal_pinned_depency_is_used(runner): +def test_triple_equal_pinned_depency_is_used(runner, make_package, make_wheel, tmpdir): """ Test that pip-compile properly emits the pinned requirement with === torchvision 0.8.2 requires torch==1.7.1 which can resolve to versions with patches (e.g. torch 1.7.1+cu110), we want torch===1.7.1 without patches """ - with open("requirements.in", "w") as reqs_in: - reqs_in.write("torch===1.7.1\n") - reqs_in.write("torchvision===0.8.2\n") - out = runner.invoke( - cli, - [ - "--find-links", - "https://download.pytorch.org/whl/torch_stable.html", - ], + dists_dir = tmpdir / "dists" + + test_package_1 = make_package("test_package_1", version="1.7.1") + make_wheel(test_package_1, dists_dir) + + test_package_2 = make_package( + "test_package_2", version="0.8.2", install_requires=["test-package-1==1.7.1"] ) + make_wheel(test_package_2, dists_dir) + + with open("requirements.in", "w") as reqs_in: + reqs_in.write("test-package-1===1.7.1\n") + reqs_in.write("test-package-2===0.8.2\n") + + out = runner.invoke(cli, ["--find-links", str(dists_dir)]) assert out.exit_code == 0, out - assert "torch===1.7.1" in out.stderr - assert "torchvision===0.8.2" in out.stderr + assert "test-package-1===1.7.1" in out.stderr + assert "test-package-2===0.8.2" in out.stderr From 27119409e226537454c3bd564fb432cf67e5d9c5 Mon Sep 17 00:00:00 2001 From: IceTDrinker <49040125+IceTDrinker@users.noreply.github.com> Date: Sun, 14 Feb 2021 11:25:54 +0100 Subject: [PATCH 07/11] Network is not required by the test anymore, remove it --- tests/test_cli_compile.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_cli_compile.py b/tests/test_cli_compile.py index 25d8fa4a3..a619ae89a 100644 --- a/tests/test_cli_compile.py +++ b/tests/test_cli_compile.py @@ -1557,7 +1557,6 @@ def test_duplicate_reqs_combined( assert "test-package-1==0.1" in out.stderr -@pytest.mark.network def test_triple_equal_pinned_depency_is_used(runner, make_package, make_wheel, tmpdir): """ Test that pip-compile properly emits the pinned requirement with === From 04154d2c5516d4b457fdd2580903db17bf13b4cd Mon Sep 17 00:00:00 2001 From: IceTDrinker <49040125+IceTDrinker@users.noreply.github.com> Date: Mon, 15 Feb 2021 09:23:45 +0100 Subject: [PATCH 08/11] Pin test_package_2 with == operator --- tests/test_cli_compile.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/test_cli_compile.py b/tests/test_cli_compile.py index a619ae89a..9b59d710a 100644 --- a/tests/test_cli_compile.py +++ b/tests/test_cli_compile.py @@ -1557,7 +1557,9 @@ def test_duplicate_reqs_combined( assert "test-package-1==0.1" in out.stderr -def test_triple_equal_pinned_depency_is_used(runner, make_package, make_wheel, tmpdir): +def test_triple_equal_pinned_dependency_is_used( + runner, make_package, make_wheel, tmpdir +): """ Test that pip-compile properly emits the pinned requirement with === torchvision 0.8.2 requires torch==1.7.1 which can resolve to versions with @@ -1576,10 +1578,10 @@ def test_triple_equal_pinned_depency_is_used(runner, make_package, make_wheel, t with open("requirements.in", "w") as reqs_in: reqs_in.write("test-package-1===1.7.1\n") - reqs_in.write("test-package-2===0.8.2\n") + reqs_in.write("test-package-2==0.8.2\n") out = runner.invoke(cli, ["--find-links", str(dists_dir)]) assert out.exit_code == 0, out assert "test-package-1===1.7.1" in out.stderr - assert "test-package-2===0.8.2" in out.stderr + assert "test-package-2==0.8.2" in out.stderr From f77201c09a3ed1e58c2fca3af727e10a009267ae Mon Sep 17 00:00:00 2001 From: IceTDrinker <49040125+IceTDrinker@users.noreply.github.com> Date: Mon, 15 Feb 2021 10:28:30 +0100 Subject: [PATCH 09/11] Add test cases to check different combinations of pins --- tests/test_cli_compile.py | 43 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/tests/test_cli_compile.py b/tests/test_cli_compile.py index 9b59d710a..4c52d2b67 100644 --- a/tests/test_cli_compile.py +++ b/tests/test_cli_compile.py @@ -1576,6 +1576,26 @@ def test_triple_equal_pinned_dependency_is_used( ) make_wheel(test_package_2, dists_dir) + # Case 1 === + with open("requirements.in", "w") as reqs_in: + reqs_in.write("test-package-1===1.7.1\n") + + out = runner.invoke(cli, ["--find-links", str(dists_dir)]) + + assert out.exit_code == 0, out + assert "test-package-1===1.7.1" in out.stderr + + # Case 2 == + with open("requirements.in", "w") as reqs_in: + reqs_in.write("test-package-1==1.7.1\n") + + out = runner.invoke(cli, ["--find-links", str(dists_dir)]) + + assert out.exit_code == 0, out + assert "test-package-1==1.7.1" in out.stderr + + # Case 3 test_package_1 pinned by user with === + # but pinned by package with ==, prefer === with open("requirements.in", "w") as reqs_in: reqs_in.write("test-package-1===1.7.1\n") reqs_in.write("test-package-2==0.8.2\n") @@ -1585,3 +1605,26 @@ def test_triple_equal_pinned_dependency_is_used( assert out.exit_code == 0, out assert "test-package-1===1.7.1" in out.stderr assert "test-package-2==0.8.2" in out.stderr + + # Case 4 test_package_1 pinned by user with === + # but pinned by package with ==, prefer === + # Different pin for test_package_2 + with open("requirements.in", "w") as reqs_in: + reqs_in.write("test-package-1===1.7.1\n") + reqs_in.write("test-package-2===0.8.2\n") + + out = runner.invoke(cli, ["--find-links", str(dists_dir)]) + + assert out.exit_code == 0, out + assert "test-package-1===1.7.1" in out.stderr + assert "test-package-2===0.8.2" in out.stderr + + # Case 5 only package 2 pinned with === + with open("requirements.in", "w") as reqs_in: + reqs_in.write("test-package-2===0.8.2\n") + + out = runner.invoke(cli, ["--find-links", str(dists_dir)]) + + assert out.exit_code == 0, out + assert "test-package-1==1.7.1" in out.stderr + assert "test-package-2===0.8.2" in out.stderr From dfb28ac38c14c1e97458f354e139600cefcb7c81 Mon Sep 17 00:00:00 2001 From: IceTDrinker <49040125+IceTDrinker@users.noreply.github.com> Date: Fri, 19 Feb 2021 20:39:40 +0100 Subject: [PATCH 10/11] Parametrize test, use simple versions --- tests/test_cli_compile.py | 93 ++++++++++++++++++--------------------- 1 file changed, 43 insertions(+), 50 deletions(-) diff --git a/tests/test_cli_compile.py b/tests/test_cli_compile.py index 4c52d2b67..96eaaf635 100644 --- a/tests/test_cli_compile.py +++ b/tests/test_cli_compile.py @@ -1557,8 +1557,44 @@ def test_duplicate_reqs_combined( assert "test-package-1==0.1" in out.stderr +@pytest.mark.parametrize( + ("pkg2_install_requires", "req_in_content", "out_expected_content"), + ( + pytest.param( + "", + ["test-package-1===0.1.0\n"], + ["test-package-1===0.1.0"], + ), + pytest.param( + "", + ["test-package-1==0.1.0\n"], + ["test-package-1==0.1.0"], + ), + pytest.param( + "test-package-1==0.1.0", + ["test-package-1===0.1.0\n", "test-package-2==0.1.0\n"], + ["test-package-1===0.1.0", "test-package-2==0.1.0"], + ), + pytest.param( + "test-package-1==0.1.0", + ["test-package-1===0.1.0\n", "test-package-2===0.1.0\n"], + ["test-package-1===0.1.0", "test-package-2===0.1.0"], + ), + pytest.param( + "test-package-1==0.1.0", + ["test-package-2===0.1.0\n"], + ["test-package-1==0.1.0", "test-package-2===0.1.0"], + ), + ), +) def test_triple_equal_pinned_dependency_is_used( - runner, make_package, make_wheel, tmpdir + runner, + make_package, + make_wheel, + tmpdir, + pkg2_install_requires, + req_in_content, + out_expected_content, ): """ Test that pip-compile properly emits the pinned requirement with === @@ -1568,63 +1604,20 @@ def test_triple_equal_pinned_dependency_is_used( dists_dir = tmpdir / "dists" - test_package_1 = make_package("test_package_1", version="1.7.1") + test_package_1 = make_package("test_package_1", version="0.1.0") make_wheel(test_package_1, dists_dir) test_package_2 = make_package( - "test_package_2", version="0.8.2", install_requires=["test-package-1==1.7.1"] + "test_package_2", version="0.1.0", install_requires=[pkg2_install_requires] ) make_wheel(test_package_2, dists_dir) - # Case 1 === - with open("requirements.in", "w") as reqs_in: - reqs_in.write("test-package-1===1.7.1\n") - - out = runner.invoke(cli, ["--find-links", str(dists_dir)]) - - assert out.exit_code == 0, out - assert "test-package-1===1.7.1" in out.stderr - - # Case 2 == - with open("requirements.in", "w") as reqs_in: - reqs_in.write("test-package-1==1.7.1\n") - - out = runner.invoke(cli, ["--find-links", str(dists_dir)]) - - assert out.exit_code == 0, out - assert "test-package-1==1.7.1" in out.stderr - - # Case 3 test_package_1 pinned by user with === - # but pinned by package with ==, prefer === - with open("requirements.in", "w") as reqs_in: - reqs_in.write("test-package-1===1.7.1\n") - reqs_in.write("test-package-2==0.8.2\n") - - out = runner.invoke(cli, ["--find-links", str(dists_dir)]) - - assert out.exit_code == 0, out - assert "test-package-1===1.7.1" in out.stderr - assert "test-package-2==0.8.2" in out.stderr - - # Case 4 test_package_1 pinned by user with === - # but pinned by package with ==, prefer === - # Different pin for test_package_2 - with open("requirements.in", "w") as reqs_in: - reqs_in.write("test-package-1===1.7.1\n") - reqs_in.write("test-package-2===0.8.2\n") - - out = runner.invoke(cli, ["--find-links", str(dists_dir)]) - - assert out.exit_code == 0, out - assert "test-package-1===1.7.1" in out.stderr - assert "test-package-2===0.8.2" in out.stderr - - # Case 5 only package 2 pinned with === with open("requirements.in", "w") as reqs_in: - reqs_in.write("test-package-2===0.8.2\n") + for line in req_in_content: + reqs_in.write(line) out = runner.invoke(cli, ["--find-links", str(dists_dir)]) assert out.exit_code == 0, out - assert "test-package-1==1.7.1" in out.stderr - assert "test-package-2===0.8.2" in out.stderr + for line in out_expected_content: + assert line in out.stderr From 83382722f8e06e55f267e39bc4863c3e3aafbe0e Mon Sep 17 00:00:00 2001 From: IceTDrinker <49040125+IceTDrinker@users.noreply.github.com> Date: Wed, 24 Feb 2021 09:38:59 +0100 Subject: [PATCH 11/11] Add test ids --- tests/test_cli_compile.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/test_cli_compile.py b/tests/test_cli_compile.py index 96eaaf635..5f50a27aa 100644 --- a/tests/test_cli_compile.py +++ b/tests/test_cli_compile.py @@ -1564,26 +1564,31 @@ def test_duplicate_reqs_combined( "", ["test-package-1===0.1.0\n"], ["test-package-1===0.1.0"], + id="pin package with ===", ), pytest.param( "", ["test-package-1==0.1.0\n"], ["test-package-1==0.1.0"], + id="pin package with ==", ), pytest.param( "test-package-1==0.1.0", ["test-package-1===0.1.0\n", "test-package-2==0.1.0\n"], ["test-package-1===0.1.0", "test-package-2==0.1.0"], + id="dep === pin preferred over == pin, main package == pin", ), pytest.param( "test-package-1==0.1.0", ["test-package-1===0.1.0\n", "test-package-2===0.1.0\n"], ["test-package-1===0.1.0", "test-package-2===0.1.0"], + id="dep === pin preferred over == pin, main package === pin", ), pytest.param( "test-package-1==0.1.0", ["test-package-2===0.1.0\n"], ["test-package-1==0.1.0", "test-package-2===0.1.0"], + id="dep == pin conserved, main package === pin", ), ), )