Skip to content

Commit

Permalink
Ban platforms and internal_only with pex_from_targets.py too (p…
Browse files Browse the repository at this point in the history
…antsbuild#16496)

We already ban `internal_only` and `platforms` in `PexRequest`:

https://github.com/pantsbuild/pants/blob/998992989bbbf48dc1057439fed3b98a47091863/src/python/pants/backend/python/util_rules/pex.py#L217-L247

But we were not eagerly checking this in `PexFromTargetsRequest`. Eagerly checking allows us to simplify reasoning about `pex_from_targets.py` as we now have fewer conditional branches to deal with.

[ci skip-rust]
[ci skip-build-wheels]
  • Loading branch information
Eric-Arellano authored Aug 12, 2022
1 parent 9989929 commit fb70c9f
Show file tree
Hide file tree
Showing 3 changed files with 103 additions and 14 deletions.
7 changes: 6 additions & 1 deletion src/python/pants/backend/helm/util_rules/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,9 @@

python_sources()

python_tests(name="tests")
python_tests(
name="tests",
overrides={
"post_renderer_test.py": {"timeout": 120},
},
)
11 changes: 11 additions & 0 deletions src/python/pants/backend/python/util_rules/pex_from_targets.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,17 @@ def __init__(
self.hardcoded_interpreter_constraints = hardcoded_interpreter_constraints
self.description = description

if self.internal_only and (self.platforms or self.complete_platforms):
raise AssertionError(
softwrap(
"""
PexFromTargetsRequest set internal_only at the same time as setting
`platforms` and/or `complete_platforms`. Platforms can only be used when
`internal_only=False`.
"""
)
)

def to_interpreter_constraints_request(self) -> InterpreterConstraintsRequest:
return InterpreterConstraintsRequest(
addresses=self.addresses,
Expand Down
99 changes: 86 additions & 13 deletions src/python/pants/backend/python/util_rules/pex_from_targets_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,9 @@ class RequirementMode(Enum):
def assert_setup(
mode: RequirementMode,
*,
internal_only: bool,
platforms: bool,
include_requirements: bool = True,
internal_only: bool = True,
run_against_entire_lockfile: bool = False,
expected: PexRequirements | PexRequest,
) -> None:
Expand All @@ -194,10 +195,12 @@ def assert_setup(
resolve_all_constraints=mode != RequirementMode.CONSTRAINTS_NO_RESOLVE_ALL,
requirement_constraints="foo.constraints" if requirement_constraints_used else None,
)
pex_from_targets_request = Mock(
pex_from_targets_request = PexFromTargetsRequest(
Addresses(),
output_filename="foo",
include_requirements=include_requirements,
platforms=PexPlatforms(["foo"] if platforms else []),
internal_only=internal_only,
addresses=Addresses(),
)
resolved_pex_requirements = PexRequirements(
req_strings,
Expand All @@ -206,12 +209,13 @@ def assert_setup(
),
)

if lockfile_used:
# NB: We recreate that platforms should turn off first creating a repository.pex.
if lockfile_used and not platforms:
mock_repository_pex_request = OptionalPexRequest(
maybe_pex_request=repository_pex_request__lockfile
)
mock_repository_pex = OptionalPex(maybe_pex=repository_pex__lockfile)
elif mode == RequirementMode.CONSTRAINTS_RESOLVE_ALL:
elif mode == RequirementMode.CONSTRAINTS_RESOLVE_ALL and not platforms:
mock_repository_pex_request = OptionalPexRequest(
maybe_pex_request=repository_pex_request__constraints
)
Expand Down Expand Up @@ -245,31 +249,48 @@ def assert_setup(
MockGet(OptionalPex, OptionalPexRequest, lambda _: mock_repository_pex),
],
)
assert requirements_or_pex_request == expected
if expected:
assert requirements_or_pex_request == expected

# If include_requirements is False, no matter what, early return.
for mode in RequirementMode:
assert_setup(mode, include_requirements=False, expected=PexRequirements())
assert_setup(
mode,
include_requirements=False,
internal_only=False,
platforms=False,
expected=PexRequirements(),
)

# Pex lockfiles: usually, return PexRequirements with from_superset as the LoadedLockfile.
# Except for when run_against_entire_lockfile is set and it's an internal_only Pex, then
# return PexRequest.
for internal_only in (False, True):
for internal_only in (True, False):
assert_setup(
RequirementMode.PEX_LOCKFILE,
internal_only=internal_only,
platforms=False,
expected=PexRequirements(req_strings, from_superset=loaded_lockfile__pex),
)
assert_setup(
RequirementMode.PEX_LOCKFILE,
internal_only=False,
run_against_entire_lockfile=True,
platforms=True,
expected=PexRequirements(req_strings, from_superset=loaded_lockfile__pex),
)
for platforms in (True, False):
assert_setup(
RequirementMode.PEX_LOCKFILE,
internal_only=False,
run_against_entire_lockfile=True,
platforms=platforms,
expected=PexRequirements(req_strings, from_superset=loaded_lockfile__pex),
)
assert_setup(
RequirementMode.PEX_LOCKFILE,
internal_only=True,
run_against_entire_lockfile=True,
platforms=False,
expected=repository_pex_request__lockfile,
)

Expand All @@ -280,22 +301,38 @@ def assert_setup(
assert_setup(
RequirementMode.NON_PEX_LOCKFILE,
internal_only=internal_only,
platforms=False,
expected=PexRequirements(
req_strings, constraints_strings=req_strings, from_superset=repository_pex__lockfile
),
)
assert_setup(
RequirementMode.NON_PEX_LOCKFILE,
internal_only=False,
platforms=True,
expected=PexRequirements(req_strings, constraints_strings=req_strings, from_superset=None),
)
assert_setup(
RequirementMode.NON_PEX_LOCKFILE,
internal_only=False,
run_against_entire_lockfile=True,
platforms=False,
expected=PexRequirements(
req_strings, constraints_strings=req_strings, from_superset=repository_pex__lockfile
),
)
assert_setup(
RequirementMode.NON_PEX_LOCKFILE,
internal_only=False,
run_against_entire_lockfile=True,
platforms=True,
expected=PexRequirements(req_strings, constraints_strings=req_strings, from_superset=None),
)
assert_setup(
RequirementMode.NON_PEX_LOCKFILE,
internal_only=True,
run_against_entire_lockfile=True,
platforms=False,
expected=repository_pex_request__lockfile,
)

Expand All @@ -306,48 +343,84 @@ def assert_setup(
assert_setup(
RequirementMode.CONSTRAINTS_RESOLVE_ALL,
internal_only=internal_only,
platforms=False,
expected=PexRequirements(
req_strings,
constraints_strings=global_requirement_constraints,
from_superset=repository_pex__constraints,
),
)
assert_setup(
RequirementMode.CONSTRAINTS_RESOLVE_ALL,
internal_only=False,
platforms=True,
expected=PexRequirements(
req_strings, constraints_strings=global_requirement_constraints, from_superset=None
),
)
assert_setup(
RequirementMode.CONSTRAINTS_RESOLVE_ALL,
internal_only=False,
run_against_entire_lockfile=True,
platforms=False,
expected=PexRequirements(
req_strings,
constraints_strings=global_requirement_constraints,
from_superset=repository_pex__constraints,
),
)
assert_setup(
RequirementMode.CONSTRAINTS_RESOLVE_ALL,
internal_only=False,
run_against_entire_lockfile=True,
platforms=True,
expected=PexRequirements(
req_strings, constraints_strings=global_requirement_constraints, from_superset=None
),
)
assert_setup(
RequirementMode.CONSTRAINTS_RESOLVE_ALL,
internal_only=True,
run_against_entire_lockfile=True,
platforms=False,
expected=repository_pex_request__constraints,
)

# Constraints file without resolve_all_constraints: always PexRequirements with
# constraint_strings as the global constraints. run_against_entire_lockfile is banned.
for internal_only in (False, True):
# constraint_strings as the global constraints.
for internal_only in (True, False):
assert_setup(
RequirementMode.CONSTRAINTS_NO_RESOLVE_ALL,
internal_only=internal_only,
platforms=platforms,
expected=PexRequirements(
req_strings, constraints_strings=global_requirement_constraints
),
)
for platforms in (True, False):
assert_setup(
RequirementMode.CONSTRAINTS_NO_RESOLVE_ALL,
internal_only=False,
platforms=platforms,
expected=PexRequirements(
req_strings, constraints_strings=global_requirement_constraints
),
)

# No constraints and lockfiles: return PexRequirements without modification.
# run_against_entire_lockfile is banned.
for internal_only in (False, True):
for internal_only in (True, False):
assert_setup(
RequirementMode.NO_LOCKS,
internal_only=internal_only,
platforms=False,
expected=PexRequirements(req_strings),
)
assert_setup(
RequirementMode.NO_LOCKS,
internal_only=False,
platforms=True,
expected=PexRequirements(req_strings),
)


@dataclass(frozen=True)
Expand Down

0 comments on commit fb70c9f

Please sign in to comment.