Extend Android path stripping testing by integrating aquery output. ## Description aquery prints command lines by calling CommandAction.getArguments(): https://github.com/bazelbuild/bazel/blob/ee9727df980f268bfb2b05637d199d55963fb6d8/src/main/java/com/google/devtools/build/lib/query2/aquery/ActionGraphTextOutputFormatterCallback.java#L262. Java commands (like JavaCompileAction) pass this to CustomCommandLine, which for path stripped actions is a PathStrippingCustomCommandLine that knows to apply path stripping appropriately. For these actions this change is a noop. Starlark action construction (including Android actions) is more complicated: the StarlarkCustomCommandLine equivalent is set before path stripping can be known. StarlarkCustomCommandLine reads Starlark code before the action is instantiated and its full inputs and outputs are known. We need that info to set path stripping. So when Bazel requests a Starlark action's command, it passes the stripPath bit as a parameter to StarlarkCustomCommandLine's arguments() call (see this cl's change to SpawnAction - that calls CommandLines.allArguments() which calls StarlarkCustomCommandLine.arguments()). This change ensures that call properly passes the bit. This lets config_stripped_outputs_test use aquery to identify path stripping in more actions, so more tests are added here. ## Wider API consequences This change affects *any* caller to SpawnAction.getArguments(). This includes callers like ActionExecutedEvent: https://github.com/bazelbuild/bazel/blob/bc2f0519a4e6f0ac3ea42ff50da2c9919c5b6c0e/src/main/java/com/google/devtools/build/lib/actions/ActionExecutedEvent.java#L250. It's unclear if we want this caller to strip paths. It also affects some action key computations, like https://github.com/bazelbuild/bazel/blob/cb01bde9369f8197cb54c34b14aa41f0dcc903cf/src/main/java/com/google/devtools/build/lib/rules/cpp/LtoBackendAction.java#L225. This seems safe according to the contract for action keys: https://github.com/bazelbuild/bazel/blob/e4c1c434d49062449c7a83dd753fe01923766b1d/src/main/java/com/google/devtools/build/lib/actions/ActionAnalysisMetadata.java#L64-L84. Note that the affected callers are noops since those actions don't use path stripping. There are also a few debugging callers, similar in spirit to aquery. All this is more complicated than I'd prefer (i.e. the different logic for constructing Java vs. Starlark actions). I'd consider better API consistency down the line but for now I think the current balance is manageable. https://github.com/bazelbuild/bazel/issues/6526 PiperOrigin-RevId: 445970260
diff --git a/src/main/java/com/google/devtools/build/lib/actions/CommandLines.java b/src/main/java/com/google/devtools/build/lib/actions/CommandLines.java index 3995626..b74bd8f 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/CommandLines.java +++ b/src/main/java/com/google/devtools/build/lib/actions/CommandLines.java
@@ -210,9 +210,15 @@ */ public ImmutableList<String> allArguments() throws CommandLineExpansionException, InterruptedException { + return allArguments(CommandAdjuster.NOOP); + } + + /** Variation of {@link #allArguments()} that supports output path stripping. */ + public ImmutableList<String> allArguments(CommandAdjuster stripPaths) + throws CommandLineExpansionException, InterruptedException { ImmutableList.Builder<String> arguments = ImmutableList.builder(); for (CommandLineAndParamFileInfo pair : getCommandLines()) { - arguments.addAll(pair.commandLine.arguments()); + arguments.addAll(pair.commandLine.arguments(/*artifactExpander=*/ null, stripPaths)); } return arguments.build(); }
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/CustomCommandLine.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/CustomCommandLine.java index 9c1dce4..59bcdef 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/actions/CustomCommandLine.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/CustomCommandLine.java
@@ -1291,9 +1291,9 @@ } @Override - public ImmutableList<String> arguments(ArtifactExpander artifactExpander) + public ImmutableList<String> arguments(@Nullable ArtifactExpander artifactExpander) throws CommandLineExpansionException, InterruptedException { - return argumentsInternal(Preconditions.checkNotNull(artifactExpander)); + return argumentsInternal(artifactExpander); } private ImmutableList<String> argumentsInternal(@Nullable ArtifactExpander artifactExpander)
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java index 45c4bbe..2a621ae 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java
@@ -53,7 +53,7 @@ import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.FilesetOutputSymlink; import com.google.devtools.build.lib.actions.ParamFileInfo; -import com.google.devtools.build.lib.actions.PathStripper; +import com.google.devtools.build.lib.actions.PathStripper.CommandAdjuster; import com.google.devtools.build.lib.actions.ResourceSetOrBuilder; import com.google.devtools.build.lib.actions.RunfilesSupplier; import com.google.devtools.build.lib.actions.Spawn; @@ -250,9 +250,16 @@ return commandLines; } + private CommandAdjuster getPathStripper() { + return CommandAdjuster.create( + stripOutputPaths, + this instanceof StarlarkAction ? getMnemonic() : null, + getPrimaryOutput().getExecPath().subFragment(0, 1)); + } + @Override public List<String> getArguments() throws CommandLineExpansionException, InterruptedException { - return ImmutableList.copyOf(commandLines.allArguments()); + return commandLines.allArguments(getPathStripper()); } @Override @@ -421,10 +428,7 @@ commandLines.expand( artifactExpander, getPrimaryOutput().getExecPath(), - PathStripper.CommandAdjuster.create( - stripOutputPaths, - this instanceof StarlarkAction ? getMnemonic() : null, - getPrimaryOutput().getExecPath().subFragment(0, 1)), + getPathStripper(), commandLineLimits); return new ActionSpawn( ImmutableList.copyOf(expandedCommandLines.arguments()),