2 files changed, 39 insertions, 24 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/flags/InvocationPolicyEnforcer.java b/src/main/java/com/google/devtools/build/lib/flags/InvocationPolicyEnforcer.java
index 5a6e324bcb..c25f100f6d 100644
--- a/src/main/java/com/google/devtools/build/lib/flags/InvocationPolicyEnforcer.java
+++ b/src/main/java/com/google/devtools/build/lib/flags/InvocationPolicyEnforcer.java
@@ -36,7 +36,10 @@ import com.google.devtools.common.options.OptionsParser.OptionValueDescription;
 import com.google.devtools.common.options.OptionsParsingException;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -96,30 +99,14 @@ public final class InvocationPolicyEnforcer {
     if (invocationPolicy == null || invocationPolicy.getFlagPoliciesCount() == 0) {
       return;
     }
-    List<FlagPolicy> effectivePolicy = getEffectivePolicy(invocationPolicy, parser);
 
-    ImmutableSet<String> commandAndParentCommands =
-        command == null
-            ? ImmutableSet.<String>of()
-            : CommandNameCache.CommandNameCacheInstance.INSTANCE.get(command);
+    // The effective policy returned is expanded, filtered for applicable commands, and cleaned of
+    // redundancies and conflicts.
+    List<FlagPolicy> effectivePolicy = getEffectivePolicy(invocationPolicy, parser, command);
+
     for (FlagPolicy flagPolicy : effectivePolicy) {
       String flagName = flagPolicy.getFlagName();
 
-      // Skip the flag policy if it doesn't apply to this command. If the commands list is empty,
-      // then the policy applies to all commands.
-      if (!flagPolicy.getCommandsList().isEmpty() && !commandAndParentCommands.isEmpty()) {
-        boolean flagApplies = false;
-        for (String policyCommand : flagPolicy.getCommandsList()) {
-          if (commandAndParentCommands.contains(policyCommand)) {
-            flagApplies = true;
-            break;
-          }
-        }
-        if (!flagApplies) {
-          continue;
-        }
-      }
-
       OptionValueDescription valueDescription;
       try {
         valueDescription = parser.getOptionValueDescription(flagName);
@@ -191,25 +178,52 @@ public final class InvocationPolicyEnforcer {
     }
   }
 
+  private static boolean policyApplies(FlagPolicy policy, ImmutableSet<String> applicableCommands) {
+    // Skip the flag policy if it doesn't apply to this command. If the commands list is empty,
+    // then the policy applies to all commands.
+    if (policy.getCommandsList().isEmpty() || applicableCommands.isEmpty()) {
+      return true;
+    }
+
+    return !Collections.disjoint(policy.getCommandsList(), applicableCommands);
+  }
+
   /**
    * Takes the provided policy and processes it to the form that can be used on the user options.
    *
    * <p>Expands any policies on expansion flags.
    */
   public static List<FlagPolicy> getEffectivePolicy(
-      InvocationPolicy invocationPolicy, OptionsParser parser) throws OptionsParsingException {
+      InvocationPolicy invocationPolicy, OptionsParser parser, String command)
+      throws OptionsParsingException {
     if (invocationPolicy == null) {
       return ImmutableList.of();
     }
 
+    ImmutableSet<String> commandAndParentCommands =
+        command == null
+            ? ImmutableSet.<String>of()
+            : CommandNameCache.CommandNameCacheInstance.INSTANCE.get(command);
+
     // Expand all policies to transfer policies on expansion flags to policies on the child flags.
     List<FlagPolicy> expandedPolicies = new ArrayList<>();
     for (FlagPolicy policy : invocationPolicy.getFlagPoliciesList()) {
+      if (!policyApplies(policy, commandAndParentCommands)) {
+        // Only keep and expand policies that are applicable to the current command.
+        continue;
+      }
       List<FlagPolicy> policies = expandPolicy(policy, parser);
       expandedPolicies.addAll(policies);
     }
 
-    return expandedPolicies;
+    // Only keep that last policy for each flag.
+    Map<String, FlagPolicy> effectivePolicy = new HashMap<>();
+    for (FlagPolicy expandedPolicy : expandedPolicies) {
+      String flagName = expandedPolicy.getFlagName();
+      effectivePolicy.put(flagName, expandedPolicy);
+    }
+
+    return new ArrayList<>(effectivePolicy.values());
   }
 
   /**
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/CanonicalizeCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/CanonicalizeCommand.java
index 2831e6babe..8033509309 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/CanonicalizeCommand.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/CanonicalizeCommand.java
@@ -70,7 +70,8 @@ public final class CanonicalizeCommand implements BlazeCommand {
       help =
           "Output the canonical policy, after expansion and filtering. To keep the output "
               + "clean, the canonicalized command arguments will NOT be shown when this option is "
-              + "set to true."
+              + "set to true. Note that the command specified by --for_command affects the "
+              + "filtered policy, and if none is specified, the default command is 'build'."
     )
     public boolean canonicalizePolicy;
 
@@ -148,7 +149,7 @@ public final class CanonicalizeCommand implements BlazeCommand {
       // Print out the canonical invocation policy if requested.
       if (canonicalizeOptions.canonicalizePolicy) {
         List<FlagPolicy> effectiveFlagPolicies =
-            InvocationPolicyEnforcer.getEffectivePolicy(policy, parser);
+            InvocationPolicyEnforcer.getEffectivePolicy(policy, parser, commandName);
         InvocationPolicy effectivePolicy =
             InvocationPolicy.newBuilder().addAllFlagPolicies(effectiveFlagPolicies).build();
         env.getReporter().getOutErr().printOutLn(effectivePolicy.toString());