Skip to content

Commit 51fb9e1

Browse files
philwophilwo
committed
Revert "Support execution constraints per exec group"
This reverts commit cbefbd9. Signed-off-by: Philipp Wollermann <[email protected]>
1 parent a165baa commit 51fb9e1

3 files changed

Lines changed: 17 additions & 273 deletions

File tree

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

Lines changed: 17 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -483,8 +483,7 @@ public ActionOwner getActionOwner(String execGroup) {
483483
aspectDescriptors,
484484
getConfiguration(),
485485
getExecProperties(execGroup, execProperties),
486-
getExecutionPlatform(execGroup),
487-
ImmutableSet.of(execGroup));
486+
getExecutionPlatform(execGroup));
488487
actionOwners.put(execGroup, actionOwner);
489488
return actionOwner;
490489
}
@@ -591,31 +590,12 @@ public ImmutableList<Artifact> getBuildInfo(BuildInfoKey key) throws Interrupted
591590
AnalysisUtils.isStampingEnabled(this, getConfiguration()), key, getConfiguration());
592591
}
593592

594-
/**
595-
* Computes a map of exec properties given the execution platform, taking only properties in exec
596-
* groups that are applicable to this action. Properties for specific exec groups take precedence
597-
* over properties that don't specify an exec group.
598-
*/
599593
private static ImmutableMap<String, String> computeExecProperties(
600-
Map<String, String> targetExecProperties,
601-
@Nullable PlatformInfo executionPlatform,
602-
Set<String> execGroups) {
594+
Map<String, String> targetExecProperties, @Nullable PlatformInfo executionPlatform) {
603595
Map<String, String> execProperties = new HashMap<>();
604596

605597
if (executionPlatform != null) {
606-
Map<String, Map<String, String>> execPropertiesPerGroup =
607-
parseExecGroups(executionPlatform.execProperties());
608-
609-
if (execPropertiesPerGroup.containsKey(DEFAULT_EXEC_GROUP_NAME)) {
610-
execProperties.putAll(execPropertiesPerGroup.get(DEFAULT_EXEC_GROUP_NAME));
611-
execPropertiesPerGroup.remove(DEFAULT_EXEC_GROUP_NAME);
612-
}
613-
614-
for (Map.Entry<String, Map<String, String>> execGroup : execPropertiesPerGroup.entrySet()) {
615-
if (execGroups.contains(execGroup.getKey())) {
616-
execProperties.putAll(execGroup.getValue());
617-
}
618-
}
598+
execProperties.putAll(executionPlatform.execProperties());
619599
}
620600

621601
// If the same key occurs both in the platform and in target-specific properties, the
@@ -631,8 +611,7 @@ public static ActionOwner createActionOwner(
631611
ImmutableList<AspectDescriptor> aspectDescriptors,
632612
BuildConfiguration configuration,
633613
Map<String, String> targetExecProperties,
634-
@Nullable PlatformInfo executionPlatform,
635-
Set<String> execGroups) {
614+
@Nullable PlatformInfo executionPlatform) {
636615
return ActionOwner.create(
637616
rule.getLabel(),
638617
aspectDescriptors,
@@ -642,7 +621,7 @@ public static ActionOwner createActionOwner(
642621
configuration.checksum(),
643622
configuration.toBuildEvent(),
644623
configuration.isHostConfiguration() ? HOST_CONFIGURATION_PROGRESS_TAG : null,
645-
computeExecProperties(targetExecProperties, executionPlatform, execGroups),
624+
computeExecProperties(targetExecProperties, executionPlatform),
646625
executionPlatform);
647626
}
648627

@@ -1279,18 +1258,20 @@ private ImmutableMap<String, ImmutableMap<String, String>> parseExecProperties(
12791258
return ImmutableMap.of(DEFAULT_EXEC_GROUP_NAME, ImmutableMap.of());
12801259
} else {
12811260
return parseExecProperties(
1282-
execProperties, toolchainContexts == null ? null : toolchainContexts.getExecGroups());
1261+
execProperties,
1262+
toolchainContexts == null ? ImmutableSet.of() : toolchainContexts.getExecGroups());
12831263
}
12841264
}
12851265

12861266
/**
12871267
* Parse raw exec properties attribute value into a map of exec group names to their properties.
12881268
* The raw map can have keys of two forms: (1) 'property' and (2) 'exec_group_name.property'. The
1289-
* former get parsed into the default exec group, the latter get parsed into their relevant exec
1290-
* groups.
1269+
* former get parsed into the target's default exec group, the latter get parsed into their
1270+
* relevant exec groups.
12911271
*/
1292-
private static Map<String, Map<String, String>> parseExecGroups(
1293-
Map<String, String> rawExecProperties) {
1272+
private static ImmutableMap<String, ImmutableMap<String, String>> parseExecProperties(
1273+
Map<String, String> rawExecProperties, Set<String> execGroups)
1274+
throws InvalidExecGroupException {
12941275
Map<String, Map<String, String>> consolidatedProperties = new HashMap<>();
12951276
consolidatedProperties.put(DEFAULT_EXEC_GROUP_NAME, new HashMap<>());
12961277
for (Map.Entry<String, String> execProperty : rawExecProperties.entrySet()) {
@@ -1303,30 +1284,14 @@ private static Map<String, Map<String, String>> parseExecGroups(
13031284
} else {
13041285
String execGroup = rawProperty.substring(0, delimiterIndex);
13051286
String property = rawProperty.substring(delimiterIndex + 1);
1306-
consolidatedProperties.putIfAbsent(execGroup, new HashMap<>());
1307-
consolidatedProperties.get(execGroup).put(property, execProperty.getValue());
1308-
}
1309-
}
1310-
return consolidatedProperties;
1311-
}
1312-
1313-
/**
1314-
* Parse raw exec properties attribute value into a map of exec group names to their properties.
1315-
* If given a set of exec groups, validates all the exec groups in the map are applicable to the
1316-
* action.
1317-
*/
1318-
private static ImmutableMap<String, ImmutableMap<String, String>> parseExecProperties(
1319-
Map<String, String> rawExecProperties, @Nullable Set<String> execGroups)
1320-
throws InvalidExecGroupException {
1321-
Map<String, Map<String, String>> consolidatedProperties = parseExecGroups(rawExecProperties);
1322-
if (execGroups != null) {
1323-
for (Map.Entry<String, Map<String, String>> execGroup : consolidatedProperties.entrySet()) {
1324-
String execGroupName = execGroup.getKey();
1325-
if (!execGroupName.equals(DEFAULT_EXEC_GROUP_NAME) && !execGroups.contains(execGroupName)) {
1287+
if (!execGroups.contains(execGroup)) {
13261288
throw new InvalidExecGroupException(
13271289
String.format(
1328-
"Tried to set properties for non-existent exec group '%s'.", execGroupName));
1290+
"Tried to set exec property '%s' for non-existent exec group '%s'.",
1291+
property, execGroup));
13291292
}
1293+
consolidatedProperties.putIfAbsent(execGroup, new HashMap<>());
1294+
consolidatedProperties.get(execGroup).put(property, execProperty.getValue());
13301295
}
13311296
}
13321297

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

Lines changed: 0 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -105,10 +105,6 @@ private void createToolchainsAndPlatforms() throws Exception {
105105
"platform(",
106106
" name = 'platform_2',",
107107
" constraint_values = [':constraint_2'],",
108-
" exec_properties = {",
109-
" 'watermelon.ripeness': 'unripe',",
110-
" 'watermelon.color': 'red',",
111-
" },",
112108
")");
113109

114110
useConfiguration(
@@ -395,54 +391,4 @@ public void testInheritsRuleRequirements() throws Exception {
395391
ImmutableSet.of(Label.parseAbsoluteUnchecked("//rule:toolchain_type_1")),
396392
ImmutableSet.of(Label.parseAbsoluteUnchecked("//platform:constraint_1"))));
397393
}
398-
399-
@Test
400-
public void testInheritsPlatformExecGroupExecProperty() throws Exception {
401-
createToolchainsAndPlatforms();
402-
writeRuleWithActionsAndWatermelonExecGroup();
403-
404-
scratch.file(
405-
"test/BUILD",
406-
"load('//test:defs.bzl', 'with_actions')",
407-
"with_actions(",
408-
" name = 'papaya',",
409-
" output = 'out.txt',",
410-
" watermelon_output = 'watermelon_out.txt',",
411-
")");
412-
413-
ConfiguredTarget target = getConfiguredTarget("//test:papaya");
414-
415-
assertThat(
416-
getGeneratingAction(target, "test/watermelon_out.txt").getOwner().getExecProperties())
417-
.containsExactly("ripeness", "unripe", "color", "red");
418-
assertThat(getGeneratingAction(target, "test/out.txt").getOwner().getExecProperties())
419-
.containsExactly();
420-
}
421-
422-
@Test
423-
public void testOverridePlatformExecGroupExecProperty() throws Exception {
424-
createToolchainsAndPlatforms();
425-
writeRuleWithActionsAndWatermelonExecGroup();
426-
427-
scratch.file(
428-
"test/BUILD",
429-
"load('//test:defs.bzl', 'with_actions')",
430-
"with_actions(",
431-
" name = 'papaya',",
432-
" output = 'out.txt',",
433-
" watermelon_output = 'watermelon_out.txt',",
434-
" exec_properties = {",
435-
" 'watermelon.ripeness': 'ripe',",
436-
" 'ripeness': 'unknown',",
437-
" },",
438-
")");
439-
440-
ConfiguredTarget target = getConfiguredTarget("//test:papaya");
441-
442-
assertThat(
443-
getGeneratingAction(target, "test/watermelon_out.txt").getOwner().getExecProperties())
444-
.containsExactly("ripeness", "ripe", "color", "red");
445-
assertThat(getGeneratingAction(target, "test/out.txt").getOwner().getExecProperties())
446-
.containsExactly("ripeness", "unknown");
447-
}
448394
}

‎src/test/shell/bazel/platforms_test.sh‎

Lines changed: 0 additions & 167 deletions
Original file line numberDiff line numberDiff line change
@@ -299,172 +299,5 @@ EOF
299299
grep "child_value" out.txt || fail "Did not find the overriding value"
300300
}
301301

302-
function test_platform_execgroup_properties_cc_test() {
303-
cat > a.cc <<'EOF'
304-
int main() {}
305-
EOF
306-
cat > BUILD <<'EOF'
307-
cc_test(
308-
name = "a",
309-
srcs = ["a.cc"],
310-
)
311-
312-
platform(
313-
name = "my_platform",
314-
parents = ["@local_config_platform//:host"],
315-
exec_properties = {
316-
"platform_key": "default_value",
317-
"test.platform_key": "test_value",
318-
}
319-
)
320-
EOF
321-
bazel build --extra_execution_platforms=":my_platform" :a --execution_log_json_file out.txt || fail "Build failed"
322-
grep "platform_key" out.txt || fail "Did not find the platform key"
323-
grep "default_value" out.txt || fail "Did not find the default value"
324-
grep "test_value" out.txt && fail "Used the test-action value when not testing"
325-
326-
bazel test --extra_execution_platforms=":my_platform" :a --execution_log_json_file out.txt || fail "Test failed"
327-
grep "platform_key" out.txt || fail "Did not find the platform key"
328-
grep "test_value" out.txt || fail "Did not find the test-action value"
329-
}
330-
331-
function test_platform_execgroup_properties_nongroup_override_cc_test() {
332-
cat > a.cc <<'EOF'
333-
int main() {}
334-
EOF
335-
cat > BUILD <<'EOF'
336-
cc_test(
337-
name = "a",
338-
srcs = ["a.cc"],
339-
exec_properties = {
340-
"platform_key": "override_value",
341-
},
342-
)
343-
344-
platform(
345-
name = "my_platform",
346-
parents = ["@local_config_platform//:host"],
347-
exec_properties = {
348-
"platform_key": "default_value",
349-
"test.platform_key": "test_value",
350-
}
351-
)
352-
EOF
353-
bazel build --extra_execution_platforms=":my_platform" :a --execution_log_json_file out.txt || fail "Build failed"
354-
grep "platform_key" out.txt || fail "Did not find the platform key"
355-
grep "override_value" out.txt || fail "Did not find the overriding value"
356-
grep "default_value" out.txt && fail "Used the default value"
357-
358-
bazel test --extra_execution_platforms=":my_platform" :a --execution_log_json_file out.txt || fail "Test failed"
359-
grep "platform_key" out.txt || fail "Did not find the platform key"
360-
grep "override_value" out.txt || fail "Did not find the overriding value"
361-
}
362-
363-
function test_platform_execgroup_properties_group_override_cc_test() {
364-
cat > a.cc <<'EOF'
365-
int main() {}
366-
EOF
367-
cat > BUILD <<'EOF'
368-
cc_test(
369-
name = "a",
370-
srcs = ["a.cc"],
371-
exec_properties = {
372-
"test.platform_key": "test_override",
373-
},
374-
)
375-
376-
platform(
377-
name = "my_platform",
378-
parents = ["@local_config_platform//:host"],
379-
exec_properties = {
380-
"platform_key": "default_value",
381-
"test.platform_key": "test_value",
382-
}
383-
)
384-
EOF
385-
bazel build --extra_execution_platforms=":my_platform" :a --execution_log_json_file out.txt || fail "Build failed"
386-
grep "platform_key" out.txt || fail "Did not find the platform key"
387-
grep "default_value" out.txt || fail "Used the default value"
388-
389-
bazel test --extra_execution_platforms=":my_platform" :a --execution_log_json_file out.txt || fail "Test failed"
390-
grep "platform_key" out.txt || fail "Did not find the platform key"
391-
grep "test_override" out.txt || fail "Did not find the overriding test-action value"
392-
}
393-
394-
function test_platform_execgroup_properties_override_group_and_default_cc_test() {
395-
cat > a.cc <<'EOF'
396-
int main() {}
397-
EOF
398-
cat > BUILD <<'EOF'
399-
cc_test(
400-
name = "a",
401-
srcs = ["a.cc"],
402-
exec_properties = {
403-
"platform_key": "override_value",
404-
"test.platform_key": "test_override",
405-
},
406-
)
407-
408-
platform(
409-
name = "my_platform",
410-
parents = ["@local_config_platform//:host"],
411-
exec_properties = {
412-
"platform_key": "default_value",
413-
"test.platform_key": "test_value",
414-
}
415-
)
416-
EOF
417-
bazel build --extra_execution_platforms=":my_platform" :a --execution_log_json_file out.txt || fail "Build failed"
418-
grep "platform_key" out.txt || fail "Did not find the platform key"
419-
grep "override_value" out.txt || fail "Did not find the overriding value"
420-
grep "default_value" out.txt && fail "Used the default value"
421-
422-
bazel test --extra_execution_platforms=":my_platform" :a --execution_log_json_file out.txt || fail "Test failed"
423-
grep "platform_key" out.txt || fail "Did not find the platform key"
424-
grep "test_override" out.txt || fail "Did not find the overriding test-action value"
425-
}
426-
427-
function test_platform_properties_only_applied_for_relevant_execgroups_cc_test() {
428-
cat > a.cc <<'EOF'
429-
int main() {}
430-
EOF
431-
cat > BUILD <<'EOF'
432-
cc_test(
433-
name = "a",
434-
srcs = ["a.cc"],
435-
)
436-
437-
platform(
438-
name = "my_platform",
439-
parents = ["@local_config_platform//:host"],
440-
exec_properties = {
441-
"platform_key": "default_value",
442-
"unknown.platform_key": "unknown_value",
443-
}
444-
)
445-
EOF
446-
bazel test --extra_execution_platforms=":my_platform" :a --execution_log_json_file out.txt || fail "Build failed"
447-
grep "platform_key" out.txt || fail "Did not find the platform key"
448-
grep "default_value" out.txt || fail "Did not find the default value"
449-
}
450-
451-
function test_cannot_set_properties_for_irrelevant_execgroup_on_target_cc_test() {
452-
cat > a.cc <<'EOF'
453-
int main() {}
454-
EOF
455-
cat > BUILD <<'EOF'
456-
cc_test(
457-
name = "a",
458-
srcs = ["a.cc"],
459-
exec_properties = {
460-
"platform_key": "default_value",
461-
"unknown.platform_key": "unknown_value",
462-
}
463-
)
464-
EOF
465-
bazel test :a &> $TEST_log && fail "Build passed when we expected an error"
466-
grep "Tried to set properties for non-existent exec group" $TEST_log || fail "Did not complain about unknown exec group"
467-
}
468-
469302
run_suite "platform mapping test"
470303

0 commit comments

Comments
 (0)