Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Formalize support for VCS-style requirements via PEP 440 #10728

Merged
merged 2 commits into from Sep 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -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',
],
)
"""
),
)
Expand All @@ -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"),
}
)

Expand Down
9 changes: 4 additions & 5 deletions 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

Expand All @@ -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.

Expand Down Expand Up @@ -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
Expand Down
10 changes: 8 additions & 2 deletions src/python/pants/backend/python/macros/python_requirements.py
Expand Up @@ -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


Expand Down Expand Up @@ -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)

Expand Down
26 changes: 22 additions & 4 deletions src/python/pants/backend/python/macros/python_requirements_test.py
Expand Up @@ -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(
Expand Down Expand Up @@ -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"),
),
],
)

Expand All @@ -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:
Expand Down
41 changes: 39 additions & 2 deletions src/python/pants/backend/python/target_types.py
Expand Up @@ -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
Expand Down Expand Up @@ -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']."""
Expand Down Expand Up @@ -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):
Expand Down
15 changes: 13 additions & 2 deletions src/python/pants/backend/python/target_types_test.py
Expand Up @@ -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
Expand All @@ -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(
Expand Down