Skip to content

Commit 34f20a1

Browse files
susinmotioncopybara-github
susinmotion
authored and
copybara-github
committed
Replace HOST transition with EXEC transition for Starlark rules.
PiperOrigin-RevId: 428808204
1 parent 8d3ff3c commit 34f20a1

File tree

13 files changed

+50
-90
lines changed

13 files changed

+50
-90
lines changed

‎src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkAttrModule.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import com.google.common.collect.ImmutableMap;
2020
import com.google.common.collect.ImmutableSet;
2121
import com.google.devtools.build.lib.analysis.config.ExecutionTransitionFactory;
22-
import com.google.devtools.build.lib.analysis.config.HostTransition;
2322
import com.google.devtools.build.lib.analysis.config.StarlarkDefinedConfigTransition;
2423
import com.google.devtools.build.lib.analysis.config.TransitionFactories;
2524
import com.google.devtools.build.lib.analysis.config.transitions.SplitTransition;
@@ -231,8 +230,9 @@ && containsNonNoneKey(arguments, ALLOW_SINGLE_FILE_ARG)) {
231230
throw Starlark.errorf(
232231
"late-bound attributes must not have a split configuration transition");
233232
}
233+
// TODO(b/203203933): Officially deprecate HOST transition and remove this.
234234
if (trans.equals("host")) {
235-
builder.cfg(HostTransition.createFactory());
235+
builder.cfg(ExecutionTransitionFactory.create());
236236
} else if (trans.equals("exec")) {
237237
builder.cfg(ExecutionTransitionFactory.create());
238238
} else if (trans instanceof ExecutionTransitionFactory) {

‎src/main/java/com/google/devtools/build/lib/rules/genrule/BUILD

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ java_library(
1818
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
1919
"//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
2020
"//src/main/java/com/google/devtools/build/lib/analysis:config/execution_transition_factory",
21-
"//src/main/java/com/google/devtools/build/lib/analysis:config/host_transition",
2221
"//src/main/java/com/google/devtools/build/lib/analysis:configured_target",
2322
"//src/main/java/com/google/devtools/build/lib/analysis:file_provider",
2423
"//src/main/java/com/google/devtools/build/lib/analysis:make_variable_supplier",

‎src/test/java/com/google/devtools/build/lib/analysis/test/TrimTestConfigurationTest.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838
import com.google.devtools.build.lib.analysis.RunfilesSupport;
3939
import com.google.devtools.build.lib.analysis.ViewCreationFailedException;
4040
import com.google.devtools.build.lib.analysis.actions.FileWriteAction;
41-
import com.google.devtools.build.lib.analysis.config.HostTransition;
41+
import com.google.devtools.build.lib.analysis.config.ExecutionTransitionFactory;
4242
import com.google.devtools.build.lib.analysis.util.AnalysisTestCase;
4343
import com.google.devtools.build.lib.analysis.util.MockRule;
4444
import com.google.devtools.build.lib.cmdline.Label;
@@ -92,7 +92,7 @@ public ConfiguredTarget create(RuleContext context)
9292
"native_test",
9393
attr("deps", LABEL_LIST).allowedFileTypes(),
9494
attr("host_deps", LABEL_LIST)
95-
.cfg(HostTransition.createFactory())
95+
.cfg(ExecutionTransitionFactory.create())
9696
.allowedFileTypes());
9797

9898
private static final RuleDefinition NATIVE_LIB_RULE =
@@ -103,7 +103,7 @@ public ConfiguredTarget create(RuleContext context)
103103
"native_lib",
104104
attr("deps", LABEL_LIST).allowedFileTypes(),
105105
attr("host_deps", LABEL_LIST)
106-
.cfg(HostTransition.createFactory())
106+
.cfg(ExecutionTransitionFactory.create())
107107
.allowedFileTypes());
108108

109109
@Before

‎src/test/java/com/google/devtools/build/lib/packages/AttributeTest.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
import com.google.common.collect.ImmutableMap;
2727
import com.google.devtools.build.lib.analysis.config.BuildOptions;
2828
import com.google.devtools.build.lib.analysis.config.BuildOptionsView;
29-
import com.google.devtools.build.lib.analysis.config.HostTransition;
29+
import com.google.devtools.build.lib.analysis.config.ExecutionTransitionFactory;
3030
import com.google.devtools.build.lib.analysis.config.TransitionFactories;
3131
import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransition;
3232
import com.google.devtools.build.lib.analysis.config.transitions.SplitTransition;
@@ -102,11 +102,12 @@ public void testDoublePropertySet() {
102102
Attribute.Builder<String> builder =
103103
attr("x", STRING)
104104
.mandatory()
105-
.cfg(HostTransition.createFactory())
105+
.cfg(ExecutionTransitionFactory.create())
106106
.undocumented("")
107107
.value("y");
108108
assertThrows(IllegalStateException.class, () -> builder.mandatory());
109-
assertThrows(IllegalStateException.class, () -> builder.cfg(HostTransition.createFactory()));
109+
assertThrows(
110+
IllegalStateException.class, () -> builder.cfg(ExecutionTransitionFactory.create()));
110111
assertThrows(IllegalStateException.class, () -> builder.undocumented(""));
111112
assertThrows(IllegalStateException.class, () -> builder.value("z"));
112113

@@ -279,8 +280,8 @@ public void testSplitTransitionProvider() throws Exception {
279280
@Test
280281
public void testHostTransition() throws Exception {
281282
Attribute attr =
282-
attr("foo", LABEL).cfg(HostTransition.createFactory()).allowedFileTypes().build();
283-
assertThat(attr.getTransitionFactory().isHost()).isTrue();
283+
attr("foo", LABEL).cfg(ExecutionTransitionFactory.create()).allowedFileTypes().build();
284+
assertThat(attr.getTransitionFactory().isTool()).isTrue();
284285
assertThat(attr.getTransitionFactory().isSplit()).isFalse();
285286
}
286287

‎src/test/java/com/google/devtools/build/lib/packages/BUILD

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,8 @@ java_test(
5656
"//src/main/java/com/google/devtools/build/lib/actions:thread_state_receiver",
5757
"//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
5858
"//src/main/java/com/google/devtools/build/lib/analysis:config/build_options",
59+
"//src/main/java/com/google/devtools/build/lib/analysis:config/execution_transition_factory",
5960
"//src/main/java/com/google/devtools/build/lib/analysis:config/fragment",
60-
"//src/main/java/com/google/devtools/build/lib/analysis:config/host_transition",
6161
"//src/main/java/com/google/devtools/build/lib/analysis:config/transition_factories",
6262
"//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/configuration_transition",
6363
"//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/no_transition",

‎src/test/java/com/google/devtools/build/lib/query2/cquery/BUILD

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,6 @@ java_test(
5959
"//src/main/java/com/google/devtools/build/lib/analysis:config/build_options",
6060
"//src/main/java/com/google/devtools/build/lib/analysis:config/execution_transition_factory",
6161
"//src/main/java/com/google/devtools/build/lib/analysis:config/fragment_options",
62-
"//src/main/java/com/google/devtools/build/lib/analysis:config/host_transition",
6362
"//src/main/java/com/google/devtools/build/lib/analysis:config/transition_factories",
6463
"//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/patch_transition",
6564
"//src/main/java/com/google/devtools/build/lib/analysis:test/test_configuration",

‎src/test/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQuerySemanticsTest.java

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
import com.google.devtools.build.lib.analysis.config.BuildOptionsView;
2929
import com.google.devtools.build.lib.analysis.config.ExecutionTransitionFactory;
3030
import com.google.devtools.build.lib.analysis.config.FragmentOptions;
31-
import com.google.devtools.build.lib.analysis.config.HostTransition;
3231
import com.google.devtools.build.lib.analysis.config.TransitionFactories;
3332
import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition;
3433
import com.google.devtools.build.lib.analysis.test.TestConfiguration.TestOptions;
@@ -208,7 +207,7 @@ public void testAlias_filtering() throws Exception {
208207
"rule_with_host_dep",
209208
attr("host_dep", LABEL)
210209
.allowedFileTypes(FileTypeSet.ANY_FILE)
211-
.cfg(HostTransition.createFactory()),
210+
.cfg(ExecutionTransitionFactory.create()),
212211
attr("$impl_dep", LABEL)
213212
.allowedFileTypes(FileTypeSet.ANY_FILE)
214213
.value(Label.parseAbsoluteUnchecked("//test:other")));
@@ -266,7 +265,7 @@ private void createConfigRulesAndBuild() throws Exception {
266265
attr("target", LABEL).allowedFileTypes(FileTypeSet.ANY_FILE),
267266
attr("host", LABEL)
268267
.allowedFileTypes(FileTypeSet.ANY_FILE)
269-
.cfg(HostTransition.createFactory()),
268+
.cfg(ExecutionTransitionFactory.create()),
270269
attr("exec", LABEL)
271270
.allowedFileTypes(FileTypeSet.ANY_FILE)
272271
.cfg(ExecutionTransitionFactory.create()),
@@ -361,7 +360,7 @@ public void testConfig_target() throws Exception {
361360
}
362361

363362
@Test
364-
public void testConfig_hostTransition() throws Exception {
363+
public void testConfig_noMoreHostTransition() throws Exception {
365364
createConfigRulesAndBuild();
366365

367366
getHelper().setWholeTestUniverseScope("test:my_rule");
@@ -371,20 +370,21 @@ public void testConfig_hostTransition() throws Exception {
371370
.isEqualTo("No target (in) //test:target_dep could be found in the 'host' configuration");
372371
assertConfigurableQueryCode(
373372
targetResult.getFailureDetail(), ConfigurableQuery.Code.TARGET_MISSING);
374-
assertThat(eval("config(//test:host_dep, host)")).isEqualTo(eval("//test:host_dep"));
375-
EvalThrowsResult hostResult = evalThrows("config(//test:exec_dep, host)", true);
376-
assertThat(hostResult.getMessage())
373+
EvalThrowsResult hostDepResult = evalThrows("config(//test:host_dep, host)", true);
374+
assertThat(hostDepResult.getMessage())
375+
.isEqualTo("No target (in) //test:host_dep could be found in the 'host' configuration");
376+
assertConfigurableQueryCode(
377+
hostDepResult.getFailureDetail(), ConfigurableQuery.Code.TARGET_MISSING);
378+
EvalThrowsResult execDepResult = evalThrows("config(//test:exec_dep, host)", true);
379+
assertThat(execDepResult.getMessage())
377380
.isEqualTo("No target (in) //test:exec_dep could be found in the 'host' configuration");
381+
assertConfigurableQueryCode(
382+
execDepResult.getFailureDetail(), ConfigurableQuery.Code.TARGET_MISSING);
383+
EvalThrowsResult hostResult = evalThrows("config(//test:dep, host)", true);
384+
assertThat(hostResult.getMessage())
385+
.isEqualTo("No target (in) //test:dep could be found in the 'host' configuration");
378386
assertConfigurableQueryCode(
379387
hostResult.getFailureDetail(), ConfigurableQuery.Code.TARGET_MISSING);
380-
381-
BuildConfigurationValue configuration =
382-
getConfiguration(Iterables.getOnlyElement(eval("config(//test:dep, host)")));
383-
384-
assertThat(configuration).isNotNull();
385-
assertThat(configuration.isHostConfiguration()).isTrue();
386-
assertThat(configuration.isExecConfiguration()).isFalse();
387-
assertThat(configuration.isToolConfiguration()).isTrue();
388388
}
389389

390390
@Test

‎src/test/java/com/google/devtools/build/lib/query2/cquery/GraphOutputFormatterCallbackTest.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@ public void factorEquivalentNodes() throws Exception {
147147
"}"));
148148
}
149149

150+
// TODO(b/203203933): Replace "host" with "exec" throughout this test.
150151
@Test
151152
public void nullAndHostDeps() throws Exception {
152153
writeFile(
@@ -159,6 +160,8 @@ public void nullAndHostDeps() throws Exception {
159160
List<String> output = getOutput("deps(//test:a)");
160161
String firstNode = output.get(2);
161162
String configHash = firstNode.substring(firstNode.indexOf("(") + 1, firstNode.length() - 2);
163+
String hostNode = output.get(6);
164+
String execConfigHash = hostNode.substring(hostNode.indexOf("(") + 1, hostNode.length() - 2);
162165
assertThat(getOutput("deps(//test:a)"))
163166
.isEqualTo(
164167
withConfigHash(
@@ -168,8 +171,8 @@ public void nullAndHostDeps() throws Exception {
168171
" \"//test:a (%s)\"",
169172
" \"//test:a (%s)\" -> \"//test:b (%s)\"",
170173
" \"//test:a (%s)\" -> \"//test:file.src (null)\"",
171-
" \"//test:a (%s)\" -> \"//test:host_dep (HOST)\"",
172-
" \"//test:host_dep (HOST)\"",
174+
" \"//test:a (%s)\" -> \"//test:host_dep (" + execConfigHash + ")\"",
175+
" \"//test:host_dep (" + execConfigHash + ")\"",
173176
" \"//test:file.src (null)\"",
174177
" \"//test:b (%s)\"",
175178
"}"));

‎src/test/java/com/google/devtools/build/lib/rules/config/FeatureFlagManualTrimmingTest.java

Lines changed: 0 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -697,41 +697,6 @@ public void featureFlagInHostConfiguration_HasNoTransitiveConfigEnforcement() th
697697
assertNoEvents();
698698
}
699699

700-
@Test
701-
public void noDistinctHostConfiguration_DoesNotResultInActionConflicts() throws Exception {
702-
scratch.file(
703-
"test/BUILD",
704-
"load(':host_transition.bzl', 'host_transition')",
705-
"load(':read_flags.bzl', 'read_flags')",
706-
"feature_flag_setter(",
707-
" name = 'target',",
708-
" deps = [':host', ':reader'],",
709-
")",
710-
"host_transition(",
711-
" name = 'host',",
712-
" srcs = [':reader'],",
713-
")",
714-
"read_flags(",
715-
" name = 'reader',",
716-
" flags = [],",
717-
")");
718-
719-
enableManualTrimmingAnd("--nodistinct_host_configuration");
720-
ConfiguredTarget target = getConfiguredTarget("//test:target");
721-
assertNoEvents();
722-
// Note that '//test:reader' is accessed (and creates actions) in both the host and target
723-
// configurations. If these are different but output to the same path (as was the case before
724-
// --nodistinct_host_configuration caused --enforce_transitive_configs_for_config_feature_flag
725-
// to become a no-op), then this causes action conflicts, as described in b/117932061 (for which
726-
// this test is a regression test).
727-
assertThat(getFilesToBuild(target).toList()).hasSize(1);
728-
// Action conflict detection is not enabled for these tests. However, the action conflict comes
729-
// from the outputs of the two configurations of //test:reader being unequal artifacts;
730-
// hence, this test checks that the nested set of artifacts reachable from //test:target only
731-
// contains one artifact, that is, they were deduplicated for being equal.
732-
}
733-
734-
735700
@Test
736701
public void noDistinctHostConfiguration_DisablesEnforcementForBothHostAndTargetConfigs()
737702
throws Exception {

‎src/test/java/com/google/devtools/build/lib/rules/java/ProguardLibraryTest.java

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@
3535
import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
3636
import com.google.devtools.build.lib.analysis.RunfilesProvider;
3737
import com.google.devtools.build.lib.analysis.actions.SpawnAction;
38-
import com.google.devtools.build.lib.analysis.config.HostTransition;
38+
import com.google.devtools.build.lib.analysis.config.ExecutionTransitionFactory;
3939
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
4040
import com.google.devtools.build.lib.cmdline.Label;
4141
import com.google.devtools.build.lib.cmdline.RepositoryName;
@@ -45,7 +45,6 @@
4545
import com.google.devtools.build.lib.packages.NativeAspectClass;
4646
import com.google.devtools.build.lib.packages.RuleClass;
4747
import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData;
48-
import com.google.devtools.build.lib.testutil.TestConstants;
4948
import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
5049
import com.google.devtools.build.lib.util.FileTypeSet;
5150
import java.util.List;
@@ -70,11 +69,11 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env)
7069
.add(attr("runtime_deps", LABEL_LIST).allowedFileTypes(FileTypeSet.NO_FILE))
7170
.add(
7271
attr("plugins", LABEL_LIST)
73-
.cfg(HostTransition.createFactory())
72+
.cfg(ExecutionTransitionFactory.create())
7473
.allowedFileTypes(FileTypeSet.NO_FILE))
7574
.add(
7675
attr("exported_plugins", LABEL_LIST)
77-
.cfg(HostTransition.createFactory())
76+
.cfg(ExecutionTransitionFactory.create())
7877
.allowedFileTypes(FileTypeSet.NO_FILE))
7978
.build();
8079
}
@@ -111,7 +110,7 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env)
111110
.add(attr("target_libs", LABEL_LIST).allowedFileTypes(FileTypeSet.NO_FILE))
112111
.add(
113112
attr("host_libs", LABEL_LIST)
114-
.cfg(HostTransition.createFactory())
113+
.cfg(ExecutionTransitionFactory.create())
115114
.allowedFileTypes(FileTypeSet.NO_FILE))
116115
.add(attr("target_attrs", STRING_LIST))
117116
.add(attr("host_attrs", STRING_LIST))
@@ -120,7 +119,7 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env)
120119
.value(Label.parseAbsoluteUnchecked("//test/implicit:implicit_target")))
121120
.add(
122121
attr("$implicit_host", LABEL)
123-
.cfg(HostTransition.createFactory())
122+
.cfg(ExecutionTransitionFactory.create())
124123
.value(Label.parseAbsoluteUnchecked("//test/implicit:implicit_host")))
125124
.build();
126125
}
@@ -283,7 +282,7 @@ public void testProguardSpecs_originalConfigSentAsInputToAllowlister() throws Ex
283282
.map(path -> path.replaceFirst(TestConstants.PRODUCT_NAME + "-out/[^/]+/", ""))
284283
.collect(Collectors.toList());
285284
List<String> expectedFilesToRun =
286-
getFilesToRun(getHostConfiguredTarget(TestConstants.PROGUARD_ALLOWLISTER_TARGET))
285+
getFilesToRun(getConfiguredTarget(TestConstants.PROGUARD_ALLOWLISTER_TARGET))
287286
.toList()
288287
.stream()
289288
.map(Artifact::getExecPathString)
@@ -348,11 +347,11 @@ public void testProguardSpecs_pickedUpFromDependencyAttributes() throws Exceptio
348347
Artifact validatedPlugin =
349348
getBinArtifact(
350349
"validated_proguard/plugin/test/plugin.cfg_valid",
351-
getHostConfiguredTarget("//test:plugin"));
350+
getDirectPrerequisite(target, "//test:plugin"));
352351
Artifact validatedExportedPlugin =
353352
getBinArtifact(
354353
"validated_proguard/exported_plugin/test/exported_plugin.cfg_valid",
355-
getHostConfiguredTarget("//test:exported_plugin"));
354+
getDirectPrerequisite(target, "//test:exported_plugin"));
356355

357356
assertThat(getFilesToBuild(target).toList())
358357
.containsExactly(
@@ -383,15 +382,16 @@ public void testProguardSpecs_customAttributes() throws Exception {
383382
getConfiguredTarget("//test:target"));
384383
Artifact validatedHost =
385384
getBinArtifact(
386-
"validated_proguard/host/test/host.cfg_valid", getHostConfiguredTarget("//test:host"));
385+
"validated_proguard/host/test/host.cfg_valid",
386+
getDirectPrerequisite(target, "//test:host"));
387387
Artifact validatedImplicitTarget =
388388
getBinArtifact(
389389
"validated_proguard/implicit_target/test/implicit/implicit_target.cfg_valid",
390390
getConfiguredTarget("//test/implicit:implicit_target"));
391391
Artifact validatedImplicitHost =
392392
getBinArtifact(
393393
"validated_proguard/implicit_host/test/implicit/implicit_host.cfg_valid",
394-
getHostConfiguredTarget("//test/implicit:implicit_host"));
394+
getDirectPrerequisite(target, "//test/implicit:implicit_host"));
395395

396396
assertThat(getFilesToBuild(target).toList())
397397
.containsExactly(

‎src/test/java/com/google/devtools/build/lib/skyframe/ConfigurationsForTargetsTest.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import com.google.devtools.build.skyframe.SkyKey;
4848
import com.google.devtools.build.skyframe.SkyValue;
4949
import java.util.List;
50+
import org.junit.Ignore;
5051
import org.junit.Test;
5152
import org.junit.runner.RunWith;
5253
import org.junit.runners.JUnit4;
@@ -274,6 +275,7 @@ public void targetDeps() throws Exception {
274275
* contexts.
275276
*/
276277
@Test
278+
@Ignore("b/219749974")
277279
public void hostDeps() throws Exception {
278280
scratch.file(
279281
"a/host_rule.bzl",

‎src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -759,9 +759,10 @@ public void testAttrBadKeywordArguments() throws Exception {
759759
}
760760

761761
@Test
762-
public void testAttrCfg() throws Exception {
762+
public void testAttrCfgNoMoreHost() throws Exception {
763763
Attribute attr = buildAttribute("a1", "attr.label(cfg = 'host', allow_files = True)");
764-
assertThat(attr.getTransitionFactory().isHost()).isTrue();
764+
assertThat(attr.getTransitionFactory().isHost()).isFalse();
765+
assertThat(attr.getTransitionFactory().isTool()).isTrue();
765766
}
766767

767768
@Test

0 commit comments

Comments
 (0)