Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Let aquery print effective environment for all CommandActions #17108

Closed
wants to merge 1 commit into from
Closed

Let aquery print effective environment for all CommandActions #17108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Let aquery print effective environment for all CommandActions 
Instead of printing the fixed part of the `ActionEnvironment` of all
`SpawnActions`, `aquery` now prints the effective environment resolved
against an empty client environment of all `AbstractAction`s that
implement `CommandAction`.

For `SpawnAction`s, this should not result in any observable change, but
C++ actions, which only override
`AbstractAction#getEffectiveEnvironment`, now have their fixed
environments (e.g. toolchain env vars such as `INCLUDE` with MSVC)
emitted.
  • Loading branch information
@fmeum
fmeum committed Jan 2, 2023
commit 04719c3c3a18f8d78f35ca4091b16c6a91255b27
12 changes: 8 additions & 4 deletions src/main/java/com/google/devtools/build/lib/skyframe/actiongraph/v2/ActionGraphDump.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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()
Expand Down
30 changes: 30 additions & 0 deletions src/test/shell/integration/aquery_test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -1679,6 +1685,30 @@ EOF
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() {
Expand Down