Skip to content

Commit

Permalink
python_requirement uses resolve: str field, not `compatible_resol…
Browse files Browse the repository at this point in the history
…ves: list[str]` (#14420)

Generally, we've realized that targets like `python_source` should use `resolve: str`: #14299. This solves several problems, particularly how to choose the resolve when operating directly on `python_source` targets e.g. with MyPy.

We wanted to keep `compatible_resovles` for `python_requirement` out of convenience, that it's nice for callers to be able to depend on `//:reqs#numpy` rather than `//:reqs#numpy@resolve=a`. The argument was that `python_requirement` is the "leaf" - it's not directly operated on. 

That was a bad assumption. `python_requirement` is directly operated on with `export` and `repl`, e.g. `./pants repl 3rdparty/python::`. We were defaulting in that case to `[python].default_resolve` -- but that doesn't make sense! If you're running `./pants repl pants-plugins/3rdparty::`, why would Pants try to use `python-default`? We don't want to get into the business of trying to disambiguate which resolve you want, either, as that's very magical and confusing.

Beyond `repl` and `export`, @stuhood has wisely pointed out that this change is important for clarity. Even though `//:reqs#numpy` would have the same raw requirement string, the actual pinned version might change depending on the resolve! For example, `numpy>=2.0,<3` might result in `==2.3.1` in resolve A, but `==2.7.4` in resolve B. _They are not the same thing_.

[ci skip-rust]
[ci skip-build-wheels]
  • Loading branch information
Eric-Arellano authored Feb 10, 2022
1 parent 3e9d830 commit e0a4275
Show file tree
Hide file tree
Showing 14 changed files with 96 additions and 151 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from pants.backend.python.target_types import (
PythonRequirementCompatibleResolvesField,
PythonRequirementModulesField,
PythonRequirementResolveField,
PythonRequirementsField,
PythonRequirementTarget,
)
Expand Down Expand Up @@ -45,7 +45,7 @@ class PantsRequirementsTargetGenerator(Target):
core_fields = (
*COMMON_TARGET_FIELDS,
PantsRequirementsTestutilField,
PythonRequirementCompatibleResolvesField,
PythonRequirementResolveField,
)


Expand Down Expand Up @@ -88,9 +88,7 @@ def create_tgt(dist: str, module: str) -> PythonRequirementTarget:
{
PythonRequirementsField.alias: (f"{dist}{version}",),
PythonRequirementModulesField.alias: (module,),
PythonRequirementCompatibleResolvesField.alias: generator[
PythonRequirementCompatibleResolvesField
].value,
PythonRequirementResolveField.alias: generator[PythonRequirementResolveField].value,
},
generator.address.create_generated(dist),
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
)
from pants.backend.python.pip_requirement import PipRequirement
from pants.backend.python.target_types import (
PythonRequirementCompatibleResolvesField,
PythonRequirementModulesField,
PythonRequirementResolveField,
PythonRequirementsField,
)
from pants.engine.addresses import Address
Expand Down Expand Up @@ -48,7 +48,7 @@ def test_target_generator() -> None:
"BUILD": (
"pants_requirements(name='default')\n"
"pants_requirements(\n"
" name='no_testutil', testutil=False, compatible_resolves=['a']\n"
" name='no_testutil', testutil=False, resolve='a'\n"
")"
)
}
Expand All @@ -72,7 +72,7 @@ def test_target_generator() -> None:
PipRequirement.parse(f"pantsbuild.pants.testutil{determine_version()}"),
)
for t in (pants_req, testutil_req):
assert not t[PythonRequirementCompatibleResolvesField].value
assert not t[PythonRequirementResolveField].value

generator = rule_runner.get_target(Address("", target_name="no_testutil"))
result = rule_runner.request(
Expand All @@ -81,4 +81,4 @@ def test_target_generator() -> None:
assert len(result) == 1
assert next(iter(result.keys())).generated_name == "pantsbuild.pants"
pants_req = next(iter(result.values()))
assert pants_req[PythonRequirementCompatibleResolvesField].value == ("a",)
assert pants_req[PythonRequirementResolveField].value == "a"
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
)
from pants.backend.python.subsystems.setup import PythonSetup
from pants.backend.python.target_types import (
PythonRequirementCompatibleResolvesField,
PythonRequirementModulesField,
PythonRequirementResolveField,
PythonRequirementsField,
PythonRequirementTypeStubModulesField,
PythonSourceField,
Expand Down Expand Up @@ -222,19 +222,18 @@ async def map_third_party_modules_to_addresses(
] = {}

for tgt in all_python_tgts.third_party:
resolves = tgt[PythonRequirementCompatibleResolvesField].normalized_value(python_setup)
resolve = tgt[PythonRequirementResolveField].normalized_value(python_setup)

def add_modules(modules: Iterable[str], *, type_stub: bool = False) -> None:
for resolve in resolves:
if resolve not in resolves_to_modules_to_providers:
resolves_to_modules_to_providers[resolve] = defaultdict(list)
for module in modules:
resolves_to_modules_to_providers[resolve][module].append(
ModuleProvider(
tgt.address,
ModuleProviderType.TYPE_STUB if type_stub else ModuleProviderType.IMPL,
)
if resolve not in resolves_to_modules_to_providers:
resolves_to_modules_to_providers[resolve] = defaultdict(list)
for module in modules:
resolves_to_modules_to_providers[resolve][module].append(
ModuleProvider(
tgt.address,
ModuleProviderType.TYPE_STUB if type_stub else ModuleProviderType.IMPL,
)
)

explicit_modules = tgt.get(PythonRequirementModulesField).value
if explicit_modules:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -299,13 +299,13 @@ def req(
*,
modules: list[str] | None = None,
stub_modules: list[str] | None = None,
resolves: list[str] | None = None,
resolve: str = "default",
) -> str:
return (
f"python_requirement(name='{tgt_name}', requirements=['{req_str}'], "
f"modules={modules or []},"
f"type_stub_modules={stub_modules or []},"
f"compatible_resolves={resolves or ['default']})"
f"resolve={repr(resolve)})"
)

build_file = "\n\n".join(
Expand All @@ -323,8 +323,8 @@ def req(
req("typed-dep5", "typed-dep5-foo", stub_modules=["typed_dep5"]),
# A 3rd-party dependency can have both a type stub and implementation.
req("multiple_owners1", "multiple_owners==1"),
req("multiple_owners2", "multiple_owners==2", resolves=["another"]),
req("multiple_owners_types", "types-multiple_owners==1", resolves=["another"]),
req("multiple_owners2", "multiple_owners==2", resolve="another"),
req("multiple_owners_types", "types-multiple_owners==1", resolve="another"),
# Only assume it's a type stubs dep if we are certain it's not an implementation.
req("looks_like_stubs", "looks-like-stubs-types", modules=["looks_like_stubs"]),
]
Expand Down Expand Up @@ -592,13 +592,13 @@ def test_map_module_considers_resolves(rule_runner: RuleRunner) -> None:
# result in ambiguity.
python_requirement(
name="dep1",
compatible_resolves=["a"],
resolve="a",
requirements=["dep"],
)
python_requirement(
name="dep2",
compatible_resolves=["b"],
resolve="b",
requirements=["dep"],
)
"""
Expand Down
8 changes: 4 additions & 4 deletions src/python/pants/backend/python/goals/lockfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
from pants.backend.python.subsystems.setup import PythonSetup
from pants.backend.python.target_types import (
EntryPoint,
PythonRequirementCompatibleResolvesField,
PythonRequirementResolveField,
PythonRequirementsField,
)
from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints
Expand Down Expand Up @@ -271,10 +271,10 @@ async def setup_user_lockfile_requests(

resolve_to_requirements_fields = defaultdict(set)
for tgt in all_targets:
if not tgt.has_fields((PythonRequirementCompatibleResolvesField, PythonRequirementsField)):
if not tgt.has_fields((PythonRequirementResolveField, PythonRequirementsField)):
continue
for resolve in tgt[PythonRequirementCompatibleResolvesField].normalized_value(python_setup):
resolve_to_requirements_fields[resolve].add(tgt[PythonRequirementsField])
resolve = tgt[PythonRequirementResolveField].normalized_value(python_setup)
resolve_to_requirements_fields[resolve].add(tgt[PythonRequirementsField])

return UserGenerateLockfiles(
GeneratePythonLockfile(
Expand Down
13 changes: 4 additions & 9 deletions src/python/pants/backend/python/goals/lockfile_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,20 +88,15 @@ def test_multiple_resolves() -> None:
{
"BUILD": dedent(
"""\
python_requirement(
name='both',
requirements=['both1', 'both2'],
compatible_resolves=['a', 'b'],
)
python_requirement(
name='a',
requirements=['a'],
compatible_resolves=['a'],
resolve='a',
)
python_requirement(
name='b',
requirements=['b'],
compatible_resolves=['b'],
resolve='b',
)
"""
),
Expand All @@ -121,15 +116,15 @@ def test_multiple_resolves() -> None:
)
assert set(result) == {
GeneratePythonLockfile(
requirements=FrozenOrderedSet(["a", "both1", "both2"]),
requirements=FrozenOrderedSet(["a"]),
interpreter_constraints=InterpreterConstraints(
PythonSetup.default_interpreter_constraints
),
resolve_name="a",
lockfile_dest="a.lock",
),
GeneratePythonLockfile(
requirements=FrozenOrderedSet(["b", "both1", "both2"]),
requirements=FrozenOrderedSet(["b"]),
interpreter_constraints=InterpreterConstraints(["==3.7.*"]),
resolve_name="b",
lockfile_dest="b.lock",
Expand Down
15 changes: 12 additions & 3 deletions src/python/pants/backend/python/goals/repl_integration_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,11 @@
from pants.backend.python.goals import repl as python_repl
from pants.backend.python.subsystems.ipython import rules as ipython_subsystem_rules
from pants.backend.python.subsystems.setup import PythonSetup
from pants.backend.python.target_types import PythonSourcesGeneratorTarget, PythonSourceTarget
from pants.backend.python.target_types import (
PythonRequirementTarget,
PythonSourcesGeneratorTarget,
PythonSourceTarget,
)
from pants.backend.python.util_rules import local_dists, pex_from_targets
from pants.backend.python.util_rules.pex import PexProcess
from pants.backend.python.util_rules.pex_from_targets import NoCompatibleResolveException
Expand Down Expand Up @@ -39,7 +43,12 @@ def rule_runner() -> RuleRunner:
*local_dists.rules(),
QueryRule(Process, (PexProcess,)),
],
target_types=[PythonSourcesGeneratorTarget, ProtobufSourceTarget, PythonSourceTarget],
target_types=[
PythonSourcesGeneratorTarget,
ProtobufSourceTarget,
PythonSourceTarget,
PythonRequirementTarget,
],
)
rule_runner.write_files(
{
Expand Down Expand Up @@ -97,7 +106,7 @@ def test_eagerly_validate_roots_have_common_resolve(rule_runner: RuleRunner) ->
{
"BUILD": dedent(
"""\
python_source(name='t1', source='f.py', resolve='a')
python_requirement(name='t1', requirements=[], resolve='a')
python_source(name='t2', source='f.py', resolve='b')
"""
)
Expand Down
9 changes: 5 additions & 4 deletions src/python/pants/backend/python/macros/pipenv_requirements.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@
from pants.backend.python.pip_requirement import PipRequirement
from pants.backend.python.subsystems.setup import PythonSetup
from pants.backend.python.target_types import (
PythonRequirementCompatibleResolvesField,
PythonRequirementModulesField,
PythonRequirementResolveField,
PythonRequirementsField,
PythonRequirementsFileSourcesField,
PythonRequirementsFileTarget,
Expand Down Expand Up @@ -54,7 +54,7 @@ class PipenvRequirementsTargetGenerator(Target):
TypeStubsModuleMappingField,
PipenvSourceField,
RequirementsOverrideField,
PythonRequirementCompatibleResolvesField,
PythonRequirementResolveField,
)


Expand Down Expand Up @@ -91,15 +91,16 @@ async def generate_from_pipenv_requirement(
)
lock_info = json.loads(digest_contents[0].content)

generator[PythonRequirementCompatibleResolvesField].normalized_value(python_setup)
# Validate the resolve is legal.
generator[PythonRequirementResolveField].normalized_value(python_setup)

module_mapping = generator[ModuleMappingField].value
stubs_mapping = generator[TypeStubsModuleMappingField].value
overrides = generator[RequirementsOverrideField].flatten_and_normalize()
inherited_fields = {
field.alias: field.value
for field in request.generator.field_values.values()
if isinstance(field, (*COMMON_TARGET_FIELDS, PythonRequirementCompatibleResolvesField))
if isinstance(field, (*COMMON_TARGET_FIELDS, PythonRequirementResolveField))
}

def generate_tgt(raw_req: str, info: dict) -> PythonRequirementTarget:
Expand Down
9 changes: 5 additions & 4 deletions src/python/pants/backend/python/macros/poetry_requirements.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@
from pants.backend.python.pip_requirement import PipRequirement
from pants.backend.python.subsystems.setup import PythonSetup
from pants.backend.python.target_types import (
PythonRequirementCompatibleResolvesField,
PythonRequirementModulesField,
PythonRequirementResolveField,
PythonRequirementsField,
PythonRequirementsFileSourcesField,
PythonRequirementsFileTarget,
Expand Down Expand Up @@ -406,7 +406,7 @@ class PoetryRequirementsTargetGenerator(Target):
TypeStubsModuleMappingField,
PoetryRequirementsSourceField,
RequirementsOverrideField,
PythonRequirementCompatibleResolvesField,
PythonRequirementResolveField,
)


Expand Down Expand Up @@ -448,15 +448,16 @@ async def generate_from_python_requirement(
)
)

generator[PythonRequirementCompatibleResolvesField].normalized_value(python_setup)
# Validate the resolve is legal.
generator[PythonRequirementResolveField].normalized_value(python_setup)

module_mapping = generator[ModuleMappingField].value
stubs_mapping = generator[TypeStubsModuleMappingField].value
overrides = generator[RequirementsOverrideField].flatten_and_normalize()
inherited_fields = {
field.alias: field.value
for field in request.generator.field_values.values()
if isinstance(field, (*COMMON_TARGET_FIELDS, PythonRequirementCompatibleResolvesField))
if isinstance(field, (*COMMON_TARGET_FIELDS, PythonRequirementResolveField))
}

def generate_tgt(parsed_req: PipRequirement) -> PythonRequirementTarget:
Expand Down
9 changes: 5 additions & 4 deletions src/python/pants/backend/python/macros/python_requirements.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
from pants.backend.python.pip_requirement import PipRequirement
from pants.backend.python.subsystems.setup import PythonSetup
from pants.backend.python.target_types import (
PythonRequirementCompatibleResolvesField,
PythonRequirementModulesField,
PythonRequirementResolveField,
PythonRequirementsField,
PythonRequirementsFileSourcesField,
PythonRequirementsFileTarget,
Expand Down Expand Up @@ -63,7 +63,7 @@ class PythonRequirementsTargetGenerator(Target):
TypeStubsModuleMappingField,
PythonRequirementsSourceField,
RequirementsOverrideField,
PythonRequirementCompatibleResolvesField,
PythonRequirementResolveField,
)


Expand Down Expand Up @@ -103,15 +103,16 @@ async def generate_from_python_requirement(
requirements, lambda parsed_req: parsed_req.project_name
)

generator[PythonRequirementCompatibleResolvesField].normalized_value(python_setup)
# Validate the resolve is legal.
generator[PythonRequirementResolveField].normalized_value(python_setup)

module_mapping = generator[ModuleMappingField].value
stubs_mapping = generator[TypeStubsModuleMappingField].value
overrides = generator[RequirementsOverrideField].flatten_and_normalize()
inherited_fields = {
field.alias: field.value
for field in request.generator.field_values.values()
if isinstance(field, (*COMMON_TARGET_FIELDS, PythonRequirementCompatibleResolvesField))
if isinstance(field, (*COMMON_TARGET_FIELDS, PythonRequirementResolveField))
}

def generate_tgt(
Expand Down
19 changes: 9 additions & 10 deletions src/python/pants/backend/python/subsystems/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,20 +125,19 @@ def register_options(cls, register):
"resolves, such as if you use two conflicting versions of a requirement in "
"your repository.\n\n"
"For now, Pants only has first-class support for disjoint resolves, meaning that "
"you cannot ergonomically set a `python_source` target, for example, to work "
"with multiple resolves. Practically, this means that you cannot yet reuse common "
"code, such as util files, across projects using different resolves. Support for "
"overlapping resolves is coming soon.\n\n"
f"If you only need a single resolve, run `{bin_name()} generate-lockfiles` to generate "
"the lockfile.\n\n"
"you cannot ergonomically set a `python_requirement` or `python_source` target, "
"for example, to work with multiple resolves. Practically, this means that you "
"cannot yet ergonomically reuse common code, such as util files, across projects "
"using different resolves. Support for overlapping resolves is coming soon.\n\n"
f"If you only need a single resolve, run `{bin_name()} generate-lockfiles` to "
"generate the lockfile.\n\n"
"If you need multiple resolves:\n\n"
" 1. Via this option, define multiple resolve "
"names and their lockfile paths. The names should be meaningful to your "
"repository, such as `data-science` or `pants-plugins`.\n"
" 2. Set the default with "
"`[python].default_resolve`.\n"
" 2. Set the default with `[python].default_resolve`.\n"
" 3. Update your `python_requirement` targets with the "
"`compatible_resolves` field to declare which resolve(s) they should "
"`resolve` field to declare which resolve they should "
"be available in. They default to `[python].default_resolve`, so you "
"only need to update targets that you want in non-default resolves. "
"(Often you'll set this via the `python_requirements` or `poetry_requirements` "
Expand All @@ -157,7 +156,7 @@ def register_options(cls, register):
type=str,
default="python-default",
help=(
"The default value used for the `resolve` and `compatible_resolves` fields.\n\n"
"The default value used for the `resolve` field.\n\n"
"The name must be defined as a resolve in `[python].resolves`."
),
)
Expand Down
Loading

0 comments on commit e0a4275

Please sign in to comment.