From c00bb0612f581e6c8ab0164408de8ecc60868031 Mon Sep 17 00:00:00 2001 From: Borodin Gregory Date: Mon, 25 Dec 2023 15:37:22 +0100 Subject: [PATCH 01/18] Generate `jvm_artifact` targets from pom.xml --- src/python/pants/jvm/target_types.py | 151 ++++++++++++++++++++++++++- 1 file changed, 149 insertions(+), 2 deletions(-) diff --git a/src/python/pants/jvm/target_types.py b/src/python/pants/jvm/target_types.py index f7e3be86c7a..72319cbd1f4 100644 --- a/src/python/pants/jvm/target_types.py +++ b/src/python/pants/jvm/target_types.py @@ -4,16 +4,20 @@ from __future__ import annotations import dataclasses +import re +import xml.etree.ElementTree as ET from abc import ABC, ABCMeta, abstractmethod from dataclasses import dataclass -from typing import Callable, ClassVar, Iterable, Optional, Tuple, Type, Union +from typing import Callable, ClassVar, Iterable, Iterator, Optional, Tuple, Type, Union from pants.build_graph.build_file_aliases import BuildFileAliases from pants.core.goals.generate_lockfiles import UnrecognizedResolveNamesError from pants.core.goals.package import OutputPathField from pants.core.goals.run import RestartableField, RunFieldSet, RunInSandboxBehavior, RunRequest from pants.core.goals.test import TestExtraEnvVarsField, TestTimeoutField +from pants.core.util_rules.source_files import SourceFiles, SourceFilesRequest from pants.engine.addresses import Address +from pants.engine.fs import Digest, DigestContents from pants.engine.internals.selectors import Get from pants.engine.rules import Rule, collect_rules, rule from pants.engine.target import ( @@ -21,8 +25,11 @@ AsyncFieldMixin, BoolField, Dependencies, + DictStringToStringSequenceField, FieldDefaultFactoryRequest, FieldDefaultFactoryResult, + GeneratedTargets, + GenerateTargetsRequest, InvalidFieldException, InvalidTargetException, OptionalSingleSourceField, @@ -32,10 +39,12 @@ StringField, StringSequenceField, Target, + TargetGenerator, ) -from pants.engine.unions import UnionRule +from pants.engine.unions import UnionMembership, UnionRule from pants.jvm.subsystems import JvmSubsystem from pants.util.docutil import git_url +from pants.util.frozendict import FrozenDict from pants.util.logging import LogLevel from pants.util.memo import memoized from pants.util.strutil import bullet_list, help_text, pluralize, softwrap @@ -426,6 +435,143 @@ def validate(self) -> None: ) +# ----------------------------------------------------------------------------------------------- +# Generate `jvm_artifact` targets from pom.xml +# ----------------------------------------------------------------------------------------------- + + +class PomXmlSourceField(SingleSourceField): + default = "pom.xml" + required = False + + +class JvmArtifactsPackageMappingField(DictStringToStringSequenceField): + alias = "package_mapping" + help = help_text( + f""" + A mapping of jvm artifacts to a list of the packages they provide. + + For example, `{{"com.google.guava:guava": ["com.google.common.**"]}}`. + + Any unspecified jvm artifacts will use a default. See the + `{JvmArtifactPackagesField.alias}` field from the `{JvmArtifactTarget.alias}` + target for more information. + """ + ) + value: FrozenDict[str, tuple[str, ...]] + default: ClassVar[FrozenDict[str, tuple[str, ...]]] = FrozenDict() + + @classmethod + def compute_value( # type: ignore[override] + cls, raw_value: dict[str, Iterable[str]], address: Address + ) -> FrozenDict[tuple[str], tuple[str, ...]]: + value_or_default = super().compute_value(raw_value, address) + return FrozenDict( + { + cls._parse_coord(coord): tuple(packages) + for coord, packages in value_or_default.items() + } + ) + + @classmethod + def _parse_coord(cls, coord: str) -> tuple[str, str]: + group, artifact = coord.split(":") + return group, artifact + + +class JvmArtifactsTargetGenerator(TargetGenerator): + alias = "jvm_artifacts" + core_fields = ( + PomXmlSourceField, + JvmArtifactsPackageMappingField, + *COMMON_TARGET_FIELDS, + ) + generated_target_cls = JvmArtifactTarget + copied_fields = COMMON_TARGET_FIELDS + moved_fields = (JvmArtifactResolveField,) + help = help_text( + """ + Generate a `jvm_artifact` target for each dependency in pom.xml file. + """ + ) + + +class GenerateFromPomXmlRequest(GenerateTargetsRequest): + generate_from = JvmArtifactsTargetGenerator + + +@rule( + desc=("Generate `jvm_artifact` targets from pom.xml"), + level=LogLevel.DEBUG, +) +async def generate_from_python_requirement( + request: GenerateFromPomXmlRequest, + union_membership: UnionMembership, +) -> GeneratedTargets: + generator = request.generator + pom_xml = await Get( + SourceFiles, + SourceFilesRequest([generator[PomXmlSourceField]]), + ) + files = await Get(DigestContents, Digest, pom_xml.snapshot.digest) + mapping = request.generator[JvmArtifactsPackageMappingField].value + coordinates = parse_pom_xml(files[0].content) + targets = ( + JvmArtifactTarget( + unhydrated_values={ + "group": coord.group, + "artifact": coord.artifact, + "version": coord.version, + "packages": mapping.get((coord.group, coord.artifact)), + **request.template, + }, + address=request.template_address.create_generated(coord.artifact), + ) + for coord in coordinates + ) + return GeneratedTargets(request.generator, targets) + + +def parse_pom_xml(content: bytes) -> Iterator[_Coordinate]: + root = ET.fromstring(content.decode("utf-8")) + match = re.match(r"^(\{.*\})project$", root.tag) + assert match, root.tag + namespace = match.group(1) + for dependency in root.iter(f"{namespace}dependency"): + yield _Coordinate( + group=get_child_text(dependency, f"{namespace}groupId"), + artifact=get_child_text(dependency, f"{namespace}artifactId"), + version=get_child_text(dependency, f"{namespace}version"), + ) + + +@dataclass +class _Coordinate: + """We can't import common.Coordinate because of circular dependencies.""" + + group: str + artifact: str + version: str + + +def get_child_text(parent: ET.Element, child: str) -> str: + tag = parent.find(child) + if tag is None: + raise ValueError(f"missing tag: {child}") + text = tag.text + if text is None: + raise ValueError(f"empty tag: {child}") + return text + + +class MissingTag(ValueError): + pass + + +class EmptyTag(ValueError): + pass + + # ----------------------------------------------------------------------------------------------- # JUnit test support field(s) # ----------------------------------------------------------------------------------------------- @@ -867,6 +1013,7 @@ async def jvm_source_run_request(request: JvmRunnableSourceFieldSet) -> RunReque def rules(): return [ *collect_rules(), + UnionRule(GenerateTargetsRequest, GenerateFromPomXmlRequest), UnionRule(FieldDefaultFactoryRequest, JvmResolveFieldDefaultFactoryRequest), *JvmArtifactFieldSet.jvm_rules(), ] From 9ddaa7eb30954fa000555b06007a477c8070cdd6 Mon Sep 17 00:00:00 2001 From: Borodin Gregory Date: Mon, 25 Dec 2023 15:44:09 +0100 Subject: [PATCH 02/18] Register JvmArtifactsTargetGenerator --- src/python/pants/jvm/jvm_common.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/python/pants/jvm/jvm_common.py b/src/python/pants/jvm/jvm_common.py index 36425b4b097..088b167e4a7 100644 --- a/src/python/pants/jvm/jvm_common.py +++ b/src/python/pants/jvm/jvm_common.py @@ -10,7 +10,12 @@ from pants.jvm.resolve import coursier_fetch, jvm_tool from pants.jvm.shading.rules import rules as shading_rules from pants.jvm.strip_jar import strip_jar -from pants.jvm.target_types import DeployJarTarget, JvmArtifactTarget, JvmWarTarget +from pants.jvm.target_types import ( + DeployJarTarget, + JvmArtifactsTargetGenerator, + JvmArtifactTarget, + JvmWarTarget, +) from pants.jvm.target_types import build_file_aliases as jvm_build_file_aliases from pants.jvm.test import junit @@ -19,6 +24,7 @@ def target_types(): return [ DeployJarTarget, JvmArtifactTarget, + JvmArtifactsTargetGenerator, JvmWarTarget, ] From 9fd182650f127e403c101381a11c1d1853758dc4 Mon Sep 17 00:00:00 2001 From: Borodin Gregory Date: Tue, 26 Dec 2023 21:00:14 +0100 Subject: [PATCH 03/18] Parse type annotation --- src/python/pants/jvm/target_types.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/python/pants/jvm/target_types.py b/src/python/pants/jvm/target_types.py index 72319cbd1f4..04a95da4ebf 100644 --- a/src/python/pants/jvm/target_types.py +++ b/src/python/pants/jvm/target_types.py @@ -464,7 +464,7 @@ class JvmArtifactsPackageMappingField(DictStringToStringSequenceField): @classmethod def compute_value( # type: ignore[override] cls, raw_value: dict[str, Iterable[str]], address: Address - ) -> FrozenDict[tuple[str], tuple[str, ...]]: + ) -> FrozenDict[tuple[str, str], tuple[str, ...]]: value_or_default = super().compute_value(raw_value, address) return FrozenDict( { From d2e16bf6167d10b3056a10e935f2e9ee1c366385 Mon Sep 17 00:00:00 2001 From: Borodin Gregory Date: Wed, 27 Dec 2023 20:31:58 +0100 Subject: [PATCH 04/18] Handle None --- src/python/pants/jvm/target_types.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/python/pants/jvm/target_types.py b/src/python/pants/jvm/target_types.py index 04a95da4ebf..099e2ad869a 100644 --- a/src/python/pants/jvm/target_types.py +++ b/src/python/pants/jvm/target_types.py @@ -466,6 +466,7 @@ def compute_value( # type: ignore[override] cls, raw_value: dict[str, Iterable[str]], address: Address ) -> FrozenDict[tuple[str, str], tuple[str, ...]]: value_or_default = super().compute_value(raw_value, address) + assert value_or_default return FrozenDict( { cls._parse_coord(coord): tuple(packages) From 83ceda1538e66c797839f61653f1ad898671f8cb Mon Sep 17 00:00:00 2001 From: Borodin Gregory Date: Sat, 30 Dec 2023 14:54:15 +0100 Subject: [PATCH 05/18] Add test --- .gitignore | 2 + src/python/pants/backend/java/BUILD | 4 + .../pants/backend/java/target_types_test.py | 149 ++++++++++++++++++ src/python/pants/jvm/target_types.py | 6 +- 4 files changed, 158 insertions(+), 3 deletions(-) create mode 100644 src/python/pants/backend/java/target_types_test.py diff --git a/.gitignore b/.gitignore index 8e008fc27f3..059c5ce8673 100644 --- a/.gitignore +++ b/.gitignore @@ -67,3 +67,5 @@ GTAGS /.pants.rc /.venv .tool-versions +.ropeproject/ +pyrightconfig.json diff --git a/src/python/pants/backend/java/BUILD b/src/python/pants/backend/java/BUILD index 760486c9dd3..98a4e0f3dfa 100644 --- a/src/python/pants/backend/java/BUILD +++ b/src/python/pants/backend/java/BUILD @@ -2,3 +2,7 @@ # Licensed under the Apache License, Version 2.0 (see LICENSE). python_sources() + +python_tests( + name="tests", +) diff --git a/src/python/pants/backend/java/target_types_test.py b/src/python/pants/backend/java/target_types_test.py new file mode 100644 index 00000000000..9d2ac30d714 --- /dev/null +++ b/src/python/pants/backend/java/target_types_test.py @@ -0,0 +1,149 @@ +# Copyright 2023 Pants project contributors (see CONTRIBUTORS.md). +# Licensed under the Apache License, Version 2.0 (see LICENSE). +from __future__ import annotations + +from textwrap import dedent + +import pytest + +from pants.backend.scala.target_types import ScalaArtifactExclusion, ScalaArtifactTarget +from pants.backend.scala.target_types import rules as target_types_rules +from pants.build_graph.address import Address +from pants.engine.internals.graph import _TargetParametrizations, _TargetParametrizationsRequest +from pants.engine.internals.parametrize import Parametrize +from pants.engine.rules import QueryRule +from pants.engine.target import Target +from pants.jvm import jvm_common +from pants.jvm.target_types import ( + JvmArtifactArtifactField, + JvmArtifactExclusion, + JvmArtifactGroupField, + JvmArtifactsTargetGenerator, + JvmArtifactTarget, + JvmArtifactVersionField, +) +from pants.testutil.rule_runner import RuleRunner + + +@pytest.fixture +def rule_runner() -> RuleRunner: + rule_runner = RuleRunner( + target_types=[ScalaArtifactTarget, JvmArtifactTarget, JvmArtifactsTargetGenerator], + rules=[ + *target_types_rules(), + *jvm_common.rules(), + QueryRule(_TargetParametrizations, [_TargetParametrizationsRequest]), + ], + objects={ + "parametrize": Parametrize, + JvmArtifactExclusion.alias: JvmArtifactExclusion, + ScalaArtifactExclusion.alias: ScalaArtifactExclusion, + }, + ) + return rule_runner + + +_JVM_RESOLVES = { + "jvm-default": "3rdparty/jvm/default.lock", +} + + +def assert_generated( + rule_runner: RuleRunner, + address: Address, + *, + build_content: str, + expected_targets: set[Target], +) -> None: + rule_runner.write_files({"BUILD": build_content}) + rule_runner.set_options( + [ + f"--jvm-resolves={repr(_JVM_RESOLVES)}", + "--jvm-default-resolve=jvm-default", + ] + ) + + parametrizations = rule_runner.request( + _TargetParametrizations, + [ + _TargetParametrizationsRequest( + address, + description_of_origin="tests", + ), + ], + ) + assert expected_targets == { + t for parametrization in parametrizations for t in parametrization.parametrization.values() + } + + +def test_generate_jvm_artifacts_from_pom_xml(rule_runner: RuleRunner) -> None: + rule_runner.write_files( + { + "pom.xml": dedent( + """\ + + 4.0.0 + com.pulsepoint + dpd-etl + 0.1.0 + + ${project.basedir}/src/jvm + ${project.basedir}/tests/jvm + + + 1.17 + 1.17 + + + + com.google.guava + guava + 14.0.1 + + + org.apache.logging.log4j + log4j-api + 2.19.0 + + + + """ + ), + } + ) + assert_generated( + rule_runner, + Address("", target_name="test"), + build_content=dedent( + """\ + jvm_artifacts(name="test") + """ + ), + expected_targets={ + JvmArtifactTarget( + { + JvmArtifactGroupField.alias: "com.google.guava", + JvmArtifactArtifactField.alias: "guava", + JvmArtifactVersionField.alias: "14.0.1", + }, + Address( + "", + target_name="test", + generated_name="guava", + ), + ), + JvmArtifactTarget( + { + JvmArtifactGroupField.alias: "org.apache.logging.log4j", + JvmArtifactArtifactField.alias: "log4j-api", + JvmArtifactVersionField.alias: "2.19.0", + }, + Address( + "", + target_name="test", + generated_name="log4j-api", + ), + ), + }, + ) diff --git a/src/python/pants/jvm/target_types.py b/src/python/pants/jvm/target_types.py index 099e2ad869a..afd02f661f0 100644 --- a/src/python/pants/jvm/target_types.py +++ b/src/python/pants/jvm/target_types.py @@ -459,14 +459,14 @@ class JvmArtifactsPackageMappingField(DictStringToStringSequenceField): """ ) value: FrozenDict[str, tuple[str, ...]] - default: ClassVar[FrozenDict[str, tuple[str, ...]]] = FrozenDict() + default: ClassVar[Optional[FrozenDict[str, tuple[str, ...]]]] = FrozenDict() @classmethod def compute_value( # type: ignore[override] cls, raw_value: dict[str, Iterable[str]], address: Address ) -> FrozenDict[tuple[str, str], tuple[str, ...]]: value_or_default = super().compute_value(raw_value, address) - assert value_or_default + assert value_or_default is not None return FrozenDict( { cls._parse_coord(coord): tuple(packages) @@ -505,7 +505,7 @@ class GenerateFromPomXmlRequest(GenerateTargetsRequest): desc=("Generate `jvm_artifact` targets from pom.xml"), level=LogLevel.DEBUG, ) -async def generate_from_python_requirement( +async def generate_from_pom_xml( request: GenerateFromPomXmlRequest, union_membership: UnionMembership, ) -> GeneratedTargets: From e58a7b592222acb846e7bfb65dede14427770c12 Mon Sep 17 00:00:00 2001 From: Borodin Gregory Date: Sat, 30 Dec 2023 15:28:27 +0100 Subject: [PATCH 06/18] Add more tests --- .../pants/backend/java/target_types_test.py | 161 +++++++++++++----- 1 file changed, 123 insertions(+), 38 deletions(-) diff --git a/src/python/pants/backend/java/target_types_test.py b/src/python/pants/backend/java/target_types_test.py index 9d2ac30d714..266e19152ff 100644 --- a/src/python/pants/backend/java/target_types_test.py +++ b/src/python/pants/backend/java/target_types_test.py @@ -18,6 +18,8 @@ JvmArtifactArtifactField, JvmArtifactExclusion, JvmArtifactGroupField, + JvmArtifactPackagesField, + JvmArtifactResolveField, JvmArtifactsTargetGenerator, JvmArtifactTarget, JvmArtifactVersionField, @@ -45,7 +47,38 @@ def rule_runner() -> RuleRunner: _JVM_RESOLVES = { "jvm-default": "3rdparty/jvm/default.lock", + "jvm-custom": "3rdparty/jvm/custom.lock", } +_POM_XML = dedent( + """\ + + 4.0.0 + com.pulsepoint + dpd-etl + 0.1.0 + + ${project.basedir}/src/jvm + ${project.basedir}/tests/jvm + + + 1.17 + 1.17 + + + + com.google.guava + guava + 14.0.1 + + + commons-collections + commons-collections + 3.2.2 + + + + """ +) def assert_generated( @@ -55,7 +88,7 @@ def assert_generated( build_content: str, expected_targets: set[Target], ) -> None: - rule_runner.write_files({"BUILD": build_content}) + rule_runner.write_files({"BUILD": build_content, "pom.xml": _POM_XML}) rule_runner.set_options( [ f"--jvm-resolves={repr(_JVM_RESOLVES)}", @@ -77,41 +110,47 @@ def assert_generated( } -def test_generate_jvm_artifacts_from_pom_xml(rule_runner: RuleRunner) -> None: - rule_runner.write_files( - { - "pom.xml": dedent( - """\ - - 4.0.0 - com.pulsepoint - dpd-etl - 0.1.0 - - ${project.basedir}/src/jvm - ${project.basedir}/tests/jvm - - - 1.17 - 1.17 - - - - com.google.guava - guava - 14.0.1 - - - org.apache.logging.log4j - log4j-api - 2.19.0 - - - - """ +def test_generate_jvm_artifacts_with_default_resolve(rule_runner: RuleRunner) -> None: + assert_generated( + rule_runner, + Address("", target_name="test"), + build_content=dedent( + """\ + jvm_artifacts(name="test", resolve="jvm-custom") + """ + ), + expected_targets={ + JvmArtifactTarget( + { + JvmArtifactGroupField.alias: "com.google.guava", + JvmArtifactArtifactField.alias: "guava", + JvmArtifactVersionField.alias: "14.0.1", + JvmArtifactResolveField.alias: "jvm-custom", + }, + Address( + "", + target_name="test", + generated_name="guava", + ), + ), + JvmArtifactTarget( + { + JvmArtifactGroupField.alias: "commons-collections", + JvmArtifactArtifactField.alias: "commons-collections", + JvmArtifactVersionField.alias: "3.2.2", + JvmArtifactResolveField.alias: "jvm-custom", + }, + Address( + "", + target_name="test", + generated_name="commons-collections", + ), ), - } + }, ) + + +def test_generate_jvm_artifacts_with_explicit_resolve(rule_runner: RuleRunner) -> None: assert_generated( rule_runner, Address("", target_name="test"), @@ -135,14 +174,60 @@ def test_generate_jvm_artifacts_from_pom_xml(rule_runner: RuleRunner) -> None: ), JvmArtifactTarget( { - JvmArtifactGroupField.alias: "org.apache.logging.log4j", - JvmArtifactArtifactField.alias: "log4j-api", - JvmArtifactVersionField.alias: "2.19.0", + JvmArtifactGroupField.alias: "commons-collections", + JvmArtifactArtifactField.alias: "commons-collections", + JvmArtifactVersionField.alias: "3.2.2", + }, + Address( + "", + target_name="test", + generated_name="commons-collections", + ), + ), + }, + ) + + +def test_generate_jvm_artifacts_with_package_mapping(rule_runner: RuleRunner) -> None: + assert_generated( + rule_runner, + Address("", target_name="test"), + build_content=dedent( + """\ + jvm_artifacts( + name="test", + package_mapping={ + "com.google.guava:guava": ["com.google.common.**"], + "commons-collections:commons-collections": ["org.apache.commons.collections.**"], + }, + ) + """ + ), + expected_targets={ + JvmArtifactTarget( + { + JvmArtifactGroupField.alias: "com.google.guava", + JvmArtifactArtifactField.alias: "guava", + JvmArtifactVersionField.alias: "14.0.1", + JvmArtifactPackagesField.alias: ("com.google.common.**",), + }, + Address( + "", + target_name="test", + generated_name="guava", + ), + ), + JvmArtifactTarget( + { + JvmArtifactGroupField.alias: "commons-collections", + JvmArtifactArtifactField.alias: "commons-collections", + JvmArtifactVersionField.alias: "3.2.2", + JvmArtifactPackagesField.alias: ("org.apache.commons.collections.**",), }, Address( "", target_name="test", - generated_name="log4j-api", + generated_name="commons-collections", ), ), }, From 93c8427c00c0cd628871ad101f0d751b81624158 Mon Sep 17 00:00:00 2001 From: Borodin Gregory Date: Fri, 5 Jan 2024 23:41:25 +0100 Subject: [PATCH 07/18] Use Coordinate --- src/python/pants/jvm/target_types.py | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/src/python/pants/jvm/target_types.py b/src/python/pants/jvm/target_types.py index afd02f661f0..7d904320217 100644 --- a/src/python/pants/jvm/target_types.py +++ b/src/python/pants/jvm/target_types.py @@ -42,6 +42,7 @@ TargetGenerator, ) from pants.engine.unions import UnionMembership, UnionRule +from pants.jvm.resolve.coordinate import Coordinate from pants.jvm.subsystems import JvmSubsystem from pants.util.docutil import git_url from pants.util.frozendict import FrozenDict @@ -533,28 +534,19 @@ async def generate_from_pom_xml( return GeneratedTargets(request.generator, targets) -def parse_pom_xml(content: bytes) -> Iterator[_Coordinate]: +def parse_pom_xml(content: bytes) -> Iterator[Coordinate]: root = ET.fromstring(content.decode("utf-8")) match = re.match(r"^(\{.*\})project$", root.tag) assert match, root.tag namespace = match.group(1) for dependency in root.iter(f"{namespace}dependency"): - yield _Coordinate( + yield Coordinate( group=get_child_text(dependency, f"{namespace}groupId"), artifact=get_child_text(dependency, f"{namespace}artifactId"), version=get_child_text(dependency, f"{namespace}version"), ) -@dataclass -class _Coordinate: - """We can't import common.Coordinate because of circular dependencies.""" - - group: str - artifact: str - version: str - - def get_child_text(parent: ET.Element, child: str) -> str: tag = parent.find(child) if tag is None: From a1325fefbf59f2b54b610a9f0d71550dd714bce2 Mon Sep 17 00:00:00 2001 From: Borodin Gregory Date: Fri, 5 Jan 2024 23:41:56 +0100 Subject: [PATCH 08/18] Revert .gitignore --- .gitignore | 2 -- 1 file changed, 2 deletions(-) diff --git a/.gitignore b/.gitignore index 059c5ce8673..8e008fc27f3 100644 --- a/.gitignore +++ b/.gitignore @@ -67,5 +67,3 @@ GTAGS /.pants.rc /.venv .tool-versions -.ropeproject/ -pyrightconfig.json From 2c580c32480531ed57848d580af54ffdf8326769 Mon Sep 17 00:00:00 2001 From: Borodin Gregory Date: Sat, 6 Jan 2024 13:21:24 +0100 Subject: [PATCH 09/18] Remove unused error types --- src/python/pants/jvm/target_types.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/python/pants/jvm/target_types.py b/src/python/pants/jvm/target_types.py index 7d904320217..f40d4f7328b 100644 --- a/src/python/pants/jvm/target_types.py +++ b/src/python/pants/jvm/target_types.py @@ -557,14 +557,6 @@ def get_child_text(parent: ET.Element, child: str) -> str: return text -class MissingTag(ValueError): - pass - - -class EmptyTag(ValueError): - pass - - # ----------------------------------------------------------------------------------------------- # JUnit test support field(s) # ----------------------------------------------------------------------------------------------- From 285986cd531767d5be70790a095f4acd1ecabefd Mon Sep 17 00:00:00 2001 From: Gregory Borodin Date: Sat, 27 Jan 2024 18:03:53 +0100 Subject: [PATCH 10/18] Update error text Co-authored-by: Benjy Weinberger --- src/python/pants/jvm/target_types.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/python/pants/jvm/target_types.py b/src/python/pants/jvm/target_types.py index f40d4f7328b..130174075ca 100644 --- a/src/python/pants/jvm/target_types.py +++ b/src/python/pants/jvm/target_types.py @@ -550,7 +550,7 @@ def parse_pom_xml(content: bytes) -> Iterator[Coordinate]: def get_child_text(parent: ET.Element, child: str) -> str: tag = parent.find(child) if tag is None: - raise ValueError(f"missing tag: {child}") + raise ValueError(f"missing element: {child}") text = tag.text if text is None: raise ValueError(f"empty tag: {child}") From 952f8a3ff86d7ef4b02278b53b3ace05990a241f Mon Sep 17 00:00:00 2001 From: Borodin Gregory Date: Tue, 30 Jan 2024 00:31:48 +0100 Subject: [PATCH 11/18] Better exception --- src/python/pants/jvm/target_types.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/python/pants/jvm/target_types.py b/src/python/pants/jvm/target_types.py index 130174075ca..dda6f1c079c 100644 --- a/src/python/pants/jvm/target_types.py +++ b/src/python/pants/jvm/target_types.py @@ -537,7 +537,9 @@ async def generate_from_pom_xml( def parse_pom_xml(content: bytes) -> Iterator[Coordinate]: root = ET.fromstring(content.decode("utf-8")) match = re.match(r"^(\{.*\})project$", root.tag) - assert match, root.tag + if not match: + raise ValueError(f"Unexpected root tag `{root.tag}`, expected project") + namespace = match.group(1) for dependency in root.iter(f"{namespace}dependency"): yield Coordinate( From cd3429ae8b3608da55638ddaeca4f1fec1a4c12e Mon Sep 17 00:00:00 2001 From: Gregory Borodin Date: Sun, 4 Feb 2024 00:32:38 +0100 Subject: [PATCH 12/18] Rename `tag` -> `element` Co-authored-by: Benjy Weinberger --- src/python/pants/jvm/target_types.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/python/pants/jvm/target_types.py b/src/python/pants/jvm/target_types.py index dda6f1c079c..bb97941e912 100644 --- a/src/python/pants/jvm/target_types.py +++ b/src/python/pants/jvm/target_types.py @@ -555,7 +555,7 @@ def get_child_text(parent: ET.Element, child: str) -> str: raise ValueError(f"missing element: {child}") text = tag.text if text is None: - raise ValueError(f"empty tag: {child}") + raise ValueError(f"empty element: {child}") return text From 540d0f2c282f1d1d0efc2bf6504da2ffdb9421f6 Mon Sep 17 00:00:00 2001 From: Borodin Gregory Date: Sun, 4 Feb 2024 00:41:07 +0100 Subject: [PATCH 13/18] Move target_types_test.py --- src/python/pants/backend/java/BUILD | 3 --- src/python/pants/{backend/java => jvm}/target_types_test.py | 0 2 files changed, 3 deletions(-) rename src/python/pants/{backend/java => jvm}/target_types_test.py (100%) diff --git a/src/python/pants/backend/java/BUILD b/src/python/pants/backend/java/BUILD index 98a4e0f3dfa..c2553d5a06f 100644 --- a/src/python/pants/backend/java/BUILD +++ b/src/python/pants/backend/java/BUILD @@ -3,6 +3,3 @@ python_sources() -python_tests( - name="tests", -) diff --git a/src/python/pants/backend/java/target_types_test.py b/src/python/pants/jvm/target_types_test.py similarity index 100% rename from src/python/pants/backend/java/target_types_test.py rename to src/python/pants/jvm/target_types_test.py From 4062f73539c707475f1145958b46ec67755a50d6 Mon Sep 17 00:00:00 2001 From: Borodin Gregory Date: Sun, 4 Feb 2024 01:07:45 +0100 Subject: [PATCH 14/18] Better error message --- src/python/pants/jvm/target_types.py | 8 +++--- src/python/pants/jvm/target_types_test.py | 30 +++++++++++++++++++++++ 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/src/python/pants/jvm/target_types.py b/src/python/pants/jvm/target_types.py index bb97941e912..9cd79677eec 100644 --- a/src/python/pants/jvm/target_types.py +++ b/src/python/pants/jvm/target_types.py @@ -517,7 +517,7 @@ async def generate_from_pom_xml( ) files = await Get(DigestContents, Digest, pom_xml.snapshot.digest) mapping = request.generator[JvmArtifactsPackageMappingField].value - coordinates = parse_pom_xml(files[0].content) + coordinates = parse_pom_xml(files[0].content, pom_xml_path=pom_xml.snapshot.files[0]) targets = ( JvmArtifactTarget( unhydrated_values={ @@ -534,11 +534,13 @@ async def generate_from_pom_xml( return GeneratedTargets(request.generator, targets) -def parse_pom_xml(content: bytes) -> Iterator[Coordinate]: +def parse_pom_xml(content: bytes, pom_xml_path: str) -> Iterator[Coordinate]: root = ET.fromstring(content.decode("utf-8")) match = re.match(r"^(\{.*\})project$", root.tag) if not match: - raise ValueError(f"Unexpected root tag `{root.tag}`, expected project") + raise ValueError( + f"Unexpected root tag `{root.tag}` in {pom_xml_path}, expected tag `project`" + ) namespace = match.group(1) for dependency in root.iter(f"{namespace}dependency"): diff --git a/src/python/pants/jvm/target_types_test.py b/src/python/pants/jvm/target_types_test.py index 266e19152ff..94e39e86a2e 100644 --- a/src/python/pants/jvm/target_types_test.py +++ b/src/python/pants/jvm/target_types_test.py @@ -11,6 +11,7 @@ from pants.build_graph.address import Address from pants.engine.internals.graph import _TargetParametrizations, _TargetParametrizationsRequest from pants.engine.internals.parametrize import Parametrize +from pants.engine.internals.scheduler import ExecutionError from pants.engine.rules import QueryRule from pants.engine.target import Target from pants.jvm import jvm_common @@ -232,3 +233,32 @@ def test_generate_jvm_artifacts_with_package_mapping(rule_runner: RuleRunner) -> ), }, ) + + +def test_invalid_root_tag(rule_runner: RuleRunner) -> None: + build_content = dedent( + """\ + jvm_artifacts(name="test") + """ + ) + pom_xml = "" + rule_runner.write_files({"dir/BUILD": build_content, "dir/pom.xml": pom_xml}) + rule_runner.set_options( + [ + f"--jvm-resolves={repr(_JVM_RESOLVES)}", + "--jvm-default-resolve=jvm-default", + ] + ) + + with pytest.raises( + ExecutionError, match=r".*ValueError.*`invalid_tag`.*dir/pom.xml.*`project`.*" + ) as e: + rule_runner.request( + _TargetParametrizations, + [ + _TargetParametrizationsRequest( + Address("dir", target_name="test"), + description_of_origin="tests", + ), + ], + ) From e45d34697979eefc827eaf43271f447894d3db03 Mon Sep 17 00:00:00 2001 From: Borodin Gregory Date: Sun, 4 Feb 2024 01:09:12 +0100 Subject: [PATCH 15/18] Fix year --- src/python/pants/jvm/target_types_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/python/pants/jvm/target_types_test.py b/src/python/pants/jvm/target_types_test.py index 94e39e86a2e..7b151d82ef9 100644 --- a/src/python/pants/jvm/target_types_test.py +++ b/src/python/pants/jvm/target_types_test.py @@ -1,4 +1,4 @@ -# Copyright 2023 Pants project contributors (see CONTRIBUTORS.md). +# Copyright 2024 Pants project contributors (see CONTRIBUTORS.md). # Licensed under the Apache License, Version 2.0 (see LICENSE). from __future__ import annotations From 7d4c26c9eb9f098f119f4430c3ba8fadf3c4debe Mon Sep 17 00:00:00 2001 From: Borodin Gregory Date: Sun, 4 Feb 2024 01:43:04 +0100 Subject: [PATCH 16/18] Fix linters --- src/python/pants/jvm/target_types_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/python/pants/jvm/target_types_test.py b/src/python/pants/jvm/target_types_test.py index 7b151d82ef9..eb3de025f90 100644 --- a/src/python/pants/jvm/target_types_test.py +++ b/src/python/pants/jvm/target_types_test.py @@ -252,7 +252,7 @@ def test_invalid_root_tag(rule_runner: RuleRunner) -> None: with pytest.raises( ExecutionError, match=r".*ValueError.*`invalid_tag`.*dir/pom.xml.*`project`.*" - ) as e: + ): rule_runner.request( _TargetParametrizations, [ From f512dc311d21e62df4e3d60ef2e9c70486305bbd Mon Sep 17 00:00:00 2001 From: Borodin Gregory Date: Sun, 4 Feb 2024 12:07:56 +0100 Subject: [PATCH 17/18] Format BUILD --- src/python/pants/backend/java/BUILD | 1 - 1 file changed, 1 deletion(-) diff --git a/src/python/pants/backend/java/BUILD b/src/python/pants/backend/java/BUILD index c2553d5a06f..760486c9dd3 100644 --- a/src/python/pants/backend/java/BUILD +++ b/src/python/pants/backend/java/BUILD @@ -2,4 +2,3 @@ # Licensed under the Apache License, Version 2.0 (see LICENSE). python_sources() - From b68ec95ee944d7366621022d49fdb24a2e1f4af8 Mon Sep 17 00:00:00 2001 From: Borodin Gregory Date: Sat, 10 Feb 2024 18:40:00 +0100 Subject: [PATCH 18/18] Handle 0 files --- src/python/pants/jvm/target_types.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/python/pants/jvm/target_types.py b/src/python/pants/jvm/target_types.py index 9cd79677eec..ea7e3e6a5fc 100644 --- a/src/python/pants/jvm/target_types.py +++ b/src/python/pants/jvm/target_types.py @@ -516,6 +516,9 @@ async def generate_from_pom_xml( SourceFilesRequest([generator[PomXmlSourceField]]), ) files = await Get(DigestContents, Digest, pom_xml.snapshot.digest) + if not files: + raise FileNotFoundError(f"pom.xml not found: {generator[PomXmlSourceField].value}") + mapping = request.generator[JvmArtifactsPackageMappingField].value coordinates = parse_pom_xml(files[0].content, pom_xml_path=pom_xml.snapshot.files[0]) targets = (