Skip to content

Commit

Permalink
Track Python indexes, find-links, and manylinux in lockfile headers (p…
Browse files Browse the repository at this point in the history
…antsbuild#16525)

Closes pantsbuild#12832. It was a bug that changing `[python-repos].{indexes,repos}` and `[python].resolver_manylinux` did not invalidate lockfiles -- those options could impact the lock result!

V2 of lockfile metadata will continue to work the same as before. Next time someone runs `generate-lockfiles` though, they will get the new v3 lockfile header.

[ci skip-rust]
[ci skip-build-wheels]
  • Loading branch information
Eric-Arellano authored Aug 16, 2022
1 parent 2a0cc9b commit c69a70c
Show file tree
Hide file tree
Showing 8 changed files with 191 additions and 16 deletions.
3 changes: 3 additions & 0 deletions src/python/pants/backend/python/goals/lockfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,9 @@ async def generate_lockfile(
valid_for_interpreter_constraints=req.interpreter_constraints,
# TODO(#12314) Improve error message on `Requirement.parse`
requirements={PipRequirement.parse(i) for i in req.requirements},
indexes=set(pip_args_setup.resolve_config.indexes),
find_links=set(pip_args_setup.resolve_config.find_links),
manylinux=pip_args_setup.resolve_config.manylinux,
requirement_constraints=(
set(pip_args_setup.resolve_config.constraints_file.constraints)
if pip_args_setup.resolve_config.constraints_file
Expand Down
43 changes: 37 additions & 6 deletions src/python/pants/backend/python/goals/lockfile_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
)
from pants.backend.python.goals.lockfile import rules as lockfile_rules
from pants.backend.python.goals.lockfile import setup_user_lockfile_requests
from pants.backend.python.subsystems.repos import PythonRepos
from pants.backend.python.subsystems.setup import RESOLVE_OPTION_KEY__DEFAULT, PythonSetup
from pants.backend.python.target_types import PythonRequirementTarget
from pants.backend.python.util_rules import pex
Expand Down Expand Up @@ -44,7 +45,8 @@ def _generate(
rule_runner: RuleRunner,
use_pex: bool,
ansicolors_version: str = "==1.1.8",
requirement_constraints_str: str = '// "requirement_constraints": []',
requirement_constraints_str: str = '// "requirement_constraints": [],\n',
only_binary_and_no_binary_str: str = '// "only_binary": [],\n// "no_binary": []',
) -> str:
result = rule_runner.request(
GenerateLockfileResult,
Expand Down Expand Up @@ -73,13 +75,20 @@ def _generate(
//
// --- BEGIN PANTS LOCKFILE METADATA: DO NOT EDIT OR REMOVE ---
// {{
// "version": 2,
// "version": 3,
// "valid_for_interpreter_constraints": [],
// "generated_with_requirements": [
// "ansicolors{ansicolors_version}"
// ]"""
// ],
// "indexes": [
// "{PythonRepos.pypi_index}"
// ],
// "find_links": [],
// "manylinux": "manylinux2014",
"""
)
# + requirement_constraints_str
+ requirement_constraints_str
+ only_binary_and_no_binary_str
+ dedent(
"""
// }
Expand Down Expand Up @@ -111,14 +120,35 @@ def test_pex_lockfile_generation(
rule_runner: RuleRunner, no_binary: bool, only_binary: bool
) -> None:
args = ["--python-resolves={'test': 'foo.lock'}"]
only_binary_lock_str = '// "only_binary": [],\n'
no_binary_lock_str = '// "no_binary": []'
no_binary_arg = f"{{'{RESOLVE_OPTION_KEY__DEFAULT}': ['ansicolors']}}"
if no_binary:
no_binary_lock_str = dedent(
"""\
// "no_binary": [
// "ansicolors"
// ]"""
)
args.append(f"--python-resolves-to-no-binary={no_binary_arg}")
if only_binary:
only_binary_lock_str = dedent(
"""\
// "only_binary": [
// "ansicolors"
// ],
"""
)
args.append(f"--python-resolves-to-only-binary={no_binary_arg}")
rule_runner.set_options(args, env_inherit=PYTHON_BOOTSTRAP_ENV)

lock_entry = json.loads(_generate(rule_runner=rule_runner, use_pex=True))
lock_entry = json.loads(
_generate(
rule_runner=rule_runner,
use_pex=True,
only_binary_and_no_binary_str=only_binary_lock_str + no_binary_lock_str,
)
)
reqs = lock_entry["locked_resolves"][0]["locked_requirements"]
assert len(reqs) == 1
assert reqs[0]["project_name"] == "ansicolors"
Expand Down Expand Up @@ -176,7 +206,8 @@ def test_constraints_file(rule_runner: RuleRunner) -> None:
"""\
// "requirement_constraints": [
// "ansicolors==1.1.7"
// ]"""
// ],
"""
),
)
)
Expand Down
50 changes: 49 additions & 1 deletion src/python/pants/backend/python/util_rules/lockfile_metadata.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ class InvalidPythonLockfileReason(Enum):
INVALIDATION_DIGEST_MISMATCH = "invalidation_digest_mismatch"
INTERPRETER_CONSTRAINTS_MISMATCH = "interpreter_constraints_mismatch"
REQUIREMENTS_MISMATCH = "requirements_mismatch"
INDEXES_MISMATCH = "indexes_mismatch"
FIND_LINKS_MISMATCH = "find_links_mismatch"
MANYLINUX_MISMATCH = "manylinux_mismatch"
CONSTRAINTS_FILE_MISMATCH = "constraints_file_mismatch"
ONLY_BINARY_MISMATCH = "only_binary_mismatch"
NO_BINARY_MISMATCH = "no_binary_mismatch"
Expand All @@ -41,6 +44,9 @@ def new(
*,
valid_for_interpreter_constraints: InterpreterConstraints,
requirements: set[PipRequirement],
indexes: set[str],
find_links: set[str],
manylinux: str | None,
requirement_constraints: set[PipRequirement],
only_binary: set[str],
no_binary: set[str],
Expand All @@ -53,7 +59,16 @@ def new(
writing, while still allowing us to support _reading_ older, deprecated metadata versions.
"""

return PythonLockfileMetadataV2(valid_for_interpreter_constraints, requirements)
return PythonLockfileMetadataV3(
valid_for_interpreter_constraints,
requirements,
indexes=indexes,
find_links=find_links,
manylinux=manylinux,
requirement_constraints=requirement_constraints,
only_binary=only_binary,
no_binary=no_binary,
)

@classmethod
def additional_header_attrs(cls, instance: LockfileMetadata) -> dict[Any, Any]:
Expand All @@ -72,6 +87,9 @@ def is_valid_for(
user_interpreter_constraints: InterpreterConstraints,
interpreter_universe: Iterable[str],
user_requirements: Iterable[PipRequirement],
indexes: Iterable[str],
find_links: Iterable[str],
manylinux: str | None,
requirement_constraints: Iterable[PipRequirement],
only_binary: Iterable[str],
no_binary: Iterable[str],
Expand Down Expand Up @@ -118,6 +136,9 @@ def is_valid_for(
interpreter_universe: Iterable[str],
# Everything below is not used by v1.
user_requirements: Iterable[PipRequirement],
indexes: Iterable[str],
find_links: Iterable[str],
manylinux: str | None,
requirement_constraints: Iterable[PipRequirement],
only_binary: Iterable[str],
no_binary: Iterable[str],
Expand Down Expand Up @@ -185,6 +206,9 @@ def is_valid_for(
interpreter_universe: Iterable[str],
user_requirements: Iterable[PipRequirement],
# Everything below is not used by V2.
indexes: Iterable[str],
find_links: Iterable[str],
manylinux: str | None,
requirement_constraints: Iterable[PipRequirement],
only_binary: Iterable[str],
no_binary: Iterable[str],
Expand Down Expand Up @@ -212,6 +236,9 @@ def is_valid_for(
class PythonLockfileMetadataV3(PythonLockfileMetadataV2):
"""Lockfile version that considers constraints files."""

indexes: set[str]
find_links: set[str]
manylinux: str | None
requirement_constraints: set[PipRequirement]
only_binary: set[str]
no_binary: set[str]
Expand All @@ -225,6 +252,9 @@ def _from_json_dict(
) -> PythonLockfileMetadataV3:
v2_metadata = super()._from_json_dict(json_dict, lockfile_description, error_suffix)
metadata = _get_metadata(json_dict, lockfile_description, error_suffix)
indexes = metadata("indexes", Set[str], lambda l: set(l))
find_links = metadata("find_links", Set[str], lambda l: set(l))
manylinux = metadata("manylinux", str, lambda l: l) # type: ignore[no-any-return]
requirement_constraints = metadata(
"requirement_constraints",
Set[PipRequirement],
Expand All @@ -236,6 +266,9 @@ def _from_json_dict(
return PythonLockfileMetadataV3(
valid_for_interpreter_constraints=v2_metadata.valid_for_interpreter_constraints,
requirements=v2_metadata.requirements,
indexes=indexes,
find_links=find_links,
manylinux=manylinux,
requirement_constraints=requirement_constraints,
only_binary=only_binary,
no_binary=no_binary,
Expand All @@ -245,6 +278,9 @@ def _from_json_dict(
def additional_header_attrs(cls, instance: LockfileMetadata) -> dict[Any, Any]:
instance = cast(PythonLockfileMetadataV3, instance)
return {
"indexes": sorted(instance.indexes),
"find_links": sorted(instance.find_links),
"manylinux": instance.manylinux,
"requirement_constraints": sorted(str(i) for i in instance.requirement_constraints),
"only_binary": sorted(instance.only_binary),
"no_binary": sorted(instance.no_binary),
Expand All @@ -258,6 +294,9 @@ def is_valid_for(
user_interpreter_constraints: InterpreterConstraints,
interpreter_universe: Iterable[str],
user_requirements: Iterable[PipRequirement],
indexes: Iterable[str],
find_links: Iterable[str],
manylinux: str | None,
requirement_constraints: Iterable[PipRequirement],
only_binary: Iterable[str],
no_binary: Iterable[str],
Expand All @@ -270,13 +309,22 @@ def is_valid_for(
user_interpreter_constraints=user_interpreter_constraints,
interpreter_universe=interpreter_universe,
user_requirements=user_requirements,
indexes=indexes,
find_links=find_links,
manylinux=manylinux,
requirement_constraints=requirement_constraints,
only_binary=only_binary,
no_binary=no_binary,
)
.failure_reasons
)

if self.indexes != set(indexes):
failure_reasons.add(InvalidPythonLockfileReason.INDEXES_MISMATCH)
if self.find_links != set(find_links):
failure_reasons.add(InvalidPythonLockfileReason.FIND_LINKS_MISMATCH)
if self.manylinux != manylinux:
failure_reasons.add(InvalidPythonLockfileReason.MANYLINUX_MISMATCH)
if self.requirement_constraints != set(requirement_constraints):
failure_reasons.add(InvalidPythonLockfileReason.CONSTRAINTS_FILE_MISMATCH)
if self.only_binary != set(only_binary):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ def test_metadata_header_round_trip() -> None:
["CPython==2.7.*", "PyPy", "CPython>=3.6,<4,!=3.7.*"]
),
requirements=reqset("ansicolors==0.1.0"),
indexes={"index"},
find_links={"find-links"},
manylinux="manylinux2014",
requirement_constraints={PipRequirement.parse("constraint")},
only_binary={"bdist"},
no_binary={"sdist"},
Expand All @@ -57,12 +60,28 @@ def test_add_header_to_lockfile() -> None:
#
# --- BEGIN PANTS LOCKFILE METADATA: DO NOT EDIT OR REMOVE ---
# {
# "version": 2,
# "version": 3,
# "valid_for_interpreter_constraints": [
# "CPython>=3.7"
# ],
# "generated_with_requirements": [
# "ansicolors==0.1.0"
# ],
# "indexes": [
# "index"
# ],
# "find_links": [
# "find-links"
# ],
# "manylinux": null,
# "requirement_constraints": [
# "constraint"
# ],
# "only_binary": [
# "bdist"
# ],
# "no_binary": [
# "sdist"
# ]
# }
# --- END PANTS LOCKFILE METADATA ---
Expand All @@ -76,6 +95,9 @@ def line_by_line(b: bytes) -> list[bytes]:
metadata = PythonLockfileMetadata.new(
valid_for_interpreter_constraints=InterpreterConstraints([">=3.7"]),
requirements=reqset("ansicolors==0.1.0"),
indexes={"index"},
find_links={"find-links"},
manylinux=None,
requirement_constraints={PipRequirement.parse("constraint")},
only_binary={"bdist"},
no_binary={"sdist"},
Expand Down Expand Up @@ -161,6 +183,9 @@ def test_is_valid_for_v1(user_digest, expected_digest, user_ic, expected_ic, mat
user_interpreter_constraints=InterpreterConstraints(user_ic),
interpreter_universe=INTERPRETER_UNIVERSE,
user_requirements=set(),
indexes=set(),
find_links=set(),
manylinux=None,
requirement_constraints=set(),
only_binary=set(),
no_binary=set(),
Expand Down Expand Up @@ -239,6 +264,9 @@ def test_is_valid_for_interpreter_constraints_and_requirements(
PythonLockfileMetadataV3(
InterpreterConstraints(lock_ics),
reqset(*lock_reqs),
indexes=set(),
find_links=set(),
manylinux=None,
requirement_constraints=set(),
only_binary=set(),
no_binary=set(),
Expand All @@ -250,6 +278,9 @@ def test_is_valid_for_interpreter_constraints_and_requirements(
user_interpreter_constraints=InterpreterConstraints(user_ics),
interpreter_universe=INTERPRETER_UNIVERSE,
user_requirements=reqset(*user_reqs),
indexes=set(),
find_links=set(),
manylinux=None,
requirement_constraints=set(),
only_binary=set(),
no_binary=set(),
Expand All @@ -263,6 +294,9 @@ def test_is_valid_for_v3_metadata(is_tool: bool) -> None:
InterpreterConstraints([]),
reqset(),
# Everything below is new to v3+.
indexes={"index"},
find_links={"find-links"},
manylinux=None,
requirement_constraints={PipRequirement.parse("c1")},
only_binary={"bdist"},
no_binary={"sdist"},
Expand All @@ -272,6 +306,9 @@ def test_is_valid_for_v3_metadata(is_tool: bool) -> None:
user_interpreter_constraints=InterpreterConstraints([]),
interpreter_universe=INTERPRETER_UNIVERSE,
user_requirements=reqset(),
indexes={"different-index"},
find_links={"different-find-links"},
manylinux="manylinux2014",
requirement_constraints={PipRequirement.parse("c2")},
only_binary={"not-bdist"},
no_binary={"not-sdist"},
Expand All @@ -280,4 +317,7 @@ def test_is_valid_for_v3_metadata(is_tool: bool) -> None:
InvalidPythonLockfileReason.CONSTRAINTS_FILE_MISMATCH,
InvalidPythonLockfileReason.ONLY_BINARY_MISMATCH,
InvalidPythonLockfileReason.NO_BINARY_MISMATCH,
InvalidPythonLockfileReason.INDEXES_MISMATCH,
InvalidPythonLockfileReason.FIND_LINKS_MISMATCH,
InvalidPythonLockfileReason.MANYLINUX_MISMATCH,
}
30 changes: 28 additions & 2 deletions src/python/pants/backend/python/util_rules/pex_requirements.py
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,9 @@ def validate_metadata(
user_interpreter_constraints=interpreter_constraints,
interpreter_universe=python_setup.interpreter_versions_universe,
user_requirements=user_requirements,
indexes=resolve_config.indexes,
find_links=resolve_config.find_links,
manylinux=resolve_config.manylinux,
requirement_constraints=(
resolve_config.constraints_file.constraints
if resolve_config.constraints_file
Expand Down Expand Up @@ -498,14 +501,37 @@ def _common_failure_reasons(
yield softwrap(
"""
- The `only_binary` arguments have changed from when the lockfile was generated.
(`only_binary` is set via the option `[python].resolves_to_only_binary`)
(`only_binary` is set via the options `[python].resolves_to_only_binary` and deprecated
`[python].only_binary`)
"""
)
if InvalidPythonLockfileReason.NO_BINARY_MISMATCH in failure_reasons:
yield softwrap(
"""
- The `no_binary` arguments have changed from when the lockfile was generated.
(`no_binary` is set via the option `[python].resolves_to_no_binary`)
(`no_binary` is set via the options `[python].resolves_to_no_binary` and deprecated
`[python].no_binary`)
"""
)
if InvalidPythonLockfileReason.INDEXES_MISMATCH in failure_reasons:
yield softwrap(
"""
- The `indexes` arguments have changed from when the lockfile was generated.
(Indexes are set via the option `[python-repos].indexes`
"""
)
if InvalidPythonLockfileReason.FIND_LINKS_MISMATCH in failure_reasons:
yield softwrap(
"""
- The `find_links` arguments have changed from when the lockfile was generated.
(Find links is set via the option `[python-repos].repos`
"""
)
if InvalidPythonLockfileReason.MANYLINUX_MISMATCH in failure_reasons:
yield softwrap(
"""
- The `manylinux` argument has changed from when the lockfile was generated.
(manylinux is set via the option `[python].resolver_manylinux`
"""
)

Expand Down
Loading

0 comments on commit c69a70c

Please sign in to comment.