Emit events when invocation policy overrides options

[illicitonion]

Prior to this commit, messages were only logged to the server log indicating that user input was being ignored. This is a place users don't tend to look (or even know about) when debugging issues.

With this commit, we still log there, but additionally emit events to the console, which users are much more likely to notice.

Example output:

% bazel --invocation_policy='flag_policies { flag_name: "test_env" set_value { flag_value: "SET_FROM_FLAG=by-policy" behavior: FINAL_VALUE_IGNORE_OVERRIDES } }' test :test --test_env=SET_FROM_FLAG=from-flag
WARNING: Setting value for option '--test_env' from invocation policy to 'SET_FROM_FLAG=by-policy', overriding value '[SET_FROM_FLAG=from-bazelrc, SET_FROM_FLAG=from-flag]' from '/path/to/.bazelrc, command line options'
INFO: Invocation ID: bdfa6f27-a7d8-4b2b-9b45-f2f4b1f905fc
...

[illicitonion]

/cc @michajlo

[michajlo]

We're probably going to need to make the console logging optional. While I agree it's useful, the incumbent is silence, and I strongly suspect that some % of users will see the new logging as spam.

That said, I think we should come up with an end-to-end story on #16997 before submitting things, this way if we change our mind or find something we overlooked our hands aren't tied.

[illicitonion]

We're probably going to need to make the console logging optional. While I agree it's useful, the incumbent is silence, and I strongly suspect that some % of users will see the new logging as spam.

I added a flag to hide this, but I defaulted it to show - I do think it's important that if Bazel is going to ignore explicit input you give it, by default it should verbosely tell you at least that it's doing so, and ideally also why.

That said, I think we should come up with an end-to-end story on #16997 before submitting things, this way if we change our mind or find something we overlooked our hands aren't tied.

I'm not sure I agree about that - I think that this is a useful standalone feature, whether or not it's used for default-setting - the use-cases are orthogonal. Telling a user that you're ignoring their input is an important feature, and I'd consider it a pretty serious UX bug of --invocation_policy not to currently be doing so by default.

[michajlo]

I generally agree with the UX concerns, and have lobbied for the same in the past. However, I've also come to recognize that the system(s) using invocation policy internally can't start throwing warnings at users without some prior cleanup work. We can flag it off internally, but that will take some effort for myself or whoever does the import, which is part of the reason I'd like to have an end-to-end story, allowing us to batch this and whatever internal adjustments we must make. On the other hand, if this effort to extend invocation policy fizzles out, I'd prefer to not have lingering ~unused flags and wasted effort.

[illicitonion]

Something that occurred to me - maybe we could add this to the proto rather than as a flag? That way we don't add to the flag proliferation, and we let the policy definer choose how important it is that someone knows what's happening?

Either we could add a new top-level field on InvocationPolicy of type:

enum OverrideBehaviour {
  UNDEFINED = 0;
  QUIET = 1;
  WARN = 2;
  ERROR = 3;
}

Or we could add some new Behavior variants:

FINAL_VALUE_IGNORE_AND_WARN_ON_OVERRIDES = 4;
FINAL_VALUE_IGNORE_AND_ERROR_ON_OVERRIDES = 5;

which act the same as FINAL_VALUE_IGNORE_OVERRIDES but with extra logging changes.

Would either of those approaches help to make this change more palatable @michajlo?