diff options
6 files changed, 93 insertions, 103 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java index 870cee733a..edc438dbb3 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidCommon.java @@ -375,8 +375,9 @@ public class AndroidCommon { ImmutableList.Builder<Artifact> jarsProducedForRuntime) throws InterruptedException { compileResourceJar(javaSemantics, resourcesJar); // Add the compiled resource jar to the classpath of the main compilation. - attributes.addDirectJars(ImmutableList.of(resourceClassJar)); - attributes.addDirectCompileTimeClassPathEntries(ImmutableList.of(resourceClassJar)); + NestedSet<Artifact> directJars = NestedSetBuilder.create(Order.STABLE_ORDER, resourceClassJar); + attributes.addDirectJars(directJars); + attributes.addCompileTimeClassPathEntries(directJars); // Add the compiled resource jar to the classpath of consuming targets. artifactsBuilder.addCompileTimeJar(resourceClassJar); // Combined resource constants needs to come even before our own classes that may contain diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java index e4f654b941..84fd903601 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationHelper.java @@ -157,7 +157,7 @@ public final class JavaCompilationHelper extends BaseJavaCompilationHelper { builder.addProcessorPaths(attributes.getProcessorPath()); builder.addProcessorNames(attributes.getProcessorNames()); builder.setStrictJavaDeps(attributes.getStrictJavaDeps()); - builder.addDirectJars(attributes.getDirectJars()); + builder.setDirectJars(attributes.getDirectJars()); builder.addCompileTimeDependencyArtifacts(attributes.getCompileTimeDependencyArtifacts()); builder.setRuleKind(attributes.getRuleKind()); builder.setTargetLabel(attributes.getTargetLabel()); @@ -279,7 +279,7 @@ public final class JavaCompilationHelper extends BaseJavaCompilationHelper { builder.setOutputDepsProto(headerDeps); builder.setStrictJavaDeps(attributes.getStrictJavaDeps()); builder.addCompileTimeDependencyArtifacts(attributes.getCompileTimeDependencyArtifacts()); - builder.addDirectJars(attributes.getDirectJars()); + builder.setDirectJars(attributes.getDirectJars()); builder.setRuleKind(attributes.getRuleKind()); builder.setTargetLabel(attributes.getTargetLabel()); builder.setJavaBaseInputs(getHostJavabaseInputsNonStatic(ruleContext)); @@ -521,10 +521,11 @@ public final class JavaCompilationHelper extends BaseJavaCompilationHelper { return jar; } - private void addArgsAndJarsToAttributes(JavaCompilationArgs args, Iterable<Artifact> directJars) { + private void addArgsAndJarsToAttributes( + JavaCompilationArgs args, NestedSet<Artifact> directJars) { // Can only be non-null when isStrict() returns true. if (directJars != null) { - attributes.addDirectCompileTimeClassPathEntries(directJars); + attributes.addCompileTimeClassPathEntries(directJars); attributes.addDirectJars(directJars); } 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 5c3d36d41e..c9abbda252 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 @@ -14,6 +14,7 @@ package com.google.devtools.build.lib.rules.java; +import static com.google.devtools.build.lib.util.Preconditions.checkNotNull; import static java.nio.charset.StandardCharsets.ISO_8859_1; import com.google.common.annotations.VisibleForTesting; @@ -68,6 +69,7 @@ import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; +import java.util.Set; /** * Action that represents a Java compilation. @@ -144,10 +146,8 @@ public final class JavaCompileAction extends AbstractAction { */ private final ImmutableList<String> javacOpts; - /** - * The subset of classpath jars provided by direct dependencies. - */ - private final ImmutableList<Artifact> directJars; + /** The subset of classpath jars provided by direct dependencies. */ + private final NestedSet<Artifact> directJars; /** * The level of strict dependency checks (off, warnings, or errors). @@ -163,12 +163,12 @@ public final class JavaCompileAction extends AbstractAction { * Constructs an action to compile a set of Java source files to class files. * * @param owner the action owner, typically a java_* RuleConfiguredTarget. - * @param baseInputs the set of the input artifacts of the compile action - * without the parameter file action; + * @param baseInputs the set of the input artifacts of the compile action without the parameter + * file action; * @param outputs the outputs of the action - * @param javaCompileCommandLine the command line for the java library - * builder - it's actually written to the parameter file, but other - * parts (for example, ide_build_info) need access to the data + * @param javaCompileCommandLine the command line for the java library builder - it's actually + * written to the parameter file, but other parts (for example, ide_build_info) need access to + * the data * @param commandLine the actual invocation command line */ private JavaCompileAction( @@ -191,7 +191,7 @@ public final class JavaCompileAction extends AbstractAction { Collection<Artifact> sourceJars, Collection<Artifact> sourceFiles, List<String> javacOpts, - Collection<Artifact> directJars, + NestedSet<Artifact> directJars, BuildConfiguration.StrictDepsMode strictJavaDeps, Collection<Artifact> compileTimeDependencyArtifacts) { super( @@ -213,7 +213,7 @@ public final class JavaCompileAction extends AbstractAction { this.javaCompileCommandLine = javaCompileCommandLine; this.commandLine = commandLine; - this.classDirectory = Preconditions.checkNotNull(classDirectory); + this.classDirectory = checkNotNull(classDirectory); this.outputJar = outputJar; this.classpathEntries = classpathEntries; this.bootclasspathEntries = ImmutableList.copyOf(bootclasspathEntries); @@ -226,7 +226,7 @@ public final class JavaCompileAction extends AbstractAction { this.sourceJars = ImmutableList.copyOf(sourceJars); this.sourceFiles = ImmutableList.copyOf(sourceFiles); this.javacOpts = ImmutableList.copyOf(javacOpts); - this.directJars = ImmutableList.copyOf(directJars); + this.directJars = checkNotNull(directJars, "directJars must not be null"); this.strictJavaDeps = strictJavaDeps; this.compileTimeDependencyArtifacts = ImmutableList.copyOf(compileTimeDependencyArtifacts); } @@ -292,7 +292,7 @@ public final class JavaCompileAction extends AbstractAction { } @VisibleForTesting - public Collection<Artifact> getDirectJars() { + public NestedSet<Artifact> getDirectJars() { return directJars; } @@ -472,13 +472,13 @@ public final class JavaCompileAction extends AbstractAction { * Creates an instance. * * @param configuration the build configuration, which provides the default options and the path - * to the compiler, etc. + * to the compiler, etc. * @param classDirectory the directory in which generated classfiles are placed relative to the - * exec root + * exec root * @param sourceGenDirectory the directory where source files generated by annotation processors - * should be stored. + * should be stored. * @param tempDirectory a directory in which the library builder can store temporary files - * relative to the exec root + * relative to the exec root * @param outputJar output jar * @param compressJar if true compress the output jar * @param outputDepsProto the proto file capturing dependency information @@ -513,13 +513,13 @@ public final class JavaCompileAction extends AbstractAction { Collection<Artifact> sourceFiles, Collection<Artifact> extdirInputs, List<String> javacOpts, - final Collection<Artifact> directJars, + final NestedSet<Artifact> directJars, BuildConfiguration.StrictDepsMode strictJavaDeps, Collection<Artifact> compileTimeDependencyArtifacts, String ruleKind, Label targetLabel) { - Preconditions.checkNotNull(classDirectory); - Preconditions.checkNotNull(tempDirectory); + checkNotNull(classDirectory); + checkNotNull(tempDirectory); CustomCommandLine.Builder result = CustomCommandLine.builder(); @@ -615,12 +615,7 @@ public final class JavaCompileAction extends AbstractAction { if (strictJavaDeps != BuildConfiguration.StrictDepsMode.OFF) { result.add("--strict_java_deps"); result.add(strictJavaDeps.toString()); - result.add(new CustomMultiArgv() { - @Override - public Iterable<String> argv() { - return addJarsToTargets(classpath, directJars); - } - }); + result.add(new JarsToTargetsArgv(classpath, directJars)); if (configuration.getFragment(JavaConfiguration.class).getReduceJavaClasspath() == JavaClasspathMode.JAVABUILDER) { @@ -673,26 +668,34 @@ public final class JavaCompileAction extends AbstractAction { } /** - * Builds the list of mappings between jars on the classpath and their - * originating targets names. + * Builds the list of mappings between jars on the classpath and their originating targets names. */ - static ImmutableList<String> addJarsToTargets( - NestedSet<Artifact> classpath, Collection<Artifact> directJars) { - ImmutableList.Builder<String> builder = ImmutableList.builder(); - for (Artifact jar : classpath) { - builder.add(directJars.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); + static class JarsToTargetsArgv extends CustomMultiArgv { + private final NestedSet<Artifact> classpath; + private final NestedSet<Artifact> directJars; + + JarsToTargetsArgv(NestedSet<Artifact> classpath, NestedSet<Artifact> directJars) { + this.classpath = classpath; + this.directJars = directJars; + } + + @Override + public Iterable<String> argv() { + Set<Artifact> directJarSet = directJars.toSet(); + ImmutableList.Builder<String> builder = ImmutableList.builder(); + 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); + } + return builder.build(); } - return builder.build(); } /** @@ -702,7 +705,7 @@ public final class JavaCompileAction extends AbstractAction { * libraries), there is no generating action, so we just return the jar name in label form. */ private static Label getTargetName(Artifact jar) { - return Preconditions.checkNotNull(jar.getOwner(), jar); + return checkNotNull(jar.getOwner(), jar); } /** @@ -712,8 +715,8 @@ public final class JavaCompileAction extends AbstractAction { Artifact langtoolsJar, ImmutableList<Artifact> instrumentationJars, Artifact paramFile, ImmutableList<String> javaBuilderJvmFlags, String javaBuilderMainClass, String pathDelimiter) { - Preconditions.checkNotNull(langtoolsJar); - Preconditions.checkNotNull(javaBuilderJar); + checkNotNull(langtoolsJar); + checkNotNull(javaBuilderJar); CustomCommandLine.Builder builder = CustomCommandLine.builder() .addPath(javaExecutable) @@ -786,7 +789,7 @@ public final class JavaCompileAction extends AbstractAction { private final Collection<Artifact> translations = new LinkedHashSet<>(); private BuildConfiguration.StrictDepsMode strictJavaDeps = BuildConfiguration.StrictDepsMode.OFF; - private final Collection<Artifact> directJars = new ArrayList<>(); + private NestedSet<Artifact> directJars = NestedSetBuilder.emptySet(Order.NAIVE_LINK_ORDER); private final Collection<Artifact> compileTimeDependencyArtifacts = new ArrayList<>(); private List<String> javacOpts = new ArrayList<>(); private ImmutableList<String> javacJvmOpts = ImmutableList.of(); @@ -855,7 +858,7 @@ public final class JavaCompileAction extends AbstractAction { // Invariant: if strictJavaDeps is OFF, then directJars and // dependencyArtifacts are ignored if (strictJavaDeps == BuildConfiguration.StrictDepsMode.OFF) { - directJars.clear(); + directJars = NestedSetBuilder.emptySet(Order.NAIVE_LINK_ORDER); compileTimeDependencyArtifacts.clear(); } @@ -1044,11 +1047,9 @@ public final class JavaCompileAction extends AbstractAction { return this; } - /** - * Accumulates the given jar artifacts as being provided by direct dependencies. - */ - public Builder addDirectJars(Collection<Artifact> directJars) { - this.directJars.addAll(directJars); + /** Accumulates the given jar artifacts as being provided by direct dependencies. */ + public Builder setDirectJars(NestedSet<Artifact> directJars) { + this.directJars = checkNotNull(directJars, "directJars must not be null"); return this; } diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileActionBuilder.java index 2dfe4a6ed9..895d1b63b5 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaHeaderCompileActionBuilder.java @@ -24,7 +24,6 @@ import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.actions.CommandLine; import com.google.devtools.build.lib.analysis.actions.CustomCommandLine; -import com.google.devtools.build.lib.analysis.actions.CustomCommandLine.CustomMultiArgv; import com.google.devtools.build.lib.analysis.actions.SpawnAction; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.config.BuildConfiguration.StrictDepsMode; @@ -71,7 +70,7 @@ public class JavaHeaderCompileActionBuilder { @Nullable private Label targetLabel; private PathFragment tempDirectory; private BuildConfiguration.StrictDepsMode strictJavaDeps = BuildConfiguration.StrictDepsMode.OFF; - private List<Artifact> directJars = new ArrayList<>(); + private NestedSet<Artifact> directJars = NestedSetBuilder.emptySet(Order.NAIVE_LINK_ORDER); private List<Artifact> compileTimeDependencyArtifacts = new ArrayList<>(); private ImmutableList<String> javacOpts; private List<Artifact> processorPath = new ArrayList<>(); @@ -92,9 +91,9 @@ public class JavaHeaderCompileActionBuilder { } /** Sets the direct dependency artifacts. */ - public JavaHeaderCompileActionBuilder addDirectJars(Collection<Artifact> directJars) { + public JavaHeaderCompileActionBuilder setDirectJars(NestedSet<Artifact> directJars) { checkNotNull(directJars, "directJars must not be null"); - this.directJars.addAll(directJars); + this.directJars = directJars; return this; } @@ -237,6 +236,13 @@ public class JavaHeaderCompileActionBuilder { checkNotNull(processorPath, "processorPath must not be null"); checkNotNull(processorNames, "processorNames must not be null"); + // Invariant: if strictJavaDeps is OFF, then directJars and + // dependencyArtifacts are ignored + if (strictJavaDeps == BuildConfiguration.StrictDepsMode.OFF) { + directJars = NestedSetBuilder.emptySet(Order.NAIVE_LINK_ORDER); + compileTimeDependencyArtifacts.clear(); + } + SpawnAction.Builder builder = new SpawnAction.Builder(); builder.addOutput(outputJar); @@ -253,7 +259,7 @@ public class JavaHeaderCompileActionBuilder { builder.addInputs(processorPath); builder.addInputs(sourceJars); builder.addInputs(sourceFiles); - builder.addInputs(directJars); + builder.addTransitiveInputs(directJars); builder.addInputs(compileTimeDependencyArtifacts); builder.addTool(javacJar); @@ -331,13 +337,7 @@ public class JavaHeaderCompileActionBuilder { } if (strictJavaDeps != BuildConfiguration.StrictDepsMode.OFF) { - result.add( - new CustomMultiArgv() { - @Override - public Iterable<String> argv() { - return JavaCompileAction.addJarsToTargets(classpathEntries, directJars); - } - }); + result.add(new JavaCompileAction.JarsToTargetsArgv(classpathEntries, directJars)); if (!compileTimeDependencyArtifacts.isEmpty()) { result.addExecPaths("--deps_artifacts", compileTimeDependencyArtifacts); diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaLibraryHelper.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaLibraryHelper.java index 4112128e65..bd4409ddc2 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaLibraryHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaLibraryHelper.java @@ -174,7 +174,7 @@ public final class JavaLibraryHelper { if (isStrict()) { directJars = getNonRecursiveCompileTimeJarsFromDeps(); if (directJars != null) { - attributes.addDirectCompileTimeClassPathEntries(directJars); + attributes.addCompileTimeClassPathEntries(directJars); attributes.addDirectJars(directJars); } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaTargetAttributes.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaTargetAttributes.java index 3a28c203be..415cb12139 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaTargetAttributes.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaTargetAttributes.java @@ -83,7 +83,7 @@ public class JavaTargetAttributes { private BuildConfiguration.StrictDepsMode strictJavaDeps = BuildConfiguration.StrictDepsMode.OFF; - private final List<Artifact> directJars = new ArrayList<>(); + private final NestedSetBuilder<Artifact> directJars = NestedSetBuilder.naiveLinkOrder(); private final List<Artifact> compileTimeDependencyArtifacts = new ArrayList<>(); private String ruleKind; private Label targetLabel; @@ -174,15 +174,6 @@ public class JavaTargetAttributes { return this; } - public Builder addDirectCompileTimeClassPathEntries(Iterable<Artifact> entries) { - Preconditions.checkArgument(!built); - // The other version is preferred as it is more memory-efficient. - for (Artifact classPathEntry : entries) { - compileTimeClassPath.add(classPathEntry); - } - return this; - } - public Builder setRuleKind(String ruleKind) { Preconditions.checkArgument(!built); this.ruleKind = ruleKind; @@ -228,23 +219,21 @@ public class JavaTargetAttributes { } /** - * In tandem with strictJavaDeps, directJars represents a subset of the - * compile-time, classpath jars that were provided by direct dependencies. - * When strictJavaDeps is OFF, there is no need to provide directJars, and - * no extra information is passed to javac. When strictJavaDeps is set to - * WARN or ERROR, the compiler command line will include extra flags to - * indicate the warning/error policy and to map the classpath jars to direct - * or transitive dependencies, using the information in directJars. The extra - * flags are formatted like this (same for --indirect_dependency): + * In tandem with strictJavaDeps, directJars represents a subset of the compile-time, classpath + * jars that were provided by direct dependencies. When strictJavaDeps is OFF, there is no need + * to provide directJars, and no extra information is passed to javac. When strictJavaDeps is + * set to WARN or ERROR, the compiler command line will include extra flags to indicate the + * warning/error policy and to map the classpath jars to direct or transitive dependencies, + * using the information in directJars. The extra flags are formatted like this (same for + * --indirect_dependency): <pre> * --direct_dependency * foo/bar/lib.jar * //java/com/google/foo:bar - * - * @param directJars + * </pre> */ - public Builder addDirectJars(Iterable<Artifact> directJars) { + public Builder addDirectJars(NestedSet<Artifact> directJars) { Preconditions.checkArgument(!built); - Iterables.addAll(this.directJars, directJars); + this.directJars.addTransitive(directJars); return this; } @@ -339,7 +328,7 @@ public class JavaTargetAttributes { messages, sourceJars, classPathResources, - directJars, + directJars.build(), compileTimeDependencyArtifacts, ruleKind, targetLabel, @@ -391,7 +380,7 @@ public class JavaTargetAttributes { private final ImmutableList<Artifact> classPathResources; - private final ImmutableList<Artifact> directJars; + private final NestedSet<Artifact> directJars; private final ImmutableList<Artifact> compileTimeDependencyArtifacts; private final String ruleKind; private final Label targetLabel; @@ -399,9 +388,7 @@ public class JavaTargetAttributes { private final NestedSet<Artifact> excludedArtifacts; private final BuildConfiguration.StrictDepsMode strictJavaDeps; - /** - * Constructor of JavaTargetAttributes. - */ + /** Constructor of JavaTargetAttributes. */ private JavaTargetAttributes( Set<Artifact> sourceFiles, Set<Artifact> compileTimeJarFiles, @@ -415,7 +402,7 @@ public class JavaTargetAttributes { List<Artifact> messages, List<Artifact> sourceJars, List<Artifact> classPathResources, - List<Artifact> directJars, + NestedSet<Artifact> directJars, List<Artifact> compileTimeDependencyArtifacts, String ruleKind, Label targetLabel, @@ -433,7 +420,7 @@ public class JavaTargetAttributes { this.messages = ImmutableList.copyOf(messages); this.sourceJars = ImmutableList.copyOf(sourceJars); this.classPathResources = ImmutableList.copyOf(classPathResources); - this.directJars = ImmutableList.copyOf(directJars); + this.directJars = directJars; this.compileTimeDependencyArtifacts = ImmutableList.copyOf(compileTimeDependencyArtifacts); this.ruleKind = ruleKind; this.targetLabel = targetLabel; @@ -441,7 +428,7 @@ public class JavaTargetAttributes { this.strictJavaDeps = strictJavaDeps; } - public List<Artifact> getDirectJars() { + public NestedSet<Artifact> getDirectJars() { return directJars; } |