Automated rollback of commit 34f20a181c0bf2c3857eeaab3f5b6031757afbcf.
*** Reason for rollback ***
See unknown commit
*** Original change description ***
Replace HOST transition with EXEC transition for Starlark rules.
PiperOrigin-RevId: 429580659
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 71ea571..4aa082c 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
@@ -19,6 +19,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.analysis.config.ExecutionTransitionFactory;
+import com.google.devtools.build.lib.analysis.config.HostTransition;
import com.google.devtools.build.lib.analysis.config.StarlarkDefinedConfigTransition;
import com.google.devtools.build.lib.analysis.config.TransitionFactories;
import com.google.devtools.build.lib.analysis.config.transitions.SplitTransition;
@@ -227,9 +228,8 @@
throw Starlark.errorf(
"late-bound attributes must not have a split configuration transition");
}
- // TODO(b/203203933): Officially deprecate HOST transition and remove this.
if (trans.equals("host")) {
- builder.cfg(ExecutionTransitionFactory.create());
+ builder.cfg(HostTransition.createFactory());
} else if (trans.equals("exec")) {
builder.cfg(ExecutionTransitionFactory.create());
} else if (trans instanceof ExecutionTransitionFactory) {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/genrule/BUILD b/src/main/java/com/google/devtools/build/lib/rules/genrule/BUILD
index d82f963..f3f2864 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/genrule/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/rules/genrule/BUILD
@@ -18,6 +18,7 @@
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
"//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
"//src/main/java/com/google/devtools/build/lib/analysis:config/execution_transition_factory",
+ "//src/main/java/com/google/devtools/build/lib/analysis:config/host_transition",
"//src/main/java/com/google/devtools/build/lib/analysis:configured_target",
"//src/main/java/com/google/devtools/build/lib/analysis:file_provider",
"//src/main/java/com/google/devtools/build/lib/analysis:make_variable_supplier",
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/test/TrimTestConfigurationTest.java b/src/test/java/com/google/devtools/build/lib/analysis/test/TrimTestConfigurationTest.java
index 9ec1c08..080d835 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/test/TrimTestConfigurationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/test/TrimTestConfigurationTest.java
@@ -38,7 +38,7 @@
import com.google.devtools.build.lib.analysis.RunfilesSupport;
import com.google.devtools.build.lib.analysis.ViewCreationFailedException;
import com.google.devtools.build.lib.analysis.actions.FileWriteAction;
-import com.google.devtools.build.lib.analysis.config.ExecutionTransitionFactory;
+import com.google.devtools.build.lib.analysis.config.HostTransition;
import com.google.devtools.build.lib.analysis.util.AnalysisTestCase;
import com.google.devtools.build.lib.analysis.util.MockRule;
import com.google.devtools.build.lib.cmdline.Label;
@@ -92,7 +92,7 @@
"native_test",
attr("deps", LABEL_LIST).allowedFileTypes(),
attr("host_deps", LABEL_LIST)
- .cfg(ExecutionTransitionFactory.create())
+ .cfg(HostTransition.createFactory())
.allowedFileTypes());
private static final RuleDefinition NATIVE_LIB_RULE =
@@ -103,7 +103,7 @@
"native_lib",
attr("deps", LABEL_LIST).allowedFileTypes(),
attr("host_deps", LABEL_LIST)
- .cfg(ExecutionTransitionFactory.create())
+ .cfg(HostTransition.createFactory())
.allowedFileTypes());
@Before
diff --git a/src/test/java/com/google/devtools/build/lib/packages/AttributeTest.java b/src/test/java/com/google/devtools/build/lib/packages/AttributeTest.java
index 49f8832..f244286 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/AttributeTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/AttributeTest.java
@@ -26,7 +26,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.analysis.config.BuildOptionsView;
-import com.google.devtools.build.lib.analysis.config.ExecutionTransitionFactory;
+import com.google.devtools.build.lib.analysis.config.HostTransition;
import com.google.devtools.build.lib.analysis.config.TransitionFactories;
import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransition;
import com.google.devtools.build.lib.analysis.config.transitions.SplitTransition;
@@ -102,12 +102,11 @@
Attribute.Builder<String> builder =
attr("x", STRING)
.mandatory()
- .cfg(ExecutionTransitionFactory.create())
+ .cfg(HostTransition.createFactory())
.undocumented("")
.value("y");
assertThrows(IllegalStateException.class, () -> builder.mandatory());
- assertThrows(
- IllegalStateException.class, () -> builder.cfg(ExecutionTransitionFactory.create()));
+ assertThrows(IllegalStateException.class, () -> builder.cfg(HostTransition.createFactory()));
assertThrows(IllegalStateException.class, () -> builder.undocumented(""));
assertThrows(IllegalStateException.class, () -> builder.value("z"));
@@ -280,8 +279,8 @@
@Test
public void testHostTransition() throws Exception {
Attribute attr =
- attr("foo", LABEL).cfg(ExecutionTransitionFactory.create()).allowedFileTypes().build();
- assertThat(attr.getTransitionFactory().isTool()).isTrue();
+ attr("foo", LABEL).cfg(HostTransition.createFactory()).allowedFileTypes().build();
+ assertThat(attr.getTransitionFactory().isHost()).isTrue();
assertThat(attr.getTransitionFactory().isSplit()).isFalse();
}
diff --git a/src/test/java/com/google/devtools/build/lib/packages/BUILD b/src/test/java/com/google/devtools/build/lib/packages/BUILD
index 4f392fd..f1b8830 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/packages/BUILD
@@ -56,8 +56,8 @@
"//src/main/java/com/google/devtools/build/lib/actions:thread_state_receiver",
"//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
"//src/main/java/com/google/devtools/build/lib/analysis:config/build_options",
- "//src/main/java/com/google/devtools/build/lib/analysis:config/execution_transition_factory",
"//src/main/java/com/google/devtools/build/lib/analysis:config/fragment",
+ "//src/main/java/com/google/devtools/build/lib/analysis:config/host_transition",
"//src/main/java/com/google/devtools/build/lib/analysis:config/transition_factories",
"//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/configuration_transition",
"//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/no_transition",
diff --git a/src/test/java/com/google/devtools/build/lib/query2/cquery/BUILD b/src/test/java/com/google/devtools/build/lib/query2/cquery/BUILD
index 53d584a..a98cf2f 100644
--- a/src/test/java/com/google/devtools/build/lib/query2/cquery/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/query2/cquery/BUILD
@@ -59,6 +59,7 @@
"//src/main/java/com/google/devtools/build/lib/analysis:config/build_options",
"//src/main/java/com/google/devtools/build/lib/analysis:config/execution_transition_factory",
"//src/main/java/com/google/devtools/build/lib/analysis:config/fragment_options",
+ "//src/main/java/com/google/devtools/build/lib/analysis:config/host_transition",
"//src/main/java/com/google/devtools/build/lib/analysis:config/transition_factories",
"//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/patch_transition",
"//src/main/java/com/google/devtools/build/lib/analysis:test/test_configuration",
diff --git a/src/test/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQuerySemanticsTest.java b/src/test/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQuerySemanticsTest.java
index 8fc102d..1c85a23 100644
--- a/src/test/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQuerySemanticsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQuerySemanticsTest.java
@@ -28,6 +28,7 @@
import com.google.devtools.build.lib.analysis.config.BuildOptionsView;
import com.google.devtools.build.lib.analysis.config.ExecutionTransitionFactory;
import com.google.devtools.build.lib.analysis.config.FragmentOptions;
+import com.google.devtools.build.lib.analysis.config.HostTransition;
import com.google.devtools.build.lib.analysis.config.TransitionFactories;
import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition;
import com.google.devtools.build.lib.analysis.test.TestConfiguration.TestOptions;
@@ -207,7 +208,7 @@
"rule_with_host_dep",
attr("host_dep", LABEL)
.allowedFileTypes(FileTypeSet.ANY_FILE)
- .cfg(ExecutionTransitionFactory.create()),
+ .cfg(HostTransition.createFactory()),
attr("$impl_dep", LABEL)
.allowedFileTypes(FileTypeSet.ANY_FILE)
.value(Label.parseAbsoluteUnchecked("//test:other")));
@@ -265,7 +266,7 @@
attr("target", LABEL).allowedFileTypes(FileTypeSet.ANY_FILE),
attr("host", LABEL)
.allowedFileTypes(FileTypeSet.ANY_FILE)
- .cfg(ExecutionTransitionFactory.create()),
+ .cfg(HostTransition.createFactory()),
attr("exec", LABEL)
.allowedFileTypes(FileTypeSet.ANY_FILE)
.cfg(ExecutionTransitionFactory.create()),
@@ -360,7 +361,7 @@
}
@Test
- public void testConfig_noMoreHostTransition() throws Exception {
+ public void testConfig_hostTransition() throws Exception {
createConfigRulesAndBuild();
getHelper().setWholeTestUniverseScope("test:my_rule");
@@ -370,21 +371,20 @@
.isEqualTo("No target (in) //test:target_dep could be found in the 'host' configuration");
assertConfigurableQueryCode(
targetResult.getFailureDetail(), ConfigurableQuery.Code.TARGET_MISSING);
- EvalThrowsResult hostDepResult = evalThrows("config(//test:host_dep, host)", true);
- assertThat(hostDepResult.getMessage())
- .isEqualTo("No target (in) //test:host_dep could be found in the 'host' configuration");
- assertConfigurableQueryCode(
- hostDepResult.getFailureDetail(), ConfigurableQuery.Code.TARGET_MISSING);
- EvalThrowsResult execDepResult = evalThrows("config(//test:exec_dep, host)", true);
- assertThat(execDepResult.getMessage())
+ assertThat(eval("config(//test:host_dep, host)")).isEqualTo(eval("//test:host_dep"));
+ EvalThrowsResult hostResult = evalThrows("config(//test:exec_dep, host)", true);
+ assertThat(hostResult.getMessage())
.isEqualTo("No target (in) //test:exec_dep could be found in the 'host' configuration");
assertConfigurableQueryCode(
- execDepResult.getFailureDetail(), ConfigurableQuery.Code.TARGET_MISSING);
- EvalThrowsResult hostResult = evalThrows("config(//test:dep, host)", true);
- assertThat(hostResult.getMessage())
- .isEqualTo("No target (in) //test:dep could be found in the 'host' configuration");
- assertConfigurableQueryCode(
hostResult.getFailureDetail(), ConfigurableQuery.Code.TARGET_MISSING);
+
+ BuildConfigurationValue configuration =
+ getConfiguration(Iterables.getOnlyElement(eval("config(//test:dep, host)")));
+
+ assertThat(configuration).isNotNull();
+ assertThat(configuration.isHostConfiguration()).isTrue();
+ assertThat(configuration.isExecConfiguration()).isFalse();
+ assertThat(configuration.isToolConfiguration()).isTrue();
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/query2/cquery/GraphOutputFormatterCallbackTest.java b/src/test/java/com/google/devtools/build/lib/query2/cquery/GraphOutputFormatterCallbackTest.java
index f10fc44..0d96445 100644
--- a/src/test/java/com/google/devtools/build/lib/query2/cquery/GraphOutputFormatterCallbackTest.java
+++ b/src/test/java/com/google/devtools/build/lib/query2/cquery/GraphOutputFormatterCallbackTest.java
@@ -147,7 +147,6 @@
"}"));
}
- // TODO(b/203203933): Replace "host" with "exec" throughout this test.
@Test
public void nullAndHostDeps() throws Exception {
writeFile(
@@ -160,8 +159,6 @@
List<String> output = getOutput("deps(//test:a)");
String firstNode = output.get(2);
String configHash = firstNode.substring(firstNode.indexOf("(") + 1, firstNode.length() - 2);
- String hostNode = output.get(6);
- String execConfigHash = hostNode.substring(hostNode.indexOf("(") + 1, hostNode.length() - 2);
assertThat(getOutput("deps(//test:a)"))
.isEqualTo(
withConfigHash(
@@ -171,8 +168,8 @@
" \"//test:a (%s)\"",
" \"//test:a (%s)\" -> \"//test:b (%s)\"",
" \"//test:a (%s)\" -> \"//test:file.src (null)\"",
- " \"//test:a (%s)\" -> \"//test:host_dep (" + execConfigHash + ")\"",
- " \"//test:host_dep (" + execConfigHash + ")\"",
+ " \"//test:a (%s)\" -> \"//test:host_dep (HOST)\"",
+ " \"//test:host_dep (HOST)\"",
" \"//test:file.src (null)\"",
" \"//test:b (%s)\"",
"}"));
diff --git a/src/test/java/com/google/devtools/build/lib/rules/config/FeatureFlagManualTrimmingTest.java b/src/test/java/com/google/devtools/build/lib/rules/config/FeatureFlagManualTrimmingTest.java
index 89d75a6..e29964a 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/config/FeatureFlagManualTrimmingTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/config/FeatureFlagManualTrimmingTest.java
@@ -698,6 +698,41 @@
}
@Test
+ public void noDistinctHostConfiguration_DoesNotResultInActionConflicts() throws Exception {
+ scratch.file(
+ "test/BUILD",
+ "load(':host_transition.bzl', 'host_transition')",
+ "load(':read_flags.bzl', 'read_flags')",
+ "feature_flag_setter(",
+ " name = 'target',",
+ " deps = [':host', ':reader'],",
+ ")",
+ "host_transition(",
+ " name = 'host',",
+ " srcs = [':reader'],",
+ ")",
+ "read_flags(",
+ " name = 'reader',",
+ " flags = [],",
+ ")");
+
+ enableManualTrimmingAnd("--nodistinct_host_configuration");
+ ConfiguredTarget target = getConfiguredTarget("//test:target");
+ assertNoEvents();
+ // Note that '//test:reader' is accessed (and creates actions) in both the host and target
+ // configurations. If these are different but output to the same path (as was the case before
+ // --nodistinct_host_configuration caused --enforce_transitive_configs_for_config_feature_flag
+ // to become a no-op), then this causes action conflicts, as described in b/117932061 (for which
+ // this test is a regression test).
+ assertThat(getFilesToBuild(target).toList()).hasSize(1);
+ // Action conflict detection is not enabled for these tests. However, the action conflict comes
+ // from the outputs of the two configurations of //test:reader being unequal artifacts;
+ // hence, this test checks that the nested set of artifacts reachable from //test:target only
+ // contains one artifact, that is, they were deduplicated for being equal.
+ }
+
+
+ @Test
public void noDistinctHostConfiguration_DisablesEnforcementForBothHostAndTargetConfigs()
throws Exception {
scratch.file(
diff --git a/src/test/java/com/google/devtools/build/lib/rules/java/ProguardLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/java/ProguardLibraryTest.java
index 235e99e..f448c7b 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/java/ProguardLibraryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/java/ProguardLibraryTest.java
@@ -35,7 +35,7 @@
import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
import com.google.devtools.build.lib.analysis.RunfilesProvider;
import com.google.devtools.build.lib.analysis.actions.SpawnAction;
-import com.google.devtools.build.lib.analysis.config.ExecutionTransitionFactory;
+import com.google.devtools.build.lib.analysis.config.HostTransition;
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.RepositoryName;
@@ -45,6 +45,7 @@
import com.google.devtools.build.lib.packages.NativeAspectClass;
import com.google.devtools.build.lib.packages.RuleClass;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData;
+import com.google.devtools.build.lib.testutil.TestConstants;
import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
import com.google.devtools.build.lib.util.FileTypeSet;
import java.util.List;
@@ -69,11 +70,11 @@
.add(attr("runtime_deps", LABEL_LIST).allowedFileTypes(FileTypeSet.NO_FILE))
.add(
attr("plugins", LABEL_LIST)
- .cfg(ExecutionTransitionFactory.create())
+ .cfg(HostTransition.createFactory())
.allowedFileTypes(FileTypeSet.NO_FILE))
.add(
attr("exported_plugins", LABEL_LIST)
- .cfg(ExecutionTransitionFactory.create())
+ .cfg(HostTransition.createFactory())
.allowedFileTypes(FileTypeSet.NO_FILE))
.build();
}
@@ -110,7 +111,7 @@
.add(attr("target_libs", LABEL_LIST).allowedFileTypes(FileTypeSet.NO_FILE))
.add(
attr("host_libs", LABEL_LIST)
- .cfg(ExecutionTransitionFactory.create())
+ .cfg(HostTransition.createFactory())
.allowedFileTypes(FileTypeSet.NO_FILE))
.add(attr("target_attrs", STRING_LIST))
.add(attr("host_attrs", STRING_LIST))
@@ -119,7 +120,7 @@
.value(Label.parseAbsoluteUnchecked("//test/implicit:implicit_target")))
.add(
attr("$implicit_host", LABEL)
- .cfg(ExecutionTransitionFactory.create())
+ .cfg(HostTransition.createFactory())
.value(Label.parseAbsoluteUnchecked("//test/implicit:implicit_host")))
.build();
}
@@ -282,7 +283,7 @@
.map(path -> path.replaceFirst(TestConstants.PRODUCT_NAME + "-out/[^/]+/", ""))
.collect(Collectors.toList());
List<String> expectedFilesToRun =
- getFilesToRun(getConfiguredTarget(TestConstants.PROGUARD_ALLOWLISTER_TARGET))
+ getFilesToRun(getHostConfiguredTarget(TestConstants.PROGUARD_ALLOWLISTER_TARGET))
.toList()
.stream()
.map(Artifact::getExecPathString)
@@ -347,11 +348,11 @@
Artifact validatedPlugin =
getBinArtifact(
"validated_proguard/plugin/test/plugin.cfg_valid",
- getDirectPrerequisite(target, "//test:plugin"));
+ getHostConfiguredTarget("//test:plugin"));
Artifact validatedExportedPlugin =
getBinArtifact(
"validated_proguard/exported_plugin/test/exported_plugin.cfg_valid",
- getDirectPrerequisite(target, "//test:exported_plugin"));
+ getHostConfiguredTarget("//test:exported_plugin"));
assertThat(getFilesToBuild(target).toList())
.containsExactly(
@@ -382,8 +383,7 @@
getConfiguredTarget("//test:target"));
Artifact validatedHost =
getBinArtifact(
- "validated_proguard/host/test/host.cfg_valid",
- getDirectPrerequisite(target, "//test:host"));
+ "validated_proguard/host/test/host.cfg_valid", getHostConfiguredTarget("//test:host"));
Artifact validatedImplicitTarget =
getBinArtifact(
"validated_proguard/implicit_target/test/implicit/implicit_target.cfg_valid",
@@ -391,7 +391,7 @@
Artifact validatedImplicitHost =
getBinArtifact(
"validated_proguard/implicit_host/test/implicit/implicit_host.cfg_valid",
- getDirectPrerequisite(target, "//test/implicit:implicit_host"));
+ getHostConfiguredTarget("//test/implicit:implicit_host"));
assertThat(getFilesToBuild(target).toList())
.containsExactly(
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 68e26e8..89e4f66 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
@@ -759,10 +759,9 @@
}
@Test
- public void testAttrCfgNoMoreHost() throws Exception {
+ public void testAttrCfg() throws Exception {
Attribute attr = buildAttribute("a1", "attr.label(cfg = 'host', allow_files = True)");
- assertThat(attr.getTransitionFactory().isHost()).isFalse();
- assertThat(attr.getTransitionFactory().isTool()).isTrue();
+ assertThat(attr.getTransitionFactory().isHost()).isTrue();
}
@Test
diff --git a/src/test/shell/integration/configured_query_test.sh b/src/test/shell/integration/configured_query_test.sh
index ff34224..7f7e625 100755
--- a/src/test/shell/integration/configured_query_test.sh
+++ b/src/test/shell/integration/configured_query_test.sh
@@ -194,6 +194,16 @@
assert_not_equals $HOST_CONFIG $TARGET_CONFIG
}
+function test_host_config_output() {
+ local -r pkg=$FUNCNAME
+ write_test_targets $pkg
+
+ bazel cquery //$pkg:host --universe_scope=//$pkg:main \
+ > output 2>"$TEST_log" || fail "Excepted success"
+
+ assert_contains "//$pkg:host (HOST)" output
+}
+
function test_transitions_lite() {
local -r pkg=$FUNCNAME
write_test_targets $pkg
@@ -202,7 +212,7 @@
> output 2>"$TEST_log" || fail "Excepted success"
assert_contains "//$pkg:main" output
- assert_contains "host_dep#//$pkg:host#(exec + (TestTrimmingTransition + ConfigFeatureFlagTaggedTrimmingTransition))" output
+ assert_contains "host_dep#//$pkg:host#HostTransition" output
}
@@ -214,7 +224,7 @@
> output 2>"$TEST_log" || fail "Excepted success"
assert_contains "//$pkg:main" output
- assert_contains "host_dep#//$pkg:host#(exec + (TestTrimmingTransition + ConfigFeatureFlagTaggedTrimmingTransition))" output
+ assert_contains "host_dep#//$pkg:host#HostTransition" output
}
function write_test_targets() {
