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.

Sign up for GitHub

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

Jump to bottom

python_requirement uses resolve: str field, not compatible_resolves: list[str] #14420

Merged
merged 1 commit into from
Feb 10, 2022
Merged

python_requirement uses resolve: str field, not compatible_resolves: list[str] #14420

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 3 additions & 5 deletions src/python/pants/backend/plugin_development/pants_requirements.py
View file
Open in desktop
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
8 changes: 4 additions & 4 deletions src/python/pants/backend/plugin_development/pants_requirements_test.py
View file
Open in desktop
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"
21 changes: 10 additions & 11 deletions src/python/pants/backend/python/dependency_inference/module_mapper.py
View file
Open in desktop
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
12 changes: 6 additions & 6 deletions src/python/pants/backend/python/dependency_inference/module_mapper_test.py
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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"
Comment on lines 127 to +133
Copy link
Member

@stuhood stuhood Feb 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This statement is still true, but for a different reason now, maybe? Basically, the same sources could have multiple resolves if you 1) declare two targets (maybe with a macro), 2) (in 2.11) use parametrize.

It might be worth calling out 1 as an option, and saying that 2 is coming...?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue is dep inference won't work with python_source, so I figured we shouldn't advertise it. But, if I can knock out #14293 in the next week we could cherry-pick it, then multiple targets is totally viable. Wdyt?

Copy link
Member

@stuhood stuhood Feb 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I supposed that if it would still mean writing your own macros, it's probably ok to wait.

"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