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.

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

[internal] Add [jvm].default_compatible_resolves #13902

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions pants.toml
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ custom_command = "build-support/bin/generate_all_lockfiles.sh"

[jvm]
default_resolve = "jvm_testprojects"
default_compatible_resolves = ["jvm_testprojects"]

[jvm.resolves]
# A shared resolve for all testproject/example code. Because this is not shipped with Pants
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/backend/java/compile/javac_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
from pants.testutil.rule_runner import PYTHON_BOOTSTRAP_ENV, QueryRule, RuleRunner, logging

NAMED_RESOLVE_OPTIONS = '--jvm-resolves={"test": "coursier_resolve.lockfile"}'
DEFAULT_RESOLVE_OPTION = "--jvm-default-resolve=test"
DEFAULT_RESOLVE_OPTION = "--jvm-default-compatible-resolves=test"


@pytest.fixture
Expand Down
6 changes: 5 additions & 1 deletion src/python/pants/backend/java/package/deploy_jar_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,11 @@ def rule_runner() -> RuleRunner:
],
)
rule_runner.set_options(
args=['--jvm-resolves={"test": "coursier_resolve.lockfile"}', "--jvm-default-resolve=test"],
args=[
'--jvm-resolves={"test": "coursier_resolve.lockfile"}',
"--jvm-default-resolve=test",
"--jvm-default-compatible-resolves=test",
],
env_inherit=PYTHON_BOOTSTRAP_ENV,
)
return rule_runner
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/backend/scala/compile/scalac_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
from pants.testutil.rule_runner import PYTHON_BOOTSTRAP_ENV, QueryRule, RuleRunner, logging

NAMED_RESOLVE_OPTIONS = '--jvm-resolves={"test": "coursier_resolve.lockfile"}'
DEFAULT_RESOLVE_OPTION = "--jvm-default-resolve=test"
DEFAULT_RESOLVE_OPTION = "--jvm-default-compatible-resolves=test"


@pytest.fixture
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/jvm/compile_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
from pants.testutil.rule_runner import PYTHON_BOOTSTRAP_ENV, QueryRule, RuleRunner

NAMED_RESOLVE_OPTIONS = '--jvm-resolves={"test": "coursier_resolve.lockfile"}'
DEFAULT_RESOLVE_OPTION = "--jvm-default-resolve=test"
DEFAULT_RESOLVE_OPTION = "--jvm-default-compatible-resolves=test"


@pytest.fixture
Expand Down
8 changes: 2 additions & 6 deletions src/python/pants/jvm/goals/coursier.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@

from collections import defaultdict
from dataclasses import dataclass
from typing import Sequence

from pants.engine.console import Console
from pants.engine.fs import (
Expand Down Expand Up @@ -67,13 +66,10 @@ async def map_resolves_to_consuming_targets(
for tgt in all_targets:
if not tgt.has_field(JvmArtifactCompatibleResolvesField):
continue
# TODO: add a `default_compatible_resolves` field.
artifact = ArtifactRequirement.from_jvm_artifact_target(tgt)
# TODO: error if no resolves for the target. Wait to change until we automatically set a
# default resolve.
resolves: Sequence[str] = tgt[JvmArtifactCompatibleResolvesField].value or (
[jvm.default_resolve] if jvm.default_resolve is not None else []
)
artifact = ArtifactRequirement.from_jvm_artifact_target(tgt)
resolves = tgt[JvmArtifactCompatibleResolvesField].value or jvm.default_compatible_resolves
for resolve in resolves:
resolve_to_artifacts[resolve].add(artifact)
return JvmResolvesToArtifacts(
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/jvm/goals/coursier_integration_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@

ARGS = [
"--jvm-resolves={'test': 'coursier_resolve.lockfile'}",
"--jvm-default-resolve=test",
"--jvm-default-compatible-resolves=test",
"--coursier-resolve-names=test",
]

Expand Down
11 changes: 7 additions & 4 deletions src/python/pants/jvm/resolve/coursier_fetch.py
Original file line number Diff line number Diff line change
Expand Up @@ -642,23 +642,26 @@ async def select_coursier_resolve_for_targets(
encountered_resolves.append(resolve)
if tgt.has_field(JvmCompatibleResolvesField):
encountered_compatible_resolves.extend(
tgt[JvmCompatibleResolvesField].value
or ([jvm.default_resolve] if jvm.default_resolve is not None else [])
tgt[JvmCompatibleResolvesField].value or jvm.default_compatible_resolves
)

# TODO: validate that all specified resolves are defined in [jvm].resolves and that all
# encountered resolves are compatible with each other. Note that we don't validate that
# dependencies are compatible, just the specified targets.

if len(encountered_resolves) > 1:
raise AssertionError(f"Encountered >1 `resolve` field, which was not expected. {targets}")
raise AssertionError(
"Encountered >1 `resolve` field, which was not expected: "
f"{sorted(tgt.address for tgt in targets)}"
)
elif len(encountered_resolves) == 1:
resolve = encountered_resolves[0]
elif encountered_compatible_resolves:
resolve = min(encountered_compatible_resolves)
else:
raise AssertionError(
f"No `resolve` or `compatible_resolves` specified for these targets: {targets}"
f"No `resolve` or `compatible_resolves` specified for these targets: "
f"{sorted(tgt.address for tgt in targets)}"
)

resolve_path = jvm.resolves[resolve]
Expand Down
24 changes: 23 additions & 1 deletion src/python/pants/jvm/subsystems.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,32 @@ def register_options(cls, register):
register(
"--resolves",
type=dict,
# TODO: Default this to something like {'jvm-default': '3rdparty/jvm/default.lockfile'}.
# TODO: expand help message
help="A dictionary mapping resolve names to the path of their lockfile.",
)
register(
"--default-resolve",
type=str,
# TODO: Default this to something like `jvm-default`.
default=None,
help="The name of the resolve to use by default.",
help=(
"The default value for the `resolve` field used by targets like `junit_test` and "
"`deploy_jar`.\n\n"
"The name must be defined as a resolve in `[jvm].resolves`.",
),
)
register(
"--default-compatible-resolves",
type=list,
member_type=str,
# TODO: Default this to something like `['jvm-default']`.
default=[],
help=(
"The default value for the `compatible_resolves` field used by targets like "
"`jvm_artifact` and `java_source`/`scala_source`.\n\n"
"Each name must be defined as a resolve in `[jvm].resolves`."
),
)

@property
Expand All @@ -35,3 +53,7 @@ def resolves(self) -> dict[str, str]:
@property
def default_resolve(self) -> str | None:
return cast(str, self.options.default_resolve)

@property
def default_compatible_resolves(self) -> tuple[str, ...]:
return tuple(self.options.default_compatible_resolves)
16 changes: 7 additions & 9 deletions src/python/pants/jvm/target_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,9 @@ class JvmCompatibleResolvesField(StringSequenceField):
required = False
help = (
"The set of resolve names that this target is compatible with.\n\n"
"Each name must be defined as a resolve in `[jvm].resolves`.\n\n"
"Any targets which depend on one another must have at least one compatible resolve in "
"common. Which resolves are actually used in a build is calculated based on a target's "
"dependees."
"If not defined, will default to `[jvm].default_compatible_resolves`.\n\n"
"Each name must be defined as a resolve in `[jvm].resolves`."
# TODO: Document expectations for dependencies once we validate that.
)


Expand All @@ -41,9 +40,9 @@ class JvmResolveField(StringField):
required = False
help = (
"The name of the resolve to use when building this target.\n\n"
"Each name must be defined as a resolve in `[jvm].resolves`.\n\n"
"If not supplied, the default resolve will be used. Otherwise, one resolve that is "
"compatible with all dependency targets will be used."
"If not defined, will default to `[jvm].default_resolve`.\n\n"
"The name must be defined as a resolve in `[jvm].resolves`."
# TODO: Document expectations for dependencies once we validate that.
)


Expand Down Expand Up @@ -157,8 +156,7 @@ class JvmProvidesTypesField(StringSequenceField):
class JvmArtifactCompatibleResolvesField(JvmCompatibleResolvesField):
help = (
"The resolves that this artifact should be included in.\n\n"
# TODO: Switch to `[jvm].default_compatible_resolves`.
"If not defined, will default to `[jvm].default_resolve`.\n\n"
"If not defined, will default to `[jvm].default_compatible_resolves`.\n\n"
"Each name must be defined as a resolve in `[jvm].resolves`.\n\n"
"When generating a lockfile for a particular resolve via the `coursier-resolve` goal, "
"it will include all artifacts that are declared compatible with that resolve. First-party "
Expand Down