Merge 681524f into 5175b2b

src/python/pants/backend/python/dependency_inference/module_mapper_test.py

@@ -142,6 +142,13 @@ def test_map_third_party_modules_to_addresses(rule_runner: RuleRunner) -> None:
               name='un_normalized',
               requirements=['Un-Normalized-Project>3', 'two_owners'],
             )
+
+            python_requirement_library(
+              name='direct_references',
+              requirements=[
+                'pip@ git+https://github.com/pypa/pip.git', 'local_dist@ file:///path/to/dist.whl',
+              ],
+            )
             """
         ),
     )
@@ -150,9 +157,11 @@ def test_map_third_party_modules_to_addresses(rule_runner: RuleRunner) -> None:
     )
     assert result.mapping == FrozenDict(
         {
-            "colors": Address.parse("3rdparty/python:ansicolors"),
-            "req1": Address.parse("3rdparty/python:req1"),
-            "un_normalized_project": Address.parse("3rdparty/python:un_normalized"),
+            "colors": Address("3rdparty/python", target_name="ansicolors"),
+            "local_dist": Address("3rdparty/python", target_name="direct_references"),
+            "pip": Address("3rdparty/python", target_name="direct_references"),
+            "req1": Address("3rdparty/python", target_name="req1"),
+            "un_normalized_project": Address("3rdparty/python", target_name="un_normalized"),
         }
     )
 

src/python/pants/backend/python/macros/pipenv_requirements.py

@@ -1,7 +1,7 @@
 # Copyright 2020 Pants project contributors (see CONTRIBUTORS.md).
 # Licensed under the Apache License, Version 2.0 (see LICENSE).
 
-from json import load
+import json
 from pathlib import Path
 from typing import Iterable, Mapping, Optional
 
@@ -10,6 +10,8 @@
 from pants.base.build_environment import get_buildroot
 
 
+# TODO(#10655): add support for PEP 440 direct references (aka VCS style).
+# TODO(#10655): differentiate between Pipfile vs. Pipfile.lock.
 class PipenvRequirements:
     """Translates a Pipenv.lock file into an equivalent set `python_requirement_library` targets.
 
@@ -46,13 +48,10 @@ def __call__(
         if the requirements_relpath value is not in the current rel_path
         """
 
-        lock_info = {}
-
         requirements_path = Path(
             get_buildroot(), self._parse_context.rel_path, requirements_relpath
         )
-        with open(requirements_path, "r") as fp:
-            lock_info = load(fp)
+        lock_info = json.loads(requirements_path.read_text())
 
         if pipfile_target:
             requirements_dep = pipfile_target

src/python/pants/backend/python/macros/python_requirements.py

@@ -6,6 +6,7 @@
 
 from pkg_resources import Requirement
 
+from pants.backend.python.target_types import format_invalid_requirement_string_error
 from pants.base.build_environment import get_buildroot
 
 
@@ -70,8 +71,13 @@ def __call__(
                 req = Requirement.parse(line)
             except Exception as e:
                 raise ValueError(
-                    f"Invalid requirement in {req_file.relative_to(get_buildroot())} at line "
-                    f"{i + 1} due to value '{line}'.\n\n{e}"
+                    format_invalid_requirement_string_error(
+                        line,
+                        e,
+                        description_of_origin=(
+                            f"{req_file.relative_to(get_buildroot())} at line {i + 1}"
+                        ),
+                    )
                 )
             requirements.append(req)
 

src/python/pants/backend/python/macros/python_requirements_test.py

@@ -69,6 +69,7 @@ def test_requirements_txt(rule_runner: RuleRunner) -> None:
             ansicolors>=1.18.0
             Django==3.2 ; python_version>'3'
             Un-Normalized-PROJECT  # Inline comment.
+            pip@ git+https://github.com/pypa/pip.git
             """
         ),
         expected_file_dep=PythonRequirementsFile(
@@ -98,6 +99,13 @@ def test_requirements_txt(rule_runner: RuleRunner) -> None:
                 },
                 address=Address("", target_name="Un-Normalized-PROJECT"),
             ),
+            PythonRequirementLibrary(
+                {
+                    "dependencies": [":requirements.txt"],
+                    "requirements": [Requirement.parse("pip@ git+https://github.com/pypa/pip.git")],
+                },
+                address=Address("", target_name="pip"),
+            ),
         ],
     )
 
@@ -112,10 +120,20 @@ def test_invalid_req(rule_runner: RuleRunner) -> None:
             expected_file_dep=PythonRequirementsFile({}, address=Address("doesnt_matter")),
             expected_targets=[],
         )
-    assert (
-        "Invalid requirement in requirements.txt at line 3 due to value 'Not A Valid Req == "
-        "3.7'."
-    ) in str(exc.value)
+    assert "Invalid requirement 'Not A Valid Req == 3.7' in requirements.txt at line 3" in str(
+        exc.value
+    )
+
+    # Give a nice error message if it looks like they're using pip VCS-style requirements.
+    with pytest.raises(ExecutionError) as exc:
+        assert_python_requirements(
+            rule_runner,
+            "python_requirements()",
+            "git+https://github.com/pypa/pip.git#egg=pip",
+            expected_file_dep=PythonRequirementsFile({}, address=Address("doesnt_matter")),
+            expected_targets=[],
+        )
+    assert "It looks like you're trying to use a pip VCS-style requirement?" in str(exc.value)
 
 
 def test_relpath_override(rule_runner: RuleRunner) -> None:

src/python/pants/backend/python/target_types.py

@@ -3,6 +3,7 @@
 
 import collections.abc
 import os.path
+from textwrap import dedent
 from typing import Iterable, Optional, Sequence, Tuple, Union, cast
 
 from pkg_resources import Requirement
@@ -331,6 +332,37 @@ class PythonLibrary(Target):
 # -----------------------------------------------------------------------------------------------
 
 
+def format_invalid_requirement_string_error(
+    value: str, e: Exception, *, description_of_origin: str
+) -> str:
+    prefix = f"Invalid requirement '{value}' in {description_of_origin}: {e}"
+    # We check if they're using Pip-style VCS requirements, and redirect them to instead use PEP
+    # 440 direct references. See https://pip.pypa.io/en/stable/reference/pip_install/#vcs-support.
+    recognized_vcs = {"git", "hg", "svn", "bzr"}
+    if all(f"{vcs}+" not in value for vcs in recognized_vcs):
+        return prefix
+    return dedent(
+        f"""\
+        {prefix}
+
+        It looks like you're trying to use a pip VCS-style requirement?
+        Instead, use a direct reference (PEP 440).
+
+        Instead of this style:
+
+            git+https://github.com/django/django.git#egg=Django
+            git+https://github.com/django/django.git@stable/2.1.x#egg=Django
+            git+https://github.com/django/django.git@fd209f62f1d83233cc634443cfac5ee4328d98b8#egg=Django
+
+        Use this style, where the first value is the name of the dependency:
+
+            Django@ git+https://github.com/django/django.git
+            Django@ git+https://github.com/django/django.git@stable/2.1.x
+            Django@ git+https://github.com/django/django.git@fd209f62f1d83233cc634443cfac5ee4328d98b8
+        """
+    )
+
+
 class PythonRequirementsField(PrimitiveField):
     """A sequence of pip-style requirement strings, e.g. ['foo==1.8', 'bar<=3 ;
     python_version<'3']."""
@@ -365,8 +397,13 @@ def compute_value(
                     parsed = Requirement.parse(v)
                 except Exception as e:
                     raise InvalidFieldException(
-                        f"Invalid requirement string '{v}' in the '{cls.alias}' field for the "
-                        f"target {address}: {e}"
+                        format_invalid_requirement_string_error(
+                            v,
+                            e,
+                            description_of_origin=(
+                                f"the '{cls.alias}' field for the target {address}"
+                            ),
+                        )
                     )
                 result.append(parsed)
             elif isinstance(v, PythonRequirement):

src/python/pants/backend/python/target_types_test.py

@@ -67,7 +67,11 @@ def test_translate_source_file_to_entry_point() -> None:
 
 
 def test_requirements_field() -> None:
-    raw_value = ("argparse==1.2.1", "configparser ; python_version<'3'")
+    raw_value = (
+        "argparse==1.2.1",
+        "configparser ; python_version<'3'",
+        "pip@ git+https://github.com/pypa/pip.git",
+    )
     parsed_value = tuple(Requirement.parse(v) for v in raw_value)
 
     assert PythonRequirementsField(raw_value, address=Address("demo")).value == parsed_value
@@ -85,10 +89,17 @@ def test_requirements_field() -> None:
     with pytest.raises(InvalidFieldException) as exc:
         PythonRequirementsField(["not valid! === 3.1"], address=Address("demo"))
     assert (
-        "Invalid requirement string 'not valid! === 3.1' in the 'requirements' field for the "
+        "Invalid requirement 'not valid! === 3.1' in the 'requirements' field for the "
         "target demo:"
     ) in str(exc.value)
 
+    # Give a nice error message if it looks like they're trying to use pip VCS-style requirements.
+    with pytest.raises(InvalidFieldException) as exc:
+        PythonRequirementsField(
+            ["git+https://github.com/pypa/pip.git#egg=pip"], address=Address("demo")
+        )
+    assert "It looks like you're trying to use a pip VCS-style requirement?" in str(exc.value)
+
     # Check that we still support the deprecated `pants_requirement` object.
     assert (
         PythonRequirementsField(