Same transition dirs when only cmd line options differs

[ulrfa]

Ignore user defined build settings only set on
command line and not affected by any transition,
when calculating transitionDirectoryNameFragment
output directory.

Native options on command line already have this
behavior, and this commit provides the same also
for user defined build settings.

This allows reducing transition scalability issues,
by combining transitions with command line options:

a) Setting a limited number of common options via
transitions, which affects large parts of the
code base, but with limited number of variants.
b) When needed, also setting a few of many
specific options, each of them affecting only
their parts of the code base, but with many
variants.

Resolves #12731
Resolves #13317

[frazze-jobb]

@gregestren have you got the time to review this yet?

[gregestren]

I'm switching to giving this more attention around this week. Thanks for your patience.

[ulrfa]

Now I waited more than 11 weeks. @gregestren, when do you think you will have time to review?

[gregestren]

I apologize for the wait and appreciate your patience.

I was looking at this issue in earnest yesterday - not just this PR but the collection of related issues I think speak to this theme (like #13591 and #12171).

I'm trying to stitch together how they'll all fit in my head. Expect a more technical update from me today. I'm also happy to share contact info if you want better ping access.

[gregestren]

Thanks again for your patience. Overall this looks great to me.

Before this review I ran through the technicalities on my own to map my own understanding of the right approach. This is exactly the change I would have made. I'm encouraged by this convergence.

I haven't reviewed the test code yet, but hopefully these comments help. No major comments - all in all the base approach looks spot on.

[ulrfa]

Thanks @gregestren for reviewing! Please, see my follow up questions.

Yes, encouraging with the convergence about the approach! 😃

[ulrfa]

The failing windows test cases seems related to python version, and not this PR. Perhaps they will be resolved when I rebase.

[ulrfa]

I tried to resolve the comments and rebased.

@gregestren, note that there is one open conversation waiting for your feedback.

@gregestren, is it something more you want from me before this can land?

[gregestren]

I also experimentally verified this at the command line with a simple project (building the same target that applies a transition twice, first time with --//foo:unrelated_flag=a and second with --//foo:unrelated_flag=b).

[ulrfa]

Hurray! 🎉 🎈 I'm very happy! Thank you @gregestren!

[gregestren]

Hey @ulrfa. #14239 (comment) suggests the null check from this PR may not have actually been necessary? Do you remember the considerations behind that line?

It seems like the relevant tests still pass with a small modification @sdtwigg made in a pending change. I wonder if the problem was

bazel/src/main/java/com/google/devtools/build/lib/analysis/starlark/FunctionTransitionUtil.java

Line 452 in a9206eb

hashStrs.add(toAdd);

failing if its value is null.

[ulrfa]

Hi @gregestren, see my answer in #14239