Treat --upgrade-packages PKGSPECs as constraints (not just minimums…

…), consistently (#1578)

* Also treat --upgrade-packages as '-c constraints.txt' style constraints

Ensure temporary constraints file gets cleaned up
in a way that avoids Windows file handling complications

* Don't add upgrade pkgs which are also existing pins as regular reqs

Remove leftovers of now unused existing_pins_to_upgrade set

* Parametrize test_upgrade_packages_option_subdependency

... to cover when the subdependency is not already pinned

* Parametrize combined --upgrade and -P test

... to cover when the -P pkgs are only subdependencies

piptools/scripts/compile.py

@@ -425,10 +425,7 @@ def cli(
         key_from_ireq(install_req): install_req for install_req in upgrade_reqs_gen
     }
 
-    existing_pins_to_upgrade = set()
-
-    # Exclude packages from --upgrade-package/-P from the existing
-    # constraints, and separately gather pins to be upgraded
+    # Exclude packages from --upgrade-package/-P from the existing constraints
     existing_pins = {}
 
     # Proxy with a LocalRequirementsRepository if --upgrade is not specified
@@ -446,9 +443,7 @@ def cli(
 
         for ireq in filter(is_pinned_requirement, ireqs):
             key = key_from_ireq(ireq)
-            if key in upgrade_install_reqs:
-                existing_pins_to_upgrade.add(key)
-            else:
+            if key not in upgrade_install_reqs:
                 existing_pins[key] = ireq
         repository = LocalRequirementsRepository(
             existing_pins, repository, reuse_hashes=reuse_hashes
@@ -515,6 +510,27 @@ def cli(
                 )
             )
 
+    if upgrade_packages:
+        constraints_file = tempfile.NamedTemporaryFile(mode="wt", delete=False)
+        constraints_file.write("\n".join(upgrade_packages))
+        constraints_file.flush()
+        try:
+            reqs = list(
+                parse_requirements(
+                    constraints_file.name,
+                    finder=repository.finder,
+                    session=repository.session,
+                    options=repository.options,
+                    constraint=True,
+                )
+            )
+        finally:
+            constraints_file.close()
+            os.unlink(constraints_file.name)
+        for req in reqs:
+            req.comes_from = None
+        constraints.extend(reqs)
+
     extras = tuple(itertools.chain.from_iterable(ex.split(",") for ex in extras))
 
     if extras and not setup_file_found:
@@ -525,9 +541,8 @@ def cli(
         key_from_ireq(ireq) for ireq in constraints if not ireq.constraint
     }
 
-    allowed_upgrades = primary_packages | existing_pins_to_upgrade
     constraints.extend(
-        ireq for key, ireq in upgrade_install_reqs.items() if key in allowed_upgrades
+        ireq for key, ireq in upgrade_install_reqs.items() if key in primary_packages
     )
 
     constraints = [req for req in constraints if req.match_markers(extras)]

tests/test_cli_compile.py

@@ -894,13 +894,20 @@ def test_upgrade_packages_version_option_no_existing_file(pip_conf, runner):
     assert "small-fake-b==0.2" in out.stderr
 
 
-def test_upgrade_packages_version_option_and_upgrade(pip_conf, runner):
+@pytest.mark.parametrize(
+    "reqs_in",
+    (
+        pytest.param("small-fake-a\nsmall-fake-b", id="direct reqs"),
+        pytest.param("small-fake-with-unpinned-deps", id="parent req"),
+    ),
+)
+def test_upgrade_packages_version_option_and_upgrade(pip_conf, runner, reqs_in):
     """
     piptools respects --upgrade-package/-P inline list with specified versions
     whilst also doing --upgrade.
     """
     with open("requirements.in", "w") as req_in:
-        req_in.write("small-fake-a\nsmall-fake-b")
+        req_in.write(reqs_in)
     with open("requirements.txt", "w") as req_in:
         req_in.write("small-fake-a==0.1\nsmall-fake-b==0.1")
 
@@ -1895,6 +1902,7 @@ def test_unreachable_index_urls(runner, cli_options, expected_message):
     assert expected_message in stderr_lines
 
 
+@pytest.mark.parametrize("subdep_already_pinned", (True, False))
 @pytest.mark.parametrize(
     ("current_package", "upgraded_package"),
     (
@@ -1903,7 +1911,7 @@ def test_unreachable_index_urls(runner, cli_options, expected_message):
     ),
 )
 def test_upgrade_packages_option_subdependency(
-    pip_conf, runner, current_package, upgraded_package
+    pip_conf, runner, current_package, upgraded_package, subdep_already_pinned
 ):
     """
     Test that pip-compile --upgrade-package/-P upgrades/downgrades subdependencies.
@@ -1914,7 +1922,8 @@ def test_upgrade_packages_option_subdependency(
 
     with open("requirements.txt", "w") as reqs:
         reqs.write("small-fake-a==0.1\n")
-        reqs.write(current_package + "\n")
+        if subdep_already_pinned:
+            reqs.write(current_package + "\n")
         reqs.write("small-fake-with-unpinned-deps==0.1\n")
 
     out = runner.invoke(