Tags propagation: added incompatible flag
reverted rollback of tags propagation: https://github.com/bazelbuild/bazel/commit/29eecb56c5f0f26b4751ec9eb79954412fb2991c
Tags declared on targets are not propagated to actions and therefore are not taken into consideration by bazel. This causes some issues, for instance, target marked with a tag 'no-remote' will still be executed remotely.
As it was agreed in the design doc (see #7766 for a link), set of tags to be propagated to actions as a first iteration.
This change is responsible for that first step for the Starlark Rules.
RELNOTES: tags 'no-remote', 'no-cache', 'no-remote-cache', 'no-remote-exec', 'no-sandbox' are propagated now to the actions from targets when '--ncompatible_allow_tags_propagation' flag set to true. See #8830.
Closes #7766
Related to #8830
Closes #8829.
PiperOrigin-RevId: 258521443
diff --git a/src/main/java/com/google/devtools/build/lib/actions/ExecutionRequirements.java b/src/main/java/com/google/devtools/build/lib/actions/ExecutionRequirements.java
index e632839..330dffa 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/ExecutionRequirements.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/ExecutionRequirements.java
@@ -18,11 +18,23 @@
import com.google.common.base.Function;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableMap;
+import com.google.devtools.build.lib.packages.Rule;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
/**
* Strings used to express requirements on action execution environments.
+ *
+ * <ol>
+ * If you are adding a new execution requirement, pay attention to the following:
+ * <li>If its name starts with one of the supported prefixes, then it can be also used as a tag on
+ * a target and will be propagated to the execution requirements, see for prefixes {@link
+ * com.google.devtools.build.lib.packages.TargetUtils#getExecutionInfo(Rule)}
+ * <li>If this is a potentially conflicting execution requirements, e.g. you are adding a pair
+ * 'requires-x' and 'block-x', you MUST take care of a potential conflict in the Executor that
+ * is using new execution requirements. As an example, see {@link
+ * Spawns#requiresNetwork(com.google.devtools.build.lib.actions.Spawn, boolean)}.
+ * </ol>
*/
public class ExecutionRequirements {
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkActionFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkActionFactory.java
index 29b7e5b..16dea75 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkActionFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkActionFactory.java
@@ -558,15 +558,14 @@
if (EvalUtils.toBoolean(useDefaultShellEnv)) {
builder.useDefaultShellEnvironment();
}
- if (executionRequirementsUnchecked != Runtime.NONE) {
- builder.setExecutionInfo(
- TargetUtils.filter(
- SkylarkDict.castSkylarkDictOrNoneToDict(
- executionRequirementsUnchecked,
- String.class,
- String.class,
- "execution_requirements")));
- }
+
+ ImmutableMap<String, String> executionInfo =
+ TargetUtils.getFilteredExecutionInfo(
+ executionRequirementsUnchecked,
+ ruleContext.getRule(),
+ starlarkSemantics.incompatibleAllowTagsPropagation());
+ builder.setExecutionInfo(executionInfo);
+
if (inputManifestsUnchecked != Runtime.NONE) {
for (RunfilesSupplier supplier : SkylarkList.castSkylarkListOrNoneToList(
inputManifestsUnchecked, RunfilesSupplier.class, "runfiles suppliers")) {
diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
index 868697d..73879e3 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
@@ -177,6 +177,21 @@
public boolean incompatibleBzlDisallowLoadAfterStatement;
@Option(
+ name = "incompatible_allow_tags_propagation",
+ defaultValue = "false",
+ documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
+ effectTags = {OptionEffectTag.BUILD_FILE_SEMANTICS},
+ metadataTags = {
+ OptionMetadataTag.INCOMPATIBLE_CHANGE,
+ OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
+ },
+ help =
+ "If set to true, tags will be propagated from a target to the actions' execution"
+ + " requirements; otherwise tags are not propagated. See"
+ + " https://github.com/bazelbuild/bazel/issues/8830 for details.")
+ public boolean incompatibleAllowTagsPropagation;
+
+ @Option(
name = "incompatible_depset_union",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
@@ -697,6 +712,7 @@
.incompatibleDisallowSplitEmptySeparator(incompatibleDisallowSplitEmptySeparator)
.incompatibleDisallowDictLookupUnhashableKeys(
incompatibleDisallowDictLookupUnhashableKeys)
+ .incompatibleAllowTagsPropagation(incompatibleAllowTagsPropagation)
.build();
return INTERNER.intern(semantics);
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/TargetUtils.java b/src/main/java/com/google/devtools/build/lib/packages/TargetUtils.java
index acb0352..3ab123a 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/TargetUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/TargetUtils.java
@@ -24,6 +24,8 @@
import com.google.common.collect.Maps;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.events.Location;
+import com.google.devtools.build.lib.syntax.EvalException;
+import com.google.devtools.build.lib.syntax.SkylarkDict;
import com.google.devtools.build.lib.syntax.Type;
import com.google.devtools.build.lib.util.Pair;
import java.util.ArrayList;
@@ -46,18 +48,15 @@
// some internal tags that we don't allow to be set on targets. We also don't want to
// exhaustively enumerate all the legal values here. Right now, only a ~small set of tags is
// recognized by Bazel.
- private static final Predicate<String> LEGAL_EXEC_INFO_KEYS = new Predicate<String>() {
- @Override
- public boolean apply(String tag) {
- return tag.startsWith("block-")
- || tag.startsWith("requires-")
- || tag.startsWith("no-")
- || tag.startsWith("supports-")
- || tag.startsWith("disable-")
- || tag.equals("local")
- || tag.startsWith("cpu:");
- }
- };
+ private static boolean legalExecInfoKeys(String tag) {
+ return tag.startsWith("block-")
+ || tag.startsWith("requires-")
+ || tag.startsWith("no-")
+ || tag.startsWith("supports-")
+ || tag.startsWith("disable-")
+ || tag.equals("local")
+ || tag.startsWith("cpu:");
+ }
private TargetUtils() {} // Uninstantiable.
@@ -220,19 +219,15 @@
}
/**
- * Returns the execution info. These include execution requirement tags ('block-*', 'requires-*',
- * 'no-*', 'supports-*', 'disable-*', 'local', and 'cpu:*') as keys with empty values.
+ * Returns the execution info from the tags declared on the target. These include only some tags
+ * {@link #legalExecInfoKeys} as keys with empty values.
*/
public static Map<String, String> getExecutionInfo(Rule rule) {
// tags may contain duplicate values.
Map<String, String> map = new HashMap<>();
for (String tag :
NonconfigurableAttributeMapper.of(rule).get(CONSTRAINTS_ATTR, Type.STRING_LIST)) {
- // We don't want to pollute the execution info with random things, and we also need to reserve
- // some internal tags that we don't allow to be set on targets. We also don't want to
- // exhaustively enumerate all the legal values here. Right now, only a ~small set of tags is
- // recognized by Bazel.
- if (LEGAL_EXEC_INFO_KEYS.apply(tag)) {
+ if (legalExecInfoKeys(tag)) {
map.put(tag, "");
}
}
@@ -240,11 +235,48 @@
}
/**
+ * Returns the execution info, obtained from the rule's tags and the execution requirements
+ * provided. Only supported tags are included into the execution info, see {@link
+ * #legalExecInfoKeys}.
+ *
+ * @param executionRequirementsUnchecked execution_requirements of a rule, expected to be of a
+ * {@code SkylarkDict<String, String>} type, null or {@link
+ * com.google.devtools.build.lib.syntax.Runtime#NONE}
+ * @param rule a rule instance to get tags from
+ * @param incompatibleAllowTagsPropagation if set to true, tags will be propagated from a target
+ * to the actions' execution requirements, for more details {@see
+ * SkylarkSematicOptions#incompatibleAllowTagsPropagation}
+ */
+ public static ImmutableMap<String, String> getFilteredExecutionInfo(
+ Object executionRequirementsUnchecked, Rule rule, boolean incompatibleAllowTagsPropagation)
+ throws EvalException {
+ Map<String, String> checkedExecutionRequirements =
+ TargetUtils.filter(
+ SkylarkDict.castSkylarkDictOrNoneToDict(
+ executionRequirementsUnchecked,
+ String.class,
+ String.class,
+ "execution_requirements"));
+
+ Map<String, String> executionInfoBuilder = new HashMap<>();
+ // adding filtered execution requirements to the execution info map
+ executionInfoBuilder.putAll(checkedExecutionRequirements);
+
+ if (incompatibleAllowTagsPropagation) {
+ Map<String, String> checkedTags = getExecutionInfo(rule);
+ // merging filtered tags to the execution info map avoiding duplicates
+ checkedTags.forEach(executionInfoBuilder::putIfAbsent);
+ }
+
+ return ImmutableMap.copyOf(executionInfoBuilder);
+ }
+
+ /**
* Returns the execution info. These include execution requirement tags ('block-*', 'requires-*',
* 'no-*', 'supports-*', 'disable-*', 'local', and 'cpu:*') as keys with empty values.
*/
public static Map<String, String> filter(Map<String, String> executionInfo) {
- return Maps.filterKeys(executionInfo, LEGAL_EXEC_INFO_KEYS);
+ return Maps.filterKeys(executionInfo, TargetUtils::legalExecInfoKeys);
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
index 87b8c5f..f2181f4 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
@@ -60,6 +60,7 @@
INCOMPATIBLE_OBJC_FRAMEWORK_CLEANUP(StarlarkSemantics::incompatibleObjcFrameworkCleanup),
INCOMPATIBLE_DISALLOW_RULE_EXECUTION_PLATFORM_CONSTRAINTS_ALLOWED(
StarlarkSemantics::incompatibleDisallowRuleExecutionPlatformConstraintsAllowed),
+ INCOMPATIBLE_ALLOW_TAGS_PROPAGATION(StarlarkSemantics::incompatibleAllowTagsPropagation),
NONE(null);
// Using a Function here makes the enum definitions far cleaner, and, since this is
@@ -210,6 +211,8 @@
public abstract boolean incompatibleDisallowDictLookupUnhashableKeys();
+ public abstract boolean incompatibleAllowTagsPropagation();
+
@Memoized
@Override
public abstract int hashCode();
@@ -287,6 +290,7 @@
.incompatibleRestrictStringEscapes(false)
.incompatibleDisallowSplitEmptySeparator(false)
.incompatibleDisallowDictLookupUnhashableKeys(false)
+ .incompatibleAllowTagsPropagation(false)
.build();
/** Builder for {@link StarlarkSemantics}. All fields are mandatory. */
@@ -312,6 +316,8 @@
public abstract Builder experimentalStarlarkUnusedInputsList(boolean value);
+ public abstract Builder incompatibleAllowTagsPropagation(boolean value);
+
public abstract Builder incompatibleBzlDisallowLoadAfterStatement(boolean value);
public abstract Builder incompatibleDepsetIsNotIterable(boolean value);
