@ahumesky

@lberki ptal

looks like @lberki is not available. @sgowroji anyone else can take a look at this change?

One diagnostic followup is to see the exact difference in the configurations expressed. So given:

Configuration: a46bb511e54d3417610f91a9e0438015d7a78d7ac4f86f7c2440dea289d49498, 88e50b932f2f46570420b692a53eb24e621234bce56b60adfcb78e0fc18f0b30

what does

$ bazel config a46bb 88e50

show? (those hashes are really long so you can pass a simple prefix to save typing)

You could also try passing --experimental_output_directory_naming_scheme=diff_against_baseline, which is a promising new feature that makes conflicting action errors basically impossible.

We got the same conflicting action errors in our Android build with Bazel 6.x when trying to run with --host_jvmopt="-XX:-MaxFDLimit".

bazelisk config ba4daa bfc79f gives us:

Displaying diff between configs ba4daa and bfc79fa751b
FragmentOptions com.google.devtools.build.lib.rules.java.JavaOptions {
  jvmopt: [-XX:ErrorFile=/dev/stderr], [-XX:-MaxFDLimit]
}

so nothing too surprising there.

Setting --experimental_output_directory_naming_scheme=diff_against_baseline resolves the issue for us and I see that's now the default setting on the master branch. It seems like a big change, so we might go with something more conservative while we're on 6.x.

@gregestren Even though this is somewhat mitigated by --experimental_output_directory_naming_scheme=diff_against_baseline, this still looks like a bug to me. Did you close this on purpose or would you consider merging?

I think I closed this because it lingered without an awaiting-user-response Awaiting a response from the author update. So it became orphaned.

Happy to re-review given these last comments.

@gregestren I fixed the conflicts and it's ready for another review.

Could #18561 be a candidate solution for 6.4.0?

It seems like a big change

It and diff_against_baseline are definitely bigger impact changes than this. But they're intended to be strict improvements that ask nothing of the user. And they've both gotten a good amount of repo testing to vet their safety.

@fmeum when you say "somewhat mitigated", which scenarios are you thinking of that still crash?

I'm pushing back some because diff_against_dynamic_baseline is intended to more principally address the source of these errors in one fell swoop. It'd be nice if we could just eliminate the class of errors vs. having to remember to pass another --host_* flag every time the issue pops up.

At this point I am quite convinced that diff_against_dynamic_baseline is safe from the point of view of correctness and performance, but I still fear path length issues on Windows and thus am not 100% convinced that it is a safe cherry pick.

@fmeum when you say "somewhat mitigated", which scenarios are you thinking of that still crash?

As far as I understand the situation, diff_against_dynamic_baseline would definitely mitigate the crash in this case. What I am worried about is correctness: Could there be a chain of transitions that results in different JVM options (host or target) due to "losing" the host options when applying the exec transition? Happy to hear from you that this isn't actually possible though :-)

At this point I am quite convinced that diff_against_dynamic_baseline is safe from the point of view of correctness and performance, but I still fear path length issues on Windows and thus am not 100% convinced that it is a safe cherry pick.

To be clear, you still support #18561 for 7.0? @sdtwigg is intending to merge.

@fmeum when you say "somewhat mitigated", which scenarios are you thinking of that still crash?

As far as I understand the situation, diff_against_dynamic_baseline would definitely mitigate the crash in this case. What I am worried about is correctness: Could there be a chain of transitions that results in different JVM options (host or target) due to "losing" the host options when applying the exec transition? Happy to hear from you that this isn't actually possible though :-)

@sdtwigg anything you could think of?

The PR is obsolete, see also 6d0716d