diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/BazelGenRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/BazelGenRule.java index 97edfaae574cca..91a600fb6db7fc 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/BazelGenRule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/BazelGenRule.java @@ -18,25 +18,15 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.AliasProvider; import com.google.devtools.build.lib.analysis.FileProvider; -import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSet; -import com.google.devtools.build.lib.packages.Type; import com.google.devtools.build.lib.rules.genrule.GenRuleBase; import java.util.List; /** An implementation of genrule for Bazel. */ public final class BazelGenRule extends GenRuleBase { - @Override - protected boolean isStampingEnabled(RuleContext ruleContext) { - if (!ruleContext.attributes().has("stamp", Type.BOOLEAN)) { - return false; - } - return ruleContext.attributes().get("stamp", Type.BOOLEAN); - } - @Override protected ImmutableMap> collectSources( List srcs) { diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/BazelGenRuleRule.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/BazelGenRuleRule.java index 685925c287ff7c..24cbe28c20ba4e 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/BazelGenRuleRule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/genrule/BazelGenRuleRule.java @@ -15,19 +15,20 @@ import static com.google.devtools.build.lib.packages.Attribute.attr; import static com.google.devtools.build.lib.packages.BuildType.LABEL; -import static com.google.devtools.build.lib.packages.Type.BOOLEAN; +import static com.google.devtools.build.lib.packages.BuildType.TRISTATE; import com.google.devtools.build.lib.analysis.RuleDefinition; import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment; import com.google.devtools.build.lib.analysis.config.ExecutionTransitionFactory; import com.google.devtools.build.lib.packages.RuleClass; +import com.google.devtools.build.lib.packages.TriState; import com.google.devtools.build.lib.rules.genrule.GenRuleBaseRule; /** * Rule definition for genrule for Bazel. */ public final class BazelGenRuleRule implements RuleDefinition { - public static final String GENRULE_SETUP_LABEL = "//tools/genrule:genrule-setup.sh"; + private static final String GENRULE_SETUP_LABEL = "//tools/genrule:genrule-setup.sh"; @Override public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env) { @@ -43,9 +44,7 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env) attr("$genrule_setup", LABEL) .cfg(ExecutionTransitionFactory.createFactory()) .value(env.getToolsLabel(GENRULE_SETUP_LABEL))) - - // TODO(bazel-team): stamping doesn't seem to work. Fix it or remove attribute. - .add(attr("stamp", BOOLEAN).value(false)) + .add(attr("stamp", TRISTATE).value(TriState.NO)) .build(); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java index af965dc1a0905c..02cbab7f138e33 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java +++ b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java @@ -43,7 +43,9 @@ 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.AttributeMap; +import com.google.devtools.build.lib.packages.BuildType; import com.google.devtools.build.lib.packages.TargetUtils; +import com.google.devtools.build.lib.packages.TriState; import com.google.devtools.build.lib.packages.Type; import com.google.devtools.build.lib.util.FileTypeSet; import com.google.devtools.build.lib.util.OnDemandString; @@ -56,7 +58,7 @@ /** * A base implementation of genrule, to be used by specific implementing rules which can change the - * semantics of {@link #isStampingEnabled} and {@link #collectSources}. + * semantics of {@link #collectSources}. */ public abstract class GenRuleBase implements RuleConfiguredTargetFactory { @@ -227,20 +229,20 @@ public String toString() { .build(); } - /** - * Returns {@code true} if the rule should be stamped. - * - *

Genrule implementations can set this based on the rule context, including by defining their - * own attributes over and above what is present in {@link GenRuleBaseRule}. - */ - @ForOverride - protected abstract boolean isStampingEnabled(RuleContext ruleContext); - /** Collects sources from src attribute. */ @ForOverride protected abstract ImmutableMap> collectSources( List srcs) throws RuleErrorException; + private static boolean isStampingEnabled(RuleContext ruleContext) { + // This intentionally does not call AnalysisUtils.isStampingEnabled(). That method returns false + // in the exec configuration (regardless of the attribute value), which is the behavior for + // binaries, but not genrules. + TriState stamp = ruleContext.attributes().get("stamp", BuildType.TRISTATE); + return stamp == TriState.YES + || (stamp == TriState.AUTO && ruleContext.getConfiguration().stampBinaries()); + } + private enum CommandType { BASH, WINDOWS_BATCH, diff --git a/src/test/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleConfiguredTargetTest.java b/src/test/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleConfiguredTargetTest.java index 5a14522976a023..b44289e8a4f54f 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleConfiguredTargetTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/rules/genrule/GenRuleConfiguredTargetTest.java @@ -530,6 +530,7 @@ private void createStampingTargets() throws Exception { "u/BUILD", "genrule(name='foo_stamp', srcs=[], outs=['uu'], stamp=1, cmd='')", "genrule(name='foo_nostamp', srcs=[], outs=['vv'], stamp=0, cmd='')", + "genrule(name='foo_autostamp', srcs=[], outs=['aa'], stamp=-1, cmd='')", "genrule(name='foo_default', srcs=[], outs=['xx'], cmd='')"); } @@ -562,6 +563,8 @@ public void testStampingWithNoStamp() throws Exception { assertStamped(getExecConfiguredTarget("//u:foo_stamp")); assertNotStamped("//u:foo_nostamp"); assertNotStamped(getExecConfiguredTarget("//u:foo_nostamp")); + assertNotStamped("//u:foo_autostamp"); + assertNotStamped(getExecConfiguredTarget("//u:foo_autostamp")); assertNotStamped("//u:foo_default"); } @@ -571,8 +574,10 @@ public void testStampingWithStamp() throws Exception { createStampingTargets(); assertStamped("//u:foo_stamp"); assertStamped(getExecConfiguredTarget("//u:foo_stamp")); - // assertStamped("//u:foo_nostamp"); + assertNotStamped("//u:foo_nostamp"); assertNotStamped(getExecConfiguredTarget("//u:foo_nostamp")); + assertStamped("//u:foo_autostamp"); + assertNotStamped(getExecConfiguredTarget("//u:foo_autostamp")); assertNotStamped("//u:foo_default"); }