Skip to content

Commit

Permalink
Fix JVM resource classification and artifact grouping (Cherry-pick of #…
Browse files Browse the repository at this point in the history
…15567) (#15573)

#15457 introduced a code generator to generate `resources` from `resources`, and that broke the classification of `resources` for the JVM, because the `JvmResourcesRequest` matched twice.

Additionally, the generator-awareness of `resources` processing was not working (depending on a `resources` generator should bring in a single jar containing the entire set of files, while depending on a single `resource` target should expose only that file) due to an inverted condition.

Add an integration test to cover both cases.

[ci skip-rust]
[ci skip-build-wheels]
  • Loading branch information
stuhood authored May 22, 2022
1 parent 8ae0ada commit 8be9f12
Show file tree
Hide file tree
Showing 4 changed files with 111 additions and 15 deletions.
36 changes: 25 additions & 11 deletions src/python/pants/jvm/compile.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
FieldSet,
GenerateSourcesRequest,
SourcesField,
Target,
TargetFilesGenerator,
)
from pants.engine.unions import UnionMembership, union
Expand Down Expand Up @@ -153,20 +154,30 @@ def classify_impl(
targets = component.members
generator_sources = self.generator_sources.get(impl) or frozenset()

compatible_direct = sum(1 for t in targets for fs in impl.field_sets if fs.is_applicable(t))
compatible_codegen = sum(1 for t in targets for g in generator_sources if t.has_field(g))
compatible_codegen_target_generator = sum(
1
for t in targets
if isinstance(t, TargetFilesGenerator)
and any(field in t.generated_target_cls.core_fields for field in generator_sources)
)
def is_compatible(target: Target) -> bool:
return (
# Is directly applicable.
any(fs.is_applicable(target) for fs in impl.field_sets)
or
# Is applicable via generated sources.
any(target.has_field(g) for g in generator_sources)
or
# Is applicable via a generator.
(
isinstance(target, TargetFilesGenerator)
and any(
field in target.generated_target_cls.core_fields
for field in generator_sources
)
)
)

compatible = compatible_direct + compatible_codegen + compatible_codegen_target_generator
compatible = sum(1 for t in targets if is_compatible(t))
if compatible == 0:
return _ClasspathEntryRequestClassification.INCOMPATIBLE
if compatible == len(targets):
return _ClasspathEntryRequestClassification.COMPATIBLE

consume_only = sum(
1 for t in targets for fs in impl.field_sets_consume_only if fs.is_applicable(t)
)
Expand Down Expand Up @@ -392,7 +403,10 @@ def classpath_dependency_requests(
classpath_entry_request: ClasspathEntryRequestFactory, request: ClasspathDependenciesRequest
) -> ClasspathEntryRequests:
def ignore_because_generated(coarsened_dep: CoarsenedTarget) -> bool:
if len(coarsened_dep.members) == 1:
if not request.ignore_generated:
return False
if len(coarsened_dep.members) != 1:
# Do not ignore a dependency which is involved in a cycle.
return False
us = request.request.component.representative.address
them = coarsened_dep.representative.address
Expand All @@ -403,7 +417,7 @@ def ignore_because_generated(coarsened_dep: CoarsenedTarget) -> bool:
component=coarsened_dep, resolve=request.request.resolve
)
for coarsened_dep in request.request.component.dependencies
if not request.ignore_generated or not ignore_because_generated(coarsened_dep)
if not ignore_because_generated(coarsened_dep)
)


Expand Down
4 changes: 2 additions & 2 deletions src/python/pants/jvm/resolve/coursier_fetch.py
Original file line number Diff line number Diff line change
Expand Up @@ -640,11 +640,11 @@ async def select_coursier_resolve_for_targets(
elif resolve != compatible_resolve:
all_compatible = False

if not compatible_resolve or not all_compatible:
if not all_compatible:
raise NoCompatibleResolve(
jvm, "The selected targets did not have a resolve in common", targets
)
resolve = compatible_resolve
resolve = compatible_resolve or jvm.default_resolve

# Load the resolve.
resolve_path = jvm.resolves[resolve]
Expand Down
8 changes: 6 additions & 2 deletions src/python/pants/jvm/resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from itertools import chain

from pants.core.target_types import ResourcesFieldSet, ResourcesGeneratorFieldSet
from pants.core.util_rules import stripped_source_files
from pants.core.util_rules.source_files import SourceFilesRequest
from pants.core.util_rules.stripped_source_files import StrippedSourceFiles
from pants.core.util_rules.system_binaries import ZipBinary
Expand All @@ -14,6 +15,7 @@
from pants.engine.rules import Get, collect_rules, rule
from pants.engine.target import SourcesField
from pants.engine.unions import UnionRule
from pants.jvm import compile
from pants.jvm.compile import (
ClasspathDependenciesRequest,
ClasspathEntry,
Expand All @@ -34,7 +36,7 @@ class JvmResourcesRequest(ClasspathEntryRequest):
)


@rule(desc="Fetch with coursier")
@rule(desc="Assemble resources")
async def assemble_resources_jar(
zip: ZipBinary,
request: JvmResourcesRequest,
Expand Down Expand Up @@ -66,7 +68,7 @@ async def assemble_resources_jar(
SourceFilesRequest([tgt.get(SourcesField) for tgt in request.component.members]),
)

output_filename = f"{request.component.representative.address.path_safe_spec}.jar"
output_filename = f"{request.component.representative.address.path_safe_spec}.resources.jar"
output_files = [output_filename]

resources_jar_input_digest = source_files.snapshot.digest
Expand Down Expand Up @@ -101,5 +103,7 @@ async def assemble_resources_jar(
def rules():
return [
*collect_rules(),
*compile.rules(),
*stripped_source_files.rules(),
UnionRule(ClasspathEntryRequest, JvmResourcesRequest),
]
78 changes: 78 additions & 0 deletions src/python/pants/jvm/resources_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
# Copyright 2022 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from __future__ import annotations

import pytest

from pants.build_graph.address import Address
from pants.core.target_types import ResourcesGeneratorTarget, ResourceTarget
from pants.core.target_types import rules as core_target_types_rules
from pants.engine.addresses import Addresses
from pants.jvm import classpath, resources, testutil
from pants.jvm.goals import lockfile
from pants.jvm.resolve.coursier_fetch import CoursierResolvedLockfile
from pants.jvm.resolve.coursier_fetch import rules as coursier_fetch_rules
from pants.jvm.resolve.coursier_test_util import TestCoursierWrapper
from pants.jvm.testutil import RenderedClasspath, maybe_skip_jdk_test
from pants.jvm.util_rules import rules as util_rules
from pants.testutil.rule_runner import PYTHON_BOOTSTRAP_ENV, QueryRule, RuleRunner

EMPTY_LOCKFILE = TestCoursierWrapper(CoursierResolvedLockfile(())).serialize([])


@pytest.fixture
def rule_runner() -> RuleRunner:
rule_runner = RuleRunner(
rules=[
*core_target_types_rules(),
*coursier_fetch_rules(),
*lockfile.rules(),
*resources.rules(),
*classpath.rules(),
*util_rules(),
*testutil.rules(),
QueryRule(RenderedClasspath, (Addresses,)),
],
target_types=[
ResourcesGeneratorTarget,
ResourceTarget,
],
)
rule_runner.set_options(args=[], env_inherit=PYTHON_BOOTSTRAP_ENV)
return rule_runner


@maybe_skip_jdk_test
def test_resources(rule_runner: RuleRunner) -> None:
rule_runner.write_files(
{
"BUILD": "resources(name='root', sources=['*.txt'])",
"one.txt": "",
"two.txt": "",
"3rdparty/jvm/default.lock": EMPTY_LOCKFILE,
}
)

# Building the generator target should exclude the individual files and result in a single jar
# for the generator.
rendered_classpath = rule_runner.request(
RenderedClasspath, [Addresses([Address(spec_path="", target_name="root")])]
)
assert rendered_classpath.content == {
".root.resources.jar": {
"one.txt",
"two.txt",
}
}

# But requesting a single file should individually package it.
rendered_classpath = rule_runner.request(
RenderedClasspath,
[Addresses([Address(spec_path="", target_name="root", relative_file_path="one.txt")])],
)
assert rendered_classpath.content == {
".one.txt.root.resources.jar": {
"one.txt",
}
}

0 comments on commit 8be9f12

Please sign in to comment.