Add incompatible flag to disable cfg="host" from Starlark
Host transitions are problematic where the host and exec environment are
different (e.g., with Remote Execution). This change adds an incompatible
flag that fails analysis if Starlark rules use `cfg = "host"`.
Migrating is pretty straigt forward (replace `host` with `exec`) and
should be automatable by buildifier (although I haven't tried to do so).
Closes #14383.
PiperOrigin-RevId: 441253060
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkAttrModule.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkAttrModule.java
index 83bc9ce..c10dfa2 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkAttrModule.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkAttrModule.java
@@ -41,6 +41,7 @@
import com.google.devtools.build.lib.packages.Type;
import com.google.devtools.build.lib.packages.Type.ConversionException;
import com.google.devtools.build.lib.packages.Type.LabelClass;
+import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
import com.google.devtools.build.lib.starlarkbuildapi.NativeComputedDefaultApi;
import com.google.devtools.build.lib.starlarkbuildapi.StarlarkAttrModuleApi;
import com.google.devtools.build.lib.util.FileType;
@@ -229,6 +230,15 @@
}
// TODO(b/203203933): Officially deprecate HOST transition and remove this.
if (trans.equals("host")) {
+ boolean disableStarlarkHostTransitions =
+ thread
+ .getSemantics()
+ .getBool(BuildLanguageOptions.INCOMPATIBLE_DISABLE_STARLARK_HOST_TRANSITIONS);
+ if (disableStarlarkHostTransitions) {
+ throw new EvalException(
+ "'cfg = \"host\"' is deprecated and should no longer be used. Please use "
+ + "'cfg = \"exec\"' instead.");
+ }
builder.cfg(ExecutionTransitionFactory.create());
} else if (trans.equals("exec")) {
builder.cfg(ExecutionTransitionFactory.create());
@@ -255,8 +265,8 @@
// android_split_transition because users of those transitions should already know about
// them.
throw Starlark.errorf(
- "cfg must be either 'host', 'target', 'exec' or a starlark defined transition defined"
- + " by the exec() or transition() functions.");
+ "cfg must be either 'target', 'exec' or a starlark defined transition defined by the "
+ + "exec() or transition() functions.");
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java
index 9fe676d..5636dca 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/semantics/BuildLanguageOptions.java
@@ -532,6 +532,17 @@
+ " providers of the aspect.")
public boolean incompatibleTopLevelAspectsRequireProviders;
+ @Option(
+ name = "incompatible_disable_starlark_host_transitions",
+ defaultValue = "false",
+ documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
+ metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
+ effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
+ help =
+ "If set to true, rule attributes cannot set 'cfg = \"host\"'. Rules should set "
+ + "'cfg = \"exec\"' instead.")
+ public boolean incompatibleDisableStarlarkHostTransitions;
+
/**
* An interner to reduce the number of StarlarkSemantics instances. A single Blaze instance should
* never accumulate a large number of these and being able to shortcut on object identity makes a
@@ -599,6 +610,9 @@
.setBool(
INCOMPATIBLE_TOP_LEVEL_ASPECTS_REQUIRE_PROVIDERS,
incompatibleTopLevelAspectsRequireProviders)
+ .setBool(
+ INCOMPATIBLE_DISABLE_STARLARK_HOST_TRANSITIONS,
+ incompatibleDisableStarlarkHostTransitions)
.build();
return INTERNER.intern(semantics);
}
@@ -666,6 +680,8 @@
"-incompatible_visibility_private_attributes_at_definition";
public static final String INCOMPATIBLE_TOP_LEVEL_ASPECTS_REQUIRE_PROVIDERS =
"-incompatible_top_level_aspects_require_providers";
+ public static final String INCOMPATIBLE_DISABLE_STARLARK_HOST_TRANSITIONS =
+ "-incompatible_disable_starlark_host_transitions";
// non-booleans
public static final StarlarkSemantics.Key<String> EXPERIMENTAL_BUILTINS_BZL_PATH =
diff --git a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkAttrModuleApi.java b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkAttrModuleApi.java
index c260340..8e6bb52 100644
--- a/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkAttrModuleApi.java
+++ b/src/main/java/com/google/devtools/build/lib/starlarkbuildapi/StarlarkAttrModuleApi.java
@@ -92,8 +92,13 @@
// TODO(b/151742236): Update when new Starlark-based configuration framework is implemented.
String CONFIGURATION_DOC =
"<a href=\"https://bazel.build/rules/rules#configurations\">"
- + "Configuration</a> of the attribute. It can be either <code>\"host\"</code>, "
- + "<code>\"exec\"</code>, or <code>\"target\"</code>.";
+ + "Configuration</a> of the attribute. It can be either <code>\"exec\"</code>, which "
+ + "indicates that the dependency is built for the <code>execution platform</code>, or "
+ + "<code>\"target\"</code>, which indicates that the dependency is build for the "
+ + "<code>target platform</code>. A typical example of the difference is when building "
+ + "mobile apps, where the <code>target platform</code> is <code>Android</code> or "
+ + "<code>iOS</code> while the <code>execution platform</code> is <code>Linux</code>, "
+ + "<code>macOS</code>, or <code>Windows</code>.";
String DEFAULT_ARG = "default";
// A trailing space is required because it's often prepended to other sentences
diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java
index 88d06f3..da9c01c 100644
--- a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java
@@ -782,6 +782,14 @@
}
@Test
+ public void testAttrCfgHostDisabled() throws Exception {
+ setBuildLanguageOptions("--incompatible_disable_starlark_host_transitions");
+
+ EvalException ex = assertThrows(EvalException.class, () -> ev.eval("attr.label(cfg = 'host')"));
+ assertThat(ex).hasMessageThat().contains("Please use 'cfg = \"exec\"' instead");
+ }
+
+ @Test
public void testAttrCfgTarget() throws Exception {
Attribute attr = buildAttribute("a1", "attr.label(cfg = 'target', allow_files = True)");
assertThat(NoTransition.isInstance(attr.getTransitionFactory())).isTrue();
@@ -791,7 +799,7 @@
public void incompatibleDataTransition() throws Exception {
EvalException expected =
assertThrows(EvalException.class, () -> ev.eval("attr.label(cfg = 'data')"));
- assertThat(expected).hasMessageThat().contains("cfg must be either 'host', 'target'");
+ assertThat(expected).hasMessageThat().contains("cfg must be either 'target', 'exec'");
}
@Test
