diff options
16 files changed, 740 insertions, 16 deletions
diff --git a/src/main/cpp/blaze.cc b/src/main/cpp/blaze.cc index bda1384e4f..329e841092 100644 --- a/src/main/cpp/blaze.cc +++ b/src/main/cpp/blaze.cc @@ -352,6 +352,13 @@ static vector<string> GetArgumentArray() { if (!globals->options.host_jvm_args.empty()) { result.push_back("--host_jvm_args=" + globals->options.host_jvm_args); } + + if (globals->options.invocation_policy != NULL && + strlen(globals->options.invocation_policy) > 0) { + result.push_back(string("--invocation_policy=") + + globals->options.invocation_policy); + } + globals->options.AddExtraOptions(&result); // The option sources are transmitted in the following format: diff --git a/src/main/cpp/blaze_startup_options.cc b/src/main/cpp/blaze_startup_options.cc index 7e09cdff43..7e0a158b43 100644 --- a/src/main/cpp/blaze_startup_options.cc +++ b/src/main/cpp/blaze_startup_options.cc @@ -55,6 +55,7 @@ BlazeStartupOptions::BlazeStartupOptions(const BlazeStartupOptions &rhs) allow_configurable_attributes(rhs.allow_configurable_attributes), option_sources(rhs.option_sources), webstatus_port(rhs.webstatus_port), + invocation_policy(rhs.invocation_policy), host_javabase(rhs.host_javabase) {} BlazeStartupOptions::~BlazeStartupOptions() { diff --git a/src/main/cpp/blaze_startup_options.h b/src/main/cpp/blaze_startup_options.h index 84db894168..f29ab3fcb0 100644 --- a/src/main/cpp/blaze_startup_options.h +++ b/src/main/cpp/blaze_startup_options.h @@ -198,6 +198,9 @@ class BlazeStartupOptions { // Port for web status server, 0 to disable int webstatus_port; + // Invocation policy proto. May be NULL. + const char* invocation_policy; + private: string host_javabase; diff --git a/src/main/cpp/blaze_startup_options_common.cc b/src/main/cpp/blaze_startup_options_common.cc index 29ba51cdda..914f7ab4e5 100644 --- a/src/main/cpp/blaze_startup_options_common.cc +++ b/src/main/cpp/blaze_startup_options_common.cc @@ -51,6 +51,7 @@ void BlazeStartupOptions::Init() { max_idle_secs = testing ? 5 : (3 * 3600); webstatus_port = 0; watchfs = false; + invocation_policy = NULL; } string BlazeStartupOptions::GetHostJavabase() { @@ -84,6 +85,7 @@ void BlazeStartupOptions::Copy( lhs->allow_configurable_attributes = rhs.allow_configurable_attributes; lhs->fatal_event_bus_exceptions = rhs.fatal_event_bus_exceptions; lhs->option_sources = rhs.option_sources; + lhs->invocation_policy = rhs.invocation_policy; } blaze_exit_code::ExitCode BlazeStartupOptions::ProcessArg( @@ -227,6 +229,16 @@ blaze_exit_code::ExitCode BlazeStartupOptions::ProcessArg( return blaze_exit_code::BAD_ARGV; } option_sources["webstatusserver"] = rcfile; + } else if ((value = GetUnaryOption(arg, next_arg, "--invocation_policy")) + != NULL) { + if (invocation_policy == NULL) { + invocation_policy = value; + option_sources["invocation_policy"] = rcfile; + } else { + *error = "The startup flag --invocation_policy cannot be specified " + "multiple times."; + return blaze_exit_code::BAD_ARGV; + } } else { bool extra_argument_processed; blaze_exit_code::ExitCode process_extra_arg_exit_code = ProcessArgExtra( diff --git a/src/main/java/BUILD b/src/main/java/BUILD index 84cbf0b81e..1e11519a24 100644 --- a/src/main/java/BUILD +++ b/src/main/java/BUILD @@ -448,10 +448,12 @@ java_library( ":util", ":vfs", "//src/main/protobuf:proto_build", + "//src/main/protobuf:proto_invocation_policy", "//src/main/protobuf:proto_test_status", "//third_party:guava", "//third_party:joda_time", "//third_party:jsr305", + "//third_party:protobuf", ], ) @@ -500,10 +502,12 @@ java_library( ":util", ":vfs", "//src/main/protobuf:proto_build", + "//src/main/protobuf:proto_invocation_policy", "//src/main/protobuf:proto_test_status", "//third_party:guava", "//third_party:joda_time", "//third_party:jsr305", + "//third_party:protobuf", ], ) 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 a441a39c4b..dbca029f67 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 @@ -148,9 +148,10 @@ public class BlazeCommandDispatcher { return ExitCode.SUCCESS; } - private CommonCommandOptions checkOptions(OptionsParser optionsParser, - Command commandAnnotation, List<String> args, List<String> rcfileNotes, OutErr outErr) + private void parseArgsAndConfigs(OptionsParser optionsParser, Command commandAnnotation, + List<String> args, List<String> rcfileNotes, OutErr outErr) throws OptionsParsingException { + Function<String, String> commandOptionSourceFunction = new Function<String, String>() { @Override public String apply(String input) { @@ -187,8 +188,6 @@ public class BlazeCommandDispatcher { configsLoaded = commonOptions.configs; commonOptions = optionsParser.getOptions(CommonCommandOptions.class); } - - return commonOptions; } /** @@ -270,13 +269,16 @@ public class BlazeCommandDispatcher { } OptionsParser optionsParser; - CommonCommandOptions commonOptions; // Delay output of notes regarding the parsed rc file, so it's possible to disable this in the // rc file. List<String> rcfileNotes = new ArrayList<>(); try { optionsParser = createOptionsParser(command); - commonOptions = checkOptions(optionsParser, commandAnnotation, args, rcfileNotes, outErr); + parseArgsAndConfigs(optionsParser, commandAnnotation, args, rcfileNotes, outErr); + + InvocationPolicyEnforcer optionsPolicyEnforcer = + InvocationPolicyEnforcer.create(getRuntime().getStartupOptionsProvider()); + optionsPolicyEnforcer.enforce(optionsParser, commandName); } catch (OptionsParsingException e) { for (String note : rcfileNotes) { outErr.printErrLn("INFO: " + note); @@ -299,6 +301,7 @@ public class BlazeCommandDispatcher { } } + CommonCommandOptions commonOptions = optionsParser.getOptions(CommonCommandOptions.class); BlazeRuntime.setupLogging(commonOptions.verbosity); // Do this before an actual crash so we don't have to worry about diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java index 244cb709f7..69d69da39d 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java @@ -235,4 +235,12 @@ public class BlazeServerStartupOptions extends OptionsBase { + "changes instead of scanning every file for a change.") public boolean watchFS; + + @Option(name = "invocation_policy", + defaultValue = "", + category = "undocumented", + help = "A base64-encoded-binary-serialized or text-formated " + + "invocation_policy.InvocationPolicy proto. Unlike other options, it is an error to " + + "specify --invocation_policy multiple times.") + public String invocationPolicy; } 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 new file mode 100644 index 0000000000..463e996684 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/runtime/InvocationPolicyEnforcer.java @@ -0,0 +1,404 @@ +// Copyright 2015 Google Inc. 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.CharMatcher; +import com.google.common.base.Function; +import com.google.common.base.Functions; +import com.google.common.base.Joiner; +import com.google.common.base.Verify; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.Sets; +import com.google.common.io.BaseEncoding; +import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.AllowValues; +import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.DisallowValues; +import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.FlagPolicy; +import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.InvocationPolicy; +import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.SetValue; +import com.google.devtools.common.options.OptionPriority; +import com.google.devtools.common.options.OptionsParser; +import com.google.devtools.common.options.OptionsParser.OptionDescription; +import com.google.devtools.common.options.OptionsParser.OptionValueDescription; +import com.google.devtools.common.options.OptionsParsingException; +import com.google.devtools.common.options.OptionsProvider; +import com.google.protobuf.InvalidProtocolBufferException; +import com.google.protobuf.TextFormat; + +import java.util.Arrays; +import java.util.List; +import java.util.Map; +import java.util.Map.Entry; +import java.util.Set; +import java.util.logging.Logger; + +import javax.annotation.Nullable; + +/** + * Given an OptionsParser and a InvocationPolicy proto, enforces the FlagPolicies on an + * OptionsParser. + * + * <p>"Flag" and "Option" are used interchangeably in this file. + */ +public final class InvocationPolicyEnforcer { + + /** + * Creates an {@link InvocationPolicyEnforcer} with the invocation policy obtained from the given + * {@link OptionsProvider}. This uses the provider only to obtain the policy from the + * --invocation_policy flag and does not enforce any policy on the flags in the provider. + * + * @param startupOptionsProvider an options provider which provides a BlazeServerStartupOptions + * options class + * + * @throws OptionsParsingException if the value of --invocation_policy is invalid + */ + public static InvocationPolicyEnforcer create(OptionsProvider startupOptionsProvider) + throws OptionsParsingException { + + BlazeServerStartupOptions blazeServerStartupOptions = + startupOptionsProvider.getOptions(BlazeServerStartupOptions.class); + return new InvocationPolicyEnforcer(parsePolicy(blazeServerStartupOptions.invocationPolicy)); + } + + /** + * Parses the given InvocationPolicy string, which may be a base64-encoded binary-serialized + * InvocationPolicy message, or a text formatted InvocationPolicy message. Note that the + * text format is not backwards compatible as the binary format is, and the option to + * provide a text formatted proto is provided only for debugging. + * + * @throws OptionsParsingException if the value of --invocation_policy is invalid + */ + private static InvocationPolicy parsePolicy(String policy) throws OptionsParsingException { + if (policy == null || policy.isEmpty()) { + return null; + } + + try { + try { + // First try decoding the policy as a base64 encoded binary proto. + return InvocationPolicy.parseFrom( + BaseEncoding.base64().decode(CharMatcher.WHITESPACE.removeFrom(policy))); + } catch (IllegalArgumentException e) { + // If the flag value can't be decoded from base64, try decoding the policy as a text + // formated proto. + InvocationPolicy.Builder builder = InvocationPolicy.newBuilder(); + TextFormat.merge(policy, builder); + return builder.build(); + } + } catch (InvalidProtocolBufferException | TextFormat.ParseException e) { + throw new OptionsParsingException("Malformed value of --invocation_policy: " + policy, e); + } + } + + private static final Logger LOG = Logger.getLogger(InvocationPolicyEnforcer.class.getName()); + + @Nullable + private final InvocationPolicy invocationPolicy; + + public InvocationPolicyEnforcer(@Nullable InvocationPolicy invocationPolicy) { + this.invocationPolicy = invocationPolicy; + } + + /** + * Applies this OptionsPolicyEnforcer's policy to the given OptionsParser. + * + * @param parser The OptionsParser to enforce policy on. + * @param command The command to which the options in the OptionsParser apply. + * @throws OptionsParsingException + */ + public void enforce(OptionsParser parser, String command) throws OptionsParsingException { + if (invocationPolicy == null) { + return; + } + + if (invocationPolicy.getFlagPoliciesCount() == 0) { + LOG.warning("InvocationPolicy contains no flag policies."); + } + + Function<Object, String> sourceFunction = Functions.constant("Invocation policy"); + + for (FlagPolicy flagPolicy : invocationPolicy.getFlagPoliciesList()) { + String flagName = flagPolicy.getFlagName(); + + // Skip the flag policy if it doesn't apply to this command. + 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; + } + + OptionValueDescription valueDescription; + try { + valueDescription = parser.getOptionValueDescription(flagName); + } 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. + LOG.info(String.format( + "Flag '%s' specified by invocation policy does not exist", flagName)); + continue; + } + + OptionDescription optionDescription = parser.getOptionDescription(flagName); + // getOptionDescription() will return null if the option does not exist, however + // getOptionValueDescription() above would have thrown an IllegalArgumentException if that + // were the case. + Verify.verifyNotNull(optionDescription); + + switch (flagPolicy.getOperationCase()) { + case SET_VALUE: + applySetValueOperation(parser, sourceFunction, flagPolicy, flagName, + valueDescription, optionDescription); + break; + + case USE_DEFAULT: + applyUseDefaultOperation(parser, flagName); + break; + + case ALLOW_VALUES: + applyAllowValuesOperation(parser, sourceFunction, flagPolicy, + flagName, valueDescription, optionDescription); + break; + + case DISALLOW_VALUES: + applyDisallowValuesOperation(parser, sourceFunction, flagPolicy, + flagName, valueDescription, optionDescription); + break; + + case OPERATION_NOT_SET: + throw new OptionsParsingException(String.format("Flag policy for flag '%s' does not " + + "have an operation", flagName)); + + default: + LOG.warning(String.format("Unknown operation '%s' from invocation policy for flag '%s'", + flagPolicy.getOperationCase(), flagName)); + break; + } + } + } + + private static void applySetValueOperation( + OptionsParser parser, + Function<Object, String> sourceFunction, + FlagPolicy flagPolicy, + String flagName, + OptionValueDescription valueDescription, + OptionDescription optionDescription) throws OptionsParsingException { + + SetValue setValue = flagPolicy.getSetValue(); + + // SetValue.flag_value must have at least 1 value. + if (setValue.getFlagValueCount() == 0) { + throw new OptionsParsingException(String.format( + "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. + LOG.info(String.format("Keeping value '%s' from source '%s' for flag '%s' " + + "because the invocation policy specifying the value(s) '%s' is overridable", + 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). + parser.clearValue(flagName); + + // Set all the flag values from the policy. + for (String flagValue : setValue.getFlagValueList()) { + if (valueDescription == null) { + LOG.info(String.format("Setting value for flag '%s' from invocation " + + "policy to '%s', overriding the default value '%s'", flagName, flagValue, + optionDescription.getDefaultValue())); + } else { + LOG.info(String.format("Setting value for flag '%s' from invocation " + + "policy to '%s', overriding value '%s' from '%s'", flagName, flagValue, + valueDescription.getValue(), valueDescription.getSource())); + } + setFlagValue(parser, flagName, flagValue, sourceFunction); + } + } + } + + private static void applyUseDefaultOperation(OptionsParser parser, String flagName) { + + Map<String, OptionValueDescription> clearedValues = parser.clearValue(flagName); + for (Entry<String, OptionValueDescription> clearedValue : clearedValues.entrySet()) { + + OptionValueDescription clearedValueDesc = clearedValue.getValue(); + String clearedFlagName = clearedValue.getKey(); + String originalValue = clearedValueDesc.getValue().toString(); + String source = clearedValueDesc.getSource(); + + OptionDescription clearedFlagDesc = parser.getOptionDescription(clearedFlagName); + Object clearedFlagdefaultValue = clearedFlagDesc.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)); + } + } + + private static void applyAllowValuesOperation( + OptionsParser parser, + Function<Object, String> sourceFunction, + FlagPolicy flagPolicy, + String flagName, + OptionValueDescription valueDescription, + OptionDescription optionDescription) throws OptionsParsingException { + + AllowValues allowValues = flagPolicy.getAllowValues(); + applyAllowDisallowValueOperation( + parser, + sourceFunction, + /*allowValues=*/ true, + allowValues.getAllowedValuesList(), + allowValues.hasNewDefaultValue() ? allowValues.getNewDefaultValue() : null, + flagName, + valueDescription, + optionDescription); + } + + private static void applyDisallowValuesOperation( + OptionsParser parser, + Function<Object, String> sourceFunction, + FlagPolicy flagPolicy, + String flagName, + OptionValueDescription valueDescription, + OptionDescription optionDescription) throws OptionsParsingException { + + DisallowValues disallowValues = flagPolicy.getDisallowValues(); + applyAllowDisallowValueOperation( + parser, + sourceFunction, + /*allowValues=*/ false, + disallowValues.getDisallowedValuesList(), + disallowValues.hasNewDefaultValue() ? disallowValues.getNewDefaultValue() : null, + flagName, + valueDescription, + optionDescription); + } + + /** + * Shared logic between AllowValues and DisallowValues operations. + * + * @param parser + * @param sourceFunction + * @param allowValues True if this is an AllowValues operation, false if DisallowValues + * @param policyValues The list of allowed or disallowed values + * @param newDefaultValue The new default to use if the default value for the flag is now allowed + * (i.e. not in the list of allowed values or in the list of disallowed values). + * @param flagName + * @param valueDescription + * @param optionDescription + * + * @throws OptionsParsingException + */ + private static void applyAllowDisallowValueOperation( + OptionsParser parser, + Function<Object, String> sourceFunction, + boolean allowValues, + List<String> policyValues, + String newDefaultValue, + String flagName, + OptionValueDescription valueDescription, + OptionDescription optionDescription) throws OptionsParsingException { + + // For error reporting. + String policyType = allowValues ? "Allow" : "Disallow"; + + // Convert all the allowed values from strings to real object using the option's + // converter so that they can be checked for equality using real .equals() instead + // of string comparison. For example, "--foo=0", "--foo=false", "--nofoo", and "-f-" + // (if the option has an abbreviation) are all equal for boolean flags. Plus converters + // can be arbitrarily complex. + Set<Object> convertedPolicyValues = Sets.newHashSet(); + for (String value : policyValues) { + convertedPolicyValues.add(optionDescription.getConverter().convert(value)); + } + + if (valueDescription == null) { + // Nothing has set the value yet, so check that the default value from the flag's + // definition is allowed. The else case below (i.e. valueDescription is not null) checks for + // the flag allowing multiple values, however, flags that allow multiple values cannot have + // default values, and their value is always the empty list if they haven't been specified, + // which is why new_default_value is not a repeated field. + // + // This is xor'ed with allowValues because if the policy is to allow these values, + // then we want to apply the new default (or throw an error) if the default value of the flag + // is not in the set of allowed values. If the policy is to disallow these values + // (allowValues is false), then we want to apply the new default (or throw an error) if + // the default value of the flag is in the set of disallowed values. This works out to xor. + if (allowValues ^ convertedPolicyValues.contains(optionDescription.getDefaultValue())) { + if (newDefaultValue != null) { + // Use the default value from the policy. + LOG.info(String.format("Overriding default value '%s' for flag '%s' with " + + "new default value '%s' specified by invocation policy. %sed values are: %s", + optionDescription.getDefaultValue(), flagName, newDefaultValue, + policyType, Joiner.on(", ").join(policyValues))); + parser.clearValue(flagName); + setFlagValue(parser, flagName, newDefaultValue, sourceFunction); + } else { + // The operation disallows the default value, but doesn't supply its own default. + throw new OptionsParsingException(String.format( + "Default flag value '%s' for flag '%s' is not allowed by invocation policy, but " + + "the policy does not provide a new default value. " + + "%sed values are: %s", optionDescription.getDefaultValue(), flagName, + policyType, Joiner.on(", ").join(policyValues))); + } + } + } else { + // Check that the flag's value is allowed. + List<?> values; + if (optionDescription.getAllowMultiple()) { + // allowMultiple requires that the type of the option be List<T>. + values = (List<?>) valueDescription.getValue(); + } else { + values = ImmutableList.of(valueDescription.getValue()); + } + + for (Object value : values) { + // See above about the xor. + if (allowValues ^ convertedPolicyValues.contains(value)) { + throw new OptionsParsingException(String.format( + "Flag value '%s' for flag '%s' is not allowed by invocation policy. " + + "%sed values are: %s", value, flagName, policyType, + Joiner.on(", ").join(policyValues))); + } + } + } + } + + private static void setFlagValue( + OptionsParser parser, + String flagName, + String flagValue, + Function<? super String, String> sourceFunction) throws OptionsParsingException { + + parser.parseWithSourceFunction(OptionPriority.INVOCATION_POLICY, sourceFunction, + Arrays.asList(String.format("--%s=%s", flagName, flagValue))); + } +} diff --git a/src/main/java/com/google/devtools/common/options/Option.java b/src/main/java/com/google/devtools/common/options/Option.java index f41a051c80..ca3add9bec 100644 --- a/src/main/java/com/google/devtools/common/options/Option.java +++ b/src/main/java/com/google/devtools/common/options/Option.java @@ -62,6 +62,9 @@ public @interface Option { * be a compile-time constant.) This special interpretation of the string * "null" is only applicable when computing the default value; if specified * on the command-line, this string will have its usual literal meaning. + * + * <p>The default value for flags that set allowMultiple to true is always + * the empty list and the value in the annotation is ignored. */ String defaultValue(); @@ -90,6 +93,9 @@ public @interface Option { * converter for this option must either match the parameter {@code T} or * {@code List<T>}. In the latter case the individual lists are concatenated * to form the full options value. + * + * <p>The {@link #defaultValue()} field of the annotation is ignored for repeatable + * flags and the default value will be the empty list. */ boolean allowMultiple() default false; diff --git a/src/main/java/com/google/devtools/common/options/OptionPriority.java b/src/main/java/com/google/devtools/common/options/OptionPriority.java index b352e88a6f..a28f012822 100644 --- a/src/main/java/com/google/devtools/common/options/OptionPriority.java +++ b/src/main/java/com/google/devtools/common/options/OptionPriority.java @@ -50,9 +50,13 @@ public enum OptionPriority { COMMAND_LINE, /** + * For options coming from invocation policy. + */ + INVOCATION_POLICY, + + /** * This priority can be used to unconditionally override any user-provided options. * This should be used rarely and with caution! */ SOFTWARE_REQUIREMENT; - } diff --git a/src/main/java/com/google/devtools/common/options/OptionsData.java b/src/main/java/com/google/devtools/common/options/OptionsData.java index d5512a1542..61d798a837 100644 --- a/src/main/java/com/google/devtools/common/options/OptionsData.java +++ b/src/main/java/com/google/devtools/common/options/OptionsData.java @@ -71,12 +71,19 @@ final class OptionsData { */ private final Map<Field, Converter<?>> converters; + /** + * Mapping from each Option-annotated field to a boolean for whether that field allows multiple + * values. + */ + private final Map<Field, Boolean> allowMultiple; + private OptionsData(Map<Class<? extends OptionsBase>, Constructor<?>> optionsClasses, Map<String, Field> nameToField, Map<Character, Field> abbrevToField, Map<Class<? extends OptionsBase>, List<Field>> allOptionsFields, Map<Field, Object> optionDefaults, - Map<Field, Converter<?>> converters) { + Map<Field, Converter<?>> converters, + Map<Field, Boolean> allowMultiple) { this.optionsClasses = ImmutableMap.copyOf(optionsClasses); this.allOptionsFields = ImmutableMap.copyOf(allOptionsFields); this.nameToField = ImmutableMap.copyOf(nameToField); @@ -84,6 +91,7 @@ final class OptionsData { // Can't use an ImmutableMap here because of null values. this.optionDefaults = Collections.unmodifiableMap(optionDefaults); this.converters = ImmutableMap.copyOf(converters); + this.allowMultiple = ImmutableMap.copyOf(allowMultiple); } public Collection<Class<? extends OptionsBase>> getOptionsClasses() { @@ -119,6 +127,10 @@ final class OptionsData { return converters.get(field); } + public boolean getAllowMultiple(Field field) { + return allowMultiple.get(field); + } + private static List<Field> getAllAnnotatedFields(Class<? extends OptionsBase> optionsClass) { List<Field> allFields = Lists.newArrayList(); for (Field field : optionsClass.getFields()) { @@ -157,6 +169,7 @@ final class OptionsData { Map<Character, Field> abbrevToFieldBuilder = Maps.newHashMap(); Map<Field, Object> optionDefaultsBuilder = Maps.newHashMap(); Map<Field, Converter<?>> convertersBuilder = Maps.newHashMap(); + Map<Field, Boolean> allowMultipleBuilder = Maps.newHashMap(); // Read all Option annotations: for (Class<? extends OptionsBase> parsedOptionsClass : classes) { @@ -256,9 +269,11 @@ final class OptionsData { optionDefaultsBuilder.put(field, retrieveDefaultFromAnnotation(field)); convertersBuilder.put(field, OptionsParserImpl.findConverter(field)); + + allowMultipleBuilder.put(field, annotation.allowMultiple()); } } return new OptionsData(constructorBuilder, nameToFieldBuilder, abbrevToFieldBuilder, - allOptionsFieldsBuilder, optionDefaultsBuilder, convertersBuilder); + allOptionsFieldsBuilder, optionDefaultsBuilder, convertersBuilder, allowMultipleBuilder); } } diff --git a/src/main/java/com/google/devtools/common/options/OptionsParser.java b/src/main/java/com/google/devtools/common/options/OptionsParser.java index 80e56cbcca..400adee114 100644 --- a/src/main/java/com/google/devtools/common/options/OptionsParser.java +++ b/src/main/java/com/google/devtools/common/options/OptionsParser.java @@ -187,6 +187,41 @@ public class OptionsParser implements OptionsProvider { } /** + * The metadata about an option. + */ + public static final class OptionDescription { + + private final String name; + private final Object defaultValue; + private final Converter<?> converter; + private final boolean allowMultiple; + + public OptionDescription(String name, Object defaultValue, Converter<?> converter, + boolean allowMultiple) { + this.name = name; + this.defaultValue = defaultValue; + this.converter = converter; + this.allowMultiple = allowMultiple; + } + + public String getName() { + return name; + } + + public Object getDefaultValue() { + return defaultValue; + } + + public Converter<?> getConverter() { + return converter; + } + + public boolean getAllowMultiple() { + return allowMultiple; + } + } + + /** * The name and value of an option with additional metadata describing its * priority, source, whether it was set via an implicit dependency, and if so, * by which other option. @@ -217,10 +252,16 @@ public class OptionsParser implements OptionsProvider { return value; } + /** + * @return the priority of the thing that set this value for this flag + */ public OptionPriority getPriority() { return priority; } + /** + * @return the thing that set this value for this flag + */ public String getSource() { return source; } @@ -450,10 +491,24 @@ public class OptionsParser implements OptionsProvider { } /** + * Returns a description of the option. + * + * @return The {@link OptionValueDescription} for the option, or null if there is no option by + * the given name. + */ + public OptionDescription getOptionDescription(String name) { + return impl.getOptionDescription(name); + } + + /** * Returns a description of the option value set by the last previous call to * {@link #parse(OptionPriority, String, List)} that successfully set the given * option. If the option is of type {@link List}, the description will * correspond to any one of the calls, but not necessarily the last. + * + * @return The {@link OptionValueDescription} for the option, or null if the value has not been + * set. + * @throws IllegalArgumentException if there is no option by the given name. */ public OptionValueDescription getOptionValueDescription(String name) { return impl.getOptionValueDescription(name); @@ -520,6 +575,23 @@ public class OptionsParser implements OptionsProvider { } } + /** + * Clears the given option. Also clears expansion arguments and implicit requirements for that + * option. + * + * <p>This will not affect options objects that have already been retrieved from this parser + * through {@link #getOptions(Class)}. + * + * @param optionName The full name of the option to clear. + * @return A map of an option name to the old value of the options that were cleared. + * @throws IllegalArgumentException If the flag does not exist. + */ + public Map<String, OptionValueDescription> clearValue(String optionName) { + Map<String, OptionValueDescription> clearedValues = Maps.newHashMap(); + impl.clearValue(optionName, clearedValues); + return clearedValues; + } + @Override public List<String> getResidue() { return ImmutableList.copyOf(residue); diff --git a/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java b/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java index bae5293439..59d4a0c524 100644 --- a/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java +++ b/src/main/java/com/google/devtools/common/options/OptionsParserImpl.java @@ -25,6 +25,7 @@ import com.google.common.collect.Iterables; import com.google.common.collect.ListMultimap; import com.google.common.collect.Lists; import com.google.common.collect.Maps; +import com.google.devtools.common.options.OptionsParser.OptionDescription; import com.google.devtools.common.options.OptionsParser.OptionValueDescription; import com.google.devtools.common.options.OptionsParser.UnparsedOptionValueDescription; @@ -433,6 +434,28 @@ class OptionsParserImpl { entry.addValue(priority, value); } + void clearValue(String optionName, Map<String, OptionValueDescription> clearedValues) { + Field field = optionsData.getFieldFromName(optionName); + if (field == null) { + throw new IllegalArgumentException("No such option '" + optionName + "'"); + } + + ParsedOptionEntry removed = parsedValues.remove(field); + if (removed != null) { + clearedValues.put(optionName, removed.asOptionValueDescription(optionName)); + } + + // Recurse to remove any implicit or expansion flags that this flag may have added when + // originally parsed. + Option option = field.getAnnotation(Option.class); + for (String implicitRequirement : option.implicitRequirements()) { + clearValue(implicitRequirement, clearedValues); + } + for (String expansion : option.expansion()) { + clearValue(expansion, clearedValues); + } + } + private Object getValue(Field field) { ParsedOptionEntry entry = parsedValues.get(field); return entry == null ? null : entry.getValue(); @@ -450,6 +473,20 @@ class OptionsParserImpl { return entry.asOptionValueDescription(name); } + OptionDescription getOptionDescription(String name) { + Field field = optionsData.getFieldFromName(name); + if (field == null) { + return null; + } + + Option optionAnnotation = field.getAnnotation(Option.class); + return new OptionDescription( + name, + optionsData.getDefaultValue(field), + optionsData.getConverter(field), + optionAnnotation.allowMultiple()); + } + boolean containsExplicitOption(String name) { Field field = optionsData.getFieldFromName(name); if (field == null) { @@ -475,7 +512,7 @@ class OptionsParserImpl { * of options; in that case, the arg seen last takes precedence. * * <p>The method uses the invariant that if an option has neither an implicit - * dependant nor an expanded from value, then it must have been explicitly + * dependent nor an expanded from value, then it must have been explicitly * set. */ private List<String> parse(OptionPriority priority, @@ -720,5 +757,4 @@ class OptionsParserImpl { throw new AssertionError(e); } } - } diff --git a/src/main/protobuf/BUILD b/src/main/protobuf/BUILD index 2fca35a0a8..6c2456a885 100644 --- a/src/main/protobuf/BUILD +++ b/src/main/protobuf/BUILD @@ -14,6 +14,7 @@ FILES = [ "bundlemerge", "xcodegen", "worker_protocol", + "invocation_policy", ] [proto_java_library( diff --git a/src/main/protobuf/invocation_policy.proto b/src/main/protobuf/invocation_policy.proto new file mode 100644 index 0000000000..8984c73c7d --- /dev/null +++ b/src/main/protobuf/invocation_policy.proto @@ -0,0 +1,147 @@ +// Copyright 2015 Google Inc. 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. + +syntax = "proto2"; +package blaze.invocation_policy; + +option java_package = "com.google.devtools.build.lib.runtime.proto"; + +// The --invocation_policy flag takes a base64-encoded binary-serialized or text +// formatted InvocationPolicy message. +message InvocationPolicy { + // Policies will be applied in order. Later policies will override + // previous policies if they conflict, which is important for flags + // that interact with each other. For example, if there is a flag "--foo" + // which is an expansion flag that expands into "--bar=x --baz=y", and the + // policy list first has a SetValue policy for "set --bar to z", and then has + // a SetDefault policy to set "--foo" to its default value, both --bar and + // --baz will get reset to their default values, overriding the SetValue + // operation. The SetDefault should come before the SetValue. + repeated FlagPolicy flag_policies = 1; +} + +// A policy for controlling the value of a flag. +message FlagPolicy { + // The name of the flag to enforce this policy on. + // + // Note that this should be the full name of the flag, not the abbreviated + // name of the flag. If the user specifies the abbreviated name of a flag, + // that flag will be matched using its full name. + // + // The "no" and "no_" prefixes will not be parsed, so for boolean flags, use + // the flag's full name and explicitly set it to true or false. + optional string flag_name = 1; + + // If set, this flag policy is applied only if one of the given commands is + // being run. If empty, this flag policy is applied for all commands. + // This allows the policy setter to add all policies to the proto without + // having to determine which Bazel command the user is actually running. + // Additionally, Bazel allows multiple flags to be defined by the same name, + // and the specific flag definition is determined by the command. + repeated string commands = 2; + + oneof operation { + SetValue set_value = 3; + UseDefault use_default = 4; + DisallowValues disallow_values = 5; + AllowValues allow_values = 6; + } +} + +message SetValue { + // Use this value for the specified flag, overriding any default or user-set + // value. + // + // This field is repeated for repeatable flags. It is an error to set + // multiple values for a flag that is not actually a repeatable flag. + // This requires at least 1 value, if even the empty string. + // + // If the flag allows multiple values, all of its values are replaced with the + // value or values from the policy (i.e., no diffing or merging is performed). + // + // Note that some flags are tricky. For example, some flags look like boolean + // flags, but are actually Void expansion flags that expand into other flags. + // The Bazel flag parser will accept "--void_flag=false", but because + // the flag is Void, the "=false" is ignored. It can get even trickier, like + // "--novoid_flag" which is also an expansion flag with the type Void whose + // name is explicitly "novoid_flag" and which expands into other flags that + // are the opposite of "--void_flag". For expansion flags, it's best to + // explicitly override the flags they expand into. + // + // Other flags may be differently tricky: A flag could have a converter that + // converts some string to a list of values, but that flag may not itself have + // allowMultiple set to true. + // + // An example is "--test_tag_filters": this flag sets its converter to + // CommaSeparatedOptionListConverter, but does not set allowMultiple to true. + // So "--test_tag_filters=foo,bar" results in ["foo", "bar"], however + // "--test_tag_filters=foo --test_tag_filters=bar" results in just ["bar"] + // since the 2nd value overrides the 1st. + // + // Similarly, "--test_tag_filters=foo,bar --test_tag_filters=baz,qux" results + // in ["baz", "qux"]. For flags like these, the policy should specify + // "foo,bar" instead of separately specifying "foo" and "bar" so that the + // converter is appropriately invoked. + // + // Note that the opposite is not necessarily + // true: for a flag that specifies allowMultiple=true, "--flag=foo,bar" + // may fail to parse or result in an unexpected value. + repeated string flag_value = 1; + + // Whether to allow this policy to be overridden by user-specified values. + // When set, if the user specified a value for this flag, use the value + // from the user, otherwise use the value specified in this policy. + optional bool overridable = 2; +} + +message UseDefault { + // Use the default value of the flag, as defined by Bazel (or equivalently, do + // not allow the user to set this flag). +} + +message DisallowValues { + // It is an error for the user to use any of these values (that is, the Bazel + // command will fail). + // + // For repeatable flags, if any one of the values in the flag matches a value + // in the list of disallowed values, an error is thrown. + // + // Care must be taken for flags with complicated converters. For example, + // it's possible for a repeated flag to be of type List<List<T>>, so that + // "--foo=a,b --foo=c,d" results in foo=[["a","b"], ["c", "d"]]. In this case, + // it is not possible to disallow just "b", nor will ["b", "a"] match, nor + // will ["b", "c"] (but ["a", "b"] will still match). + repeated string disallowed_values = 1; + + // If the default value of the flag is a disallowed value, use this + // as the default if the user doesn't specify the flag. + // Similar to doing a SetValue with overridable set to true, but also + // wanting to limit the available values. Note that flags that set + // allowMultiple to true cannot have default values (they default to the + // empty list), which is why this field is optional and not repeated. + optional string new_default_value = 2; +} + +message AllowValues { + // It is an error for the user to use any value not in this list. + repeated string allowed_values = 1; + + // If the default value of the flag is a not an allowed value, use this + // as the default if the user doesn't specify the flag. + // Similar to doing a SetValue with overridable set to true, but also + // wanting to limit the available values. Note that flags that set + // allowMultiple to true cannot have default values (they default to the + // empty list), which is why this field is optional and not repeated. + optional string new_default_value = 2; +} diff --git a/src/test/java/com/google/devtools/common/options/OptionsParserTest.java b/src/test/java/com/google/devtools/common/options/OptionsParserTest.java index ecb8bda54e..0bb8a15758 100644 --- a/src/test/java/com/google/devtools/common/options/OptionsParserTest.java +++ b/src/test/java/com/google/devtools/common/options/OptionsParserTest.java @@ -715,11 +715,12 @@ public class OptionsParserTest { // in the code. @Test public void optionPrioritiesAreCorrectlyOrdered() throws Exception { - assertEquals(5, OptionPriority.values().length); - assertEquals(-1, OptionPriority.DEFAULT.compareTo(OptionPriority.COMPUTED_DEFAULT)); - assertEquals(-1, OptionPriority.COMPUTED_DEFAULT.compareTo(OptionPriority.RC_FILE)); - assertEquals(-1, OptionPriority.RC_FILE.compareTo(OptionPriority.COMMAND_LINE)); - assertEquals(-1, OptionPriority.COMMAND_LINE.compareTo(OptionPriority.SOFTWARE_REQUIREMENT)); + assertEquals(6, OptionPriority.values().length); + assertThat(OptionPriority.DEFAULT).isLessThan(OptionPriority.COMPUTED_DEFAULT); + assertThat(OptionPriority.COMPUTED_DEFAULT).isLessThan(OptionPriority.RC_FILE); + assertThat(OptionPriority.RC_FILE).isLessThan(OptionPriority.COMMAND_LINE); + assertThat(OptionPriority.COMMAND_LINE).isLessThan(OptionPriority.INVOCATION_POLICY); + assertThat(OptionPriority.INVOCATION_POLICY).isLessThan(OptionPriority.SOFTWARE_REQUIREMENT); } public static class IntrospectionExample extends OptionsBase { |