Skip to content

Commit

Permalink
Remove transitive InterpreterConstraint merging deprecation (pantsb…
Browse files Browse the repository at this point in the history
…uild#15531)

Removes the deprecation from pantsbuild#15373 by converting it into an error, and then assuming the condition in `mypy`, `pylint`, and `pytest` (all of which used transitive dependencies, and previously AND'd their constraints).

[ci skip-rust]
  • Loading branch information
stuhood authored May 20, 2022
1 parent 931101e commit 2d4131b
Show file tree
Hide file tree
Showing 15 changed files with 101 additions and 271 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -389,11 +389,17 @@ async def infer_python_dependencies_via_source(

_wrapped_tgt = await Get(WrappedTarget, Address, request.sources_field.address)
tgt = _wrapped_tgt.target
interpreter_constraints = InterpreterConstraints.create_from_targets([tgt], python_setup)
if interpreter_constraints is None:
# TODO: This would represent a target with a PythonSource field, but no
# InterpreterConstraints field. #15400 would allow inference to require both
# fields.
return InferredDependencies([])
parsed_dependencies = await Get(
ParsedPythonDependencies,
ParsePythonDependenciesRequest(
cast(PythonSourceField, request.sources_field),
InterpreterConstraints.create_from_targets([tgt], python_setup),
interpreter_constraints,
string_imports=python_infer_subsystem.string_imports,
string_imports_min_dots=python_infer_subsystem.string_imports_min_dots,
assets=python_infer_subsystem.assets,
Expand Down
4 changes: 3 additions & 1 deletion src/python/pants/backend/python/goals/pytest_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,9 @@ async def setup_pytest_for_target(
)
all_targets = transitive_targets.closure

interpreter_constraints = InterpreterConstraints.create_from_targets(all_targets, python_setup)
interpreter_constraints = InterpreterConstraints.create_from_compatibility_fields(
[request.field_set.interpreter_constraints], python_setup
)

requirements_pex_get = Get(Pex, RequirementsPexRequest([request.field_set.address]))
pytest_pex_get = Get(
Expand Down
4 changes: 3 additions & 1 deletion src/python/pants/backend/python/lint/bandit/subsystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,9 @@ async def _bandit_interpreter_constraints(python_setup: PythonSetup) -> Interpre
for tgt in all_tgts
if BanditFieldSet.is_applicable(tgt)
}
constraints = InterpreterConstraints(itertools.chain.from_iterable(unique_constraints))
constraints = InterpreterConstraints(
itertools.chain.from_iterable(ic for ic in unique_constraints if ic)
)
return constraints or InterpreterConstraints(python_setup.interpreter_constraints)


Expand Down
4 changes: 3 additions & 1 deletion src/python/pants/backend/python/lint/black/subsystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,9 @@ async def _black_interpreter_constraints(
code_constraints = InterpreterConstraints.create_from_targets(
(tgt for tgt in all_tgts if not tgt.get(SkipBlackField).value), python_setup
)
if code_constraints.requires_python38_or_newer(python_setup.interpreter_universe):
if code_constraints is not None and code_constraints.requires_python38_or_newer(
python_setup.interpreter_universe
):
constraints = code_constraints
return constraints

Expand Down
22 changes: 11 additions & 11 deletions src/python/pants/backend/python/lint/flake8/subsystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -241,17 +241,17 @@ async def _flake8_interpreter_constraints(
# This ORs all unique interpreter constraints. The net effect is that every possible Python
# interpreter used will be covered.
all_tgts = await Get(AllTargets, AllTargetsRequest())
relevant_targets = tuple(tgt for tgt in all_tgts if Flake8FieldSet.is_applicable(tgt))
unique_constraints = set()
for tgt in relevant_targets:
if tgt.has_field(InterpreterConstraintsField):
constraints_field = tgt[InterpreterConstraintsField]
unique_constraints.add(
InterpreterConstraints.create_from_compatibility_fields(
(constraints_field, *first_party_plugins.interpreter_constraints_fields),
python_setup,
)
)
unique_constraints = {
InterpreterConstraints.create_from_compatibility_fields(
(
tgt[InterpreterConstraintsField],
*first_party_plugins.interpreter_constraints_fields,
),
python_setup,
)
for tgt in all_tgts
if Flake8FieldSet.is_applicable(tgt)
}
if not unique_constraints:
unique_constraints.add(
InterpreterConstraints.create_from_compatibility_fields(
Expand Down
34 changes: 13 additions & 21 deletions src/python/pants/backend/python/lint/pylint/subsystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,13 @@
from pants.core.util_rules.config_files import ConfigFilesRequest
from pants.engine.addresses import Addresses, UnparsedAddressInputs
from pants.engine.fs import EMPTY_DIGEST, AddPrefix, Digest
from pants.engine.rules import Get, MultiGet, collect_rules, rule, rule_helper
from pants.engine.rules import Get, collect_rules, rule, rule_helper
from pants.engine.target import (
AllTargets,
AllTargetsRequest,
Dependencies,
DependenciesRequest,
FieldSet,
Target,
Targets,
TransitiveTargets,
TransitiveTargetsRequest,
)
Expand All @@ -62,6 +60,7 @@ class PylintFieldSet(FieldSet):

source: PythonSourceField
dependencies: Dependencies
interpreter_constraints: InterpreterConstraintsField

@classmethod
def opt_out(cls, tgt: Target) -> bool:
Expand Down Expand Up @@ -239,29 +238,22 @@ async def _pylint_interpreter_constraints(
# While Pylint will run in partitions, we need a set of constraints that works with every
# partition. We must also consider any 3rd-party requirements used by 1st-party plugins.
#
# This first computes the constraints for each individual target, including its direct
# dependencies (which will AND across each target in the closure). Then, it ORs all unique
# This first computes the constraints for each individual target. Then, it ORs all unique
# resulting interpreter constraints. The net effect is that every possible Python interpreter
# used will be covered.
all_tgts = await Get(AllTargets, AllTargetsRequest())
relevant_targets = tuple(tgt for tgt in all_tgts if PylintFieldSet.is_applicable(tgt))
direct_deps_per_target = await MultiGet(
Get(Targets, DependenciesRequest(tgt.get(Dependencies))) for tgt in relevant_targets
)

unique_constraints = set()
for tgt, direct_deps in zip(relevant_targets, direct_deps_per_target):
constraints_fields = (
t[InterpreterConstraintsField]
for t in (tgt, *direct_deps)
if t.has_field(InterpreterConstraintsField)
)
unique_constraints.add(
InterpreterConstraints.create_from_compatibility_fields(
(*constraints_fields, *first_party_plugins.interpreter_constraints_fields),
python_setup,
)
unique_constraints = {
InterpreterConstraints.create_from_compatibility_fields(
(
tgt[InterpreterConstraintsField],
*first_party_plugins.interpreter_constraints_fields,
),
python_setup,
)
for tgt in all_tgts
if PylintFieldSet.is_applicable(tgt)
}
if not unique_constraints:
unique_constraints.add(
InterpreterConstraints.create_from_compatibility_fields(
Expand Down
36 changes: 0 additions & 36 deletions src/python/pants/backend/python/lint/pylint/subsystem_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,42 +180,6 @@ def assert_lockfile_request(
["==2.7.*", global_constraint, ">=3.8"],
)

# Also consider direct deps (but not transitive). They should be ANDed within each target's
# closure like normal, but then ORed across each closure.
assert_lockfile_request(
dedent(
"""\
python_sources(
name='transitive_dep', interpreter_constraints=['==99'], skip_pylint=True,
)
python_sources(
name='dep',
dependencies=[":transitive_dep"],
interpreter_constraints=['==2.7.*', '==3.8.*'],
skip_pylint=True,
)
python_sources(name='app', dependencies=[":dep"], interpreter_constraints=['==2.7.*'])
"""
),
["==2.7.*", "==2.7.*,==3.8.*"],
)
assert_lockfile_request(
dedent(
"""\
python_sources(
name='lib1', interpreter_constraints=['==2.7.*', '==3.8.*'], skip_pylint=True
)
python_sources(name='app1', dependencies=[":lib1"], interpreter_constraints=['==2.7.*'])
python_sources(
name='lib2', interpreter_constraints=['>=3.7'], skip_pylint=True
)
python_sources(name='app2', dependencies=[":lib2"], interpreter_constraints=['==3.9.*'])
"""
),
["==2.7.*", "==2.7.*,==3.8.*", ">=3.7,==3.9.*"],
)

# Check that source_plugins are included, even if they aren't linted directly. Plugins
# consider transitive deps.
assert_lockfile_request(
Expand Down
6 changes: 5 additions & 1 deletion src/python/pants/backend/python/subsystems/pytest.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
from pants.backend.python.subsystems.setup import PythonSetup
from pants.backend.python.target_types import (
ConsoleScript,
InterpreterConstraintsField,
PythonTestsExtraEnvVarsField,
PythonTestSourceField,
PythonTestsTimeoutField,
Expand Down Expand Up @@ -48,6 +49,7 @@ class PythonTestFieldSet(TestFieldSet):
required_fields = (PythonTestSourceField,)

source: PythonTestSourceField
interpreter_constraints: InterpreterConstraintsField
timeout: PythonTestsTimeoutField
runtime_package_dependencies: RuntimePackageDependenciesField
extra_env_vars: PythonTestsExtraEnvVarsField
Expand Down Expand Up @@ -208,7 +210,9 @@ async def _pytest_interpreter_constraints(python_setup: PythonSetup) -> Interpre
InterpreterConstraints.create_from_targets(transitive_targets.closure, python_setup)
for transitive_targets in transitive_targets_per_test
}
constraints = InterpreterConstraints(itertools.chain.from_iterable(unique_constraints))
constraints = InterpreterConstraints(
itertools.chain.from_iterable(ic for ic in unique_constraints if ic)
)
return constraints or InterpreterConstraints(python_setup.interpreter_constraints)


Expand Down
24 changes: 6 additions & 18 deletions src/python/pants/backend/python/target_types_rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@
ResolvePythonDistributionEntryPointsRequest,
)
from pants.backend.python.util_rules.interpreter_constraints import interpreter_constraints_contains
from pants.base.deprecated import deprecated_conditional
from pants.engine.addresses import Address, Addresses, UnparsedAddressInputs
from pants.engine.fs import GlobMatchErrorBehavior, PathGlobs, Paths
from pants.engine.rules import Get, MultiGet, collect_rules, rule
Expand Down Expand Up @@ -476,21 +475,10 @@ async def validate_python_dependencies(
):
non_subset_items.append(f"{dep_ics}: {dep.target.address}")

# TODO: When this deprecation triggers, it should be converted into an exception, and
# all usages of the InterpreterConstraints methods:
# * compute_for_targets
# * create_from_compatibility_fields
# * create_from_targets
# ... should be replaced with calls to InterpreterConstraintsField.value_or_global_default.
deprecated_conditional(
lambda: bool(non_subset_items),
removal_version="2.13.0.dev3",
entity=(
"the `interpreter_constraints` of a target not being a subset "
"of its dependencies' `interpreter_constraints`"
),
hint=softwrap(
f"""
if non_subset_items:
raise InvalidFieldException(
softwrap(
f"""
The target {request.field_set.address} has the `interpreter_constraints` {target_ics},
which are not a subset of the `interpreter_constraints` of some of its dependencies:
Expand All @@ -499,8 +487,8 @@ async def validate_python_dependencies(
To fix this, you should likely adjust {request.field_set.address}'s
`interpreter_constraints` to match the narrowest range in the above list.
"""
),
)
)
)

return ValidatedDependencies()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,15 @@ def add(x: int, y: int) -> int:
f"{PACKAGE}/__init__.py": "",
f"{PACKAGE}/uses_py2.py": "from project.py2 import add\nassert add(2, 2) == 4\n",
f"{PACKAGE}/uses_py3.py": "from project.py3 import add\nassert add(2, 2) == 4\n",
f"{PACKAGE}/BUILD": "python_sources(interpreter_constraints=['==2.7.*', '>=3.6'])",
f"{PACKAGE}/BUILD": dedent(
"""python_sources(
overrides={
'uses_py2.py': {'interpreter_constraints': ['==2.7.*']},
'uses_py3.py': {'interpreter_constraints': ['>=3.6']},
}
)
"""
),
}
)
py2_tgt = rule_runner.get_target(Address(PACKAGE, relative_file_path="uses_py2.py"))
Expand All @@ -458,11 +466,11 @@ def add(x: int, y: int) -> int:
py2_result, py3_result = sorted(result, key=lambda res: res.partition_description or "")

assert py2_result.exit_code == 0
assert py2_result.partition_description == "['CPython==2.7.*', 'CPython==2.7.*,>=3.6']"
assert py2_result.partition_description == "['CPython==2.7.*']"
assert "Success: no issues found" in py2_result.stdout

assert py3_result.exit_code == 0
assert py3_result.partition_description == "['CPython==2.7.*,>=3.6', 'CPython>=3.6']"
assert py3_result.partition_description == "['CPython>=3.6']"
assert "Success: no issues found" in py3_result.stdout


Expand Down
29 changes: 9 additions & 20 deletions src/python/pants/backend/python/typecheck/mypy/subsystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from pants.backend.python.subsystems.setup import PythonSetup
from pants.backend.python.target_types import (
ConsoleScript,
InterpreterConstraintsField,
PythonRequirementsField,
PythonSourceField,
)
Expand All @@ -33,8 +34,6 @@
from pants.engine.target import (
AllTargets,
AllTargetsRequest,
CoarsenedTargets,
CoarsenedTargetsRequest,
FieldSet,
Target,
TransitiveTargets,
Expand Down Expand Up @@ -62,6 +61,7 @@ class MyPyFieldSet(FieldSet):
required_fields = (PythonSourceField,)

sources: PythonSourceField
interpreter_constraints: InterpreterConstraintsField

@classmethod
def opt_out(cls, tgt: Target) -> bool:
Expand Down Expand Up @@ -283,25 +283,14 @@ async def _mypy_interpreter_constraints(
constraints = mypy.interpreter_constraints
if mypy.options.is_default("interpreter_constraints"):
all_tgts = await Get(AllTargets, AllTargetsRequest())

coarsened_targets = await Get(
CoarsenedTargets,
CoarsenedTargetsRequest(
(tgt.address for tgt in all_tgts if MyPyFieldSet.is_applicable(tgt)),
expanded_targets=True,
),
unique_constraints = {
InterpreterConstraints.create_from_targets([tgt], python_setup)
for tgt in all_tgts
if MyPyFieldSet.is_applicable(tgt)
}
code_constraints = InterpreterConstraints(
itertools.chain.from_iterable(ic for ic in unique_constraints if ic)
)

ics = InterpreterConstraints.compute_for_targets(coarsened_targets, python_setup)
if ics is None:
unique_constraints = {
InterpreterConstraints.create_from_targets(ct.closure(), python_setup)
for ct in coarsened_targets
}
else:
unique_constraints = {ic for ic in ics if ic}

code_constraints = InterpreterConstraints(itertools.chain.from_iterable(unique_constraints))
if code_constraints.requires_python38_or_newer(python_setup.interpreter_universe):
constraints = code_constraints
return constraints
Expand Down
Loading

0 comments on commit 2d4131b

Please sign in to comment.