From 736b955ce19c7c1d82de18b3bece5d0f09dc66d4 Mon Sep 17 00:00:00 2001 From: cushon Date: Thu, 19 Apr 2018 10:07:20 -0700 Subject: Consolidate handling of jar files in deps PiperOrigin-RevId: 193533061 --- .../lib/rules/android/AndroidLocalTestBase.java | 11 +-- .../devtools/build/lib/rules/java/JavaCommon.java | 19 +++-- .../build/lib/rules/java/JavaCompilationArgs.java | 87 +--------------------- .../lib/rules/java/JavaCompilationArgsHelper.java | 19 +---- .../rules/java/JavaCompilationArgsProvider.java | 41 ++++++++++ .../lib/rules/java/JavaCompilationHelper.java | 15 ++-- .../build/lib/rules/java/JavaLibraryHelper.java | 29 ++------ 7 files changed, 79 insertions(+), 142 deletions(-) (limited to 'src/main/java/com/google/devtools/build/lib/rules') diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java index de8505553b..3ae6f66beb 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidLocalTestBase.java @@ -39,8 +39,8 @@ import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.rules.java.ClasspathConfiguredFragment; import com.google.devtools.build.lib.rules.java.DeployArchiveBuilder; import com.google.devtools.build.lib.rules.java.JavaCommon; -import com.google.devtools.build.lib.rules.java.JavaCompilationArgs; import com.google.devtools.build.lib.rules.java.JavaCompilationArgs.ClasspathType; +import com.google.devtools.build.lib.rules.java.JavaCompilationArgsProvider; import com.google.devtools.build.lib.rules.java.JavaCompilationArtifacts; import com.google.devtools.build.lib.rules.java.JavaCompilationHelper; import com.google.devtools.build.lib.rules.java.JavaConfiguration; @@ -511,12 +511,9 @@ public abstract class AndroidLocalTestBase implements RuleConfiguredTargetFactor // The dep may be a simple JAR and not a java rule, hence we can't simply do // dep.getProvider(JavaCompilationArgsProvider.class).getRecursiveJavaCompilationArgs(), // so we reuse the logic within JavaCompilationArgs to handle both scenarios. - JavaCompilationArgs args = - JavaCompilationArgs.builder() - .addTransitiveTargets( - ImmutableList.of(deps), /*recursive=*/ true, ClasspathType.RUNTIME_ONLY) - .build(); - return args.getRuntimeJars(); + return JavaCompilationArgsProvider.legacyFromTargets(ImmutableList.of(deps)) + .getRecursiveJavaCompilationArgs() + .getRuntimeJars(); } private static String getAndCheckTestClass( diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java index 3933ad7e78..7de525e0c8 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCommon.java @@ -272,9 +272,16 @@ public class JavaCommon { .setIsNeverLink(isNeverLink) .setSrcLessDepsExport(srcLessDepsExport) .setCompilationArtifacts(getJavaCompilationArtifacts()) - .setDeps(targetsTreatedAsDeps(ClasspathType.COMPILE_ONLY)) - .setRuntimeDeps(getRuntimeDeps(ruleContext)) - .setExports(getExports(ruleContext)) + .setDepsCompilationArgs( + ImmutableList.of( + JavaCompilationArgsProvider.legacyFromTargets( + targetsTreatedAsDeps(ClasspathType.COMPILE_ONLY)))) + .setRuntimeDepsCompilationArgs( + ImmutableList.of( + JavaCompilationArgsProvider.legacyFromTargets(getRuntimeDeps(ruleContext)))) + .setExportsCompilationArgs( + ImmutableList.of( + JavaCompilationArgsProvider.legacyFromTargets(getExports(ruleContext)))) .build()); } @@ -752,9 +759,9 @@ public class JavaCommon { private void processRuntimeDeps(JavaTargetAttributes.Builder attributes) { List runtimeDepInfo = getRuntimeDeps(ruleContext); checkRuntimeDeps(ruleContext, runtimeDepInfo); - JavaCompilationArgs args = JavaCompilationArgs.builder() - .addTransitiveTargets(runtimeDepInfo, true, ClasspathType.RUNTIME_ONLY) - .build(); + JavaCompilationArgs args = + JavaCompilationArgsProvider.legacyFromTargets(runtimeDepInfo) + .getRecursiveJavaCompilationArgs(); attributes.addRuntimeClassPathEntries(args.getRuntimeJars()); attributes.addInstrumentationMetadataEntries(args.getInstrumentationMetadata()); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArgs.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArgs.java index 74e2cac387..166d9edb2f 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArgs.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArgs.java @@ -16,13 +16,10 @@ package com.google.devtools.build.lib.rules.java; import com.google.auto.value.AutoValue; import com.google.devtools.build.lib.actions.Artifact; -import com.google.devtools.build.lib.analysis.FileProvider; -import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; -import com.google.devtools.build.lib.util.FileType; /** A container of Java compilation artifacts. */ @AutoValue @@ -195,9 +192,8 @@ public abstract class JavaCompilationArgs { public Builder addTransitiveCompilationArgs( JavaCompilationArgsProvider dep, boolean recursive, ClasspathType type) { - JavaCompilationArgs args = recursive - ? dep.getRecursiveJavaCompilationArgs() - : dep.getJavaCompilationArgs(); + JavaCompilationArgs args = + recursive ? dep.getRecursiveJavaCompilationArgs() : dep.getJavaCompilationArgs(); addTransitiveArgs(args, type); return this; } @@ -210,85 +206,6 @@ public abstract class JavaCompilationArgs { return this; } - /** - * Merges the artifacts of another target. - */ - public Builder addTransitiveTarget(TransitiveInfoCollection dep, boolean recursive, - ClasspathType type) { - JavaCompilationArgsProvider provider = - JavaInfo.getProvider(JavaCompilationArgsProvider.class, dep); - if (provider != null) { - addTransitiveCompilationArgs(provider, recursive, type); - return this; - } else { - NestedSet filesToBuild = - dep.getProvider(FileProvider.class).getFilesToBuild(); - for (Artifact jar : FileType.filter(filesToBuild, JavaSemantics.JAR)) { - addCompileTimeJarAsFullJar(jar); - addRuntimeJar(jar); - } - } - return this; - } - - /** - * Merges the artifacts of a collection of targets. - */ - public Builder addTransitiveTargets(Iterable deps, - boolean recursive, ClasspathType type) { - for (TransitiveInfoCollection dep : deps) { - addTransitiveTarget(dep, recursive, type); - } - return this; - } - - /** - * Merges the artifacts of a collection of targets. - */ - public Builder addTransitiveTargets(Iterable deps, - boolean recursive) { - return addTransitiveTargets(deps, recursive, ClasspathType.BOTH); - } - - /** - * Merges the artifacts of a collection of targets. - */ - public Builder addTransitiveTargets(Iterable deps) { - return addTransitiveTargets(deps, /*recursive=*/true, ClasspathType.BOTH); - } - - /** - * Merges the artifacts of a collection of targets. - */ - public Builder addTransitiveDependencies(Iterable deps, - boolean recursive) { - for (JavaCompilationArgsProvider dep : deps) { - addTransitiveDependency(dep, recursive, ClasspathType.BOTH); - } - return this; - } - - /** Merges the artifacts of a collection of targets. */ - public Builder addTransitiveDependencies( - Iterable deps, boolean recursive, ClasspathType type) { - for (JavaCompilationArgsProvider dep : deps) { - addTransitiveDependency(dep, recursive, type); - } - return this; - } - - /** - * Merges the artifacts of another target. - */ - private Builder addTransitiveDependency(JavaCompilationArgsProvider dep, boolean recursive, - ClasspathType type) { - JavaCompilationArgs args = recursive - ? dep.getRecursiveJavaCompilationArgs() - : dep.getJavaCompilationArgs(); - addTransitiveArgs(args, type); - return this; - } - /** * Includes the contents of another instance of {@link JavaCompilationArgs}. * diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArgsHelper.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArgsHelper.java index b25234d86b..23f423259b 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArgsHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArgsHelper.java @@ -16,9 +16,9 @@ package com.google.devtools.build.lib.rules.java; import com.google.auto.value.AutoValue; import com.google.common.collect.ImmutableList; -import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import java.util.List; +// TODO(cushon): delete this class, see discussion in cl/193146318 @AutoValue abstract class JavaCompilationArgsHelper { abstract boolean recursive(); @@ -31,24 +31,15 @@ abstract class JavaCompilationArgsHelper { abstract List depsCompilationArgs(); - abstract Iterable deps(); - abstract List runtimeDepsCompilationArgs(); - abstract Iterable runtimeDeps(); - abstract List exportsCompilationArgs(); - abstract Iterable exports(); - static Builder builder() { return new AutoValue_JavaCompilationArgsHelper.Builder() .setDepsCompilationArgs(ImmutableList.of()) - .setDeps(ImmutableList.of()) .setRuntimeDepsCompilationArgs(ImmutableList.of()) - .setRuntimeDeps(ImmutableList.of()) - .setExportsCompilationArgs(ImmutableList.of()) - .setExports(ImmutableList.of()); + .setExportsCompilationArgs(ImmutableList.of()); } public abstract Builder toBuilder(); @@ -65,16 +56,10 @@ abstract class JavaCompilationArgsHelper { abstract Builder setDepsCompilationArgs(List value); - abstract Builder setDeps(Iterable value); - abstract Builder setRuntimeDepsCompilationArgs(List value); - abstract Builder setRuntimeDeps(Iterable value); - abstract Builder setExportsCompilationArgs(List value); - abstract Builder setExports(Iterable value); - abstract JavaCompilationArgsHelper build(); } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArgsProvider.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArgsProvider.java index de476e970d..7ca70e2095 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArgsProvider.java +++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompilationArgsProvider.java @@ -17,12 +17,16 @@ package com.google.devtools.build.lib.rules.java; import com.google.auto.value.AutoValue; import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.Artifact; +import com.google.devtools.build.lib.analysis.FileProvider; +import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.analysis.TransitiveInfoProvider; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; +import com.google.devtools.build.lib.rules.java.JavaCompilationArgs.ClasspathType; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; +import com.google.devtools.build.lib.util.FileType; import java.util.Collection; /** An interface for objects that provide information on how to include them in Java builds. */ @@ -100,4 +104,41 @@ public abstract class JavaCompilationArgsProvider implements TransitiveInfoProvi recursiveJavaCompilationArgs.build(), compileTimeJavaDepArtifacts.build()); } + + /** + * Returns a {@link JavaCompilationArgsProvider} for the given {@link TransitiveInfoCollection}s. + * + *

If the given targets have a {@link JavaCompilationArgsProvider}, the information from that + * provider will be returned. Otherwise, any jar files provided by the targets will be wrapped in + * the returned provider. + * + * @deprecated The handling of raw jar files is present for legacy compatibility. All new + * Java-based rules should require their dependencies to provide {@link + * JavaCompilationArgsProvider}, and that precompiled jar files be wrapped in {@code + * java_import}. New rules should not use this method, and existing rules should be cleaned up + * to disallow jar files in their deps. + */ + // TODO(b/11285003): disallow jar files in deps, require java_import instead + @Deprecated + public static JavaCompilationArgsProvider legacyFromTargets( + Iterable infos) { + JavaCompilationArgs.Builder argsBuilder = JavaCompilationArgs.builder(); + JavaCompilationArgs.Builder recursiveArgsBuilder = JavaCompilationArgs.builder(); + for (TransitiveInfoCollection info : infos) { + JavaCompilationArgsProvider provider = + JavaInfo.getProvider(JavaCompilationArgsProvider.class, info); + if (provider != null) { + argsBuilder.addTransitiveArgs(provider.getJavaCompilationArgs(), ClasspathType.BOTH); + recursiveArgsBuilder.addTransitiveArgs( + provider.getRecursiveJavaCompilationArgs(), ClasspathType.BOTH); + } else { + NestedSet filesToBuild = info.getProvider(FileProvider.class).getFilesToBuild(); + for (Artifact jar : FileType.filter(filesToBuild, JavaSemantics.JAR)) { + argsBuilder.addRuntimeJar(jar).addCompileTimeJarAsFullJar(jar); + recursiveArgsBuilder.addRuntimeJar(jar).addCompileTimeJarAsFullJar(jar); + } + } + } + return create(argsBuilder.build(), recursiveArgsBuilder.build()); + } } 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 fdd0c26082..e209ebc23d 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 @@ -39,6 +39,7 @@ import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.collect.nestedset.Order; +import com.google.devtools.build.lib.rules.java.JavaCompilationArgs.ClasspathType; import com.google.devtools.build.lib.rules.java.JavaConfiguration.JavaClasspathMode; import com.google.devtools.build.lib.util.FileType; import com.google.devtools.build.lib.vfs.FileSystemUtils; @@ -708,12 +709,11 @@ public final class JavaCompilationHelper { } private void addLibrariesToAttributesInternal(Iterable deps) { - JavaCompilationArgs args = JavaCompilationArgs.builder() - .addTransitiveTargets(deps).build(); + JavaCompilationArgs args = + JavaCompilationArgsProvider.legacyFromTargets(deps).getRecursiveJavaCompilationArgs(); - NestedSet directJars = isStrict() - ? getNonRecursiveCompileTimeJarsFromCollection(deps) - : null; + NestedSet directJars = + isStrict() ? getNonRecursiveCompileTimeJarsFromCollection(deps) : null; addArgsAndJarsToAttributes(args, directJars); } @@ -724,7 +724,10 @@ public final class JavaCompilationHelper { private NestedSet getNonRecursiveCompileTimeJarsFromCollection( Iterable deps) { JavaCompilationArgs.Builder builder = JavaCompilationArgs.builder(); - builder.addTransitiveTargets(deps, /*recursive=*/false); + builder.addTransitiveCompilationArgs( + JavaCompilationArgsProvider.legacyFromTargets(deps), + /*recursive=*/ false, + ClasspathType.BOTH); return builder.build().getCompileTimeJars(); } 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 0a153af354..fb6668040b 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 @@ -332,28 +332,19 @@ public final class JavaLibraryHelper { } private void addDepsToAttributes(JavaTargetAttributes.Builder attributes) { - NestedSet directJars; + JavaCompilationArgsProvider argsProvider = JavaCompilationArgsProvider.merge(deps); + if (isStrict()) { - directJars = getNonRecursiveCompileTimeJarsFromDeps(); + NestedSet directJars = argsProvider.getJavaCompilationArgs().getCompileTimeJars(); if (directJars != null) { attributes.addDirectJars(directJars); } } - JavaCompilationArgs args = - JavaCompilationArgs.builder() - .addTransitiveDependencies(deps, true) - .build(); - attributes.addCompileTimeClassPathEntries(args.getCompileTimeJars()); - attributes.addRuntimeClassPathEntries(args.getRuntimeJars()); - attributes.addInstrumentationMetadataEntries(args.getInstrumentationMetadata()); - } - - private NestedSet getNonRecursiveCompileTimeJarsFromDeps() { - return JavaCompilationArgs.builder() - .addTransitiveDependencies(deps, false) - .build() - .getCompileTimeJars(); + JavaCompilationArgs recursiveArgs = argsProvider.getRecursiveJavaCompilationArgs(); + attributes.addCompileTimeClassPathEntries(recursiveArgs.getCompileTimeJars()); + attributes.addRuntimeClassPathEntries(recursiveArgs.getRuntimeJars()); + attributes.addInstrumentationMetadataEntries(recursiveArgs.getInstrumentationMetadata()); } static JavaCompilationArgs getJavaCompilationArgs(JavaCompilationArgsHelper helper) { @@ -361,18 +352,14 @@ public final class JavaLibraryHelper { JavaCompilationArgs.Builder builder = JavaCompilationArgs.builder() .merge(helper.compilationArtifacts(), helper.isNeverLink()) - .addTransitiveTargets(helper.exports(), helper.recursive(), type) .addTransitiveCompilationArgs( helper.exportsCompilationArgs(), helper.recursive(), type); // TODO(bazel-team): remove srcs-less behaviour after android_library users are refactored if (helper.recursive() || helper.srcLessDepsExport()) { builder .addTransitiveCompilationArgs(helper.depsCompilationArgs(), helper.recursive(), type) - .addTransitiveTargets(helper.deps(), helper.recursive(), type) .addTransitiveCompilationArgs( - helper.runtimeDepsCompilationArgs(), helper.recursive(), ClasspathType.RUNTIME_ONLY) - .addTransitiveTargets( - helper.runtimeDeps(), helper.recursive(), ClasspathType.RUNTIME_ONLY); + helper.runtimeDepsCompilationArgs(), helper.recursive(), ClasspathType.RUNTIME_ONLY); } return builder.build(); } -- cgit v1.2.3