Unexpected evaluation order of user defined flags

[chsigg]

Description of the problem / feature request:

User flags on the command line, expanded from command line configs, and from platform-specific configs are not applied in the correct order.

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

build_defs.bzl:

BuildSettingInfo = provider()

def _string_impl(ctx):
    value = ctx.build_setting_value
    print(value)
    return BuildSettingInfo(value = value)

string_flag = rule(
    implementation = _string_impl,
    build_setting = config.string(flag = True),
)

BUILD:

load(":build_defs.bzl", "string_flag")

string_flag(
  name = "flag",
  build_setting_default = "default",
)

.bazelrc:

build:linux --//:flag=linux
build:config --//:flag=config

Repro 1
--enable_platform_specific_config gets expanded last:

$ bazel build --enable_platform_specific_config --config=config --//:flag=cmdline :flag
DEBUG: build_defs.bzl:5:10: linux

The correct output would be cmdline.

Repro 2
--config=config gets expanded last:

$ bazel build --config=linux --config=config --//:flag=cmdline :flag
DEBUG: build_defs.bzl:5:10: config

The correct output would be cmdline.

Repro 3
This works as expected:

$ bazel build --//:flag=foo --//:flag=cmdline :flag
DEBUG: build_defs.bzl:5:10: cmdline

What operating system are you running Bazel on?

Ubuntu

What's the output of bazel info release?

release 4.1.0

Have you found anything relevant by searching the web?

Somewhat related: issue #10781

[aiuto]

I don't think of this as "incorrect", so much as "what it does is not what most people would expect.
The point about platform specific flags is a secondary issue.

So we really have one or more issues.

• should enable_platform_specific_config go into effect when it appears in the command line or before any other flags are processed
• why is enable_platform_specific_config even an option. IMO we should redefine it in a way so that it can always be on. AND..... it should be made clear that it applies to the host platform, not the exec or target platforms.
• what is the correct expansion of setting a flag via config or directly. That is repro 2. That one looks like a bug to me. It should not matter.

[chsigg]

should enable_platform_specific_config go into effect when it appears in the command line or before any other flags are processed

It looks inconsistent to me in the current form. Looking at issue #10781, --enable_platform_specific_config is either expanded first or in-place. For repro 1 here, it is expanded after --config=config or last.

it should be made clear that it applies to the host platform, not the exec or target platforms.

This is beyond my bazel knowledge, but aren't --configs and .bazelrc entries just command line flags and therefore agnostic to platforms? Are you saying build:windows --platforms=//:my_win_platform wouldn't work with --enable_platform_specific_config?

[aiuto]

From looking at the code and thinking about it

• --enable_platform_specific_config has to be processed before expanding any --config commands
• if there is a platform specific config, it should override the less specific config, so in Repro Update the Readme to give a short description of bazel. #1 above, cmdline may be what we both expect, but linux is still the better value than config.
• Most people would expect, however, that any trailing command line flags would override anything from the config.

https://github.com/aiuto/bazel_samples/tree/main/flag_evaluation_order
has a broader reproduction showing the problem.

It seems that built-in flags, like cpu do win from the command line, while user defined flags do not.

TL;DR; You can not override user defined flags that are set in configs from the command line.
Workaround: Don't do that. That might sound glib, but it's possible to arrange what you want if you know the behavior.

[chsigg]

Thanks a lot Tony for investigating.

if there is a platform specific config, it should override the less specific config

Couldn't --enable_platform_specific_config expand in place to e.g. --config=linux, which would in turn expand in place again to the *:linux entries in .bazelrc? This would seem the most obvious choice (but I'm by far no bazel expert).

[aiuto]

I don't think that would make sense. If we did that - there would be no need for the flag - you might as well just write bazel build --config=$(uname) ... - it would not give you the conceptual layering of 1. build the baseline set of flags from the rc files and --config 1. automatically splice in my platform specific things. In any case, --enable_platform_specific_config is something that belongs in .bazelrc and not on the command line. Letting you choose it or not on a per invocation basis is a foot-gun. It's a policy thing that should be chosen project wide or not.

…

On Thu, Jul 8, 2021 at 6:40 AM Christian Sigg ***@***.***> wrote: Thanks a lot Tony for investigating. if there is a platform specific config, it should override the less specific config Couldn't --enable_platform_specific_config expand in place to e.g. --config=linux, which would in turn expand in place again to the *:linux entries in .bazelrc? This would seem the most obvious choice (but I'm by far no bazel expert). — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub <#13603 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAXHHHHNCALQVM6D5YKUAALTWV6DFANCNFSM47HHMYEQ> .

[chsigg]

My comment seems to have implied that I would have --enable_platform_specific_config on the command line. That was not intentional, I agree that it belongs in the .bazelrc. It could still expand in place though to the appropriate --config, right?

automatically splice in my platform specific things.

Isn't that unnecessarily imposing a specific pattern on the user? For example, I would expect

.bazelrc:

build --enable_platform_specific_config
build:linux --nofoo
build:foo --foo

$ bazel build --config=foo to have foo==True.
You seem to think that it should be foo==False. Is that correct?

[aiuto]

Yea. I think it should be --nofoo when run on linux.
Having the platform config always win is the actual feature. If we didn't want that, one could add an overridable platform flag by always adding it to the command line. Or, if you will,

alias bazel='/usr/bin/bazel --config=$(uname)'

[github-actions]

Thank you for contributing to the Bazel repository! This issue has been marked as stale since it has not had any activity in the last 1+ years. It will be closed in the next 14 days unless any other activity occurs or one of the following labels is added: "not stale", "awaiting-bazeler". Please reach out to the triage team (@bazelbuild/triage) if you think this issue is still relevant or you are interested in getting the issue resolved.

[github-actions]

This issue has been automatically closed due to inactivity. If you're still interested in pursuing this, please reach out to the triage team (@bazelbuild/triage). Thanks!