[6.4.0] Add --incompatible_merge_fixed_and_default_shell_env

[fmeum]

With the new flag, Starlark actions that specify both env and use_default_shell_env will no longer have env ignored. Instead, the values of env will override the default shell environment.

This allows Starlark actions to both pick up user-configured variables such as PATH from the shell environment as well as set variables to fixed values required for the action, e.g., variables provided by the C++ toolchain.

Rationale for having env override the default shell env: The rules know best which values they have to set specific environment variables to in order to successfully execute an action, so a situation where users break an action by a globally applied --action_env is prevented. If users really do have to be able to modify an environment variables fixed by the rule, the rule can always make this configurable via an attribute.

Work towards #5980
Fixes #12049

Closes #18235.

Commit d1fdc53

PiperOrigin-RevId: 559506535
Change-Id: I7ec6ae17b076bbca72fab8394f3a8b3e4f9ea9d8

Fixes #19312

[fmeum]

@comius The cherry pick didn't apply cleanly and I had to make minor modifications (a new if branch). Let me know if you think that this is too complex for a cherry pick.

[comius]

Can you also cherry-pick d5a963e?
That's where the actionEnvironment was removed from SpawnAction. I think cherry-picking is safer than adding an if branch. I suspect everything is fine, but tests after cherry-pick will show if there are any users.

[fmeum]

@iancha1992 If cherry-picking d5a963e isn't straightforward let me know and I can try.

[iancha1992]

@fmeum There's conflicts when d5a963e cherry-pick was attempted.

1. src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
   In release-6.4.0 branch currently, public SpawnAction build(ActionOwner owner, BuildConfigurationValue configuration) returns

return buildSpawnAction(
          owner, commandLines, configuration.getCommandLineLimits(), configuration, env);

but we expected below before the cherry-pick

return buildSpawnAction(owner, commandLines, configuration, env);

Also,
SpawnAction buildForActionTemplate(ActionOwner owner) returns in release-6.4.0

return buildSpawnAction(
          owner,
          result.build(),
          CommandLineLimits.UNLIMITED,
          null,
          ActionEnvironment.create(environment, inheritedEnvironment));

but we expected below before the cherry-pick

return buildSpawnAction(
          owner,
          result.build(),
          null,
          ActionEnvironment.create(environment, inheritedEnvironment));

Also, in the release-6.4.0 branch, we have

@CanIgnoreReturnValue
    private Builder setJavaExecutable(
        PathFragment javaExecutable,
        Artifact deployJar,
        NestedSet<String> jvmArgs,
        String... launchArgs) {
      this.executableArgs =
          CustomCommandLine.builder()
              .addPath(javaExecutable)
              .addAll(jvmArgs)
              .addAll(ImmutableList.copyOf(launchArgs));
      this.executableArg = null;
      toolsBuilder.add(deployJar);
      this.isShellCommand = false;
      return this;
    }

but we expected below to not be there before the cherry-pick
this.isShellCommand = false;

2. src/test/java/com/google/devtools/build/lib/analysis/actions/SpawnActionTest.java
   In public void testWelcomeArtifactIsInput() function, in the current release-6.4.0 branch, we have:
   Iterable<Artifact> inputs = copyFromWelcomeToDestination.getInputs().toList();

We expected:
ImmutableList<Artifact> inputs = copyFromWelcomeToDestination.getInputs().toList();

cc: @bazelbuild/triage

[fmeum]

Thanks for trying!

@comius Looks like the original cherry-pick will be cleaner even though it has an additional branch. What do you think?

[comius]

Thanks for trying!

@comius Looks like the original cherry-pick will be cleaner even though it has an additional branch. What do you think?

Yes, probably.

Is there a guarantee that actionEnvironment is always null in 6.4.0 branch? I think that's the case, right?

[fmeum]

@comius Looks like it. I pushed a new commit that removes it, if all tests pass with that the cherry-pick will be even cleaner.

[iancha1992]

@fmeum can you please resolve the conflicts? Thanks!

[fmeum]

@iancha1992 Done