Filegroups can optionally export sources from a given output group. -- MOS_MIGRATED_REVID=125562946
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/OutputGroupProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/OutputGroupProvider.java index e600550..113fa3f 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/OutputGroupProvider.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/OutputGroupProvider.java
@@ -53,24 +53,31 @@ public static final String HIDDEN_OUTPUT_GROUP_PREFIX = "_"; /** + * Suffix for output groups that are internal to bazel and may not be referenced from a filegroup. + */ + public static final String INTERNAL_SUFFIX = "_INTERNAL_"; + + /** * Building these artifacts only results in the compilation (and not e.g. linking) of the * associated target. Mostly useful for C++, less so for e.g. Java. */ - public static final String FILES_TO_COMPILE = "files_to_compile"; + public static final String FILES_TO_COMPILE = "files_to_compile" + INTERNAL_SUFFIX; /** * These artifacts are the direct requirements for compilation, but building these does not * actually compile the target. Mostly useful when IDEs want Blaze to emit generated code so that * they can do the compilation in their own way. */ - public static final String COMPILATION_PREREQUISITES = "compilation_prerequisites"; + public static final String COMPILATION_PREREQUISITES = + "compilation_prerequisites" + INTERNAL_SUFFIX; /** * These files are built when a target is mentioned on the command line, but are not reported to * the user. This is mostly runfiles, which is necessary because we don't want a target to * successfully build if a file in its runfiles is broken. */ - public static final String HIDDEN_TOP_LEVEL = HIDDEN_OUTPUT_GROUP_PREFIX + "hidden_top_level"; + public static final String HIDDEN_TOP_LEVEL = + HIDDEN_OUTPUT_GROUP_PREFIX + "hidden_top_level" + INTERNAL_SUFFIX; /** * Temporary files created during building a rule, for example, .i, .d and .s files for C++ @@ -81,7 +88,7 @@ * not creating the associated actions and artifacts if we don't need them or just historical * baggage. */ - public static final String TEMP_FILES = "temp_files"; + public static final String TEMP_FILES = "temp_files" + INTERNAL_SUFFIX; /** * The default group of files built by a target when it is mentioned on the command line.
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/common/BazelFilegroupRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/common/BazelFilegroupRule.java index cc49cab..d2c1588 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/common/BazelFilegroupRule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/common/BazelFilegroupRule.java
@@ -46,6 +46,15 @@ </p> <!-- #END_BLAZE_RULE.ATTRIBUTE -->*/ .add(attr("srcs", LABEL_LIST).allowedFileTypes(FileTypeSet.ANY_FILE)) + /*<!-- #BLAZE_RULE(filegroup).ATTRIBUTE(output_group) --> + The output group from which to gather artifacts from sources. If this attribute is + specified, artifacts from the specified output group of the dependencies will be exported + instead of the default output group. + <p>An "output group" is a category of output artifacts of a target, specified in that + rule's implementation. + </p> + <!-- #END_BLAZE_RULE.ATTRIBUTE -->*/ + .add(attr("output_group", STRING)) /*<!-- #BLAZE_RULE(filegroup).ATTRIBUTE(data) --> The list of files needed by this rule at runtime. <p> @@ -67,8 +76,10 @@ <code>filegroup</code> to find the name of the directory holding the files. </p> <!-- #END_BLAZE_RULE.ATTRIBUTE -->*/ - .add(attr("path", STRING) - .undocumented("only used to expose FilegroupPathProvider, which is not currently used")) + .add( + attr("path", STRING) + .undocumented( + "only used to expose FilegroupPathProvider, which is not currently used")) .build(); }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java index e876d2f..b074379 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidBinary.java
@@ -15,6 +15,7 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Strings.isNullOrEmpty; +import static com.google.devtools.build.lib.analysis.OutputGroupProvider.INTERNAL_SUFFIX; import com.google.common.base.Function; import com.google.common.base.Optional; @@ -718,8 +719,8 @@ RunfilesProvider.class, RunfilesProvider.simple( new Runfiles.Builder( - ruleContext.getWorkspaceName(), - ruleContext.getConfiguration().legacyExternalRunfiles()) + ruleContext.getWorkspaceName(), + ruleContext.getConfiguration().legacyExternalRunfiles()) .addRunfiles(ruleContext, RunfilesProvider.DEFAULT_RUNFILES) .addTransitiveArtifacts(filesToBuild) .build())) @@ -731,12 +732,12 @@ new ApkProvider( NestedSetBuilder.create(Order.STABLE_ORDER, zipAlignedApk), coverageMetadata, - NestedSetBuilder.create(Order.STABLE_ORDER, applicationManifest.getManifest()) - )) + NestedSetBuilder.create(Order.STABLE_ORDER, applicationManifest.getManifest()))) .add(AndroidPreDexJarProvider.class, new AndroidPreDexJarProvider(jarToDex)) - .addOutputGroup("mobile_install_full", fullInstallOutputGroup) - .addOutputGroup("mobile_install_incremental", incrementalInstallOutputGroup) - .addOutputGroup("mobile_install_split", splitInstallOutputGroup) + .addOutputGroup("mobile_install_full" + INTERNAL_SUFFIX, fullInstallOutputGroup) + .addOutputGroup( + "mobile_install_incremental" + INTERNAL_SUFFIX, incrementalInstallOutputGroup) + .addOutputGroup("mobile_install_split" + INTERNAL_SUFFIX, splitInstallOutputGroup) .addOutputGroup("apk_manifest", apkManifest) .addOutputGroup("apk_manifest_text", apkManifestText) .addOutputGroup("android_deploy_info", deployInfo);
diff --git a/src/main/java/com/google/devtools/build/lib/rules/filegroup/Filegroup.java b/src/main/java/com/google/devtools/build/lib/rules/filegroup/Filegroup.java index cbbc0fc..92aac6f 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/filegroup/Filegroup.java +++ b/src/main/java/com/google/devtools/build/lib/rules/filegroup/Filegroup.java
@@ -14,20 +14,23 @@ package com.google.devtools.build.lib.rules.filegroup; +import static com.google.devtools.build.lib.analysis.OutputGroupProvider.INTERNAL_SUFFIX; + import com.google.devtools.build.lib.actions.Actions; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.CompilationHelper; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.MiddlemanProvider; +import com.google.devtools.build.lib.analysis.OutputGroupProvider; import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode; import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.Runfiles; import com.google.devtools.build.lib.analysis.RunfilesProvider; +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.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException; import com.google.devtools.build.lib.rules.RuleConfiguredTargetFactory; import com.google.devtools.build.lib.rules.test.InstrumentedFilesCollector; import com.google.devtools.build.lib.rules.test.InstrumentedFilesCollector.InstrumentationSpec; @@ -37,18 +40,37 @@ import com.google.devtools.build.lib.vfs.PathFragment; import java.util.Iterator; +import java.util.List; /** * ConfiguredTarget for "filegroup". */ public class Filegroup implements RuleConfiguredTargetFactory { + /** Error message for output groups that are explicitly blacklisted for filegroup reference. */ + public static final String ILLEGAL_OUTPUT_GROUP_ERROR = + "Output group %s is not permitted for " + "reference in filegroups."; + @Override public ConfiguredTarget create(RuleContext ruleContext) throws RuleErrorException { - NestedSet<Artifact> filesToBuild = NestedSetBuilder.wrap(Order.STABLE_ORDER, - ruleContext.getPrerequisiteArtifacts("srcs", Mode.TARGET).list()); - NestedSet<Artifact> middleman = CompilationHelper.getAggregatingMiddleman( - ruleContext, Actions.escapeLabel(ruleContext.getLabel()), filesToBuild); + String outputGroupName = ruleContext.attributes().get("output_group", Type.STRING); + + if (outputGroupName.endsWith(INTERNAL_SUFFIX)) { + ruleContext.throwWithAttributeError( + "output_group", String.format(ILLEGAL_OUTPUT_GROUP_ERROR, outputGroupName)); + } + + NestedSet<Artifact> filesToBuild = + outputGroupName.isEmpty() + ? NestedSetBuilder.wrap( + Order.STABLE_ORDER, + ruleContext.getPrerequisiteArtifacts("srcs", Mode.TARGET).list()) + : getArtifactsForOutputGroup( + outputGroupName, ruleContext.getPrerequisites("srcs", Mode.TARGET)); + + NestedSet<Artifact> middleman = + CompilationHelper.getAggregatingMiddleman( + ruleContext, Actions.escapeLabel(ruleContext.getLabel()), filesToBuild); InstrumentedFilesProvider instrumentedFilesProvider = InstrumentedFilesCollector.collect(ruleContext, @@ -102,4 +124,19 @@ return ruleContext.getLabel().getPackageIdentifier().getPathFragment().getRelative(attr); } } + + /** Returns the artifacts from the given targets that are members of the given output group. */ + private static NestedSet<Artifact> getArtifactsForOutputGroup( + String outputGroupName, List<? extends TransitiveInfoCollection> deps) { + NestedSetBuilder<Artifact> result = NestedSetBuilder.stableOrder(); + + for (TransitiveInfoCollection dep : deps) { + OutputGroupProvider outputGroupProvider = dep.getProvider(OutputGroupProvider.class); + if (outputGroupProvider != null) { + result.addTransitive(outputGroupProvider.getOutputGroup(outputGroupName)); + } + } + + return result.build(); + } }
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/MobileInstallCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/MobileInstallCommand.java index 67ad56b..c936673 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/commands/MobileInstallCommand.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/MobileInstallCommand.java
@@ -13,6 +13,8 @@ // limitations under the License. package com.google.devtools.build.lib.runtime.commands; +import static com.google.devtools.build.lib.analysis.OutputGroupProvider.INTERNAL_SUFFIX; + import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.buildtool.BuildRequest; import com.google.devtools.build.lib.buildtool.BuildTool; @@ -88,9 +90,11 @@ throws AbruptExitException { try { String outputGroup = - optionsParser.getOptions(Options.class).splitApks ? "mobile_install_split" - : optionsParser.getOptions(Options.class).incremental ? "mobile_install_incremental" - : "mobile_install_full"; + optionsParser.getOptions(Options.class).splitApks + ? "mobile_install_split" + INTERNAL_SUFFIX + : optionsParser.getOptions(Options.class).incremental + ? "mobile_install_incremental" + INTERNAL_SUFFIX + : "mobile_install_full" + INTERNAL_SUFFIX; optionsParser.parse(OptionPriority.COMMAND_LINE, "Options required by the mobile-install command",
diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD index b675d77..09631c3 100644 --- a/src/test/java/com/google/devtools/build/lib/BUILD +++ b/src/test/java/com/google/devtools/build/lib/BUILD
@@ -767,6 +767,7 @@ "//src/main/java/com/google/devtools/build/lib:bazel-main", "//src/main/java/com/google/devtools/build/lib:bazel-rules", "//src/main/java/com/google/devtools/build/lib:build-base", + "//src/main/java/com/google/devtools/build/lib:java-rules", "//src/main/java/com/google/devtools/build/lib:util", "//src/main/java/com/google/devtools/build/lib:vfs", "//third_party:guava",
diff --git a/src/test/java/com/google/devtools/build/lib/rules/filegroup/FilegroupConfiguredTargetTest.java b/src/test/java/com/google/devtools/build/lib/rules/filegroup/FilegroupConfiguredTargetTest.java index f70a2fc..a466c95 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/filegroup/FilegroupConfiguredTargetTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/filegroup/FilegroupConfiguredTargetTest.java
@@ -16,11 +16,14 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; +import static org.junit.Assert.fail; import com.google.devtools.build.lib.actions.util.ActionsTestUtil; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.FileConfiguredTarget; +import com.google.devtools.build.lib.analysis.OutputGroupProvider; import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; +import com.google.devtools.build.lib.rules.java.JavaSemantics; import com.google.devtools.build.lib.util.FileType; import org.junit.Test; @@ -145,4 +148,43 @@ "filegroup(name = 'my_rule', srcs = glob(['file_or_rule']))"); assertThat(ActionsTestUtil.baseArtifactNames(getFilesToBuild(target))).containsExactly("a.txt"); } + + @Test + public void testOutputGroupExtractsCorrectArtifacts() 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/BUILD", + "java_library(name='lib_a', srcs=['a.java'])", + "java_library(name='lib_b', srcs=['b.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"); + } + + @Test + public void testErrorForIllegalOutputGroup() throws Exception { + scratch.file("pkg/a.cc"); + scratch.file( + "pkg/BUILD", + "cc_library(name='lib_a', srcs=['a.cc'])", + String.format( + "filegroup(name='group', srcs=[':lib_a'], output_group='%s')", + OutputGroupProvider.HIDDEN_TOP_LEVEL)); + try { + getConfiguredTarget("//pkg:group"); + fail("Should throw AssertionError"); + } catch (AssertionError e) { + assertThat(e.getMessage()) + .contains( + String.format( + Filegroup.ILLEGAL_OUTPUT_GROUP_ERROR, OutputGroupProvider.HIDDEN_TOP_LEVEL)); + } + } }
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkAspectsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkAspectsTest.java index 3604865..8677771 100644 --- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkAspectsTest.java +++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkAspectsTest.java
@@ -16,6 +16,7 @@ import static com.google.common.collect.Iterables.transform; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertWithMessage; +import static com.google.devtools.build.lib.analysis.OutputGroupProvider.INTERNAL_SUFFIX; import static org.junit.Assert.fail; import com.google.common.base.Function; @@ -36,8 +37,8 @@ import com.google.devtools.build.lib.rules.java.Jvm; import com.google.devtools.build.lib.skyframe.AspectValue; import com.google.devtools.build.lib.syntax.SkylarkNestedSet; - import com.google.devtools.build.lib.vfs.FileSystemUtils; + import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -254,7 +255,7 @@ scratch.file( "test/aspect.bzl", "def _impl(target, ctx):", - " f = target.output_group('_hidden_top_level')", + " f = target.output_group('_hidden_top_level" + INTERNAL_SUFFIX + "')", " return struct(output_groups = { 'my_result' : f })", "", "MyAspect = aspect(",