Skip to content

Commit

Permalink
Flatten JavaCompilationAgs into JavaCompilationArgsProvider
Browse files Browse the repository at this point in the history
Consolidate the creation of JavaCompilationArgsProviders, and avoid explicit
handling of the 'direct' and 'recursive' cases in clients. Also add some
higher-level methods to the builder API to support adding dependencies
with dep/export/runtime_dep-like semantics.

PiperOrigin-RevId: 202166383
  • Loading branch information
cushon authored and Copybara-Service committed Jun 26, 2018
1 parent dc041d1 commit 4a20020
Show file tree
Hide file tree
Showing 21 changed files with 448 additions and 583 deletions.
1 change: 0 additions & 1 deletion src/main/java/com/google/devtools/build/lib/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -1001,7 +1001,6 @@ java_library(
"rules/java/ImportDepsCheckActionBuilder.java",
"rules/java/JavaBuildInfoFactory.java",
"rules/java/JavaCommon.java",
"rules/java/JavaCompilationArgs.java",
"rules/java/JavaCompilationArgsProvider.java",
"rules/java/JavaCompilationArtifacts.java",
"rules/java/JavaCompilationHelper.java",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@
import com.google.devtools.build.lib.rules.java.DeployArchiveBuilder.Compression;
import com.google.devtools.build.lib.rules.java.JavaCcLinkParamsProvider;
import com.google.devtools.build.lib.rules.java.JavaCommon;
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.JavaCompilationArgsProvider.ClasspathType;
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;
Expand Down Expand Up @@ -509,7 +509,7 @@ private TransitiveInfoCollection getTestSupport(RuleContext ruleContext) {
private static NestedSet<Artifact> getRuntimeJarsForTargets(TransitiveInfoCollection... deps) {
// 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.
// so we reuse the logic within JavaCompilationArgsProvider to handle both scenarios.
return JavaCompilationArgsProvider.legacyFromTargets(ImmutableList.copyOf(deps))
.getRuntimeJars();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,6 @@
import com.google.devtools.build.lib.rules.android.AndroidConfiguration.AndroidAaptVersion;
import com.google.devtools.build.lib.rules.java.ImportDepsCheckActionBuilder;
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.JavaConfiguration;
Expand Down Expand Up @@ -166,10 +164,13 @@ public ConfiguredTarget create(RuleContext ruleContext)
/* bothDeps = */ targets);
javaSemantics.checkRule(ruleContext, common);

Artifact jdepsArtifact = createAarArtifact(ruleContext, "jdeps.proto");

common.setJavaCompilationArtifacts(
new JavaCompilationArtifacts.Builder()
.addRuntimeJar(mergedJar)
.addCompileTimeJarAsFullJar(mergedJar)
.setCompileTimeDependencies(jdepsArtifact)
.build());

JavaConfiguration javaConfig = ruleContext.getFragment(JavaConfiguration.class);
Expand All @@ -178,7 +179,6 @@ public ConfiguredTarget create(RuleContext ruleContext)
NestedSet<Artifact> bootclasspath = getBootclasspath(ruleContext);
Artifact depsCheckerResult =
createAarArtifact(ruleContext, "aar_import_deps_checker_result.txt");
Artifact jdepsArtifact = createAarArtifact(ruleContext, "jdeps.proto");
ImportDepsCheckActionBuilder.newBuilder()
.bootcalsspath(bootclasspath)
.declareDeps(directDeps)
Expand All @@ -188,8 +188,6 @@ public ConfiguredTarget create(RuleContext ruleContext)
.jdepsOutputArtifact(jdepsArtifact)
.ruleLabel(ruleContext.getLabel().getName())
.buildAndRegister(ruleContext);
NestedSet<Artifact> compileTimeDependencyArtifacts =
common.collectCompileTimeDependencyArtifacts(jdepsArtifact);

// We pass depsCheckerResult to create the action of extracting ANDROID_MANIFEST. Note that
// this action does not need depsCheckerResult. The only reason is that we need to check the
Expand All @@ -200,16 +198,9 @@ public ConfiguredTarget create(RuleContext ruleContext)
ruleContext, aar, ANDROID_MANIFEST, depsCheckerResult, androidManifestArtifact));

JavaCompilationArgsProvider javaCompilationArgsProvider =
JavaCompilationArgsProvider.create(
common.collectJavaCompilationArgs(
/* recursive = */ false,
JavaCommon.isNeverLink(ruleContext),
/* srcLessDepsExport = */ false),
common.collectJavaCompilationArgs(
/* recursive = */ true,
JavaCommon.isNeverLink(ruleContext),
/* srcLessDepsExport = */ false),
compileTimeDependencyArtifacts);
common.collectJavaCompilationArgs(
/* isNeverLink = */ JavaCommon.isNeverLink(ruleContext),
/* srcLessDepsExport = */ false);

JavaInfo.Builder javaInfoBuilder =
JavaInfo.Builder.create()
Expand Down Expand Up @@ -246,13 +237,8 @@ public ConfiguredTarget create(RuleContext ruleContext)

private NestedSet<Artifact> getCompileTimeJarsFromCollection(
ImmutableList<TransitiveInfoCollection> deps, boolean isStrict) {
return JavaCompilationArgs.builder()
.addTransitiveCompilationArgs(
JavaCompilationArgsProvider.legacyFromTargets(deps),
/*recursive=*/ !isStrict,
ClasspathType.BOTH)
.build()
.getCompileTimeJars();
JavaCompilationArgsProvider provider = JavaCompilationArgsProvider.legacyFromTargets(deps);
return isStrict ? provider.getDirectCompileTimeJars() : provider.getTransitiveCompileTimeJars();
}

private NestedSet<Artifact> getBootclasspath(RuleContext ruleContext) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,8 @@
import com.google.devtools.build.lib.rules.java.ClasspathConfiguredFragment;
import com.google.devtools.build.lib.rules.java.JavaCcLinkParamsProvider;
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.JavaCompilationArgsProvider.ClasspathType;
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.JavaInfo;
Expand Down Expand Up @@ -130,12 +129,10 @@ public static final <T extends Info> Iterable<T> getTransitivePrerequisites(
private final boolean asNeverLink;
private final boolean exportDeps;

private NestedSet<Artifact> compileTimeDependencyArtifacts;
private NestedSet<Artifact> filesToBuild;
private NestedSet<Artifact> transitiveNeverlinkLibraries =
NestedSetBuilder.emptySet(Order.STABLE_ORDER);
private JavaCompilationArgs javaCompilationArgs = JavaCompilationArgs.EMPTY_ARGS;
private JavaCompilationArgs recursiveJavaCompilationArgs = JavaCompilationArgs.EMPTY_ARGS;
private JavaCompilationArgsProvider javaCompilationArgs = JavaCompilationArgsProvider.EMPTY;
private NestedSet<Artifact> jarsProducedForRuntime;
private Artifact classJar;
private Artifact nativeHeaderOutput;
Expand Down Expand Up @@ -639,9 +636,7 @@ private void initJava(
}

JavaCompilationArtifacts javaArtifacts = javaArtifactsBuilder.build();
compileTimeDependencyArtifacts =
javaCommon.collectCompileTimeDependencyArtifacts(
javaArtifacts.getCompileTimeDependencyArtifact());
javaCommon.setJavaCompilationArtifacts(javaArtifacts);
javaCommon.setJavaCompilationArtifacts(javaArtifacts);

javaCommon.setClassPathFragment(
Expand All @@ -658,9 +653,10 @@ private void initJava(
javaCommon.getJavaCompilationArtifacts().getRuntimeJars());
if (collectJavaCompilationArgs) {
boolean hasSources = attributes.hasSources();
this.javaCompilationArgs = collectJavaCompilationArgs(exportDeps, asNeverLink, hasSources);
this.recursiveJavaCompilationArgs =
collectJavaCompilationArgs(true, asNeverLink, /* hasSources */ true);
this.javaCompilationArgs = collectJavaCompilationArgs(asNeverLink, hasSources);
if (exportDeps) {
this.javaCompilationArgs = JavaCompilationArgsProvider.makeNonStrict(javaCompilationArgs);
}
}
}

Expand Down Expand Up @@ -694,9 +690,7 @@ public RuleConfiguredTargetBuilder addTransitiveInfoProviders(
.setNativeHeaders(nativeHeaderOutput)
.build();
JavaSourceJarsProvider sourceJarsProvider = javaSourceJarsProviderBuilder.build();
JavaCompilationArgsProvider compilationArgsProvider =
JavaCompilationArgsProvider.create(
javaCompilationArgs, recursiveJavaCompilationArgs, compileTimeDependencyArtifacts);
JavaCompilationArgsProvider compilationArgsProvider = javaCompilationArgs;

JavaInfo.Builder javaInfoBuilder = JavaInfo.Builder.create();

Expand Down Expand Up @@ -762,18 +756,17 @@ private Runfiles getRunfiles() {
/**
* Collects Java compilation arguments for this target.
*
* @param recursive Whether to scan dependencies recursively.
* @param isNeverLink Whether the target has the 'neverlink' attr.
* @param hasSrcs If false, deps are exported (deprecated behaviour)
*/
private JavaCompilationArgs collectJavaCompilationArgs(
boolean recursive, boolean isNeverLink, boolean hasSrcs) {
private JavaCompilationArgsProvider collectJavaCompilationArgs(
boolean isNeverLink, boolean hasSrcs) {
boolean exportDeps =
!hasSrcs
&& ruleContext
.getFragment(AndroidConfiguration.class)
.allowSrcsLessAndroidLibraryDeps();
return javaCommon.collectJavaCompilationArgs(recursive, isNeverLink, exportDeps);
return javaCommon.collectJavaCompilationArgs(isNeverLink, exportDeps);
}

public ImmutableList<String> getJavacOpts() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@
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.ClasspathType;
import com.google.devtools.build.lib.rules.java.JavaCompilationArgsProvider;
import com.google.devtools.build.lib.rules.java.JavaCompilationArgsProvider.ClasspathType;
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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
import com.google.devtools.build.lib.rules.cpp.CppConfiguration;
import com.google.devtools.build.lib.rules.cpp.CppHelper;
import com.google.devtools.build.lib.rules.cpp.LinkerInput;
import com.google.devtools.build.lib.rules.java.JavaCompilationArgs.ClasspathType;
import com.google.devtools.build.lib.rules.java.JavaCompilationArgsProvider.ClasspathType;
import com.google.devtools.build.lib.rules.java.JavaConfiguration.OneVersionEnforcementLevel;
import com.google.devtools.build.lib.rules.java.ProguardHelper.ProguardOutput;
import com.google.devtools.build.lib.rules.java.proto.GeneratedExtensionRegistryProvider;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.packages.TargetUtils;
import com.google.devtools.build.lib.rules.cpp.LinkerInput;
import com.google.devtools.build.lib.rules.java.JavaCompilationArgs.ClasspathType;
import com.google.devtools.build.lib.rules.java.JavaCompilationArgsProvider.ClasspathType;
import com.google.devtools.build.lib.syntax.Type;
import com.google.devtools.build.lib.util.FileTypeSet;
import com.google.devtools.build.lib.util.Pair;
Expand Down Expand Up @@ -111,7 +111,9 @@ public void collectMetadataArtifacts(
private JavaCompilationHelper javaCompilationHelper;

public JavaCommon(RuleContext ruleContext, JavaSemantics semantics) {
this(ruleContext, semantics,
this(
ruleContext,
semantics,
ruleContext.getPrerequisiteArtifacts("srcs", Mode.TARGET).list(),
collectTargetsTreatedAsDeps(ruleContext, semantics, ClasspathType.COMPILE_ONLY),
collectTargetsTreatedAsDeps(ruleContext, semantics, ClasspathType.RUNTIME_ONLY),
Expand All @@ -120,7 +122,9 @@ public JavaCommon(RuleContext ruleContext, JavaSemantics semantics) {

public JavaCommon(RuleContext ruleContext, JavaSemantics semantics,
ImmutableList<Artifact> sources) {
this(ruleContext, semantics,
this(
ruleContext,
semantics,
sources,
collectTargetsTreatedAsDeps(ruleContext, semantics, ClasspathType.COMPILE_ONLY),
collectTargetsTreatedAsDeps(ruleContext, semantics, ClasspathType.RUNTIME_ONLY),
Expand All @@ -147,10 +151,11 @@ public JavaCommon(RuleContext ruleContext,
this.javaToolchain = JavaToolchainProvider.from(ruleContext);
this.semantics = semantics;
this.sources = sources;
this.targetsTreatedAsDeps = ImmutableMap.of(
ClasspathType.COMPILE_ONLY, compileDeps,
ClasspathType.RUNTIME_ONLY, runtimeDeps,
ClasspathType.BOTH, bothDeps);
this.targetsTreatedAsDeps =
ImmutableMap.of(
ClasspathType.COMPILE_ONLY, compileDeps,
ClasspathType.RUNTIME_ONLY, runtimeDeps,
ClasspathType.BOTH, bothDeps);
}

public JavaSemantics getJavaSemantics() {
Expand Down Expand Up @@ -242,15 +247,13 @@ public static String javaLibraryPath(
/**
* Collects Java compilation arguments for this target.
*
* @param recursive Whether to scan dependencies recursively.
* @param isNeverLink Whether the target has the 'neverlink' attr.
* @param srcLessDepsExport If srcs is omitted, deps are exported (deprecated behaviour for
* android_library only)
*/
public JavaCompilationArgs collectJavaCompilationArgs(
boolean recursive, boolean isNeverLink, boolean srcLessDepsExport) {
public JavaCompilationArgsProvider collectJavaCompilationArgs(
boolean isNeverLink, boolean srcLessDepsExport) {
return collectJavaCompilationArgs(
/* recursive= */ recursive,
/* isNeverLink= */ isNeverLink,
/* srcLessDepsExport= */ srcLessDepsExport,
getJavaCompilationArtifacts(),
Expand All @@ -262,25 +265,26 @@ public JavaCompilationArgs collectJavaCompilationArgs(
ImmutableList.of(JavaCompilationArgsProvider.legacyFromTargets(getExports(ruleContext))));
}

static JavaCompilationArgs collectJavaCompilationArgs(
boolean recursive,
static JavaCompilationArgsProvider collectJavaCompilationArgs(
boolean isNeverLink,
boolean srcLessDepsExport,
JavaCompilationArtifacts compilationArtifacts,
List<JavaCompilationArgsProvider> deps,
List<JavaCompilationArgsProvider> runtimeDeps,
List<JavaCompilationArgsProvider> exports) {
ClasspathType type = isNeverLink ? ClasspathType.COMPILE_ONLY : ClasspathType.BOTH;
JavaCompilationArgs.Builder builder =
JavaCompilationArgs.builder()
.merge(compilationArtifacts, isNeverLink)
.addTransitiveCompilationArgs(exports, recursive, type);
// TODO(bazel-team): remove srcs-less behaviour after android_library users are refactored
if (recursive || srcLessDepsExport) {
builder
.addTransitiveCompilationArgs(deps, recursive, type)
.addTransitiveCompilationArgs(runtimeDeps, recursive, ClasspathType.RUNTIME_ONLY);
JavaCompilationArgsProvider.Builder builder =
JavaCompilationArgsProvider.builder().merge(compilationArtifacts, isNeverLink);
exports.forEach(export -> builder.addExports(export, type));
if (srcLessDepsExport) {
deps.forEach(dep -> builder.addExports(dep, type));
} else {
deps.forEach(dep -> builder.addDeps(dep, type));
}
runtimeDeps.forEach(dep -> builder.addDeps(dep, ClasspathType.RUNTIME_ONLY));
builder.addCompileTimeJavaDependencyArtifacts(
collectCompileTimeDependencyArtifacts(
compilationArtifacts.getCompileTimeDependencyArtifact(), exports));
return builder.build();
}

Expand All @@ -303,13 +307,15 @@ public NestedSet<Artifact> collectCompileTimeDependencyArtifacts(@Nullable Artif
* @param exports dependencies with export-like semantics
*/
public static NestedSet<Artifact> collectCompileTimeDependencyArtifacts(
@Nullable Artifact jdeps, Iterable<JavaCompilationArgsProvider> exports) {
@Nullable Artifact jdeps, Collection<JavaCompilationArgsProvider> exports) {
NestedSetBuilder<Artifact> builder = NestedSetBuilder.stableOrder();
if (jdeps != null) {
builder.add(jdeps);
}
exports.forEach(
export -> builder.addTransitive(export.getCompileTimeJavaDependencyArtifacts()));
exports
.stream()
.map(JavaCompilationArgsProvider::getCompileTimeJavaDependencyArtifacts)
.forEach(builder::addTransitive);
return builder.build();
}

Expand Down
Loading

0 comments on commit 4a20020

Please sign in to comment.