Skip to content

Commit

Permalink
Revert "Clean up RuleContext to use a Table instead of a Map of Maps."
Browse files Browse the repository at this point in the history 
This reverts commit 1aa544b.

Signed-off-by: Philipp Wollermann <philwo@google.com>
  • Loading branch information
@philwo
philwo committed May 20, 2021
1 parent e09f274 commit a165baa
Showing 1 changed file with 41 additions and 25 deletions.
66 changes: 41 additions & 25 deletions src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedSet;
import com.google.common.collect.ImmutableTable;
import com.google.common.collect.Iterables;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Lists;
Expand Down Expand Up @@ -211,7 +210,7 @@ public ImmutableList<TransitiveInfoCollection> getFiles() {
private final ConstraintSemantics<RuleContext> constraintSemantics;
private final ImmutableSet<String> requiredConfigFragments;
private final List<Expander> makeVariableExpanders = new ArrayList<>();
private final ImmutableTable<String, String, String> execProperties;
private final ImmutableMap<String, ImmutableMap<String, String>> execProperties;

/** Map of exec group names to ActionOwners. */
private final Map<String, ActionOwner> actionOwners = new HashMap<>();
Expand Down Expand Up @@ -604,16 +603,17 @@ private static ImmutableMap<String, String> computeExecProperties(
Map<String, String> execProperties = new HashMap<>();

if (executionPlatform != null) {
ImmutableTable<String, String, String> execPropertiesPerGroup =
Map<String, Map<String, String>> execPropertiesPerGroup =
parseExecGroups(executionPlatform.execProperties());

if (execPropertiesPerGroup.containsRow(DEFAULT_EXEC_GROUP_NAME)) {
execProperties.putAll(execPropertiesPerGroup.row(DEFAULT_EXEC_GROUP_NAME));
if (execPropertiesPerGroup.containsKey(DEFAULT_EXEC_GROUP_NAME)) {
execProperties.putAll(execPropertiesPerGroup.get(DEFAULT_EXEC_GROUP_NAME));
execPropertiesPerGroup.remove(DEFAULT_EXEC_GROUP_NAME);
}

for (String execGroup : execPropertiesPerGroup.rowKeySet()) {
if (execGroups.contains(execGroup)) {
execProperties.putAll(execPropertiesPerGroup.row(execGroup));
for (Map.Entry<String, Map<String, String>> execGroup : execPropertiesPerGroup.entrySet()) {
if (execGroups.contains(execGroup.getKey())) {
execProperties.putAll(execGroup.getValue());
}
}
}
Expand Down Expand Up @@ -1273,10 +1273,10 @@ public ImmutableSortedSet<String> getRequiredConfigFragments() {
return ans.build();
}

private ImmutableTable<String, String, String> parseExecProperties(
private ImmutableMap<String, ImmutableMap<String, String>> parseExecProperties(
Map<String, String> execProperties) throws InvalidExecGroupException {
if (execProperties.isEmpty()) {
return ImmutableTable.of();
return ImmutableMap.of(DEFAULT_EXEC_GROUP_NAME, ImmutableMap.of());
} else {
return parseExecProperties(
execProperties, toolchainContexts == null ? null : toolchainContexts.getExecGroups());
Expand All @@ -1289,35 +1289,39 @@ private ImmutableTable<String, String, String> parseExecProperties(
* former get parsed into the default exec group, the latter get parsed into their relevant exec
* groups.
*/
private static ImmutableTable<String, String, String> parseExecGroups(
private static Map<String, Map<String, String>> parseExecGroups(
Map<String, String> rawExecProperties) {
ImmutableTable.Builder<String, String, String> execProperties = ImmutableTable.builder();
Map<String, Map<String, String>> consolidatedProperties = new HashMap<>();
consolidatedProperties.put(DEFAULT_EXEC_GROUP_NAME, new HashMap<>());
for (Map.Entry<String, String> execProperty : rawExecProperties.entrySet()) {
String rawProperty = execProperty.getKey();
int delimiterIndex = rawProperty.indexOf('.');
if (delimiterIndex == -1) {
execProperties.put(DEFAULT_EXEC_GROUP_NAME, rawProperty, execProperty.getValue());
consolidatedProperties
.get(DEFAULT_EXEC_GROUP_NAME)
.put(rawProperty, execProperty.getValue());
} else {
String execGroup = rawProperty.substring(0, delimiterIndex);
String property = rawProperty.substring(delimiterIndex + 1);
execProperties.put(execGroup, property, execProperty.getValue());
consolidatedProperties.putIfAbsent(execGroup, new HashMap<>());
consolidatedProperties.get(execGroup).put(property, execProperty.getValue());
}
}
return execProperties.build();
return consolidatedProperties;
}

/**
* Parse raw exec properties attribute value into a map of exec group names to their properties.
* If given a set of exec groups, validates all the exec groups in the map are applicable to the
* action.
*/
private static ImmutableTable<String, String, String> parseExecProperties(
private static ImmutableMap<String, ImmutableMap<String, String>> parseExecProperties(
Map<String, String> rawExecProperties, @Nullable Set<String> execGroups)
throws InvalidExecGroupException {
ImmutableTable<String, String, String> consolidatedProperties =
parseExecGroups(rawExecProperties);
Map<String, Map<String, String>> consolidatedProperties = parseExecGroups(rawExecProperties);
if (execGroups != null) {
for (String execGroupName : consolidatedProperties.rowKeySet()) {
for (Map.Entry<String, Map<String, String>> execGroup : consolidatedProperties.entrySet()) {
String execGroupName = execGroup.getKey();
if (!execGroupName.equals(DEFAULT_EXEC_GROUP_NAME) && !execGroups.contains(execGroupName)) {
throw new InvalidExecGroupException(
String.format(
Expand All @@ -1326,7 +1330,14 @@ private static ImmutableTable<String, String, String> parseExecProperties(
}
}

return consolidatedProperties;
// Copy everything to immutable maps.
ImmutableMap.Builder<String, ImmutableMap<String, String>> execProperties =
new ImmutableMap.Builder<>();
for (Map.Entry<String, Map<String, String>> execGroupMap : consolidatedProperties.entrySet()) {
execProperties.put(execGroupMap.getKey(), ImmutableMap.copyOf(execGroupMap.getValue()));
}

return execProperties.build();
}

/**
Expand All @@ -1338,16 +1349,16 @@ private static ImmutableTable<String, String, String> parseExecProperties(
* @param execProperties Map of exec group name to map of properties and values
*/
private static ImmutableMap<String, String> getExecProperties(
String execGroup, ImmutableTable<String, String, String> execProperties) {
if (!execProperties.containsRow(execGroup) || execGroup.equals(DEFAULT_EXEC_GROUP_NAME)) {
return execProperties.row(DEFAULT_EXEC_GROUP_NAME);
String execGroup, Map<String, ImmutableMap<String, String>> execProperties) {
if (!execProperties.containsKey(execGroup) || execGroup.equals(DEFAULT_EXEC_GROUP_NAME)) {
return execProperties.get(DEFAULT_EXEC_GROUP_NAME);
}

// Use a HashMap to build here because we expect duplicate keys to happen
// (and rewrite previous entries).
Map<String, String> targetAndGroupProperties =
new HashMap<>(execProperties.row(DEFAULT_EXEC_GROUP_NAME));
targetAndGroupProperties.putAll(execProperties.row(execGroup));
new HashMap<>(execProperties.get(DEFAULT_EXEC_GROUP_NAME));
targetAndGroupProperties.putAll(execProperties.get(execGroup));
return ImmutableMap.copyOf(targetAndGroupProperties);
}

Expand All @@ -1368,6 +1379,11 @@ public DetailedExitCode getDetailedExitCode() {
}
}

@VisibleForTesting
public ImmutableMap<String, ImmutableMap<String, String>> getExecPropertiesForTesting() {
return execProperties;
}

private void checkAttributeIsDependency(String attributeName) {
Attribute attributeDefinition = attributes.getAttributeDefinition(attributeName);
if (attributeDefinition == null) {
Expand Down

0 comments on commit a165baa

Please sign in to comment.