Don't stamp binaries built in tool configuration
Link-stamping should only be used for binaries built for the target. This change unifies the logic for cc_common.link actions defined in Starlark with the logic used for native rules (in AnalysisUtils.isStampingEnabled), which is otherwise the same.
RELNOTES: Don't stamp cc_common.link actions for tool dependencies.
PiperOrigin-RevId: 438027624
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisUtils.java b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisUtils.java
index 9f79dc3..f77d859 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisUtils.java
@@ -50,6 +50,20 @@
}
/**
+ * Returns whether link stamping is enabled for a TriState stamp attribute.
+ *
+ * <p>This returns false for unstampable rule classes and for rules used to build tools. Otherwise
+ * it returns the value of the stamp attribute, or of the stamp option if the attribute value is
+ * AUTO.
+ */
+ public static boolean isStampingEnabled(TriState stamp, BuildConfigurationValue config) {
+ if (config.isToolConfiguration()) {
+ return false;
+ }
+ return stamp.equals(TriState.YES) || (stamp.equals(TriState.AUTO) && config.stampBinaries());
+ }
+
+ /**
* Returns whether link stamping is enabled for a rule.
*
* <p>This returns false for unstampable rule classes and for rules used to build tools. Otherwise
@@ -57,19 +71,15 @@
* -1.
*/
public static boolean isStampingEnabled(RuleContext ruleContext, BuildConfigurationValue config) {
- if (config.isToolConfiguration()) {
- return false;
- }
- TriState stamp;
if (ruleContext.attributes().has("stamp", BuildType.TRISTATE)) {
- stamp = ruleContext.attributes().get("stamp", BuildType.TRISTATE);
- } else if (ruleContext.attributes().has("stamp", Type.INTEGER)) {
- int value = ruleContext.attributes().get("stamp", Type.INTEGER).toIntUnchecked();
- stamp = TriState.fromInt(value);
- } else {
- return false;
+ TriState stamp = ruleContext.attributes().get("stamp", BuildType.TRISTATE);
+ return isStampingEnabled(stamp, config);
}
- return stamp == TriState.YES || (stamp == TriState.AUTO && config.stampBinaries());
+ if (ruleContext.attributes().has("stamp", Type.INTEGER)) {
+ int stamp = ruleContext.attributes().get("stamp", Type.INTEGER).toIntUnchecked();
+ return isStampingEnabled(TriState.fromInt(stamp), config);
+ }
+ return false;
}
public static boolean isStampingEnabled(RuleContext ruleContext) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java
index 696cb56..04bc3ad 100755
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcModule.java
@@ -25,6 +25,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.analysis.AnalysisUtils;
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
@@ -45,6 +46,7 @@
import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
import com.google.devtools.build.lib.packages.StarlarkInfo;
import com.google.devtools.build.lib.packages.TargetUtils;
+import com.google.devtools.build.lib.packages.TriState;
import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
import com.google.devtools.build.lib.rules.cpp.CcCommon.CoptsFilter;
import com.google.devtools.build.lib.rules.cpp.CcCompilationHelper.CompilationInfo;
@@ -1954,17 +1956,12 @@
private static boolean isStampingEnabled(int stamp, BuildConfigurationValue config)
throws EvalException {
- if (stamp == 0) {
- return false;
- } else if (stamp == 1) {
- return true;
- } else if (stamp == -1) {
- return config.stampBinaries();
- } else {
- throw Starlark.errorf(
- "stamp value %d is not supported, must be 0 (disabled), 1 (enabled), or -1 (default)",
- stamp);
+ if (stamp == 0 || stamp == 1 || stamp == -1) {
+ return AnalysisUtils.isStampingEnabled(TriState.fromInt(stamp), config);
}
+ throw Starlark.errorf(
+ "stamp value %d is not supported, must be 0 (disabled), 1 (enabled), or -1 (default)",
+ stamp);
}
protected Label getCallerLabel(StarlarkActionFactory actions, String name) throws EvalException {
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/AnalysisUtilsTest.java b/src/test/java/com/google/devtools/build/lib/analysis/AnalysisUtilsTest.java
index 634ec364..71b3b61 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/AnalysisUtilsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/AnalysisUtilsTest.java
@@ -19,12 +19,14 @@
import static org.junit.Assert.assertThrows;
import com.google.auto.value.AutoValue;
+import com.google.devtools.build.lib.analysis.util.ConfigurationTestCase;
+import com.google.devtools.build.lib.packages.TriState;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@RunWith(JUnit4.class)
-public class AnalysisUtilsTest {
+public class AnalysisUtilsTest extends ConfigurationTestCase {
@Test
public void checkProviderSucceedsOnClassAnnotatedWithAutoValue() {
@@ -46,4 +48,44 @@
abstract static class AutoValuedClass implements TransitiveInfoProvider {
abstract int foo();
}
+
+ @Test
+ public void stampingEnabled_yesIgnoresNostamp() throws Exception {
+ assertThat(AnalysisUtils.isStampingEnabled(TriState.YES, create("--nostamp"))).isTrue();
+ }
+
+ @Test
+ public void stampingEnabled_yesPlusStamp() throws Exception {
+ assertThat(AnalysisUtils.isStampingEnabled(TriState.YES, create("--stamp"))).isTrue();
+ }
+
+ @Test
+ public void stampingEnabled_noPlusNostamp() throws Exception {
+ assertThat(AnalysisUtils.isStampingEnabled(TriState.NO, create("--nostamp"))).isFalse();
+ }
+
+ @Test
+ public void stampingEnabled_noIgnoresStamp() throws Exception {
+ assertThat(AnalysisUtils.isStampingEnabled(TriState.NO, create("--stamp"))).isFalse();
+ }
+
+ @Test
+ public void stampingEnabled_autoUsesNostamp() throws Exception {
+ assertThat(AnalysisUtils.isStampingEnabled(TriState.AUTO, create("--nostamp"))).isFalse();
+ }
+
+ @Test
+ public void stampingEnabled_autoUsesStamp() throws Exception {
+ assertThat(AnalysisUtils.isStampingEnabled(TriState.AUTO, create("--stamp"))).isTrue();
+ }
+
+ @Test
+ public void stampingEnabled_stampDisabledInToolConfig_attributeYes() throws Exception {
+ assertThat(AnalysisUtils.isStampingEnabled(TriState.YES, createHost("--stamp"))).isFalse();
+ }
+
+ @Test
+ public void stampingEnabled_stampDisabledInToolConfig_attributeAuto() throws Exception {
+ assertThat(AnalysisUtils.isStampingEnabled(TriState.AUTO, createHost("--stamp"))).isFalse();
+ }
}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/BUILD b/src/test/java/com/google/devtools/build/lib/analysis/BUILD
index 6d44863..ab43b53 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/analysis/BUILD
@@ -219,6 +219,8 @@
deps = [
"//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
"//src/main/java/com/google/devtools/build/lib/analysis:transitive_info_provider",
+ "//src/main/java/com/google/devtools/build/lib/packages",
+ "//src/test/java/com/google/devtools/build/lib/analysis/util",
"//third_party:auto_value",
"//third_party:junit4",
"//third_party:truth",
