Allow ImplicitOutputsFunctions to be overriden on Rule creation.
--
MOS_MIGRATED_REVID=129787305
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Package.java b/src/main/java/com/google/devtools/build/lib/packages/Package.java
index b05e6ae..e3f5954 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Package.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Package.java
@@ -1021,7 +1021,32 @@
RuleClass ruleClass,
Location location,
AttributeContainer attributeContainer) {
- return new Rule(pkg, label, ruleClass, location, attributeContainer);
+ return new Rule(
+ pkg,
+ label,
+ ruleClass,
+ location,
+ attributeContainer);
+ }
+
+ /**
+ * Same to {@link #createRule(Label, RuleClass, Location, AttributeContainer)}, except
+ * allows specifying an {@link ImplicitOutputsFunction} override. Only use if you know what
+ * you're doing.
+ */
+ Rule createRule(
+ Label label,
+ RuleClass ruleClass,
+ Location location,
+ AttributeContainer attributeContainer,
+ ImplicitOutputsFunction implicitOutputsFunction) {
+ return new Rule(
+ pkg,
+ label,
+ ruleClass,
+ location,
+ attributeContainer,
+ implicitOutputsFunction);
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Rule.java b/src/main/java/com/google/devtools/build/lib/packages/Rule.java
index 02971e0..b99e55e 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Rule.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Rule.java
@@ -77,17 +77,40 @@
private final Location location;
+ private final ImplicitOutputsFunction implicitOutputsFunction;
+
// Initialized in the call to populateOutputFiles.
private List<OutputFile> outputFiles;
private ListMultimap<String, OutputFile> outputFileMap;
- Rule(Package pkg, Label label, RuleClass ruleClass, Location location,
+ Rule(
+ Package pkg,
+ Label label,
+ RuleClass ruleClass,
+ Location location,
AttributeContainer attributeContainer) {
+ this(
+ pkg,
+ label,
+ ruleClass,
+ location,
+ attributeContainer,
+ ruleClass.getDefaultImplicitOutputsFunction());
+ }
+
+ Rule(
+ Package pkg,
+ Label label,
+ RuleClass ruleClass,
+ Location location,
+ AttributeContainer attributeContainer,
+ ImplicitOutputsFunction implicitOutputsFunction) {
this.pkg = Preconditions.checkNotNull(pkg);
this.label = label;
this.ruleClass = Preconditions.checkNotNull(ruleClass);
this.location = Preconditions.checkNotNull(location);
this.attributes = attributeContainer;
+ this.implicitOutputsFunction = implicitOutputsFunction;
this.containsErrors = false;
}
@@ -252,6 +275,10 @@
return location;
}
+ public ImplicitOutputsFunction getImplicitOutputsFunction() {
+ return implicitOutputsFunction;
+ }
+
@Override
public Rule getAssociatedRule() {
return this;
@@ -478,7 +505,7 @@
throws InterruptedException {
try {
RawAttributeMapper attributeMap = RawAttributeMapper.of(this);
- for (String out : ruleClass.getImplicitOutputsFunction().getImplicitOutputs(attributeMap)) {
+ for (String out : implicitOutputsFunction.getImplicitOutputs(attributeMap)) {
try {
addOutputFile(pkgBuilder.createLabel(out), eventHandler);
} catch (LabelSyntaxException e) {
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
index 62f8a24..751e6cf 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
@@ -1136,18 +1136,21 @@
}
/**
- * Returns the function which determines the set of implicit outputs
- * generated by a given rule.
+ * Returns the default function for determining the set of implicit outputs generated by a given
+ * rule. If not otherwise specified, this will be the implementation used by {@link Rule}s
+ * created with this {@link RuleClass}.
*
- * <p>An implicit output is an OutputFile that automatically comes into
- * existence when a rule of this class is declared, and whose name is derived
- * from the name of the rule.
+ * <p>Do not use this value to calculate implicit outputs for a rule, instead use
+ * {@link Rule#getImplicitOutputsFunction()}.
*
- * <p>Implicit outputs are a widely-relied upon. All ".so",
- * and "_deploy.jar" targets referenced in BUILD files are examples.
+ * <p>An implicit output is an OutputFile that automatically comes into existence when a rule of
+ * this class is declared, and whose name is derived from the name of the rule.
+ *
+ * <p>Implicit outputs are a widely-relied upon. All ".so", and "_deploy.jar" targets referenced
+ * in BUILD files are examples.
*/
@VisibleForTesting
- public ImplicitOutputsFunction getImplicitOutputsFunction() {
+ public ImplicitOutputsFunction getDefaultImplicitOutputsFunction() {
return implicitOutputsFunction;
}
@@ -1349,9 +1352,15 @@
Label ruleLabel,
AttributeValuesMap attributeValues,
Location location,
- AttributeContainer attributeContainer)
+ AttributeContainer attributeContainer,
+ ImplicitOutputsFunction implicitOutputsFunction)
throws LabelSyntaxException, InterruptedException {
- Rule rule = pkgBuilder.createRule(ruleLabel, this, location, attributeContainer);
+ Rule rule = pkgBuilder.createRule(
+ ruleLabel,
+ this,
+ location,
+ attributeContainer,
+ implicitOutputsFunction);
populateRuleAttributeValues(rule, pkgBuilder, attributeValues, NullEventHandler.INSTANCE);
rule.populateOutputFiles(NullEventHandler.INSTANCE, pkgBuilder);
return rule;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleContext.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleContext.java
index b95554c..994a1cd 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleContext.java
@@ -174,12 +174,11 @@
addOutput(outputsBuilder, "executable", ruleContext.createOutputArtifact());
}
ImplicitOutputsFunction implicitOutputsFunction =
- ruleContext.getRule().getRuleClassObject().getImplicitOutputsFunction();
+ ruleContext.getRule().getImplicitOutputsFunction();
if (implicitOutputsFunction instanceof SkylarkImplicitOutputsFunction) {
SkylarkImplicitOutputsFunction func =
- (SkylarkImplicitOutputsFunction)
- ruleContext.getRule().getRuleClassObject().getImplicitOutputsFunction();
+ (SkylarkImplicitOutputsFunction) implicitOutputsFunction;
for (Map.Entry<String, String> entry :
func.calculateOutputs(RawAttributeMapper.of(ruleContext.getRule())).entrySet()) {
addOutput(
@@ -655,7 +654,7 @@
}
@SkylarkCallable(name = "info_file", structField = true, documented = false,
- doc = "Returns the file that is used to hold the non-volatile workspace status for the "
+ doc = "Returns the file that is used to hold the non-volatile workspace status for the "
+ "current build request.")
public Artifact getStableWorkspaceStatus() {
return ruleContext.getAnalysisEnvironment().getStableWorkspaceStatusArtifact();
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java
index 4ebdb4f..0785b36 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLibrary.java
@@ -131,8 +131,7 @@
// wrt. implicit output files, if the contract says so. Behavior here differs between Bazel
// and Blaze.
.setGenerateLinkActionsIfEmpty(
- ruleContext.getRule().getRuleClassObject().getImplicitOutputsFunction()
- != ImplicitOutputsFunction.NONE)
+ ruleContext.getRule().getImplicitOutputsFunction() != ImplicitOutputsFunction.NONE)
.setLinkType(linkType)
.setNeverLink(neverLink)
.addPrecompiledFiles(precompiledFiles);
@@ -195,9 +194,9 @@
} else if (!createDynamicLibrary
&& ruleContext.attributes().isConfigurable("srcs", BuildType.LABEL_LIST)) {
// If "srcs" is configurable, the .so output is always declared because the logic that
- // determines implicit outs doesn't know which value of "srcs" will ultimately get chosen.
- // Here, where we *do* have the correct value, it may not contain any source files to
- // generate an .so with. If that's the case, register a fake generating action to prevent
+ // determines implicit outs doesn't know which value of "srcs" will ultimately get chosen.
+ // Here, where we *do* have the correct value, it may not contain any source files to
+ // generate an .so with. If that's the case, register a fake generating action to prevent
// a "no generating action for this artifact" error.
Artifact solibArtifact =
CppHelper.getLinuxLinkedArtifact(ruleContext, LinkTargetType.DYNAMIC_LIBRARY);
diff --git a/src/test/java/com/google/devtools/build/lib/pkgcache/TargetPatternEvaluatorTest.java b/src/test/java/com/google/devtools/build/lib/pkgcache/TargetPatternEvaluatorTest.java
index aeb297e..33e736a 100644
--- a/src/test/java/com/google/devtools/build/lib/pkgcache/TargetPatternEvaluatorTest.java
+++ b/src/test/java/com/google/devtools/build/lib/pkgcache/TargetPatternEvaluatorTest.java
@@ -67,7 +67,7 @@
public final void createFiles() throws Exception {
// TODO(ulfjack): Also disable the implicit C++ outputs in Google's internal version.
boolean hasImplicitCcOutputs = ruleClassProvider.getRuleClassMap().get("cc_library")
- .getImplicitOutputsFunction() != ImplicitOutputsFunction.NONE;
+ .getDefaultImplicitOutputsFunction() != ImplicitOutputsFunction.NONE;
scratch.file("BUILD",
"filegroup(name = 'fg', srcs = glob(['*.cc']))");
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java
index 4462417..c4a312f 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java
@@ -446,7 +446,7 @@
}
/**
- * Returns the header module artifacts in {@code input}.
+ * Returns the header module artifacts in {@code input}.
*/
private Iterable<Artifact> getHeaderModules(Iterable<Artifact> input) {
return Iterables.filter(input, new Predicate<Artifact>() {
@@ -456,9 +456,9 @@
}
});
}
-
+
/**
- * Returns the flags in {@code input} that reference a header module.
+ * Returns the flags in {@code input} that reference a header module.
*/
private Iterable<String> getHeaderModuleFlags(Iterable<String> input) {
List<String> names = new ArrayList<>();
@@ -469,7 +469,7 @@
}
return names;
}
-
+
@Test
public void testCompileHeaderModules() throws Exception {
AnalysisMock.get()
@@ -532,7 +532,7 @@
" deps = ['//nomodule:c', '//nomodule:i'],",
")");
scratch.file("nomodule/BUILD",
- "package(features = ['-header_modules'"
+ "package(features = ['-header_modules'"
+ (useHeaderModules ? ", 'use_header_modules'" : "") + "])",
"cc_library(",
" name = 'y',",
@@ -652,7 +652,7 @@
assertThat(getHeaderModuleFlags(jObjectAction.getCompilerOptions()))
.containsExactly("b.pic.pcm", "g.pic.pcm");
}
-
+
@Test
public void testCompileUsingHeaderModulesTransitivelyWithTranstiveModuleMaps() throws Exception {
AnalysisMock.get()
@@ -663,7 +663,7 @@
+ "feature { name: 'transitive_module_maps' }");
useConfiguration("--features=transitive_module_maps");
setupPackagesForModuleTests(/*useHeaderModules=*/true);
-
+
getConfiguredTarget("//nomodule:f");
Artifact fObjectArtifact = getBinArtifact("_objs/f/nomodule/f.pic.o", "//nomodule:f");
CppCompileAction fObjectAction = (CppCompileAction) getGeneratingAction(fObjectArtifact);
@@ -688,7 +688,7 @@
.containsExactly(getBinArtifact("_objs/b/module/b.pic.pcm", "//module:b"));
assertThat(getHeaderModuleFlags(cObjectAction.getCompilerOptions()))
.containsExactly("b.pic.pcm");
-
+
getConfiguredTarget("//nomodule:d");
Artifact dObjectArtifact = getBinArtifact("_objs/d/nomodule/d.pic.o", "//nomodule:d");
CppCompileAction dObjectAction = (CppCompileAction) getGeneratingAction(dObjectArtifact);
@@ -834,7 +834,7 @@
assertThat(artifactsToStrings(getFilesToBuild(hello)))
.doesNotContain("src precompiled/missing.a");
}
-
+
@Test
public void testAllowDuplicateNonCompiledSources() throws Exception {
ConfiguredTarget x =
@@ -974,7 +974,7 @@
" linkopts = ['-shared'],",
")");
}
-
+
private static final String COMPILATION_MODE_FEATURES = ""
+ "feature {"
+ " name: 'dbg'"
@@ -997,7 +997,7 @@
+ " flag_group { flag: '-opt' }"
+ " }"
+ "}";
-
+
private List<String> getCompilationModeFlags(String... flags) throws Exception {
AnalysisMock.get().ccSupport().setupCrosstool(mockToolsConfig, COMPILATION_MODE_FEATURES);
useConfiguration(flags);
@@ -1007,27 +1007,27 @@
CppCompileAction action = (CppCompileAction) getGeneratingAction(objectArtifact);
return action.getCompilerOptions();
}
-
+
@Test
public void testCompilationModeFeatures() throws Exception {
List<String> flags;
flags = getCompilationModeFlags();
assertThat(flags).contains("-fastbuild");
assertThat(flags).containsNoneOf("-opt", "-dbg");
-
+
flags = getCompilationModeFlags("-c", "fastbuild");
assertThat(flags).contains("-fastbuild");
assertThat(flags).containsNoneOf("-opt", "-dbg");
-
+
flags = getCompilationModeFlags("-c", "opt");
assertThat(flags).contains("-opt");
assertThat(flags).containsNoneOf("-fastbuild", "-dbg");
-
+
flags = getCompilationModeFlags("-c", "dbg");
assertThat(flags).contains("-dbg");
assertThat(flags).containsNoneOf("-fastbuild", "-opt");
}
-
+
private List<String> getHostAndTargetFlags(boolean useHost) throws Exception {
AnalysisMock.get()
.ccSupport()
@@ -1068,7 +1068,7 @@
"The include path 'd/../../somewhere' references a path outside of the execution root.",
"cc_library(name='a', srcs=['a.cc'], copts=['-Id/../../somewhere'])");
}
-
+
@Test
public void testAbsoluteIncludePathsOutsideExecutionRoot() throws Exception {
checkError("root", "a",
@@ -1077,14 +1077,14 @@
}
@Test
- public void testSystemIncludePathsOutsideExecutionRoot() throws Exception {
+ public void testSystemIncludePathsOutsideExecutionRoot() throws Exception {
checkError("root", "a",
"The include path '../system' references a path outside of the execution root.",
"cc_library(name='a', srcs=['a.cc'], copts=['-isystem../system'])");
}
@Test
- public void testAbsoluteSystemIncludePathsOutsideExecutionRoot() throws Exception {
+ public void testAbsoluteSystemIncludePathsOutsideExecutionRoot() throws Exception {
checkError("root", "a",
"The include path '/system' references a path outside of the execution root.",
"cc_library(name='a', srcs=['a.cc'], copts=['-isystem/system'])");
@@ -1122,7 +1122,7 @@
ConfiguredTarget target =
scratchConfiguredTarget("a", "b", "cc_library(name = 'b', srcs = ['libb.so'])");
- if (target.getTarget().getAssociatedRule().getRuleClassObject().getImplicitOutputsFunction()
+ if (target.getTarget().getAssociatedRule().getImplicitOutputsFunction()
!= ImplicitOutputsFunction.NONE) {
assertThat(artifactsToStrings(getFilesToBuild(target))).containsExactly("bin a/libb.a");
} else {
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
index 11f7083..9edfe35 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
@@ -288,7 +288,7 @@
Attribute attribute = Iterables.getOnlyElement(aspect.getAttributes());
assertThat(attribute.getName()).isEqualTo("param");
}
-
+
@Test
public void testAspectParameterRequiresValues() throws Exception {
checkErrorContains(
@@ -511,7 +511,7 @@
"def impl(ctx): return None",
"r1 = rule(impl, outputs = {'a': 'a.txt'})");
RuleClass c = ((RuleFunction) lookup("r1")).getRuleClass();
- ImplicitOutputsFunction function = c.getImplicitOutputsFunction();
+ ImplicitOutputsFunction function = c.getDefaultImplicitOutputsFunction();
assertEquals("a.txt", Iterables.getOnlyElement(function.getImplicitOutputs(null)));
}