Skip to content

Commit 8c6382a

Browse files
committed
Create a new ExecGroupCollection container to manage exec group inheritance and exec property parsing.
Fixes bazelbuild#13459. PiperOrigin-RevId: 373388266
1 parent 7d5493d commit 8c6382a

21 files changed

+735
-250
lines changed

src/main/java/com/google/devtools/build/lib/analysis/BUILD

+18-1
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,7 @@ java_library(
324324
":dependency_key",
325325
":dependency_kind",
326326
":duplicate_exception",
327+
":exec_group_collection",
327328
":extra/extra_action_info_file_write_action",
328329
":extra_action_artifacts_provider",
329330
":file_provider",
@@ -418,7 +419,6 @@ java_library(
418419
"//src/main/java/com/google/devtools/build/lib/skyframe:configured_value_creation_exception",
419420
"//src/main/java/com/google/devtools/build/lib/skyframe:package_value",
420421
"//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value",
421-
"//src/main/java/com/google/devtools/build/lib/skyframe:sane_analysis_exception",
422422
"//src/main/java/com/google/devtools/build/lib/skyframe:toolchain_context_key",
423423
"//src/main/java/com/google/devtools/build/lib/skyframe:transitive_target_key",
424424
"//src/main/java/com/google/devtools/build/lib/skyframe:transitive_target_value",
@@ -758,6 +758,23 @@ java_library(
758758
srcs = ["DuplicateException.java"],
759759
)
760760

761+
java_library(
762+
name = "exec_group_collection",
763+
srcs = ["ExecGroupCollection.java"],
764+
deps = [
765+
":resolved_toolchain_context",
766+
":toolchain_collection",
767+
"//src/main/java/com/google/devtools/build/lib/analysis/platform",
768+
"//src/main/java/com/google/devtools/build/lib/packages:exec_group",
769+
"//src/main/java/com/google/devtools/build/lib/skyframe:sane_analysis_exception",
770+
"//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
771+
"//src/main/protobuf:failure_details_java_proto",
772+
"//third_party:auto_value",
773+
"//third_party:guava",
774+
"//third_party:jsr305",
775+
],
776+
)
777+
761778
java_library(
762779
name = "extra_action_artifacts_provider",
763780
srcs = ["ExtraActionArtifactsProvider.java"],

src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java

+9-4
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
import com.google.devtools.build.lib.actions.ArtifactFactory;
2626
import com.google.devtools.build.lib.actions.FailAction;
2727
import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
28-
import com.google.devtools.build.lib.analysis.RuleContext.InvalidExecGroupException;
28+
import com.google.devtools.build.lib.analysis.ExecGroupCollection.InvalidExecGroupException;
2929
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
3030
import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
3131
import com.google.devtools.build.lib.analysis.config.Fragment;
@@ -186,7 +186,8 @@ public final ConfiguredTarget createConfiguredTarget(
186186
ConfiguredTargetKey configuredTargetKey,
187187
OrderedSetMultimap<DependencyKind, ConfiguredTargetAndData> prerequisiteMap,
188188
ImmutableMap<Label, ConfigMatchingProvider> configConditions,
189-
@Nullable ToolchainCollection<ResolvedToolchainContext> toolchainContexts)
189+
@Nullable ToolchainCollection<ResolvedToolchainContext> toolchainContexts,
190+
ExecGroupCollection.Builder execGroupCollectionBuilder)
190191
throws InterruptedException, ActionConflictException, InvalidExecGroupException {
191192
if (target instanceof Rule) {
192193
try {
@@ -199,7 +200,8 @@ public final ConfiguredTarget createConfiguredTarget(
199200
configuredTargetKey,
200201
prerequisiteMap,
201202
configConditions,
202-
toolchainContexts);
203+
toolchainContexts,
204+
execGroupCollectionBuilder);
203205
} finally {
204206
CurrentRuleTracker.endConfiguredTarget();
205207
}
@@ -292,7 +294,8 @@ private ConfiguredTarget createRule(
292294
ConfiguredTargetKey configuredTargetKey,
293295
OrderedSetMultimap<DependencyKind, ConfiguredTargetAndData> prerequisiteMap,
294296
ImmutableMap<Label, ConfigMatchingProvider> configConditions,
295-
@Nullable ToolchainCollection<ResolvedToolchainContext> toolchainContexts)
297+
@Nullable ToolchainCollection<ResolvedToolchainContext> toolchainContexts,
298+
ExecGroupCollection.Builder execGroupCollectionBuilder)
296299
throws InterruptedException, ActionConflictException, InvalidExecGroupException {
297300
ConfigurationFragmentPolicy configurationFragmentPolicy =
298301
rule.getRuleClassObject().getConfigurationFragmentPolicy();
@@ -312,6 +315,7 @@ private ConfiguredTarget createRule(
312315
.setConfigConditions(configConditions)
313316
.setUniversalFragments(ruleClassProvider.getUniversalFragments())
314317
.setToolchainContexts(toolchainContexts)
318+
.setExecGroupCollectionBuilder(execGroupCollectionBuilder)
315319
.setConstraintSemantics(ruleClassProvider.getConstraintSemantics())
316320
.setRequiredConfigFragments(
317321
RequiredFragmentsUtil.getRequiredFragments(
@@ -527,6 +531,7 @@ public ConfiguredAspect createAspect(
527531
.setToolchainContext(toolchainContext)
528532
// TODO(b/161222568): Implement the exec_properties attr for aspects and read its value
529533
// here.
534+
.setExecGroupCollectionBuilder(ExecGroupCollection.emptyBuilder())
530535
.setExecProperties(ImmutableMap.of())
531536
.setConstraintSemantics(ruleClassProvider.getConstraintSemantics())
532537
.setRequiredConfigFragments(
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,267 @@
1+
// Copyright 2021 The Bazel Authors. All rights reserved.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
package com.google.devtools.build.lib.analysis;
15+
16+
import static com.google.common.collect.ImmutableSet.toImmutableSet;
17+
import static com.google.devtools.build.lib.packages.ExecGroup.DEFAULT_EXEC_GROUP_NAME;
18+
import static java.util.stream.Collectors.joining;
19+
20+
import com.google.auto.value.AutoValue;
21+
import com.google.common.collect.HashBasedTable;
22+
import com.google.common.collect.ImmutableMap;
23+
import com.google.common.collect.ImmutableSet;
24+
import com.google.common.collect.ImmutableTable;
25+
import com.google.common.collect.Table;
26+
import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
27+
import com.google.devtools.build.lib.packages.ExecGroup;
28+
import com.google.devtools.build.lib.server.FailureDetails.Analysis;
29+
import com.google.devtools.build.lib.server.FailureDetails.Analysis.Code;
30+
import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
31+
import com.google.devtools.build.lib.skyframe.SaneAnalysisException;
32+
import com.google.devtools.build.lib.util.DetailedExitCode;
33+
import java.util.Collection;
34+
import java.util.LinkedHashMap;
35+
import java.util.Map;
36+
import javax.annotation.Nullable;
37+
38+
/**
39+
* A container class for groups of {@link ExecGroup} instances. This correctly handles exec group
40+
* inheritance between rules and targets. See
41+
* https://docs.bazel.build/versions/master/exec-groups.html for further details.
42+
*/
43+
@AutoValue
44+
public abstract class ExecGroupCollection {
45+
46+
public static Builder emptyBuilder() {
47+
return new AutoValue_ExecGroupCollection_Builder(ImmutableMap.of());
48+
}
49+
50+
public static Builder builder(
51+
ExecGroup defaultExecGroup, ImmutableMap<String, ExecGroup> execGroups) {
52+
// Post-process the groups to handle inheritance.
53+
Map<String, ExecGroup> processedGroups = new LinkedHashMap<>();
54+
for (Map.Entry<String, ExecGroup> entry : execGroups.entrySet()) {
55+
String name = entry.getKey();
56+
ExecGroup execGroup = entry.getValue();
57+
58+
if (execGroup.copyFrom() != null) {
59+
if (execGroup.copyFrom().equals(DEFAULT_EXEC_GROUP_NAME)) {
60+
execGroup = execGroup.inheritFrom(defaultExecGroup);
61+
} else {
62+
execGroup = execGroup.inheritFrom(execGroups.get(execGroup.copyFrom()));
63+
}
64+
}
65+
66+
processedGroups.put(name, execGroup);
67+
}
68+
69+
return new AutoValue_ExecGroupCollection_Builder(ImmutableMap.copyOf(processedGroups));
70+
}
71+
72+
/** Builder class for correctly constructing ExecGroupCollection instances. */
73+
// Note that this is _not_ an actual @AutoValue.Builder: it provides more logic and has different
74+
// fields.
75+
@AutoValue
76+
public abstract static class Builder {
77+
public abstract ImmutableMap<String, ExecGroup> execGroups();
78+
79+
public ImmutableSet<String> getExecGroupNames() {
80+
return execGroups().keySet();
81+
}
82+
83+
public ExecGroup getExecGroup(String name) {
84+
return execGroups().get(name);
85+
}
86+
87+
public ExecGroupCollection build(
88+
@Nullable ToolchainCollection<ResolvedToolchainContext> toolchainContexts,
89+
ImmutableMap<String, String> rawExecProperties)
90+
throws InvalidExecGroupException {
91+
92+
// For each exec group, compute the combined execution properties.
93+
ImmutableTable<String, String, String> combinedExecProperties =
94+
computeCombinedExecProperties(toolchainContexts, rawExecProperties);
95+
96+
return new AutoValue_ExecGroupCollection(execGroups(), combinedExecProperties);
97+
}
98+
}
99+
100+
/**
101+
* Gets the combined exec properties of the platform and the target's exec properties. If a
102+
* property is set in both, the target properties take precedence.
103+
*/
104+
private static ImmutableTable<String, String, String> computeCombinedExecProperties(
105+
@Nullable ToolchainCollection<ResolvedToolchainContext> toolchainContexts,
106+
ImmutableMap<String, String> rawExecProperties)
107+
throws InvalidExecGroupException {
108+
109+
ImmutableSet<String> execGroupNames;
110+
if (toolchainContexts == null) {
111+
execGroupNames = ImmutableSet.of(DEFAULT_EXEC_GROUP_NAME);
112+
} else {
113+
execGroupNames = toolchainContexts.getExecGroupNames();
114+
}
115+
116+
// Parse the target-level exec properties.
117+
ImmutableTable<String, String, String> parsedTargetProperties =
118+
parseExecProperties(rawExecProperties);
119+
// Validate the exec group names in the properties.
120+
if (toolchainContexts != null) {
121+
ImmutableSet<String> unknownTargetExecGroupNames =
122+
parsedTargetProperties.rowKeySet().stream()
123+
.filter(name -> !name.equals(DEFAULT_EXEC_GROUP_NAME))
124+
.filter(name -> !execGroupNames.contains(name))
125+
.collect(toImmutableSet());
126+
if (!unknownTargetExecGroupNames.isEmpty()) {
127+
throw new InvalidExecGroupException(unknownTargetExecGroupNames);
128+
}
129+
}
130+
131+
// Parse each execution platform's exec properties.
132+
ImmutableSet<PlatformInfo> executionPlatforms;
133+
if (toolchainContexts == null) {
134+
executionPlatforms = ImmutableSet.of();
135+
} else {
136+
executionPlatforms =
137+
execGroupNames.stream()
138+
.map(name -> toolchainContexts.getToolchainContext(name).executionPlatform())
139+
.distinct()
140+
.collect(toImmutableSet());
141+
}
142+
Map<PlatformInfo, ImmutableTable<String, String, String>> parsedPlatformProperties =
143+
new LinkedHashMap<>();
144+
for (PlatformInfo executionPlatform : executionPlatforms) {
145+
ImmutableTable<String, String, String> parsed =
146+
parseExecProperties(executionPlatform.execProperties());
147+
parsedPlatformProperties.put(executionPlatform, parsed);
148+
}
149+
150+
// First, get the defaults.
151+
ImmutableMap<String, String> defaultExecProperties =
152+
parsedTargetProperties.row(DEFAULT_EXEC_GROUP_NAME);
153+
Table<String, String, String> result = HashBasedTable.create();
154+
putAll(result, DEFAULT_EXEC_GROUP_NAME, defaultExecProperties);
155+
156+
for (String execGroupName : execGroupNames) {
157+
ImmutableMap<String, String> combined =
158+
computeProperties(
159+
execGroupName,
160+
defaultExecProperties,
161+
toolchainContexts,
162+
parsedPlatformProperties,
163+
parsedTargetProperties);
164+
putAll(result, execGroupName, combined);
165+
}
166+
167+
return ImmutableTable.copyOf(result);
168+
}
169+
170+
private static <R, C, V> void putAll(Table<R, C, V> builder, R row, Map<C, V> values) {
171+
for (Map.Entry<C, V> entry : values.entrySet()) {
172+
builder.put(row, entry.getKey(), entry.getValue());
173+
}
174+
}
175+
176+
private static ImmutableMap<String, String> computeProperties(
177+
String execGroupName,
178+
ImmutableMap<String, String> defaultExecProperties,
179+
@Nullable ToolchainCollection<ResolvedToolchainContext> toolchainContexts,
180+
Map<PlatformInfo, ImmutableTable<String, String, String>> parsedPlatformProperties,
181+
ImmutableTable<String, String, String> parsedTargetProperties) {
182+
183+
ImmutableMap<String, String> defaultExecGroupPlatformProperties;
184+
ImmutableMap<String, String> platformProperties;
185+
if (toolchainContexts == null) {
186+
defaultExecGroupPlatformProperties = ImmutableMap.of();
187+
platformProperties = ImmutableMap.of();
188+
} else {
189+
PlatformInfo executionPlatform =
190+
toolchainContexts.getToolchainContext(execGroupName).executionPlatform();
191+
defaultExecGroupPlatformProperties =
192+
parsedPlatformProperties.get(executionPlatform).row(DEFAULT_EXEC_GROUP_NAME);
193+
platformProperties = parsedPlatformProperties.get(executionPlatform).row(execGroupName);
194+
}
195+
Map<String, String> targetProperties =
196+
new LinkedHashMap<>(parsedTargetProperties.row(execGroupName));
197+
for (String propertyName : defaultExecProperties.keySet()) {
198+
// If the property exists in the default and not in the target, copy it.
199+
targetProperties.computeIfAbsent(propertyName, defaultExecProperties::get);
200+
}
201+
202+
// Combine the target and exec platform properties. Target properties take precedence.
203+
// Use a HashMap instead of an ImmutableMap.Builder because we expect duplicate keys.
204+
Map<String, String> combined = new LinkedHashMap<>();
205+
combined.putAll(defaultExecGroupPlatformProperties);
206+
combined.putAll(defaultExecProperties);
207+
combined.putAll(platformProperties);
208+
combined.putAll(targetProperties);
209+
return ImmutableMap.copyOf(combined);
210+
}
211+
212+
protected abstract ImmutableMap<String, ExecGroup> execGroups();
213+
214+
protected abstract ImmutableTable<String, String, String> execProperties();
215+
216+
public ExecGroup getExecGroup(String execGroupName) {
217+
return execGroups().get(execGroupName);
218+
}
219+
220+
public ImmutableMap<String, String> getExecProperties(String execGroupName) {
221+
return execProperties().row(execGroupName);
222+
}
223+
224+
/**
225+
* Parse raw exec properties attribute value into a map of exec group names to their properties.
226+
* The raw map can have keys of two forms: (1) 'property' and (2) 'exec_group_name.property'. The
227+
* former get parsed into the default exec group, the latter get parsed into their relevant exec
228+
* groups.
229+
*/
230+
private static ImmutableTable<String, String, String> parseExecProperties(
231+
Map<String, String> rawExecProperties) {
232+
ImmutableTable.Builder<String, String, String> execProperties = ImmutableTable.builder();
233+
for (Map.Entry<String, String> execProperty : rawExecProperties.entrySet()) {
234+
String rawProperty = execProperty.getKey();
235+
int delimiterIndex = rawProperty.indexOf('.');
236+
if (delimiterIndex == -1) {
237+
execProperties.put(DEFAULT_EXEC_GROUP_NAME, rawProperty, execProperty.getValue());
238+
} else {
239+
String execGroup = rawProperty.substring(0, delimiterIndex);
240+
String property = rawProperty.substring(delimiterIndex + 1);
241+
execProperties.put(execGroup, property, execProperty.getValue());
242+
}
243+
}
244+
return execProperties.build();
245+
}
246+
247+
/** An error for when the user tries to access a non-existent exec group. */
248+
public static final class InvalidExecGroupException extends Exception
249+
implements SaneAnalysisException {
250+
251+
public InvalidExecGroupException(Collection<String> invalidNames) {
252+
super(
253+
String.format(
254+
"Tried to set properties for non-existent exec groups: %s.",
255+
invalidNames.stream().collect(joining(","))));
256+
}
257+
258+
@Override
259+
public DetailedExitCode getDetailedExitCode() {
260+
return DetailedExitCode.of(
261+
FailureDetail.newBuilder()
262+
.setMessage(getMessage())
263+
.setAnalysis(Analysis.newBuilder().setCode(Code.EXEC_GROUP_MISSING))
264+
.build());
265+
}
266+
}
267+
}

0 commit comments

Comments
 (0)