From 59755455034a998cdedfb7b086aea3ad78419381 Mon Sep 17 00:00:00 2001 From: Yun Peng Date: Tue, 27 Aug 2019 02:26:35 -0700 Subject: [PATCH] bazelrc: support --enable_platform_specific_config When the value of this flag is true, a host platform specific config section will be enabled if it exists. Bazel recognizes your host platform as linux, macos, windows or freebsd. It's equivalent to add --config=linux on Linux platforms and --config=windows on Windows, etc. Fixes https://github.com/bazelbuild/bazel/issues/5055 RELNOTES[NEW]: Platform-specific bazelrc: with --enable_platform_specific_config you can enable flags in bazelrc according to your host platform. PiperOrigin-RevId: 265641013 --- .../build/lib/runtime/BlazeOptionHandler.java | 74 ++++++++++++++---- .../lib/runtime/CommonCommandOptions.java | 13 ++++ .../lib/runtime/BlazeOptionHandlerTest.java | 76 ++++++++++++++++++- 3 files changed, 146 insertions(+), 17 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeOptionHandler.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeOptionHandler.java index 9944a75537cb43..2584bfb78d8973 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeOptionHandler.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeOptionHandler.java @@ -18,6 +18,7 @@ import com.google.common.collect.ArrayListMultimap; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; import com.google.common.collect.ListMultimap; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventHandler; @@ -25,6 +26,7 @@ import com.google.devtools.build.lib.runtime.commands.ProjectFileSupport; import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.InvocationPolicy; import com.google.devtools.build.lib.util.ExitCode; +import com.google.devtools.build.lib.util.OS; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.common.options.InvocationPolicyEnforcer; @@ -315,6 +317,42 @@ ExitCode parseOptions(List args, ExtendedEventHandler eventHandler) { return ExitCode.SUCCESS; } + private static String getPlatformName() { + switch (OS.getCurrent()) { + case LINUX: + return "linux"; + case DARWIN: + return "macos"; + case WINDOWS: + return "windows"; + case FREEBSD: + return "freebsd"; + default: + return OS.getCurrent().getCanonicalName(); + } + } + + /** + * If --enable_platform_specific_config is true and the corresponding config definition exists, we + * should enable the platform specific config. + */ + private boolean shouldEnablePlatformSpecificConfig( + OptionValueDescription enablePlatformSpecificConfigDescription, + ListMultimap commandToRcArgs) { + if (enablePlatformSpecificConfigDescription == null + || !(boolean) enablePlatformSpecificConfigDescription.getValue()) { + return false; + } + + for (String commandName : getCommandNamesToParse(commandAnnotation)) { + String defaultConfigDef = commandName + ":" + getPlatformName(); + if (commandToRcArgs.containsKey(defaultConfigDef)) { + return true; + } + } + return false; + } + /** * Expand the values of --config according to the definitions provided in the rc files and the * applicable command. @@ -325,23 +363,31 @@ void expandConfigOptions( OptionValueDescription configValueDescription = optionsParser.getOptionValueDescription("config"); - if (configValueDescription == null || configValueDescription.getCanonicalInstances() == null) { - // No --config values were set, we can avoid this whole thing. - return; + if (configValueDescription != null && configValueDescription.getCanonicalInstances() != null) { + // Find the base set of configs. This does not include the config options that might be + // recursively included. + ImmutableList configInstances = + ImmutableList.copyOf(configValueDescription.getCanonicalInstances()); + + // Expand the configs that are mentioned in the input. Flatten these expansions before parsing + // them, to preserve order. + for (ParsedOptionDescription configInstance : configInstances) { + String configValueToExpand = (String) configInstance.getConvertedValue(); + List expansion = getExpansion(eventHandler, commandToRcArgs, configValueToExpand); + optionsParser.parseArgsAsExpansionOfOption( + configInstance, String.format("expanded from --%s", configValueToExpand), expansion); + } } - // Find the base set of configs. This does not include the config options that might be - // recursively incuded. - ImmutableList configInstances = - ImmutableList.copyOf(configValueDescription.getCanonicalInstances()); - - // Expand the configs that are mentioned in the input. Flatten these expansions before parsing - // them, to preserve order. - for (ParsedOptionDescription configInstance : configInstances) { - String configValueToExpand = (String) configInstance.getConvertedValue(); - List expansion = getExpansion(eventHandler, commandToRcArgs, configValueToExpand); + OptionValueDescription enablePlatformSpecificConfigDescription = + optionsParser.getOptionValueDescription("enable_platform_specific_config"); + if (shouldEnablePlatformSpecificConfig( + enablePlatformSpecificConfigDescription, commandToRcArgs)) { + List expansion = getExpansion(eventHandler, commandToRcArgs, getPlatformName()); optionsParser.parseArgsAsExpansionOfOption( - configInstance, String.format("expanded from --%s", configValueToExpand), expansion); + Iterables.getOnlyElement(enablePlatformSpecificConfigDescription.getCanonicalInstances()), + String.format("enabled by --enable_platform_specific_config"), + expansion); } // At this point, we've expanded everything, identify duplicates, if any, to warn about diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CommonCommandOptions.java b/src/main/java/com/google/devtools/build/lib/runtime/CommonCommandOptions.java index 1ce51636186c64..706be9339bd769 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/CommonCommandOptions.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/CommonCommandOptions.java @@ -56,6 +56,19 @@ public class CommonCommandOptions extends OptionsBase { + "your build may break in the future due to deprecations or other changes.") public Void allIncompatibleChanges; + @Option( + name = "enable_platform_specific_config", + defaultValue = "false", + documentationCategory = OptionDocumentationCategory.UNCATEGORIZED, + effectTags = {OptionEffectTag.UNKNOWN}, + help = + "If true, Bazel picks up host-OS-specific config lines from bazelrc files. For example, " + + "if the host OS is Linux and you run bazel build, Bazel picks up lines starting " + + "with build:linux. Supported OS identifiers are linux, macos, windows, and " + + "freebsd. Enabling this flag is equivalent to using --config=linux on Linux, " + + "--config=windows on Windows, etc.") + public boolean enablePlatformSpecificConfig; + @Option( name = "config", defaultValue = "", diff --git a/src/test/java/com/google/devtools/build/lib/runtime/BlazeOptionHandlerTest.java b/src/test/java/com/google/devtools/build/lib/runtime/BlazeOptionHandlerTest.java index 21726009d0e842..37718097c56214 100644 --- a/src/test/java/com/google/devtools/build/lib/runtime/BlazeOptionHandlerTest.java +++ b/src/test/java/com/google/devtools/build/lib/runtime/BlazeOptionHandlerTest.java @@ -29,6 +29,7 @@ import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.InvocationPolicy; import com.google.devtools.build.lib.testutil.Scratch; import com.google.devtools.build.lib.testutil.TestConstants; +import com.google.devtools.build.lib.util.OS; import com.google.devtools.common.options.OptionsParser; import com.google.devtools.common.options.OptionsParsingException; import com.google.devtools.common.options.OptionsParsingResult; @@ -112,7 +113,7 @@ public BlazeCommandResult exec(CommandEnvironment env, OptionsParsingResult opti public void editOptions(OptionsParser optionsParser) {} } - private ListMultimap structuredArgsFrom2SimpleRcsWithOnlyResidue() { + private static ListMultimap structuredArgsFrom2SimpleRcsWithOnlyResidue() { ListMultimap structuredArgs = ArrayListMultimap.create(); // first add all lines of rc1, then rc2, to simulate a simple, import free, 2 rc file setup. structuredArgs.put("c0", new RcChunkOfArgs("rc1", ImmutableList.of("a"))); @@ -124,7 +125,7 @@ private ListMultimap structuredArgsFrom2SimpleRcsWithOnly return structuredArgs; } - private ListMultimap structuredArgsFrom2SimpleRcsWithFlags() { + private static ListMultimap structuredArgsFrom2SimpleRcsWithFlags() { ListMultimap structuredArgs = ArrayListMultimap.create(); structuredArgs.put( "c0", new RcChunkOfArgs("rc1", ImmutableList.of("--test_multiple_string=foo"))); @@ -140,7 +141,8 @@ private ListMultimap structuredArgsFrom2SimpleRcsWithFlag return structuredArgs; } - private ListMultimap structuredArgsFromImportedRcsWithOnlyResidue() { + private static ListMultimap + structuredArgsFromImportedRcsWithOnlyResidue() { ListMultimap structuredArgs = ArrayListMultimap.create(); // first add all lines of rc1, then rc2, but then jump back to 1 as if rc2 was loaded in an // import statement halfway through rc1. @@ -155,6 +157,18 @@ private ListMultimap structuredArgsFromImportedRcsWithOnl return structuredArgs; } + private static ListMultimap structuredArgsForDifferentPlatforms() { + ListMultimap structuredArgs = ArrayListMultimap.create(); + structuredArgs.put("c0:linux", new RcChunkOfArgs("rc1", ImmutableList.of("command_linux"))); + structuredArgs.put("c0:windows", new RcChunkOfArgs("rc1", ImmutableList.of("command_windows"))); + structuredArgs.put("c0:macos", new RcChunkOfArgs("rc1", ImmutableList.of("command_macos"))); + structuredArgs.put("c0:freebsd", new RcChunkOfArgs("rc1", ImmutableList.of("command_freebsd"))); + structuredArgs.put( + "c0:platform_config", + new RcChunkOfArgs("rc1", ImmutableList.of("--enable_platform_specific_config"))); + return structuredArgs; + } + @Test public void testStructureRcOptionsAndConfigs_argumentless() throws Exception { ListMultimap structuredRc = @@ -297,6 +311,62 @@ public void testExpandConfigOptions_withConfig() throws Exception { .containsExactly("Found applicable config definition c0:config in file rc1: b"); } + @Test + public void testExpandConfigOptions_withPlatformSpecificConfigEnabled() throws Exception { + parser.parse("--enable_platform_specific_config"); + optionHandler.expandConfigOptions(eventHandler, structuredArgsForDifferentPlatforms()); + switch (OS.getCurrent()) { + case LINUX: + assertThat(parser.getResidue()).containsExactly("command_linux"); + break; + case DARWIN: + assertThat(parser.getResidue()).containsExactly("command_macos"); + break; + case WINDOWS: + assertThat(parser.getResidue()).containsExactly("command_windows"); + break; + case FREEBSD: + assertThat(parser.getResidue()).containsExactly("command_freebsd"); + break; + default: + assertThat(parser.getResidue()).isEmpty(); + } + } + + @Test + public void testExpandConfigOptions_withPlatformSpecificConfigEnabledInConfig() throws Exception { + // --enable_platform_specific_config itself will affect the selecting of config sections. + // Because Bazel expands config sections recursively, we want to make sure it's fine to enable + // --enable_platform_specific_config via another config section. + parser.parse("--config=platform_config"); + optionHandler.expandConfigOptions(eventHandler, structuredArgsForDifferentPlatforms()); + switch (OS.getCurrent()) { + case LINUX: + assertThat(parser.getResidue()).containsExactly("command_linux"); + break; + case DARWIN: + assertThat(parser.getResidue()).containsExactly("command_macos"); + break; + case WINDOWS: + assertThat(parser.getResidue()).containsExactly("command_windows"); + break; + case FREEBSD: + assertThat(parser.getResidue()).containsExactly("command_freebsd"); + break; + default: + assertThat(parser.getResidue()).isEmpty(); + } + } + + @Test + public void testExpandConfigOptions_withPlatformSpecificConfigEnabledWhenNothingSpecified() + throws Exception { + parser.parse("--enable_platform_specific_config"); + optionHandler.parseRcOptions(eventHandler, ArrayListMultimap.create()); + assertThat(eventHandler.getEvents()).isEmpty(); + assertThat(parser.getResidue()).isEmpty(); + } + @Test public void testExpandConfigOptions_withConfigForUnapplicableCommand() throws Exception { parser.parse("--config=other");