Repository: pypa/pip
Pull request #12306 (2023-10-01 12:47:52)

Use `-r=...` instead of `-r ...` for VCS CLI
This ensures that the resulting revision can not be misinterpreted as an option.

<!---
Thank you for your soon to be pull request. Before you submit this, please
double check to make sure that you've added a news file fragment. In pip we
generate our NEWS.rst from multiple news fragment files, and all pull requests
require either a news file fragment or a marker to indicate they don't require
one.

To read more about adding a news file fragment for your PR, please check out
our documentation at: https://pip.pypa.io/en/latest/development/contributing/#news-entries
-->


sbidoul: Are we 100% sure this change works for svn and bzr too? Otherwise I'd do it for mercurial only.
pradyunsg: I figured we'd find out via CI, but doing it just for hg seems reasonable to me!

Patch:
diff --git a/news/12306.bugfix.rst b/news/12306.bugfix.rst
new file mode 100644
index 000000000..eb6eecaaf
--- /dev/null
+++ b/news/12306.bugfix.rst
@@ -0,0 +1,1 @@
+Use ``-r=...`` instead of ``-r ...`` to specify references with Mercurial.
diff --git a/news/1F54AB69-21F3-49F6-B938-AB16E326F82C.trivial.rst b/news/1F54AB69-21F3-49F6-B938-AB16E326F82C.trivial.rst
deleted file mode 100644
index e69de29bb..000000000
diff --git a/src/pip/_internal/cli/base_command.py b/src/pip/_internal/cli/base_command.py
index db9d5cc66..6a3b8e6c2 100644
--- a/src/pip/_internal/cli/base_command.py
+++ b/src/pip/_internal/cli/base_command.py
@@ -181,7 +181,7 @@ class Command(CommandContextMixIn):
                     assert isinstance(status, int)
                     return status
                 except DiagnosticPipError as exc:
-                    logger.error("%s", exc, extra={"rich": True})
+                    logger.error("[present-rich] %s", exc)
                     logger.debug("Exception information:", exc_info=True)
 
                     return ERROR
diff --git a/src/pip/_internal/cli/req_command.py b/src/pip/_internal/cli/req_command.py
index 6f2f79c6b..02910604b 100644
--- a/src/pip/_internal/cli/req_command.py
+++ b/src/pip/_internal/cli/req_command.py
@@ -265,7 +265,7 @@ class RequirementCommand(IndexGroupCommand):
         if "legacy-resolver" in options.deprecated_features_enabled:
             return "legacy"
 
-        return "resolvelib"
+        return "2020-resolver"
 
     @classmethod
     def make_requirement_preparer(
@@ -287,7 +287,7 @@ class RequirementCommand(IndexGroupCommand):
         legacy_resolver = False
 
         resolver_variant = cls.determine_resolver_variant(options)
-        if resolver_variant == "resolvelib":
+        if resolver_variant == "2020-resolver":
             lazy_wheel = "fast-deps" in options.features_enabled
             if lazy_wheel:
                 logger.warning(
@@ -349,7 +349,7 @@ class RequirementCommand(IndexGroupCommand):
         # The long import name and duplicated invocation is needed to convince
         # Mypy into correctly typechecking. Otherwise it would complain the
         # "Resolver" class being redefined.
-        if resolver_variant == "resolvelib":
+        if resolver_variant == "2020-resolver":
             import pip._internal.resolution.resolvelib.resolver
 
             return pip._internal.resolution.resolvelib.resolver.Resolver(
diff --git a/src/pip/_internal/commands/install.py b/src/pip/_internal/commands/install.py
index d53bbd059..f6a300804 100644
--- a/src/pip/_internal/commands/install.py
+++ b/src/pip/_internal/commands/install.py
@@ -595,7 +595,7 @@ class InstallCommand(RequirementCommand):
                 "source of the following dependency conflicts."
             )
         else:
-            assert resolver_variant == "resolvelib"
+            assert resolver_variant == "2020-resolver"
             parts.append(
                 "pip's dependency resolver does not currently take into account "
                 "all the packages that are installed. This behaviour is the "
@@ -628,7 +628,7 @@ class InstallCommand(RequirementCommand):
                     requirement=req,
                     dep_name=dep_name,
                     dep_version=dep_version,
-                    you=("you" if resolver_variant == "resolvelib" else "you'll"),
+                    you=("you" if resolver_variant == "2020-resolver" else "you'll"),
                 )
                 parts.append(message)
 
diff --git a/src/pip/_internal/self_outdated_check.py b/src/pip/_internal/self_outdated_check.py
index cb18edbed..eefbc498b 100644
--- a/src/pip/_internal/self_outdated_check.py
+++ b/src/pip/_internal/self_outdated_check.py
@@ -233,7 +233,7 @@ def pip_self_version_check(session: PipSession, options: optparse.Values) -> Non
             ),
         )
         if upgrade_prompt is not None:
-            logger.warning("%s", upgrade_prompt, extra={"rich": True})
+            logger.warning("[present-rich] %s", upgrade_prompt)
     except Exception:
         logger.warning("There was an error checking the latest version of pip.")
         logger.debug("See below for error", exc_info=True)
diff --git a/src/pip/_internal/utils/logging.py b/src/pip/_internal/utils/logging.py
index 95982dfb6..c10e1f4ce 100644
--- a/src/pip/_internal/utils/logging.py
+++ b/src/pip/_internal/utils/logging.py
@@ -155,8 +155,8 @@ class RichPipStreamHandler(RichHandler):
 
         # If we are given a diagnostic error to present, present it with indentation.
         assert isinstance(record.args, tuple)
-        if getattr(record, "rich", False):
-            (rich_renderable,) = record.args
+        if record.msg == "[present-rich] %s" and len(record.args) == 1:
+            rich_renderable = record.args[0]
             assert isinstance(
                 rich_renderable, (ConsoleRenderable, RichCast, str)
             ), f"{rich_renderable} is not rich-console-renderable"
diff --git a/src/pip/_internal/utils/subprocess.py b/src/pip/_internal/utils/subprocess.py
index 79580b053..1e8ff50ed 100644
--- a/src/pip/_internal/utils/subprocess.py
+++ b/src/pip/_internal/utils/subprocess.py
@@ -209,7 +209,7 @@ def call_subprocess(
                 output_lines=all_output if not showing_subprocess else None,
             )
             if log_failed_cmd:
-                subprocess_logger.error("%s", error, extra={"rich": True})
+                subprocess_logger.error("[present-rich] %s", error)
                 subprocess_logger.verbose(
                     "[bold magenta]full command[/]: [blue]%s[/]",
                     escape(format_command_args(cmd)),
diff --git a/src/pip/_internal/vcs/mercurial.py b/src/pip/_internal/vcs/mercurial.py
index 4595960b5..e440c1221 100644
--- a/src/pip/_internal/vcs/mercurial.py
+++ b/src/pip/_internal/vcs/mercurial.py
@@ -31,7 +31,7 @@ class Mercurial(VersionControl):
 
     @staticmethod
     def get_base_rev_args(rev: str) -> List[str]:
-        return ["-r", rev]
+        return [f"-r={rev}"]
 
     def fetch_new(
         self, dest: str, url: HiddenText, rev_options: RevOptions, verbosity: int


Test patch:
diff --git a/tests/conftest.py b/tests/conftest.py
index 07cd468c1..cd9931c66 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -76,8 +76,8 @@ def pytest_addoption(parser: Parser) -> None:
     parser.addoption(
         "--resolver",
         action="store",
-        default="resolvelib",
-        choices=["resolvelib", "legacy"],
+        default="2020-resolver",
+        choices=["2020-resolver", "legacy"],
         help="use given resolver in tests",
     )
     parser.addoption(
diff --git a/tests/functional/test_freeze.py b/tests/functional/test_freeze.py
index 9a5937df3..d6122308a 100644
--- a/tests/functional/test_freeze.py
+++ b/tests/functional/test_freeze.py
@@ -629,7 +629,7 @@ _freeze_req_opts = textwrap.dedent(
     --extra-index-url http://ignore
     --find-links http://ignore
     --index-url http://ignore
-    --use-feature resolvelib
+    --use-feature 2020-resolver
 """
 )
 
diff --git a/tests/functional/test_install.py b/tests/functional/test_install.py
index 485710eaa..bf0512943 100644
--- a/tests/functional/test_install.py
+++ b/tests/functional/test_install.py
@@ -1209,9 +1209,9 @@ def test_install_nonlocal_compatible_wheel_path(
         "--no-index",
         "--only-binary=:all:",
         Path(data.packages) / "simplewheel-2.0-py3-fakeabi-fakeplat.whl",
-        expect_error=(resolver_variant == "resolvelib"),
+        expect_error=(resolver_variant == "2020-resolver"),
     )
-    if resolver_variant == "resolvelib":
+    if resolver_variant == "2020-resolver":
         assert result.returncode == ERROR
     else:
         assert result.returncode == SUCCESS
@@ -1825,14 +1825,14 @@ def test_install_editable_with_wrong_egg_name(
         "install",
         "--editable",
         f"file://{pkga_path}#egg=pkgb",
-        expect_error=(resolver_variant == "resolvelib"),
+        expect_error=(resolver_variant == "2020-resolver"),
     )
     assert (
         "Generating metadata for package pkgb produced metadata "
         "for project name pkga. Fix your #egg=pkgb "
         "fragments."
     ) in result.stderr
-    if resolver_variant == "resolvelib":
+    if resolver_variant == "2020-resolver":
         assert "has inconsistent" in result.stdout, str(result)
     else:
         assert "Successfully installed pkga" in str(result), str(result)
diff --git a/tests/functional/test_install_extras.py b/tests/functional/test_install_extras.py
index 1dd67be0a..8ccbcf199 100644
--- a/tests/functional/test_install_extras.py
+++ b/tests/functional/test_install_extras.py
@@ -242,7 +242,7 @@ def test_install_extra_merging(
         expect_error=(fails_on_legacy and resolver_variant == "legacy"),
     )
 
-    if not fails_on_legacy or resolver_variant == "resolvelib":
+    if not fails_on_legacy or resolver_variant == "2020-resolver":
         expected = f"Successfully installed pkga-0.1 simple-{simple_version}"
         assert expected in result.stdout
 
diff --git a/tests/functional/test_install_reqs.py b/tests/functional/test_install_reqs.py
index c21b9ba83..96cff0dc5 100644
--- a/tests/functional/test_install_reqs.py
+++ b/tests/functional/test_install_reqs.py
@@ -392,8 +392,11 @@ def test_constraints_local_editable_install_causes_error(
         to_install,
         expect_error=True,
     )
-    # Because singlemodule only has 0.0.1 available.
-    assert "Cannot install singlemodule 0.0.1" in result.stderr, str(result)
+    if resolver_variant == "legacy-resolver":
+        assert "Could not satisfy constraints" in result.stderr, str(result)
+    else:
+        # Because singlemodule only has 0.0.1 available.
+        assert "Cannot install singlemodule 0.0.1" in result.stderr, str(result)
 
 
 @pytest.mark.network
@@ -423,8 +426,11 @@ def test_constraints_local_install_causes_error(
         to_install,
         expect_error=True,
     )
-    # Because singlemodule only has 0.0.1 available.
-    assert "Cannot install singlemodule 0.0.1" in result.stderr, str(result)
+    if resolver_variant == "legacy-resolver":
+        assert "Could not satisfy constraints" in result.stderr, str(result)
+    else:
+        # Because singlemodule only has 0.0.1 available.
+        assert "Cannot install singlemodule 0.0.1" in result.stderr, str(result)
 
 
 def test_constraints_constrain_to_local_editable(
@@ -445,9 +451,9 @@ def test_constraints_constrain_to_local_editable(
         script.scratch_path / "constraints.txt",
         "singlemodule",
         allow_stderr_warning=True,
-        expect_error=(resolver_variant == "resolvelib"),
+        expect_error=(resolver_variant == "2020-resolver"),
     )
-    if resolver_variant == "resolvelib":
+    if resolver_variant == "2020-resolver":
         assert "Editable requirements are not allowed as constraints" in result.stderr
     else:
         assert "Running setup.py develop for singlemodule" in result.stdout
@@ -545,9 +551,9 @@ def test_install_with_extras_from_constraints(
         script.scratch_path / "constraints.txt",
         "LocalExtras",
         allow_stderr_warning=True,
-        expect_error=(resolver_variant == "resolvelib"),
+        expect_error=(resolver_variant == "2020-resolver"),
     )
-    if resolver_variant == "resolvelib":
+    if resolver_variant == "2020-resolver":
         assert "Constraints cannot have extras" in result.stderr
     else:
         result.did_create(script.site_packages / "simple")
@@ -583,9 +589,9 @@ def test_install_with_extras_joined(
         script.scratch_path / "constraints.txt",
         "LocalExtras[baz]",
         allow_stderr_warning=True,
-        expect_error=(resolver_variant == "resolvelib"),
+        expect_error=(resolver_variant == "2020-resolver"),
     )
-    if resolver_variant == "resolvelib":
+    if resolver_variant == "2020-resolver":
         assert "Constraints cannot have extras" in result.stderr
     else:
         result.did_create(script.site_packages / "simple")
@@ -604,9 +610,9 @@ def test_install_with_extras_editable_joined(
         script.scratch_path / "constraints.txt",
         "LocalExtras[baz]",
         allow_stderr_warning=True,
-        expect_error=(resolver_variant == "resolvelib"),
+        expect_error=(resolver_variant == "2020-resolver"),
     )
-    if resolver_variant == "resolvelib":
+    if resolver_variant == "2020-resolver":
         assert "Editable requirements are not allowed as constraints" in result.stderr
     else:
         result.did_create(script.site_packages / "simple")
@@ -648,9 +654,9 @@ def test_install_distribution_union_with_constraints(
         script.scratch_path / "constraints.txt",
         f"{to_install}[baz]",
         allow_stderr_warning=True,
-        expect_error=(resolver_variant == "resolvelib"),
+        expect_error=(resolver_variant == "2020-resolver"),
     )
-    if resolver_variant == "resolvelib":
+    if resolver_variant == "2020-resolver":
         msg = "Unnamed requirements are not allowed as constraints"
         assert msg in result.stderr
     else:
@@ -668,9 +674,9 @@ def test_install_distribution_union_with_versions(
     result = script.pip_install_local(
         f"{to_install_001}[bar]",
         f"{to_install_002}[baz]",
-        expect_error=(resolver_variant == "resolvelib"),
+        expect_error=(resolver_variant == "2020-resolver"),
     )
-    if resolver_variant == "resolvelib":
+    if resolver_variant == "2020-resolver":
         assert "Cannot install localextras[bar]" in result.stderr
         assert ("localextras[bar] 0.0.1 depends on localextras 0.0.1") in result.stdout
         assert ("localextras[baz] 0.0.2 depends on localextras 0.0.2") in result.stdout
diff --git a/tests/functional/test_install_upgrade.py b/tests/functional/test_install_upgrade.py
index 6556fcdf5..09c01d7eb 100644
--- a/tests/functional/test_install_upgrade.py
+++ b/tests/functional/test_install_upgrade.py
@@ -172,7 +172,7 @@ def test_upgrade_with_newest_already_installed(
         "install", "--upgrade", "-f", data.find_links, "--no-index", "simple"
     )
     assert not result.files_created, "simple upgraded when it should not have"
-    if resolver_variant == "resolvelib":
+    if resolver_variant == "2020-resolver":
         msg = "Requirement already satisfied"
     else:
         msg = "already up-to-date"
diff --git a/tests/lib/__init__.py b/tests/lib/__init__.py
index a48423570..2e8b239ac 100644
--- a/tests/lib/__init__.py
+++ b/tests/lib/__init__.py
@@ -46,7 +46,9 @@ if TYPE_CHECKING:
     # Literal was introduced in Python 3.8.
     from typing import Literal
 
-    ResolverVariant = Literal["resolvelib", "legacy"]
+    ResolverVariant = Literal[
+        "resolvelib", "legacy", "2020-resolver", "legacy-resolver"
+    ]
 else:
     ResolverVariant = str
 
diff --git a/tests/unit/test_req_file.py b/tests/unit/test_req_file.py
index 7a196eb8d..439c41563 100644
--- a/tests/unit/test_req_file.py
+++ b/tests/unit/test_req_file.py
@@ -471,7 +471,9 @@ class TestProcessLine:
     ) -> None:
         """--use-feature triggers error when parsing requirements files."""
         with pytest.raises(RequirementsFileParseError):
-            line_processor("--use-feature=resolvelib", "filename", 1, options=options)
+            line_processor(
+                "--use-feature=2020-resolver", "filename", 1, options=options
+            )
 
     def test_relative_local_find_links(
         self,
diff --git a/tests/unit/test_utils_subprocess.py b/tests/unit/test_utils_subprocess.py
index 2dbd5d77e..a694b717f 100644
--- a/tests/unit/test_utils_subprocess.py
+++ b/tests/unit/test_utils_subprocess.py
@@ -260,9 +260,9 @@ class TestCallSubprocess:
         expected = (
             None,
             [
-                # pytest's caplog overrides the formatter, which means that we
+                # pytest's caplog overrides th formatter, which means that we
                 # won't see the message formatted through our formatters.
-                ("pip.subprocessor", ERROR, "subprocess error exited with 1"),
+                ("pip.subprocessor", ERROR, "[present-rich]"),
             ],
         )
         # The spinner should spin three times in this case since the
diff --git a/tests/unit/test_vcs.py b/tests/unit/test_vcs.py
index fb6c3ea31..4a3750f2d 100644
--- a/tests/unit/test_vcs.py
+++ b/tests/unit/test_vcs.py
@@ -66,7 +66,7 @@ def test_rev_options_repr() -> None:
         # First check VCS-specific RevOptions behavior.
         (Bazaar, [], ["-r", "123"], {}),
         (Git, ["HEAD"], ["123"], {}),
-        (Mercurial, [], ["-r", "123"], {}),
+        (Mercurial, [], ["-r=123"], {}),
         (Subversion, [], ["-r", "123"], {}),
         # Test extra_args.  For this, test using a single VersionControl class.
         (