[6.1.0]Let aquery print effective environment for all CommandActions

src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/ActionGraphDump.java

@@ -18,6 +18,7 @@
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
+import com.google.devtools.build.lib.actions.AbstractAction;
 import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
 import com.google.devtools.build.lib.actions.ActionExecutionMetadata;
 import com.google.devtools.build.lib.actions.ActionKeyContext;
@@ -32,7 +33,6 @@
 import com.google.devtools.build.lib.analysis.SourceManifestAction;
 import com.google.devtools.build.lib.analysis.actions.FileWriteAction;
 import com.google.devtools.build.lib.analysis.actions.ParameterFileWriteAction;
-import com.google.devtools.build.lib.analysis.actions.SpawnAction;
 import com.google.devtools.build.lib.analysis.actions.Substitution;
 import com.google.devtools.build.lib.analysis.actions.TemplateExpansionAction;
 import com.google.devtools.build.lib.analysis.actions.TemplateExpansionException;
@@ -174,11 +174,15 @@ private void dumpSingleAction(ConfiguredTarget configuredTarget, ActionAnalysisM
     }
 
     // store environment
-    if (action instanceof SpawnAction) {
-      SpawnAction spawnAction = (SpawnAction) action;
+    if (action instanceof AbstractAction && action instanceof CommandAction) {
+      AbstractAction spawnAction = (AbstractAction) action;
+      // Some actions (e.g. CppCompileAction) don't override getEnvironment, but only
+      // getEffectiveEnvironment. Since calling the latter with an empty client env returns the
+      // fixed part of the full ActionEnvironment with the default implementations provided by
+      // AbstractAction, we can call getEffectiveEnvironment here to handle these actions as well.
       // TODO(twerth): This handles the fixed environment. We probably want to output the inherited
       // environment as well.
-      ImmutableMap<String, String> fixedEnvironment = spawnAction.getEnvironment().getFixedEnv();
+      ImmutableMap<String, String> fixedEnvironment = spawnAction.getEffectiveEnvironment(Map.of());
       for (Map.Entry<String, String> environmentVariable : fixedEnvironment.entrySet()) {
         actionBuilder.addEnvironmentVariables(
             AnalysisProtosV2.KeyValuePair.newBuilder()

src/test/shell/integration/aquery_test.sh

@@ -30,9 +30,15 @@ source "$(rlocation "io_bazel/src/test/shell/integration_test_setup.sh")" \
 
 case "$(uname -s | tr [:upper:] [:lower:])" in
 msys*|mingw*|cygwin*)
+  declare -r is_macos=false
   declare -r is_windows=true
   ;;
+darwin)
+  declare -r is_macos=true
+  declare -r is_windows=false
+  ;;
 *)
+  declare -r is_macos=false
   declare -r is_windows=false
   ;;
 esac
@@ -1659,6 +1665,49 @@ EOF
   fi
 }
 
+function test_does_not_fail_horribly_with_file() {
+  rm -rf peach
+  mkdir -p peach
+  cat > "peach/BUILD" <<'EOF'
+genrule(
+    name = "bar",
+    srcs = ["dummy.txt"],
+    outs = ["bar_out.txt"],
+    cmd = "echo unused > bar_out.txt",
+)
+EOF
+
+  echo "//peach:bar" > query_file
+  bazel aquery --query_file=query_file > $TEST_log
+
+  expect_log "Target: //peach:bar" "look in $TEST_log"
+  expect_log "ActionKey:"
+}
+
+function test_cpp_compile_action_env() {
+  local pkg="${FUNCNAME[0]}"
+  mkdir -p "$pkg"
+
+  touch "$pkg/main.cpp"
+  cat > "$pkg/BUILD" <<'EOF'
+cc_binary(
+    name = "main",
+    srcs = ["main.cpp"],
+)
+EOF
+  bazel aquery --output=textproto \
+     "mnemonic(CppCompile,//$pkg:main)" >output 2> "$TEST_log" || fail "Expected success"
+  cat output >> "$TEST_log"
+
+  if "$is_macos"; then
+    assert_contains '  key: "XCODE_VERSION_OVERRIDE"' output
+  elif "$is_windows"; then
+    assert_contains '  key: "INCLUDE"' output
+  else
+    assert_contains '  key: "PWD"' output
+  fi
+}
+
 # TODO(bazel-team): The non-text aquery output formats don't correctly handle
 # non-ASCII fields (input/output paths, environment variables, etc).
 function DISABLED_test_unicode_textproto() {