bazelrc: support --enable_platform_specific_config #9246

meteorcloudy · 2019-08-26T13:02:41Z

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, osx, windows or freebsd. It's equivalent to add
--config=linux on Linux platforms and --config=windows on Windows, etc.

Fixes #5055

RELNOTES[NEW]: Platform-specific bazelrc: with --enable_platform_specific_config you can
enable flags in bazelrc according to your host platform.
GOOGLE:

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, osx, windows or freebsd. It's equivalent to add --config=linux on Linux platforms and --config=windows on Windows, etc. Fixes bazelbuild#5055 RELNOTES[NEW]: Support --enable_platform_specific_config, you can use this flag to enable flags in bazelrc according to your host platform. GOOGLE:

laszlocsomor · 2019-08-26T13:04:46Z

RELNOTES[NEW]: Support --enable_platform_specific_config, you can use
this flag to enable flags in bazelrc according to your host platform.
GOOGLE:

How about this instead:

RELNOTES[NEW]: Platform-specific bazelrc: with --enable_platform_specific_config you can use
enable flags in bazelrc according to your host platform.

meteorcloudy · 2019-08-26T13:10:51Z

Great, thanks!

laszlocsomor · 2019-08-26T13:06:34Z

src/main/java/com/google/devtools/build/lib/runtime/BlazeOptionHandler.java

+      OptionValueDescription enablePlatformSpecificConfigDescription,
+      ListMultimap<String, RcChunkOfArgs> commandToRcArgs) {
+    if (enablePlatformSpecificConfigDescription == null ||
+        !(boolean) enablePlatformSpecificConfigDescription.getValue()) {


Is this cast safe? Shouldn't it be (Boolean) and check for null?

Yes, as long as there is a default value, it won't be null and will be converted to the correct type.
See

bazel/src/main/java/com/google/devtools/common/options/OptionDefinition.java

Line 255 in 1f684e1

public Object getDefaultValue() {

src/main/java/com/google/devtools/build/lib/runtime/BlazeOptionHandler.java

laszlocsomor · 2019-08-26T13:28:30Z

src/main/java/com/google/devtools/build/lib/runtime/BlazeOptionHandler.java

+      // Find the base set of configs. This does not include the config options that might be
+      // recursively included.
+      ImmutableList<ParsedOptionDescription> configInstances =
+          ImmutableList.copyOf(configValueDescription.getCanonicalInstances());


Why make a copy?

(I realize this is old code, just asking.)

Sorry, I actually don't know the reason..

src/test/java/com/google/devtools/build/lib/runtime/BlazeOptionHandlerTest.java

laszlocsomor · 2019-08-26T13:46:28Z

src/main/java/com/google/devtools/build/lib/runtime/CommonCommandOptions.java

+      documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+      effectTags = {OptionEffectTag.UNKNOWN},
+      help =
+          "When the value of this flag is true, a host platform specific config section will be "


When the value of this flag is true

Could be rewritten as "If true".

... a host platform specific config section will be enabled if it exists

I find that confusing. Using active voice and straight word order feels clearer to me. How about: "... Bazel picks up host-OS-specific config lines from the bazelrc files."

I feel like an example would greatly help too: "For example, if the host OS is Linux and you run bazel build, Bazel would pick up lines starting with build:linux"

linux, osx, windows

macOS is the official name so let's use "macos" instead of "osx".

It's equivalent to add --config=linux on Linux platforms and --config=windows on Windows, etc.

We could be a little clearer here: "Enabling this flag is equivalent to using --config=linux on Linux, --config=windows on Windows, etc."

Putting it together:

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.

Wow, thanks for helping with the documentation. This is great improvement!

As for the name for mac, it seems we still use osx in a lot of places in our code base. For example,

bazel/tools/platforms/BUILD

Line 63 in 71a213c

actual = "@platforms//os:osx",

It prefer to keep the consistence util we decided to migrate everything to macos. WDYT?

Could you please ask the Apple team what's their preference?

laszlocsomor

LGTM modulo the "osx" vs "macos" question.

laszlocsomor · 2019-08-26T14:47:13Z

src/main/java/com/google/devtools/build/lib/runtime/BlazeOptionHandler.java

+      OptionValueDescription enablePlatformSpecificConfigDescription,
+      ListMultimap<String, RcChunkOfArgs> commandToRcArgs) {
+    if (enablePlatformSpecificConfigDescription == null ||
+        !(boolean) enablePlatformSpecificConfigDescription.getValue()) {


src/test/java/com/google/devtools/build/lib/runtime/BlazeOptionHandlerTest.java

laszlocsomor · 2019-08-26T14:50:55Z

src/main/java/com/google/devtools/build/lib/runtime/CommonCommandOptions.java

+      documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
+      effectTags = {OptionEffectTag.UNKNOWN},
+      help =
+          "When the value of this flag is true, a host platform specific config section will be "


Could you please ask the Apple team what's their preference?

philwo · 2019-08-27T13:40:51Z

This was merged via 5975545.

gunan · 2019-11-19T01:02:37Z

This looks quite useful to me.
Is there documentation on how to use this?

meteorcloudy · 2019-11-19T09:43:20Z

@gunan The documentation is here: https://docs.bazel.build/versions/master/command-line-reference.html#flag--enable_platform_specific_config

Here is an example in .bazelrc file:

--enable_platform_specific_config
build:windows --workspace_status_command=command.bat
build:linux --workspace_status_command=command.sh
build:macos --workspace_status_command=command.sh

Then the corresponding option will be enabled for each platform. Be aware, Bazel will only enable those options based on your host platform, instead of execution platform or target platform.

gunan · 2019-11-19T18:07:44Z

Is there a way to use this to change definition of certain build configs?

such as:
I want --config=avx to be "--copt=-mavx" in linux, but "--copt=/arch=AVX" on windows.

meteorcloudy · 2019-11-20T11:15:41Z

No, there is no way to do that currently. But it sounds like a very useful feature.

googlebot added the cla: yes label Aug 26, 2019

meteorcloudy requested review from laszlocsomor and philwo August 26, 2019 13:02

laszlocsomor suggested changes Aug 26, 2019

View reviewed changes

Address reviewer comments

65f0aff

laszlocsomor approved these changes Aug 26, 2019

View reviewed changes

philwo closed this Aug 27, 2019

kevingessner mentioned this pull request Feb 13, 2020

bazelrc, --announce_rc, --enable_platform_specific_config disagree on precedence of platform-specific flags #10781

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bazelrc: support --enable_platform_specific_config #9246

bazelrc: support --enable_platform_specific_config #9246

meteorcloudy commented Aug 26, 2019 •

edited

Loading

laszlocsomor commented Aug 26, 2019

meteorcloudy commented Aug 26, 2019

laszlocsomor Aug 26, 2019

meteorcloudy Aug 26, 2019

laszlocsomor Aug 26, 2019

laszlocsomor Aug 26, 2019

meteorcloudy Aug 26, 2019

laszlocsomor Aug 26, 2019

meteorcloudy Aug 26, 2019

laszlocsomor Aug 26, 2019

laszlocsomor left a comment

laszlocsomor Aug 26, 2019

laszlocsomor Aug 26, 2019

philwo commented Aug 27, 2019

gunan commented Nov 19, 2019

meteorcloudy commented Nov 19, 2019

gunan commented Nov 19, 2019

meteorcloudy commented Nov 20, 2019

bazelrc: support --enable_platform_specific_config #9246

bazelrc: support --enable_platform_specific_config #9246

Conversation

meteorcloudy commented Aug 26, 2019 • edited Loading

laszlocsomor commented Aug 26, 2019

meteorcloudy commented Aug 26, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

laszlocsomor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

philwo commented Aug 27, 2019

gunan commented Nov 19, 2019

meteorcloudy commented Nov 19, 2019

gunan commented Nov 19, 2019

meteorcloudy commented Nov 20, 2019

meteorcloudy commented Aug 26, 2019 •

edited

Loading