Skip to content

Commit

Permalink
BEGIN_PUBLIC
Browse files Browse the repository at this point in the history
Add _direct_source_jars output group to rules that have _source_jars output group.

Output group _source_jars provides all transitive source jars, whereas there is a need to provide only the direct source jar.

We're trying to eliminate implicit outputs from Bazel. Implicit output `libNAME-src.jar` provides direct source jar. There are targets that collect direct sources and pack them together into an archive. Currently those targets use an implicit output. A nice substitution for that, that doesn't use implicit output is a `filegroup` target with `output_group` attribute set to `_direct_source_jars`.

We're concerned cleaning up Java rules, but for the sake of consistency and because one cleanup is easier, output group is added also to other Java related rules.
RELNOTES: Added _direct_source_jars output group to Java related targets.
END_PUBLIC
PiperOrigin-RevId: 358346735
  • Loading branch information
comius authored and copybara-github committed Feb 19, 2021
1 parent cf33174 commit f7d5d53
Show file tree
Hide file tree
Showing 9 changed files with 63 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -723,7 +723,10 @@ public RuleConfiguredTargetBuilder addTransitiveInfoProviders(
.addOutputGroup(
OutputGroupInfo.HIDDEN_TOP_LEVEL, collectHiddenTopLevelArtifacts(ruleContext))
.addOutputGroup(
JavaSemantics.SOURCE_JARS_OUTPUT_GROUP, sourceJarsProvider.getTransitiveSourceJars());
JavaSemantics.SOURCE_JARS_OUTPUT_GROUP, sourceJarsProvider.getTransitiveSourceJars())
.addOutputGroup(
JavaSemantics.DIRECT_SOURCE_JARS_OUTPUT_GROUP,
NestedSetBuilder.wrap(Order.STABLE_ORDER, sourceJarsProvider.getSourceJars()));
}

private Runfiles getRunfiles() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,9 @@ public ConfiguredTarget create(RuleContext ruleContext)
new JavaRuntimeClasspathProvider(javaCommon.getRuntimeClasspath()))
.addProvider(JavaPrimaryClassProvider.class, new JavaPrimaryClassProvider(testClass))
.addOutputGroup(JavaSemantics.SOURCE_JARS_OUTPUT_GROUP, transitiveSourceJars)
.addOutputGroup(
JavaSemantics.DIRECT_SOURCE_JARS_OUTPUT_GROUP,
NestedSetBuilder.wrap(Order.STABLE_ORDER, sourceJarsProvider.getSourceJars()))
.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,9 @@ public ConfiguredTarget create(RuleContext ruleContext)
JavaRuntimeClasspathProvider.class,
new JavaRuntimeClasspathProvider(common.getRuntimeClasspath()))
.addOutputGroup(JavaSemantics.SOURCE_JARS_OUTPUT_GROUP, transitiveSourceJars)
.addOutputGroup(
JavaSemantics.DIRECT_SOURCE_JARS_OUTPUT_GROUP,
NestedSetBuilder.wrap(Order.STABLE_ORDER, sourceJarsProvider.getSourceJars()))
.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
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.rules.cpp.LibraryToLink;
import java.util.LinkedHashSet;
import java.util.Set;
Expand Down Expand Up @@ -151,6 +152,9 @@ public ConfiguredTarget create(RuleContext ruleContext)
.addNativeDeclaredProvider(new JavaNativeLibraryInfo(transitiveJavaNativeLibraries))
.addNativeDeclaredProvider(new ProguardSpecProvider(proguardSpecs))
.addOutputGroup(JavaSemantics.SOURCE_JARS_OUTPUT_GROUP, transitiveJavaSourceJars)
.addOutputGroup(
JavaSemantics.DIRECT_SOURCE_JARS_OUTPUT_GROUP,
NestedSetBuilder.wrap(Order.STABLE_ORDER, sourceJarsProvider.getSourceJars()))
.addOutputGroup(OutputGroupInfo.HIDDEN_TOP_LEVEL, proguardSpecs)
.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import com.google.devtools.build.lib.analysis.RunfilesProvider;
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.cpp.LibraryToLink;
import com.google.devtools.build.lib.rules.java.JavaCompilationArgsProvider.ClasspathType;
import com.google.devtools.build.lib.rules.java.proto.GeneratedExtensionRegistryProvider;
Expand Down Expand Up @@ -205,6 +206,9 @@ final ConfiguredTarget init(
.addNativeDeclaredProvider(new ProguardSpecProvider(proguardSpecs))
.addNativeDeclaredProvider(javaInfo)
.addOutputGroup(JavaSemantics.SOURCE_JARS_OUTPUT_GROUP, transitiveSourceJars)
.addOutputGroup(
JavaSemantics.DIRECT_SOURCE_JARS_OUTPUT_GROUP,
NestedSetBuilder.wrap(Order.STABLE_ORDER, sourceJarsProvider.getSourceJars()))
.addOutputGroup(OutputGroupInfo.HIDDEN_TOP_LEVEL, proguardSpecs);

Artifact validation =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,13 @@ static Label javaToolchainAttribute(RuleDefinitionEnvironment environment) {
return environment.getToolsLabel("//tools/jdk:current_java_toolchain");
}

/** Name of the output group used for source jars. */
/** Name of the output group used for transitive source jars. */
String SOURCE_JARS_OUTPUT_GROUP = OutputGroupInfo.HIDDEN_OUTPUT_GROUP_PREFIX + "source_jars";

/** Name of the output group used for direct source jars. */
String DIRECT_SOURCE_JARS_OUTPUT_GROUP =
OutputGroupInfo.HIDDEN_OUTPUT_GROUP_PREFIX + "direct_source_jars";

/** Implementation for the :jvm attribute. */
static Label jvmAttribute(RuleDefinitionEnvironment env) {
return env.getToolsLabel("//tools/jdk:current_java_runtime");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4482,12 +4482,20 @@ public void starlarkJavaInfoToAndroidBinaryAttributes() throws Exception {
// TODO(b/75051107): Remove the following line when fixed.
" incremental_dexing = 0,",
")");

// Test that all bottom jars are on the runtime classpath of the app.
ConfiguredTarget target = getConfiguredTarget("//java/r/android:foo_app");

ImmutableList<Artifact> transitiveSrcJars =
OutputGroupInfo.get(target).getOutputGroup(JavaSemantics.SOURCE_JARS_OUTPUT_GROUP).toList();
assertThat(ActionsTestUtil.baseArtifactNames(transitiveSrcJars))
.containsExactly("libal_bottom_for_deps-src.jar", "libfoo_app-src.jar");
ImmutableList<Artifact> directSrcJar =
OutputGroupInfo.get(target)
.getOutputGroup(JavaSemantics.DIRECT_SOURCE_JARS_OUTPUT_GROUP)
.toList();
assertThat(ActionsTestUtil.baseArtifactNames(directSrcJar))
.containsExactly("libfoo_app-src.jar");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2530,15 +2530,23 @@ public void starlarkJavaInfoToAndroidLibraryAttributes() throws Exception {
" deps = [':mya'],",
" exports = [':myb'],",
")");

// Test that all bottom jars are on the runtime classpath of lib_android.
ConfiguredTarget target = getConfiguredTarget("//foo:lib_foo");

ImmutableList<Artifact> transitiveSrcJars =
OutputGroupInfo.get(target).getOutputGroup(JavaSemantics.SOURCE_JARS_OUTPUT_GROUP).toList();
assertThat(ActionsTestUtil.baseArtifactNames(transitiveSrcJars))
.containsExactly(
"libjl_bottom_for_exports-src.jar",
"libal_bottom_for_deps-src.jar",
"liblib_foo-src.jar");
ImmutableList<Artifact> directSrcJars =
OutputGroupInfo.get(target)
.getOutputGroup(JavaSemantics.DIRECT_SOURCE_JARS_OUTPUT_GROUP)
.toList();
assertThat(ActionsTestUtil.baseArtifactNames(directSrcJars))
.containsExactly("liblib_foo-src.jar");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,20 +151,39 @@ public void testGlobMatchesRuleOutputsInsteadOfFileWithTheSameName() throws Exce
}

@Test
public void testOutputGroupExtractsCorrectArtifacts() throws Exception {
public void outputGroupSourceJars_extractsTransitiveSources() throws Exception {
scratch.file("pkg/a.java");
scratch.file("pkg/b.java");
scratch.file("pkg/in_ouput_group_a");
scratch.file("pkg/in_ouput_group_b");

scratch.file("pkg/c.java");
scratch.file(
"pkg/BUILD",
"java_library(name='lib_a', srcs=['a.java'])",
"java_library(name='lib_b', srcs=['b.java'])",
"java_library(name='lib_b', srcs=['b.java'], deps = [':lib_c'])",
"java_library(name='lib_c', srcs=['c.java'])",
"filegroup(name='group', srcs=[':lib_a', ':lib_b'],"
+ String.format("output_group='%s')", JavaSemantics.SOURCE_JARS_OUTPUT_GROUP));

ConfiguredTarget group = getConfiguredTarget("//pkg:group");

assertThat(ActionsTestUtil.prettyArtifactNames(getFilesToBuild(group)))
.containsExactly("pkg/liblib_a-src.jar", "pkg/liblib_b-src.jar", "pkg/liblib_c-src.jar");
}

@Test
public void outputGroupDirectSourceJars_extractsDirectSources() throws Exception {
scratch.file("pkg/a.java");
scratch.file("pkg/b.java");
scratch.file("pkg/c.java");
scratch.file(
"pkg/BUILD",
"java_library(name='lib_a', srcs=['a.java'])",
"java_library(name='lib_b', srcs=['b.java'], deps = [':lib_c'])",
"java_library(name='lib_c', srcs=['c.java'])",
"filegroup(name='group', srcs=[':lib_a', ':lib_b'],"
+ String.format("output_group='%s')", JavaSemantics.DIRECT_SOURCE_JARS_OUTPUT_GROUP));

ConfiguredTarget group = getConfiguredTarget("//pkg:group");

assertThat(ActionsTestUtil.prettyArtifactNames(getFilesToBuild(group)))
.containsExactly("pkg/liblib_a-src.jar", "pkg/liblib_b-src.jar");
}
Expand Down

0 comments on commit f7d5d53

Please sign in to comment.