Update ANDROID_DEFAULT_CROSSTOOL to default to //external:android/crosstool. Remove the constant.

--
MOS_MIGRATED_REVID=115555161

12 files changed, 113 insertions, 27 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/Constants.java b/src/main/java/com/google/devtools/build/lib/Constants.java
index cdf54dba83..e080f26a3d 100644
--- a/src/main/java/com/google/devtools/build/lib/Constants.java
+++ b/src/main/java/com/google/devtools/build/lib/Constants.java
@@ -37,9 +37,9 @@ public final class Constants {
   // Native Java deps are all linked into a single file, which is named with this value + ".so".
   public static final String NATIVE_DEPS_LIB_SUFFIX = "_nativedeps";
 
-  // Locations of implicit Android SDK and NDK dependencies.
+  // Locations of implicit Android SDK dependencies.
   public static final String ANDROID_DEFAULT_SDK = "//external:android/sdk".toString();
-  public static final String ANDROID_DEFAULT_CROSSTOOL = "//external:android/crosstool".toString();
+
   // If the --fat_apk_cpu flag is not set, we use this as the default value.
   public static final ImmutableList<String> ANDROID_DEFAULT_FAT_APK_CPUS =
       ImmutableList.<String>of("armeabi-v7a");
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
index 135d339239..96a79209d8 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildConfiguration.java
@@ -317,6 +317,22 @@ public final class BuildConfiguration {
   }
 
   /**
+   * A converter that returns null if the input string is empty, otherwise it converts
+   * the input to a label.
+   */
+  public static class EmptyToNullLabelConverter implements Converter<Label> {
+    @Override
+    public Label convert(String input) throws OptionsParsingException {
+      return input.isEmpty() ? null : convertLabel(input);
+    }
+
+    @Override
+    public String getTypeDescription() {
+      return "a build target label";
+    }
+  }
+
+  /**
    * A label converter that returns a default value if the input string is empty.
    */
   public static class DefaultLabelConverter implements Converter<Label> {
diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java
index 7a52637383..d29a57fe74 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidConfiguration.java
@@ -20,6 +20,7 @@ import com.google.common.collect.Multimap;
 import com.google.devtools.build.lib.Constants;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration.DefaultLabelConverter;
+import com.google.devtools.build.lib.analysis.config.BuildConfiguration.EmptyToNullLabelConverter;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Fragment;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration.StrictDepsConverter;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration.StrictDepsMode;
@@ -43,13 +44,6 @@ import java.util.List;
 @Immutable
 public class AndroidConfiguration extends BuildConfiguration.Fragment {
 
-  /** Converter for --android_crosstool_top. */
-  public static class AndroidCrosstoolTopConverter extends DefaultLabelConverter {
-    public AndroidCrosstoolTopConverter() {
-      super(Constants.ANDROID_DEFAULT_CROSSTOOL);
-    }
-  }
-
   /** Converter for --android_sdk. */
   public static class AndroidSdkConverter extends DefaultLabelConverter {
     public AndroidSdkConverter() {
@@ -109,9 +103,9 @@ public class AndroidConfiguration extends BuildConfiguration.Fragment {
     public boolean incrementalNativeLibs;
 
     @Option(name = "android_crosstool_top",
-        defaultValue = "",
+        defaultValue = "//external:android/crosstool",
         category = "semantics",
-        converter = AndroidCrosstoolTopConverter.class,
+        converter = EmptyToNullLabelConverter.class,
         help = "The location of the C++ compiler used for Android builds.")
     public Label androidCrosstoolTop;
 
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java
index fc8a225271..a426f47fd5 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java
@@ -288,6 +288,9 @@ public class BlazeCommandDispatcher {
       parseArgsAndConfigs(optionsParser, commandAnnotation, args, rcfileNotes, outErr);
 
       InvocationPolicyEnforcer optionsPolicyEnforcer =
+          new InvocationPolicyEnforcer(runtime.getInvocationPolicy());
+      optionsPolicyEnforcer.enforce(optionsParser, commandName);
+      optionsPolicyEnforcer =
           InvocationPolicyEnforcer.create(getRuntime().getStartupOptionsProvider());
       optionsPolicyEnforcer.enforce(optionsParser, commandName);
     } catch (OptionsParsingException e) {
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java
index 2c23469f1a..66739ad603 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java
@@ -33,6 +33,7 @@ import com.google.devtools.build.lib.packages.Preprocessor;
 import com.google.devtools.build.lib.query2.engine.QueryEnvironment.QueryFunction;
 import com.google.devtools.build.lib.query2.output.OutputFormatter;
 import com.google.devtools.build.lib.rules.test.CoverageReportActionFactory;
+import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.InvocationPolicy;
 import com.google.devtools.build.lib.skyframe.DiffAwareness;
 import com.google.devtools.build.lib.skyframe.PrecomputedValue.Injected;
 import com.google.devtools.build.lib.skyframe.SkyValueDirtinessChecker;
@@ -414,4 +415,12 @@ public abstract class BlazeModule {
   public CoverageReportActionFactory getCoverageReportFactory() {
     return null;
   }
+
+  /**
+   * Optionally returns the invocation policy to override options in blaze.
+   */
+  @Nullable
+  public InvocationPolicy getInvocationPolicy() {
+    return null;
+  }
 }
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 9cd7a56edc..d33baeb7f7 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
@@ -71,6 +71,7 @@ import com.google.devtools.build.lib.runtime.commands.RunCommand;
 import com.google.devtools.build.lib.runtime.commands.ShutdownCommand;
 import com.google.devtools.build.lib.runtime.commands.TestCommand;
 import com.google.devtools.build.lib.runtime.commands.VersionCommand;
+import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.InvocationPolicy;
 import com.google.devtools.build.lib.server.RPCServer;
 import com.google.devtools.build.lib.server.ServerCommand;
 import com.google.devtools.build.lib.server.signal.InterruptSignalHandler;
@@ -168,6 +169,8 @@ public final class BlazeRuntime {
   private final BinTools binTools;
   private final WorkspaceStatusAction.Factory workspaceStatusActionFactory;
   private final ProjectFile.Provider projectFileProvider;
+  @Nullable
+  private final InvocationPolicy invocationPolicy;
 
   // Workspace state (currently exactly one workspace per server)
   private final BlazeDirectories directories;
@@ -187,7 +190,7 @@ public final class BlazeRuntime {
       TimestampGranularityMonitor timestampGranularityMonitor,
       SubscriberExceptionHandler eventBusExceptionHandler,
       BinTools binTools, ProjectFile.Provider projectFileProvider,
-      Iterable<BlazeCommand> commands) {
+      InvocationPolicy invocationPolicy, Iterable<BlazeCommand> commands) {
     // Server state
     this.blazeModules = blazeModules;
     overrideCommands(commands);
@@ -196,6 +199,7 @@ public final class BlazeRuntime {
     this.packageFactory = pkgFactory;
     this.binTools = binTools;
     this.projectFileProvider = projectFileProvider;
+    this.invocationPolicy = invocationPolicy;
 
     this.ruleClassProvider = ruleClassProvider;
     this.configurationFactory = configurationFactory;
@@ -215,6 +219,21 @@ public final class BlazeRuntime {
     setupExecRoot();
   }
 
+  private static InvocationPolicy createInvocationPolicyFromModules(
+      InvocationPolicy initialInvocationPolicy,
+      Iterable<BlazeModule> modules) {
+    InvocationPolicy.Builder builder = InvocationPolicy.newBuilder();
+    builder.mergeFrom(initialInvocationPolicy);
+    // Merge the policies from the modules
+    for (BlazeModule module : modules) {
+      InvocationPolicy modulePolicy = module.getInvocationPolicy();
+      if (modulePolicy != null) {
+        builder.mergeFrom(module.getInvocationPolicy());
+      }
+    }
+    return builder.build();
+  }
+
   @Nullable CoverageReportActionFactory getCoverageReportActionFactory() {
     CoverageReportActionFactory firstFactory = null;
     for (BlazeModule module : blazeModules) {
@@ -266,6 +285,11 @@ public final class BlazeRuntime {
     skyframeExecutor.setEventBus(null);
   }
 
+  @Nullable
+  public InvocationPolicy getInvocationPolicy() {
+    return invocationPolicy;
+  }
+
   /**
    * Conditionally enable profiling.
    */
@@ -1305,6 +1329,7 @@ public final class BlazeRuntime {
     private BinTools binTools;
     private UUID instanceId;
     private final List<BlazeCommand> commands = new ArrayList<>();
+    private InvocationPolicy invocationPolicy = InvocationPolicy.getDefaultInstance();
 
     public BlazeRuntime build() throws AbruptExitException {
       Preconditions.checkNotNull(directories);
@@ -1453,10 +1478,13 @@ public final class BlazeRuntime {
         }
       }
 
+      invocationPolicy = createInvocationPolicyFromModules(invocationPolicy, blazeModules);
+
       return new BlazeRuntime(directories, workspaceStatusActionFactory, skyframeExecutor,
           pkgFactory, ruleClassProvider, configurationFactory,
           clock, startupOptionsProvider, ImmutableList.copyOf(blazeModules),
-          timestampMonitor, eventBusExceptionHandler, binTools, projectFileProvider, commands);
+          timestampMonitor, eventBusExceptionHandler, binTools, projectFileProvider,
+          invocationPolicy, commands);
     }
 
     public Builder setBinTools(BinTools binTools) {
@@ -1464,6 +1492,11 @@ public final class BlazeRuntime {
       return this;
     }
 
+    public Builder setInvocationPolicy(InvocationPolicy invocationPolicy) {
+      this.invocationPolicy = invocationPolicy;
+      return this;
+    }
+
     public Builder setDirectories(BlazeDirectories directories) {
       this.directories = directories;
       return this;
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/InvocationPolicyEnforcer.java b/src/main/java/com/google/devtools/build/lib/runtime/InvocationPolicyEnforcer.java
index 7426272281..a87575d728 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/InvocationPolicyEnforcer.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/InvocationPolicyEnforcer.java
@@ -109,7 +109,7 @@ public final class InvocationPolicyEnforcer {
 
   private static final Function<Object, String> INVOCATION_POLICY_SOURCE =
       Functions.constant("Invocation policy");
-  
+
   @Nullable
   private final InvocationPolicy invocationPolicy;
 
@@ -124,6 +124,16 @@ public final class InvocationPolicyEnforcer {
   }
 
   /**
+   * Applies this OptionsPolicyEnforcer's policy to the given OptionsParser for all blaze commands.
+   *
+   * @param parser The OptionsParser to enforce policy on.
+   * @throws OptionsParsingException if any flag policy is invalid.
+   */
+  public void enforce(OptionsParser parser) throws OptionsParsingException {
+    enforce(parser, "");
+  }
+
+  /**
    * Applies this OptionsPolicyEnforcer's policy to the given OptionsParser.
    *
    * @param parser The OptionsParser to enforce policy on.
@@ -160,7 +170,7 @@ public final class InvocationPolicyEnforcer {
       } catch (IllegalArgumentException e) {
         // This flag doesn't exist. We are deliberately lenient if the flag policy has a flag
         // we don't know about. This is for better future proofing so that as new flags are added,
-        // new policies can use the new flags without worrying about older versions of Bazel. 
+        // new policies can use the new flags without worrying about older versions of Bazel.
         log.info(String.format(
             "Flag '%s' specified by invocation policy does not exist", flagName));
         continue;
@@ -231,14 +241,14 @@ public final class InvocationPolicyEnforcer {
           "SetValue operation from invocation policy for flag '%s' does not have a value",
           flagName));
     }
-  
+
     // Flag must allow multiple values if multiple values are specified by the policy.
     if (setValue.getFlagValueCount() > 1 && !optionDescription.getAllowMultiple()) {
       throw new OptionsParsingException(String.format(
           "SetValue operation from invocation policy sets multiple values for flag '%s' which "
               + "does not allow multiple values", flagName));
     }
-  
+
     if (setValue.getOverridable() && valueDescription != null) {
       // The user set the value for the flag but the flag policy is overridable, so keep the user's
       // value.
@@ -247,7 +257,7 @@ public final class InvocationPolicyEnforcer {
           valueDescription.getValue(), valueDescription.getSource(), flagName,
           setValue.getFlagValueList()));
     } else {
-  
+
       // Clear the value in case the flag is a repeated flag (so that values don't accumulate), and
       // in case the flag is an expansion flag or has implicit flags (so that the additional flags
       // also get cleared).
@@ -273,15 +283,15 @@ public final class InvocationPolicyEnforcer {
 
     Map<String, OptionValueDescription> clearedValues = parser.clearValue(flagName);
     for (Entry<String, OptionValueDescription> clearedValue : clearedValues.entrySet()) {
-  
+
       OptionValueDescription clearedValueDescription = clearedValue.getValue();
       String clearedFlagName = clearedValue.getKey();
       String originalValue = clearedValueDescription.getValue().toString();
       String source = clearedValueDescription.getSource();
-  
+
       Object clearedFlagDefaultValue = parser.getOptionDescription(clearedFlagName)
           .getDefaultValue();
-  
+
       log.info(String.format("Using default value '%s' for flag '%s' as "
               + "specified by invocation policy, overriding original value '%s' from '%s'",
           clearedFlagDefaultValue, clearedFlagName, originalValue, source));
@@ -310,7 +320,7 @@ public final class InvocationPolicyEnforcer {
         return !convertedPolicyValues.contains(value);
       }
     };
-    
+
     private final String policyType;
 
     FilterValueOperation(String policyType) {
@@ -325,7 +335,7 @@ public final class InvocationPolicyEnforcer {
      * @return True if the value should be allowed, false if it should not.
      */
     abstract boolean filter(Set<Object> convertedPolicyValues, Object value);
-    
+
     void apply(
         OptionsParser parser,
         List<String> policyValues,
@@ -333,7 +343,7 @@ public final class InvocationPolicyEnforcer {
         String flagName,
         OptionValueDescription valueDescription,
         OptionDescription optionDescription) throws OptionsParsingException {
-      
+
       // Convert all the allowed values from strings to real objects using the options'
       // converters so that they can be checked for equality using real .equals() instead
       // of string comparison. For example, "--foo=0", "--foo=false", "--nofoo", and "-f-"
@@ -366,7 +376,7 @@ public final class InvocationPolicyEnforcer {
             convertedPolicyValues);
       }
     }
-    
+
     void checkDefaultValue(
         OptionsParser parser,
         List<String> policyValues,
@@ -394,7 +404,7 @@ public final class InvocationPolicyEnforcer {
         }
       }
     }
-    
+
     void checkUserValue(
         List<String> policyValues,
         String flagName,
@@ -427,7 +437,7 @@ public final class InvocationPolicyEnforcer {
       OptionsParser parser,
       String flagName,
       String flagValue) throws OptionsParsingException {
- 
+
     parser.parseWithSourceFunction(OptionPriority.INVOCATION_POLICY, INVOCATION_POLICY_SOURCE,
         Arrays.asList(String.format("--%s=%s", flagName, flagValue)));
   }
diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD
index 7d2a2d0637..506bc41fc3 100644
--- a/src/test/java/com/google/devtools/build/lib/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/BUILD
@@ -28,6 +28,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib:util",
         "//src/main/java/com/google/devtools/build/lib:vfs",
         "//src/main/java/com/google/devtools/build/lib/actions",
+        "//src/main/protobuf:invocation_policy_java_proto",
         "//third_party:guava",
         "//third_party:guava-testlib",
         "//third_party:junit4",
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
index 3177249cb8..4d16cec628 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java
@@ -46,6 +46,7 @@ import com.google.devtools.build.lib.pkgcache.LoadingResult;
 import com.google.devtools.build.lib.pkgcache.PackageCacheOptions;
 import com.google.devtools.build.lib.pkgcache.PackageManager;
 import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
+import com.google.devtools.build.lib.runtime.InvocationPolicyEnforcer;
 import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
 import com.google.devtools.build.lib.skyframe.DiffAwareness;
 import com.google.devtools.build.lib.skyframe.PrecomputedValue;
@@ -200,6 +201,10 @@ public abstract class AnalysisTestCase extends FoundationTestCase {
         ruleClassProvider.getConfigurationOptions()));
     optionsParser.parse(new String[] {"--default_visibility=public" });
     optionsParser.parse(args);
+
+    InvocationPolicyEnforcer optionsPolicyEnforcer =
+        new InvocationPolicyEnforcer(TestConstants.TEST_INVOCATION_POLICY);
+    optionsPolicyEnforcer.enforce(optionsParser);
   }
 
   protected FlagBuilder defaultFlags() {
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
index d32a0c68d1..a369cf55b7 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
@@ -117,6 +117,7 @@ import com.google.devtools.build.lib.pkgcache.TransitivePackageLoader;
 import com.google.devtools.build.lib.rules.extra.ExtraAction;
 import com.google.devtools.build.lib.rules.test.BaselineCoverageAction;
 import com.google.devtools.build.lib.rules.test.InstrumentedFilesProvider;
+import com.google.devtools.build.lib.runtime.InvocationPolicyEnforcer;
 import com.google.devtools.build.lib.skyframe.AspectValue;
 import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
 import com.google.devtools.build.lib.skyframe.DiffAwareness;
@@ -277,6 +278,10 @@ public abstract class BuildViewTestCase extends FoundationTestCase {
       optionsParser.parse(configurationArgs);
       optionsParser.parse(args);
 
+      InvocationPolicyEnforcer optionsPolicyEnforcer =
+            new InvocationPolicyEnforcer(TestConstants.TEST_INVOCATION_POLICY);
+      optionsPolicyEnforcer.enforce(optionsParser, "");
+
       configurationFactory.forbidSanityCheck();
       BuildOptions buildOptions = ruleClassProvider.createBuildOptions(optionsParser);
       ensureTargetsVisited(buildOptions.getAllLabels().values());
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java
index 1d0f6384ae..654f838614 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/ConfigurationTestCase.java
@@ -34,6 +34,7 @@ import com.google.devtools.build.lib.packages.Preprocessor;
 import com.google.devtools.build.lib.packages.util.MockToolsConfig;
 import com.google.devtools.build.lib.pkgcache.PackageCacheOptions;
 import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
+import com.google.devtools.build.lib.runtime.InvocationPolicyEnforcer;
 import com.google.devtools.build.lib.skyframe.DiffAwareness;
 import com.google.devtools.build.lib.skyframe.PrecomputedValue;
 import com.google.devtools.build.lib.skyframe.SequencedSkyframeExecutor;
@@ -149,6 +150,11 @@ public abstract class ConfigurationTestCase extends FoundationTestCase {
         .add(TestOptions.class)
         .build());
     parser.parse(args);
+
+    InvocationPolicyEnforcer optionsPolicyEnforcer =
+        new InvocationPolicyEnforcer(TestConstants.TEST_INVOCATION_POLICY);
+    optionsPolicyEnforcer.enforce(parser);
+
     ImmutableSortedSet<String> multiCpu = ImmutableSortedSet.copyOf(
         parser.getOptions(TestOptions.class).multiCpus);
 
diff --git a/src/test/java/com/google/devtools/build/lib/testutil/TestConstants.java b/src/test/java/com/google/devtools/build/lib/testutil/TestConstants.java
index 96bcfcaf00..6d4fb51bef 100644
--- a/src/test/java/com/google/devtools/build/lib/testutil/TestConstants.java
+++ b/src/test/java/com/google/devtools/build/lib/testutil/TestConstants.java
@@ -15,6 +15,7 @@
 package com.google.devtools.build.lib.testutil;
 
 import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.InvocationPolicy;
 
 /**
  * Various constants required by the tests.
@@ -76,4 +77,7 @@ public class TestConstants {
 
   public static final ImmutableList<String> DOCS_RULES_PATHS = ImmutableList.of(
       "src/main/java/com/google/devtools/build/lib/rules");
+
+  public static final InvocationPolicy TEST_INVOCATION_POLICY =
+      InvocationPolicy.getDefaultInstance();
 }