In InvocationPolicyEnforcer#enforce, instead of just checking the direct command to see if a flag policy applies, check whether the flag applies by seeing if its list of commands matches one of the commands in the hierarchy.

This avoids the tedious and brittle specification of all commands that may use a flag, while providing better filtering out of inapplicable flags.

RELNOTES: A FlagPolicy specified via the --invocation_policy flag will now match the current command if any of its commands matches any of the commands the current command inherits from, as opposed to just the current command.

--
MOS_MIGRATED_REVID=121159131

5 files changed, 140 insertions, 13 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD
index 0faafca8da..e1b65493c7 100644
--- a/src/main/java/com/google/devtools/build/lib/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/BUILD
@@ -111,6 +111,7 @@ java_library(
     ]),
     deps = [
         ":io",
+        ":preconditions",
         "//src/main/java/com/google/devtools/common/options",
         "//src/main/protobuf:invocation_policy_java_proto",
         "//third_party:guava",
diff --git a/src/main/java/com/google/devtools/build/lib/flags/CommandNameCache.java b/src/main/java/com/google/devtools/build/lib/flags/CommandNameCache.java
new file mode 100644
index 0000000000..9c3b90aca0
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/flags/CommandNameCache.java
@@ -0,0 +1,41 @@
+// Copyright 2016 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.package com.google.devtools.build.lib.flags;
+package com.google.devtools.build.lib.flags;
+
+import com.google.common.collect.ImmutableSet;
+
+/** Cache mapping a command to the names of all commands it inherits from, including itself. */
+public interface CommandNameCache {
+  /** Class that exists only to expose a static instance variable that can be set and retrieved. */
+  class CommandNameCacheInstance implements CommandNameCache {
+    public static final CommandNameCacheInstance INSTANCE = new CommandNameCacheInstance();
+    private CommandNameCache delegate;
+
+    private CommandNameCacheInstance() {}
+
+    /** Only for use by {@code BlazeRuntime}. */
+    public void setCommandNameCache(CommandNameCache cache) {
+      // Can be set multiple times in tests.
+      this.delegate = cache;
+    }
+
+    @Override
+    public ImmutableSet<String> get(String commandName) {
+      return delegate.get(commandName);
+    }
+  }
+
+  /** Returns the names of all commands {@code commandName} inherits from, including itself. */
+  ImmutableSet<String> get(String commandName);
+}
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 abeaa59e49..a61c432f0c 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
@@ -19,6 +19,7 @@ import com.google.common.base.Functions;
 import com.google.common.base.Strings;
 import com.google.common.base.Verify;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
 import com.google.common.io.BaseEncoding;
 import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.AllowValues;
@@ -110,7 +111,7 @@ public final class InvocationPolicyEnforcer {
    * @throws OptionsParsingException if any flag policy is invalid.
    */
   public void enforce(OptionsParser parser) throws OptionsParsingException {
-    enforce(parser, "");
+    enforce(parser, null);
   }
 
   /**
@@ -118,29 +119,36 @@ public final class InvocationPolicyEnforcer {
    *
    * @param parser The OptionsParser to enforce policy on.
    * @param command The current blaze command, for flag policies that apply to only specific
-   *     commands.
+   *     commands. Such policies will be enforced only if they contain this command or a command
+   *     they inherit from
    * @throws OptionsParsingException if any flag policy is invalid.
    */
-  public void enforce(OptionsParser parser, String command) throws OptionsParsingException {
+  public void enforce(OptionsParser parser, @Nullable String command)
+      throws OptionsParsingException {
     if (invocationPolicy == null || invocationPolicy.getFlagPoliciesCount() == 0) {
       return;
     }
 
+    ImmutableSet<String> commandAndParentCommands =
+        command == null
+            ? ImmutableSet.<String>of()
+            : CommandNameCache.CommandNameCacheInstance.INSTANCE.get(command);
     for (FlagPolicy flagPolicy : invocationPolicy.getFlagPoliciesList()) {
       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()
-          && !flagPolicy.getCommandsList().contains(command)) {
-        log.info(
-            String.format(
-                "Skipping flag policy for flag '%s' because it "
-                    + "applies only to commands %s and the current command is '%s'",
-                flagName,
-                flagPolicy.getCommandsList(),
-                command));
-        continue;
+      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;
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
index 25f38d5fd2..58f09d8ce5 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
@@ -36,6 +36,7 @@ import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.analysis.config.ConfigurationFactory;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.OutputFilter;
+import com.google.devtools.build.lib.flags.CommandNameCache;
 import com.google.devtools.build.lib.packages.PackageFactory;
 import com.google.devtools.build.lib.packages.Preprocessor;
 import com.google.devtools.build.lib.packages.RuleClassProvider;
@@ -185,6 +186,8 @@ public final class BlazeRuntime {
 
     this.defaultsPackageContent =
         ruleClassProvider.getDefaultsPackageContent(getInvocationPolicy());
+    CommandNameCache.CommandNameCacheInstance.INSTANCE.setCommandNameCache(
+        new CommandNameCacheImpl(getCommandMap()));
   }
 
   private static InvocationPolicy createInvocationPolicyFromModules(
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CommandNameCacheImpl.java b/src/main/java/com/google/devtools/build/lib/runtime/CommandNameCacheImpl.java
new file mode 100644
index 0000000000..dd40207ecb
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/runtime/CommandNameCacheImpl.java
@@ -0,0 +1,74 @@
+// Copyright 2016 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.runtime;
+
+import com.google.common.base.Function;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Maps;
+import com.google.devtools.build.lib.flags.CommandNameCache;
+import com.google.devtools.build.lib.util.Preconditions;
+
+import java.util.ArrayDeque;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Queue;
+import java.util.Set;
+
+class CommandNameCacheImpl implements CommandNameCache {
+  private final Map<String, Command> commandMap;
+  private final Map<String, ImmutableSet<String>> cache = new HashMap<>();
+
+  CommandNameCacheImpl(Map<String, BlazeCommand> commandMap) {
+    // Note: it is important that this map is live, since the commandMap may be altered
+    // post-creation.
+    this.commandMap =
+        Maps.transformValues(
+            commandMap,
+            new Function<BlazeCommand, Command>() {
+              @Override
+              public Command apply(BlazeCommand blazeCommand) {
+                return blazeCommand.getClass().getAnnotation(Command.class);
+              }
+            });
+  }
+
+  @Override
+  public ImmutableSet<String> get(String commandName) {
+    ImmutableSet<String> cachedResult = cache.get(commandName);
+    if (cachedResult != null) {
+      return cachedResult;
+    }
+    ImmutableSet.Builder<String> builder = ImmutableSet.builder();
+
+    Command command = Preconditions.checkNotNull(commandMap.get(commandName), commandName);
+    Set<Command> visited = new HashSet<>();
+    visited.add(command);
+    Queue<Command> queue = new ArrayDeque<>();
+    queue.add(command);
+    while (!queue.isEmpty()) {
+      Command cur = queue.remove();
+      builder.add(cur.name());
+      for (Class<? extends BlazeCommand> clazz : cur.inherits()) {
+        Command parent = clazz.getAnnotation(Command.class);
+        if (visited.add(parent)) {
+          queue.add(parent);
+        }
+      }
+    }
+    cachedResult = builder.build();
+    cache.put(commandName, cachedResult);
+    return cachedResult;
+  }
+}