Skip to content

Commit

Permalink
Fix Java / Scala cycle artifact filename collision. (#13759)
Browse files Browse the repository at this point in the history
When actually consumed, the cyclic classpaths from #13653 collided by using the [same artifact filename with different contents](https://github.com/pantsbuild/pants/blob/8fc8d0bd757cb4700185f1867ec67b9a157430cf/src/rust/engine/fs/store/src/snapshot_ops.rs#L270-L276).

Adjust the test to trigger the failure, and then fix artifact names.

[ci skip-rust]
[ci skip-build-wheels]
  • Loading branch information
stuhood authored Dec 1, 2021
1 parent 9c76136 commit 4b066ac
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 23 deletions.
2 changes: 1 addition & 1 deletion src/python/pants/backend/java/compile/javac.py
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ async def compile_java_source(
# invoking via a `bash` wrapper (since the trailing portion of the command is executed by
# the nailgun server). We might be able to resolve this in the future via a Javac wrapper shim.
output_snapshot = await Get(Snapshot, Digest, compile_result.output_digest)
output_file = f"{request.component.representative.address.path_safe_spec}.jar"
output_file = f"{request.component.representative.address.path_safe_spec}.javac.jar"
if output_snapshot.files:
jar_result = await Get(
ProcessResult,
Expand Down
25 changes: 16 additions & 9 deletions src/python/pants/backend/java/compile/javac_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ def test_compile_no_deps(rule_runner: RuleRunner) -> None:

classpath = rule_runner.request(RenderedClasspath, [compiled_classfiles.digest])
assert classpath.content == {
".ExampleLib.java.lib.jar": {"org/pantsbuild/example/lib/ExampleLib.class"}
".ExampleLib.java.lib.javac.jar": {"org/pantsbuild/example/lib/ExampleLib.class"}
}

# Additionally validate that `check` works.
Expand Down Expand Up @@ -186,7 +186,7 @@ def test_compile_jdk_versions(rule_runner: RuleRunner) -> None:
compiled_classfiles = rule_runner.request(ClasspathEntry, [request])
classpath = rule_runner.request(RenderedClasspath, [compiled_classfiles.digest])
assert classpath.content == {
".ExampleLib.java.lib.jar": {"org/pantsbuild/example/lib/ExampleLib.class"}
".ExampleLib.java.lib.javac.jar": {"org/pantsbuild/example/lib/ExampleLib.class"}
}

rule_runner.set_options(
Expand Down Expand Up @@ -252,7 +252,7 @@ def test_compile_multiple_source_files(rule_runner: RuleRunner) -> None:
compiled_classfiles0 = rule_runner.request(ClasspathEntry, [request0])
classpath0 = rule_runner.request(RenderedClasspath, [compiled_classfiles0.digest])
assert classpath0.content == {
".ExampleLib.java.lib.jar": {
".ExampleLib.java.lib.javac.jar": {
"org/pantsbuild/example/lib/ExampleLib.class",
}
}
Expand All @@ -263,7 +263,7 @@ def test_compile_multiple_source_files(rule_runner: RuleRunner) -> None:
compiled_classfiles1 = rule_runner.request(ClasspathEntry, [request1])
classpath1 = rule_runner.request(RenderedClasspath, [compiled_classfiles1.digest])
assert classpath1.content == {
".OtherLib.java.lib.jar": {
".OtherLib.java.lib.javac.jar": {
"org/pantsbuild/example/lib/OtherLib.class",
}
}
Expand Down Expand Up @@ -347,7 +347,7 @@ class C implements A {}
compiled_classfiles = rule_runner.request(ClasspathEntry, [request])
classpath = rule_runner.request(RenderedClasspath, [compiled_classfiles.digest])
assert classpath.content == {
"a.A.java.jar": {
"a.A.java.javac.jar": {
"org/pantsbuild/a/A.class",
"org/pantsbuild/a/C.class",
"org/pantsbuild/b/B.class",
Expand Down Expand Up @@ -437,7 +437,7 @@ class C implements A {}
],
)
classpath = rule_runner.request(RenderedClasspath, [compiled_classfiles.digest])
assert classpath.content == {".Main.java.main.jar": {"org/pantsbuild/main/Main.class"}}
assert classpath.content == {".Main.java.main.javac.jar": {"org/pantsbuild/main/Main.class"}}


@logging
Expand Down Expand Up @@ -508,7 +508,10 @@ class C implements A {}
)
classpath = rule_runner.request(RenderedClasspath, [compiled_classfiles.digest])
assert classpath.content == {
".Main.java.main.jar": {"org/pantsbuild/main/Main.class", "org/pantsbuild/main/Other.class"}
".Main.java.main.javac.jar": {
"org/pantsbuild/main/Main.class",
"org/pantsbuild/main/Other.class",
}
}


Expand Down Expand Up @@ -554,7 +557,9 @@ def test_compile_with_deps(rule_runner: RuleRunner) -> None:
],
)
classpath = rule_runner.request(RenderedClasspath, [compiled_classfiles.digest])
assert classpath.content == {".Example.java.main.jar": {"org/pantsbuild/example/Example.class"}}
assert classpath.content == {
".Example.java.main.javac.jar": {"org/pantsbuild/example/Example.class"}
}


@maybe_skip_jdk_test
Expand Down Expand Up @@ -688,7 +693,9 @@ def test_compile_with_maven_deps(rule_runner: RuleRunner) -> None:
)
compiled_classfiles = rule_runner.request(ClasspathEntry, [request])
classpath = rule_runner.request(RenderedClasspath, [compiled_classfiles.digest])
assert classpath.content == {".Example.java.main.jar": {"org/pantsbuild/example/Example.class"}}
assert classpath.content == {
".Example.java.main.javac.jar": {"org/pantsbuild/example/Example.class"}
}


@maybe_skip_jdk_test
Expand Down
2 changes: 1 addition & 1 deletion src/python/pants/backend/scala/compile/scalac.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ async def compile_scala_source(
ClasspathEntry.closure(direct_dependency_classpath_entries), prefix=usercp
)

output_file = f"{request.component.representative.address.path_safe_spec}.jar"
output_file = f"{request.component.representative.address.path_safe_spec}.scalac.jar"
process_result = await Get(
FallibleProcessResult,
Process(
Expand Down
9 changes: 6 additions & 3 deletions src/python/pants/backend/scala/compile/scalac_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,10 @@ def test_compile_no_deps(rule_runner: RuleRunner) -> None:

classpath = rule_runner.request(RenderedClasspath, [compiled_classfiles.digest])
assert classpath.content == {
".ExampleLib.scala.lib.jar": {"META-INF/MANIFEST.MF", "org/pantsbuild/example/lib/C.class"}
".ExampleLib.scala.lib.scalac.jar": {
"META-INF/MANIFEST.MF",
"org/pantsbuild/example/lib/C.class",
}
}

# Additionally validate that `check` works.
Expand Down Expand Up @@ -189,7 +192,7 @@ def test_compile_with_deps(rule_runner: RuleRunner) -> None:
)
classpath = rule_runner.request(RenderedClasspath, [compiled_classfiles.digest])
assert classpath.content == {
".Example.scala.main.jar": {
".Example.scala.main.scalac.jar": {
"META-INF/MANIFEST.MF",
"org/pantsbuild/example/Main$.class",
"org/pantsbuild/example/Main.class",
Expand Down Expand Up @@ -288,7 +291,7 @@ def main(args: Array[String]): Unit = {
compiled_classfiles = rule_runner.request(ClasspathEntry, [request])
classpath = rule_runner.request(RenderedClasspath, [compiled_classfiles.digest])
assert classpath.content == {
".Example.scala.main.jar": {
".Example.scala.main.scalac.jar": {
"META-INF/MANIFEST.MF",
"org/pantsbuild/example/Main$.class",
"org/pantsbuild/example/Main.class",
Expand Down
19 changes: 13 additions & 6 deletions src/python/pants/jvm/classpath.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

from __future__ import annotations

import logging
import os
from dataclasses import dataclass
from typing import Callable, Iterator
Expand All @@ -17,6 +18,9 @@
_USERCP_RELPATH = "__cp"


logger = logging.getLogger(__name__)


@dataclass(frozen=True)
class Classpath:
"""A transitive classpath which is sufficient to launch the target(s) it was generated for.
Expand Down Expand Up @@ -60,20 +64,23 @@ async def classpath(
coarsened_targets: CoarsenedTargets,
union_membership: UnionMembership,
) -> Classpath:
targets = Targets(t for ct in coarsened_targets.closure() for t in ct.members)

resolve = await Get(CoursierResolveKey, Targets, targets)
resolve = await Get(
CoursierResolveKey,
Targets,
Targets(t for ct in coarsened_targets.closure() for t in ct.members),
)

transitive_classpath_entries = await MultiGet(
classpath_entries = await MultiGet(
Get(
ClasspathEntry,
ClasspathEntryRequest,
ClasspathEntryRequest.for_targets(union_membership, component=t, resolve=resolve),
)
for t in coarsened_targets.closure()
for t in coarsened_targets
)
merged_transitive_classpath_entries_digest = await Get(
Digest, MergeDigests(classfiles.digest for classfiles in transitive_classpath_entries)
Digest,
MergeDigests(classfiles.digest for classfiles in ClasspathEntry.closure(classpath_entries)),
)

return Classpath(
Expand Down
7 changes: 4 additions & 3 deletions src/python/pants/jvm/compile_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -226,12 +226,12 @@ def test_compile_mixed(rule_runner: RuleRunner) -> None:
)
rendered_classpath = rule_runner.request(RenderedClasspath, [classpath.content.digest])
assert rendered_classpath.content == {
"__cp/.Example.scala.main.jar": {
"__cp/.Example.scala.main.scalac.jar": {
"META-INF/MANIFEST.MF",
"org/pantsbuild/example/Main$.class",
"org/pantsbuild/example/Main.class",
},
"__cp/lib.C.java.jar": {
"__cp/lib.C.java.javac.jar": {
"org/pantsbuild/example/lib/C.class",
},
}
Expand All @@ -251,5 +251,6 @@ def test_compile_mixed_cycle(rule_runner: RuleRunner) -> None:
)

main_address = Address(spec_path="", target_name="main")
lib_address = Address(spec_path="lib")
assert len(expect_single_expanded_coarsened_target(rule_runner, main_address).members) == 2
rule_runner.request(Classpath, [Addresses([main_address])])
rule_runner.request(Classpath, [Addresses([main_address, lib_address])])

0 comments on commit 4b066ac

Please sign in to comment.