From a165baa250652fdc865ae0df39160be1f7f74c47 Mon Sep 17 00:00:00 2001 From: Philipp Wollermann Date: Thu, 20 May 2021 12:03:40 +0200 Subject: [PATCH] Revert "Clean up RuleContext to use a Table instead of a Map of Maps." This reverts commit 1aa544b60465765f77b714cdbf40fa082b0573a7. Signed-off-by: Philipp Wollermann --- .../build/lib/analysis/RuleContext.java | 66 ++++++++++++------- 1 file changed, 41 insertions(+), 25 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java index 44d90212dda57b..29fc484bad7c71 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java @@ -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; @@ -211,7 +210,7 @@ public ImmutableList getFiles() { private final ConstraintSemantics constraintSemantics; private final ImmutableSet requiredConfigFragments; private final List makeVariableExpanders = new ArrayList<>(); - private final ImmutableTable execProperties; + private final ImmutableMap> execProperties; /** Map of exec group names to ActionOwners. */ private final Map actionOwners = new HashMap<>(); @@ -604,16 +603,17 @@ private static ImmutableMap computeExecProperties( Map execProperties = new HashMap<>(); if (executionPlatform != null) { - ImmutableTable execPropertiesPerGroup = + Map> 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> execGroup : execPropertiesPerGroup.entrySet()) { + if (execGroups.contains(execGroup.getKey())) { + execProperties.putAll(execGroup.getValue()); } } } @@ -1273,10 +1273,10 @@ public ImmutableSortedSet getRequiredConfigFragments() { return ans.build(); } - private ImmutableTable parseExecProperties( + private ImmutableMap> parseExecProperties( Map 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()); @@ -1289,21 +1289,25 @@ private ImmutableTable parseExecProperties( * former get parsed into the default exec group, the latter get parsed into their relevant exec * groups. */ - private static ImmutableTable parseExecGroups( + private static Map> parseExecGroups( Map rawExecProperties) { - ImmutableTable.Builder execProperties = ImmutableTable.builder(); + Map> consolidatedProperties = new HashMap<>(); + consolidatedProperties.put(DEFAULT_EXEC_GROUP_NAME, new HashMap<>()); for (Map.Entry 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; } /** @@ -1311,13 +1315,13 @@ private static ImmutableTable parseExecGroups( * If given a set of exec groups, validates all the exec groups in the map are applicable to the * action. */ - private static ImmutableTable parseExecProperties( + private static ImmutableMap> parseExecProperties( Map rawExecProperties, @Nullable Set execGroups) throws InvalidExecGroupException { - ImmutableTable consolidatedProperties = - parseExecGroups(rawExecProperties); + Map> consolidatedProperties = parseExecGroups(rawExecProperties); if (execGroups != null) { - for (String execGroupName : consolidatedProperties.rowKeySet()) { + for (Map.Entry> execGroup : consolidatedProperties.entrySet()) { + String execGroupName = execGroup.getKey(); if (!execGroupName.equals(DEFAULT_EXEC_GROUP_NAME) && !execGroups.contains(execGroupName)) { throw new InvalidExecGroupException( String.format( @@ -1326,7 +1330,14 @@ private static ImmutableTable parseExecProperties( } } - return consolidatedProperties; + // Copy everything to immutable maps. + ImmutableMap.Builder> execProperties = + new ImmutableMap.Builder<>(); + for (Map.Entry> execGroupMap : consolidatedProperties.entrySet()) { + execProperties.put(execGroupMap.getKey(), ImmutableMap.copyOf(execGroupMap.getValue())); + } + + return execProperties.build(); } /** @@ -1338,16 +1349,16 @@ private static ImmutableTable parseExecProperties( * @param execProperties Map of exec group name to map of properties and values */ private static ImmutableMap getExecProperties( - String execGroup, ImmutableTable execProperties) { - if (!execProperties.containsRow(execGroup) || execGroup.equals(DEFAULT_EXEC_GROUP_NAME)) { - return execProperties.row(DEFAULT_EXEC_GROUP_NAME); + String execGroup, Map> 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 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); } @@ -1368,6 +1379,11 @@ public DetailedExitCode getDetailedExitCode() { } } + @VisibleForTesting + public ImmutableMap> getExecPropertiesForTesting() { + return execProperties; + } + private void checkAttributeIsDependency(String attributeName) { Attribute attributeDefinition = attributes.getAttributeDefinition(attributeName); if (attributeDefinition == null) {