diff --git a/src/python/pants/jvm/goals/coursier.py b/src/python/pants/jvm/goals/coursier.py index 1f7057df3f0..993a6f6ec72 100644 --- a/src/python/pants/jvm/goals/coursier.py +++ b/src/python/pants/jvm/goals/coursier.py @@ -7,7 +7,6 @@ from dataclasses import dataclass from typing import Sequence -from pants.base.glob_match_error_behavior import GlobMatchErrorBehavior from pants.engine.addresses import Address from pants.engine.console import Console from pants.engine.fs import ( @@ -47,7 +46,9 @@ def register_options(cls, register): "--names", type=list, help=( - "A list of resolve names to resolve. If not provided, resolve all known resolves." + "A list of resolve names to resolve.\n\n" + "Each name must be defined as a resolve in `[jvm].resolves`.\n\n" + "If not provided, resolve all known resolves." ), ) @@ -123,55 +124,23 @@ async def coursier_generate_lockfile( ) resolved_lockfile = await Get( - CoursierResolvedLockfile, - ArtifactRequirements, - artifact_requirements, + CoursierResolvedLockfile, ArtifactRequirements, artifact_requirements ) resolved_lockfile_json = resolved_lockfile.to_json() - - lockfile_path = jvm.options.resolves[request.resolve] - - # Materialise the existing lockfile, and check for changes. We don't want to re-write - # identical lockfiles - existing_lockfile_source = PathGlobs( - [lockfile_path], - glob_match_error_behavior=GlobMatchErrorBehavior.ignore, - ) - existing_lockfile_digest_contents = await Get( - DigestContents, PathGlobs, existing_lockfile_source - ) - - if not existing_lockfile_digest_contents: - # The user defined the target and resolved it, but hasn't created a lockfile yet. - # For convenience, create the initial lockfile for them in the specified path. - return CoursierGenerateLockfileResult( - digest=await Get( - Digest, - CreateDigest( - ( - FileContent( - path=lockfile_path, - content=resolved_lockfile_json, - ), - ) - ), - ) - ) - - existing_lockfile_json = existing_lockfile_digest_contents[0].content - - if resolved_lockfile_json != existing_lockfile_json: - # The generated lockfile differs from the existing one, so return the digest of the generated one. - return CoursierGenerateLockfileResult( - digest=await Get( - Digest, - CreateDigest((FileContent(path=lockfile_path, content=resolved_lockfile_json),)), - ) - ) - # The generated lockfile didn't change, so return an empty digest. - return CoursierGenerateLockfileResult( - digest=EMPTY_DIGEST, + lockfile_path = jvm.resolves[request.resolve] + + # If the lockfile hasn't changed, don't overwrite it. + existing_lockfile_digest_contents = await Get(DigestContents, PathGlobs([lockfile_path])) + if ( + existing_lockfile_digest_contents + and resolved_lockfile_json == existing_lockfile_digest_contents[0].content + ): + return CoursierGenerateLockfileResult(EMPTY_DIGEST) + + new_lockfile = await Get( + Digest, CreateDigest((FileContent(lockfile_path, resolved_lockfile_json),)) ) + return CoursierGenerateLockfileResult(new_lockfile) @goal_rule @@ -181,34 +150,33 @@ async def coursier_resolve_lockfiles( jvm: JvmSubsystem, workspace: Workspace, ) -> CoursierResolve: - resolves = resolve_subsystem.options.names - available_resolves = set(jvm.options.resolves.keys()) + available_resolves = set(jvm.resolves.keys()) if not resolves: - # Default behaviour is to reconcile every known resolve (this is expensive, but *shrug*) + # Default behaviour is to resolve everything. resolves = available_resolves else: invalid_resolve_names = set(resolves) - available_resolves if invalid_resolve_names: raise CoursierError( - "The following resolve names are not names of actual resolves: " - f"{invalid_resolve_names}. The valid resolve names are {available_resolves}." + "The following resolve names are not defined in `[jvm].resolves`: " + f"{invalid_resolve_names}\n\n" + f"The valid resolve names are: {available_resolves}" ) results = await MultiGet( - Get(CoursierGenerateLockfileResult, CoursierGenerateLockfileRequest(resolve=resolve)) + Get(CoursierGenerateLockfileResult, CoursierGenerateLockfileRequest(resolve)) for resolve in resolves ) # For performance reasons, avoid writing out files to the workspace that haven't changed. results_to_write = tuple(result for result in results if result.digest != EMPTY_DIGEST) if results_to_write: - merged_digest = await Get( - Digest, MergeDigests(result.digest for result in results_to_write) + merged_snapshot = await Get( + Snapshot, MergeDigests(result.digest for result in results_to_write) ) - workspace.write_digest(merged_digest) - merged_digest_snapshot = await Get(Snapshot, Digest, merged_digest) - for path in merged_digest_snapshot.files: + workspace.write_digest(merged_snapshot.digest) + for path in merged_snapshot.files: console.print_stderr(f"Updated lockfile at: {path}") return CoursierResolve(exit_code=0) diff --git a/src/python/pants/jvm/resolve/coursier_fetch.py b/src/python/pants/jvm/resolve/coursier_fetch.py index 3091f8a1d0c..319c610e2e2 100644 --- a/src/python/pants/jvm/resolve/coursier_fetch.py +++ b/src/python/pants/jvm/resolve/coursier_fetch.py @@ -29,7 +29,7 @@ ) from pants.engine.process import BashBinary, Process, ProcessResult from pants.engine.rules import Get, MultiGet, collect_rules, rule -from pants.engine.target import InvalidTargetException, Target, Targets +from pants.engine.target import Target, Targets from pants.engine.unions import UnionRule from pants.jvm.compile import ( ClasspathEntry, @@ -136,46 +136,22 @@ class ArtifactRequirement: @classmethod def from_jvm_artifact_target(cls, target: Target) -> ArtifactRequirement: if not JvmArtifactFieldSet.is_applicable(target): - raise CoursierError( + raise AssertionError( "`ArtifactRequirement.from_jvm_artifact_target()` only works on targets with " "`JvmArtifactFieldSet` fields present." ) - - group = target[JvmArtifactGroupField].value - if not group: - raise InvalidTargetException( - f"The `group` field of {target.alias} target {target.address} must be set." - ) - - artifact = target[JvmArtifactArtifactField].value - if not artifact: - raise InvalidTargetException( - f"The `artifact` field of {target.alias} target {target.address} must be set." - ) - - version = target[JvmArtifactVersionField].value - if not version: - raise InvalidTargetException( - f"The `version` field of {target.alias} target {target.address} must be set." - ) - - url = target[JvmArtifactUrlField].value - jar_ = target[JvmArtifactJarSourceField] - jar = jar_ if jar_.value else None - - if url and url.startswith("file:"): - raise CoursierError( - "Pants does not support `file:` URLS. Instead, use the `jar` field to specify the " - "relative path to the local jar file." - ) - - if url and jar: - raise CoursierError( - "You cannot specify both a `url` and `jar` for the same `jvm_artifact`." - ) - return ArtifactRequirement( - coordinate=Coordinate(group=group, artifact=artifact, version=version), url=url, jar=jar + coordinate=Coordinate( + group=target[JvmArtifactGroupField].value, + artifact=target[JvmArtifactArtifactField].value, + version=target[JvmArtifactVersionField].value, + ), + url=target[JvmArtifactUrlField].value, + jar=( + target[JvmArtifactJarSourceField] + if target[JvmArtifactJarSourceField].value + else None + ), ) def to_coord_str(self, versioned: bool = True) -> str: diff --git a/src/python/pants/jvm/target_types.py b/src/python/pants/jvm/target_types.py index 29ada19f603..09928cd0b27 100644 --- a/src/python/pants/jvm/target_types.py +++ b/src/python/pants/jvm/target_types.py @@ -4,10 +4,14 @@ from __future__ import annotations from abc import ABCMeta +from typing import Optional +from pants.engine.addresses import Address from pants.engine.target import ( COMMON_TARGET_FIELDS, FieldSet, + InvalidFieldException, + InvalidTargetException, SingleSourceField, StringField, StringSequenceField, @@ -15,6 +19,34 @@ ) from pants.util.docutil import git_url +# ----------------------------------------------------------------------------------------------- +# Generic resolve support fields +# ----------------------------------------------------------------------------------------------- + + +class JvmCompatibleResolvesField(StringSequenceField): + alias = "compatible_resolves" + 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." + ) + + +class JvmResolveField(StringField): + alias = "resolve" + 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." + ) + + # ----------------------------------------------------------------------------------------------- # `jvm_artifact` targets # ----------------------------------------------------------------------------------------------- @@ -27,6 +59,7 @@ class JvmArtifactGroupField(StringField): alias = "group" required = True + value: str help = ( "The 'group' part of a Maven-compatible coordinate to a third-party JAR artifact.\n\n" "For the JAR coordinate `com.google.guava:guava:30.1.1-jre`, the group is " @@ -37,6 +70,7 @@ class JvmArtifactGroupField(StringField): class JvmArtifactArtifactField(StringField): alias = "artifact" required = True + value: str help = ( "The 'artifact' part of a Maven-compatible coordinate to a third-party JAR artifact.\n\n" "For the JAR coordinate `com.google.guava:guava:30.1.1-jre`, the artifact is `guava`." @@ -46,6 +80,7 @@ class JvmArtifactArtifactField(StringField): class JvmArtifactVersionField(StringField): alias = "version" required = True + value: str help = ( "The 'version' part of a Maven-compatible coordinate to a third-party JAR artifact.\n\n" "For the JAR coordinate `com.google.guava:guava:30.1.1-jre`, the version is `30.1.1-jre`." @@ -76,6 +111,17 @@ class JvmArtifactJarSourceField(SingleSourceField): "Use the `url` field for remote artifacts." ) + @classmethod + def compute_value(cls, raw_value: Optional[str], address: Address) -> Optional[str]: + value_or_default = super().compute_value(raw_value, address) + if value_or_default and value_or_default.startswith("file:"): + raise InvalidFieldException( + f"The `{cls.alias}` field does not support `file:` URLS, but the target " + f"{address} sets the field to `{value_or_default}`.\n\n" + "Instead, use the `jar` field to specify the relative path to the local jar file." + ) + return value_or_default + class JvmArtifactPackagesField(StringSequenceField): alias = "packages" @@ -109,7 +155,6 @@ class JvmProvidesTypesField(StringSequenceField): class JvmArtifactFieldSet(FieldSet): - group: JvmArtifactGroupField artifact: JvmArtifactArtifactField version: JvmArtifactVersionField @@ -137,6 +182,13 @@ class JvmArtifactTarget(Target): "That is, an artifact identified by its `group`, `artifact`, and `version` components." ) + def validate(self) -> None: + if self[JvmArtifactJarSourceField].value and self[JvmArtifactUrlField].value: + raise InvalidTargetException( + f"You cannot specify both the `url` and `jar` fields, but both were set on the " + f"`{self.alias}` target {self.address}." + ) + # ----------------------------------------------------------------------------------------------- # JUnit test support field(s) @@ -145,31 +197,3 @@ class JvmArtifactTarget(Target): class JunitTestSourceField(SingleSourceField, metaclass=ABCMeta): """A marker that indicates that a source field represents a JUnit test.""" - - -# ----------------------------------------------------------------------------------------------- -# Generic resolve support fields -# ----------------------------------------------------------------------------------------------- - - -class JvmCompatibleResolvesField(StringSequenceField): - alias = "compatible_resolves" - 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." - ) - - -class JvmResolveField(StringField): - alias = "resolve" - 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." - )