diff options
| -rw-r--r-- | src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java | 44 | ||||
| -rw-r--r-- | src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java | 35 |
2 files changed, 42 insertions, 37 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java index 2508d1d810..028e9d1810 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java @@ -30,6 +30,7 @@ import com.google.devtools.build.lib.actions.ActionExecutionContext; import com.google.devtools.build.lib.actions.ActionExecutionException; import com.google.devtools.build.lib.actions.ActionOwner; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.actions.ArtifactOwner; import com.google.devtools.build.lib.actions.BaseSpawn; import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.Executor; @@ -56,12 +57,12 @@ import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadCompatible; import com.google.devtools.build.lib.rules.java.JavaConfiguration.JavaClasspathMode; +import com.google.devtools.build.lib.skyframe.AspectValue; import com.google.devtools.build.lib.util.Fingerprint; import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.util.ShellEscaper; import com.google.devtools.build.lib.util.StringCanonicalizer; import com.google.devtools.build.lib.vfs.PathFragment; - import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -680,11 +681,13 @@ public final class JavaCompileAction extends AbstractAction { /** * Builds the list of mappings between jars on the classpath and their originating targets names. */ - static class JarsToTargetsArgv extends CustomMultiArgv { + @VisibleForTesting + public static class JarsToTargetsArgv extends CustomMultiArgv { private final NestedSet<Artifact> classpath; private final NestedSet<Artifact> directJars; - JarsToTargetsArgv(NestedSet<Artifact> classpath, NestedSet<Artifact> directJars) { + @VisibleForTesting + public JarsToTargetsArgv(NestedSet<Artifact> classpath, NestedSet<Artifact> directJars) { this.classpath = classpath; this.directJars = directJars; } @@ -696,26 +699,29 @@ public final class JavaCompileAction extends AbstractAction { for (Artifact jar : classpath) { builder.add(directJarSet.contains(jar) ? "--direct_dependency" : "--indirect_dependency"); builder.add(jar.getExecPathString()); - Label label = getTargetName(jar); - builder.add( - label.getPackageIdentifier().getRepository().isDefault() - || label.getPackageIdentifier().getRepository().isMain() - ? label.toString() - // Escape '@' prefix for .params file. - : "@" + label); + builder.add(getArtifactOwnerGeneralizedLabel(jar)); } return builder.build(); } - } - /** - * Gets the name of the target that produced the given jar artifact. - * - * <p>When specifying jars directly in the "srcs" attribute of a rule (mostly for third_party - * libraries), there is no generating action, so we just return the jar name in label form. - */ - private static Label getTargetName(Artifact jar) { - return checkNotNull(jar.getOwner(), jar); + private String getArtifactOwnerGeneralizedLabel(Artifact artifact) { + ArtifactOwner owner = checkNotNull(artifact.getArtifactOwner(), artifact); + StringBuilder result = new StringBuilder(); + Label label = owner.getLabel(); + result.append( + label.getPackageIdentifier().getRepository().isDefault() + || label.getPackageIdentifier().getRepository().isMain() + ? label.toString() + // Escape '@' prefix for .params file. + : "@" + label); + + if (owner instanceof AspectValue.AspectKey) { + AspectValue.AspectKey aspectOwner = (AspectValue.AspectKey) owner; + result.append(" ").append(aspectOwner.getAspectClass().getName()); + } + + return result.toString(); + } } /** diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java index f84fd302d9..bb630a3c55 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java @@ -92,6 +92,7 @@ import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.events.StoredEventHandler; import com.google.devtools.build.lib.exec.ExecutionOptions; import com.google.devtools.build.lib.flags.InvocationPolicyEnforcer; +import com.google.devtools.build.lib.packages.AspectClass; import com.google.devtools.build.lib.packages.AspectParameters; import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition; import com.google.devtools.build.lib.packages.AttributeMap; @@ -137,9 +138,6 @@ import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.skyframe.SkyFunction; import com.google.devtools.common.options.Options; import com.google.devtools.common.options.OptionsParser; - -import org.junit.Before; - import java.io.ByteArrayOutputStream; import java.io.IOException; import java.nio.charset.StandardCharsets; @@ -152,6 +150,7 @@ import java.util.List; import java.util.Map; import java.util.Set; import java.util.UUID; +import org.junit.Before; /** * Common test code that creates a BuildView instance. @@ -976,32 +975,32 @@ public abstract class BuildViewTestCase extends FoundationTestCase { /** * Gets a derived Artifact for testing in the subdirectory of the {@link - * BuildConfiguration#getBinDirectory()} corresponding to the package of {@code owner}, - * where the given artifact belongs to the given ConfiguredTarget together with the given Aspect. - * So to specify a file foo/foo.o owned by target //foo:foo with an aspect from FooAspect, - * {@code packageRelativePath} should just be "foo.o", and aspectOfOwner should be - * FooAspect.class. This method is necessary when an Aspect of the target, not the target itself, - * is creating an Artifact. + * BuildConfiguration#getBinDirectory()} corresponding to the package of {@code owner}, where the + * given artifact belongs to the given ConfiguredTarget together with the given Aspect. So to + * specify a file foo/foo.o owned by target //foo:foo with an aspect from FooAspect, {@code + * packageRelativePath} should just be "foo.o", and aspectOfOwner should be FooAspect.class. This + * method is necessary when an Aspect of the target, not the target itself, is creating an + * Artifact. */ - protected Artifact getBinArtifact(String packageRelativePath, ConfiguredTarget owner, - NativeAspectClass creatingAspectFactory) { + protected Artifact getBinArtifact( + String packageRelativePath, ConfiguredTarget owner, AspectClass creatingAspectFactory) { return getBinArtifact( packageRelativePath, owner, creatingAspectFactory, AspectParameters.EMPTY); } /** * Gets a derived Artifact for testing in the subdirectory of the {@link - * BuildConfiguration#getBinDirectory()} corresponding to the package of {@code owner}, - * where the given artifact belongs to the given ConfiguredTarget together with the given Aspect. - * So to specify a file foo/foo.o owned by target //foo:foo with an aspect from FooAspect, - * {@code packageRelativePath} should just be "foo.o", and aspectOfOwner should be - * FooAspect.class. This method is necessary when an Aspect of the target, not the target itself, - * is creating an Artifact. + * BuildConfiguration#getBinDirectory()} corresponding to the package of {@code owner}, where the + * given artifact belongs to the given ConfiguredTarget together with the given Aspect. So to + * specify a file foo/foo.o owned by target //foo:foo with an aspect from FooAspect, {@code + * packageRelativePath} should just be "foo.o", and aspectOfOwner should be FooAspect.class. This + * method is necessary when an Aspect of the target, not the target itself, is creating an + * Artifact. */ protected Artifact getBinArtifact( String packageRelativePath, ConfiguredTarget owner, - NativeAspectClass creatingAspectFactory, + AspectClass creatingAspectFactory, AspectParameters parameters) { return getPackageRelativeDerivedArtifact( packageRelativePath, |