From 23dab722b7764ae89dfae401f84e99ad1d70ed35 Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Wed, 15 Dec 2021 18:27:37 -0700 Subject: [PATCH 1/3] [internal] Add `[jvm].default_compatible_resolves` [ci skip-rust] [ci skip-build-wheels] --- src/python/pants/jvm/goals/coursier.py | 8 ++----- .../jvm/goals/coursier_integration_test.py | 2 +- .../pants/jvm/resolve/coursier_fetch.py | 3 +-- src/python/pants/jvm/subsystems.py | 24 ++++++++++++++++++- src/python/pants/jvm/target_types.py | 16 ++++++------- 5 files changed, 34 insertions(+), 19 deletions(-) diff --git a/src/python/pants/jvm/goals/coursier.py b/src/python/pants/jvm/goals/coursier.py index 6f06e5284d9..fa415f51186 100644 --- a/src/python/pants/jvm/goals/coursier.py +++ b/src/python/pants/jvm/goals/coursier.py @@ -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 ( @@ -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( diff --git a/src/python/pants/jvm/goals/coursier_integration_test.py b/src/python/pants/jvm/goals/coursier_integration_test.py index b4b1b2afc9b..909a782942c 100644 --- a/src/python/pants/jvm/goals/coursier_integration_test.py +++ b/src/python/pants/jvm/goals/coursier_integration_test.py @@ -58,7 +58,7 @@ ARGS = [ "--jvm-resolves={'test': 'coursier_resolve.lockfile'}", - "--jvm-default-resolve=test", + "--jvm-default-compatible-resolves=test", "--coursier-resolve-names=test", ] diff --git a/src/python/pants/jvm/resolve/coursier_fetch.py b/src/python/pants/jvm/resolve/coursier_fetch.py index 249dacb2975..1e449c2772b 100644 --- a/src/python/pants/jvm/resolve/coursier_fetch.py +++ b/src/python/pants/jvm/resolve/coursier_fetch.py @@ -642,8 +642,7 @@ 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 diff --git a/src/python/pants/jvm/subsystems.py b/src/python/pants/jvm/subsystems.py index 5abfacda479..60bcf402d43 100644 --- a/src/python/pants/jvm/subsystems.py +++ b/src/python/pants/jvm/subsystems.py @@ -18,6 +18,8 @@ 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( @@ -25,7 +27,23 @@ def register_options(cls, register): 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 @@ -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) diff --git a/src/python/pants/jvm/target_types.py b/src/python/pants/jvm/target_types.py index e415eeda087..86cffaf387a 100644 --- a/src/python/pants/jvm/target_types.py +++ b/src/python/pants/jvm/target_types.py @@ -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. ) @@ -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. ) @@ -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 " From 42b3dd792e190cff684aa69a802dbbad5e3aa416 Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Thu, 16 Dec 2021 13:00:38 -0700 Subject: [PATCH 2/3] Set default compatible resolve for pantsbuild/pants # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] --- pants.toml | 1 + src/python/pants/jvm/resolve/coursier_fetch.py | 8 ++++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/pants.toml b/pants.toml index 8364185cfd5..d2532fd823e 100644 --- a/pants.toml +++ b/pants.toml @@ -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 diff --git a/src/python/pants/jvm/resolve/coursier_fetch.py b/src/python/pants/jvm/resolve/coursier_fetch.py index 1e449c2772b..b46a432070a 100644 --- a/src/python/pants/jvm/resolve/coursier_fetch.py +++ b/src/python/pants/jvm/resolve/coursier_fetch.py @@ -650,14 +650,18 @@ async def select_coursier_resolve_for_targets( # 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] From 517a09e2f3e7693248d8eb04e6a2d4fc26dc4cd3 Mon Sep 17 00:00:00 2001 From: Eric Arellano Date: Thu, 16 Dec 2021 14:05:41 -0700 Subject: [PATCH 3/3] Fix tests # Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels] --- src/python/pants/backend/java/compile/javac_test.py | 2 +- src/python/pants/backend/java/package/deploy_jar_test.py | 6 +++++- src/python/pants/backend/scala/compile/scalac_test.py | 2 +- src/python/pants/jvm/compile_test.py | 2 +- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/python/pants/backend/java/compile/javac_test.py b/src/python/pants/backend/java/compile/javac_test.py index 7a45e9e376b..9e919ab209e 100644 --- a/src/python/pants/backend/java/compile/javac_test.py +++ b/src/python/pants/backend/java/compile/javac_test.py @@ -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 diff --git a/src/python/pants/backend/java/package/deploy_jar_test.py b/src/python/pants/backend/java/package/deploy_jar_test.py index 0a9d7903b47..5fbe87aef4d 100644 --- a/src/python/pants/backend/java/package/deploy_jar_test.py +++ b/src/python/pants/backend/java/package/deploy_jar_test.py @@ -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 diff --git a/src/python/pants/backend/scala/compile/scalac_test.py b/src/python/pants/backend/scala/compile/scalac_test.py index 6cf934880b5..0a2d75593da 100644 --- a/src/python/pants/backend/scala/compile/scalac_test.py +++ b/src/python/pants/backend/scala/compile/scalac_test.py @@ -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 diff --git a/src/python/pants/jvm/compile_test.py b/src/python/pants/jvm/compile_test.py index fabdceaa3b8..ca707f6fac2 100644 --- a/src/python/pants/jvm/compile_test.py +++ b/src/python/pants/jvm/compile_test.py @@ -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