Skip to content

Commit

Permalink
Merge ActionStager and CommandAdjuster into PathMapper
Browse files Browse the repository at this point in the history 
Closes #17808.

PiperOrigin-RevId: 523898505
Change-Id: I0c93d7afc860de6cb6b91ad9577b05b689f472da
  • Loading branch information
@fmeum @copybara-github
fmeum authored and copybara-github committed Apr 13, 2023
1 parent 9587d2d commit acec6c0
Show file tree
Hide file tree
Showing 9 changed files with 159 additions and 179 deletions.
4 changes: 2 additions & 2 deletions src/main/java/com/google/devtools/build/lib/actions/CommandLine.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import com.google.common.base.Joiner;
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander;
import com.google.devtools.build.lib.actions.PathStripper.CommandAdjuster;
import com.google.devtools.build.lib.actions.PathStripper.PathMapper;
import com.google.devtools.build.lib.collect.CollectionUtils;
import com.google.devtools.build.lib.collect.IterablesChain;
import com.google.devtools.build.lib.util.Fingerprint;
Expand Down Expand Up @@ -51,7 +51,7 @@ public Iterable<String> arguments(ArtifactExpander artifactExpander)
return arguments();
}

public Iterable<String> arguments(ArtifactExpander artifactExpander, CommandAdjuster pathStripper)
public Iterable<String> arguments(ArtifactExpander artifactExpander, PathMapper pathMapper)
throws CommandLineExpansionException, InterruptedException {
return arguments(artifactExpander);
}
Expand Down
26 changes: 13 additions & 13 deletions src/main/java/com/google/devtools/build/lib/actions/CommandLines.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander;
import com.google.devtools.build.lib.actions.ParameterFile.ParameterFileType;
import com.google.devtools.build.lib.actions.PathStripper.CommandAdjuster;
import com.google.devtools.build.lib.actions.PathStripper.PathMapper;
import com.google.devtools.build.lib.actions.cache.VirtualActionInput;
import com.google.devtools.build.lib.collect.IterablesChain;
import com.google.devtools.build.lib.util.Fingerprint;
Expand Down Expand Up @@ -110,27 +110,27 @@ private CommandLines(Object commandLines) {
*
* @param artifactExpander The artifact expander to use.
* @param paramFileBasePath Used to derive param file names. Often the first output of an action
* @param pathStripper function to strip configuration prefixes from output paths, in accordance
* @param pathMapper function to strip configuration prefixes from output paths, in accordance
* with the logic in {@link PathStripper}
* @param limits The command line limits the host OS can support.
* @return The expanded command line and its param files (if any).
*/
public ExpandedCommandLines expand(
ArtifactExpander artifactExpander,
PathFragment paramFileBasePath,
PathStripper.CommandAdjuster pathStripper,
PathMapper pathMapper,
CommandLineLimits limits)
throws CommandLineExpansionException, InterruptedException {
return expand(
artifactExpander, paramFileBasePath, limits, pathStripper, PARAM_FILE_ARG_LENGTH_ESTIMATE);
artifactExpander, paramFileBasePath, limits, pathMapper, PARAM_FILE_ARG_LENGTH_ESTIMATE);
}

@VisibleForTesting
ExpandedCommandLines expand(
ArtifactExpander artifactExpander,
PathFragment paramFileBasePath,
CommandLineLimits limits,
PathStripper.CommandAdjuster pathStripper,
PathMapper pathMapper,
int paramFileArgLengthEstimate)
throws CommandLineExpansionException, InterruptedException {
// Optimize for simple case of single command line
Expand All @@ -150,12 +150,12 @@ ExpandedCommandLines expand(
CommandLine commandLine = pair.commandLine;
ParamFileInfo paramFileInfo = pair.paramFileInfo;
if (paramFileInfo == null) {
Iterable<String> args = commandLine.arguments(artifactExpander, pathStripper);
Iterable<String> args = commandLine.arguments(artifactExpander, pathMapper);
arguments.add(args);
cmdLineLength += totalArgLen(args);
} else {
Preconditions.checkNotNull(paramFileInfo); // If null, we would have just had a CommandLine
Iterable<String> args = commandLine.arguments(artifactExpander, pathStripper);
Iterable<String> args = commandLine.arguments(artifactExpander, pathMapper);
boolean useParamFile = true;
if (!paramFileInfo.always()) {
int tentativeCmdLineLength = cmdLineLength + totalArgLen(args);
Expand All @@ -173,7 +173,7 @@ ExpandedCommandLines expand(
String paramArg =
SingleStringArgFormatter.format(
paramFileInfo.getFlagFormatString(),
pathStripper.map(paramFileExecPath).getPathString());
pathMapper.map(paramFileExecPath).getPathString());
arguments.addElement(paramArg);
cmdLineLength += paramArg.length() + 1;

Expand Down Expand Up @@ -212,11 +212,11 @@ ExpandedCommandLines expand(
*/
public ImmutableList<String> allArguments()
throws CommandLineExpansionException, InterruptedException {
return allArguments(CommandAdjuster.NOOP);
return allArguments(PathMapper.NOOP);
}

/** Variation of {@link #allArguments()} that supports output path stripping. */
public ImmutableList<String> allArguments(CommandAdjuster stripPaths)
public ImmutableList<String> allArguments(PathMapper stripPaths)
throws CommandLineExpansionException, InterruptedException {
ImmutableList.Builder<String> arguments = ImmutableList.builder();
for (CommandLineAndParamFileInfo pair : getCommandLines()) {
Expand Down Expand Up @@ -486,15 +486,15 @@ public SingletonCommandLine(Object arg) {

@Override
public Iterable<String> arguments() throws CommandLineExpansionException, InterruptedException {
return arguments(null, PathStripper.CommandAdjuster.NOOP);
return arguments(null, PathMapper.NOOP);
}

@Override
public Iterable<String> arguments(
@Nullable ArtifactExpander artifactExpander, CommandAdjuster pathStripper)
@Nullable ArtifactExpander artifactExpander, PathMapper pathMapper)
throws CommandLineExpansionException, InterruptedException {
if (arg instanceof PathStrippable) {
return ImmutableList.of(((PathStrippable) arg).expand(pathStripper::map));
return ImmutableList.of(((PathStrippable) arg).expand(pathMapper::map));
}
return ImmutableList.of(CommandLineItem.expandToCommandLine(arg));
}
Expand Down
208 changes: 93 additions & 115 deletions src/main/java/com/google/devtools/build/lib/actions/PathStripper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,12 @@
* <li>"Qualifying" actions strip config paths from their command lines. An action qualifies if
* its implementation logic checks {@code --experimental_output_paths=strip}, creates a {@link
* Spawn} with {@link Spawn#stripOutputPaths()} == true, and removes config prefixes from its
* command line with the help of {@link PathStripper.CommandAdjuster}. Action logic should
* also check {@link PathStripper#isPathStrippable}: see that method's javadoc for why.
* command line with the help of {@link PathStripper#createForAction(boolean, String,
* PathFragment)}. Action logic should also check {@link PathStripper#isPathStrippable}: see
* that method's javadoc for why.
* <li>A supporting executor strips paths from qualifying actions' inputs and outputs before
* staging for execution, with the help of {@link PathStripper.ActionStager}.
* staging for execution, with the help of {@link PathStripper#createForExecutor(Spawn,
* PathFragment)}.
* </ol>
*
* <p>So an action is responsible for declaring that it strips paths and adjusting its command line
Expand All @@ -72,58 +74,105 @@
*/
public final class PathStripper {
/**
* Support for mapping config parts of exec paths of an action's inputs and outputs.
* Creates a new {@link PathMapper} for action implementation logic to use.
*
* <p>The executor should use this to correctly stage an action for execution.
* @param stripOutputPaths should this action strip config prefixes?
* @param starlarkMnemonic this action's mnemonic if it's a Starlark action, else null
* @param outputRoot the root path where outputs are written (e.g. "bazel-out"). Actions that
* don't strip outputs can set this to null.
*/
public interface ActionStager {
/**
* Returns the exec path where the executor should stage an action input or output.
*
* <p>If the action should be config-stripped ({@link PathStripper}), removes "k8-fastbuild"
* from paths like "bazel-out/k8-fastbuild/foo/bar".
*
* <p>Else returns the artifact's original exec path.
*/
default String getMappedExecPathString(ActionInput artifact) {
return map(artifact.getExecPath()).getPathString();
}

/** Same as {@link #getMappedExecPathString(ActionInput)} but for a {@link PathFragment}. */
PathFragment map(PathFragment execPath);

/**
* Creates a new action stager for executor implementation logic to use.
*
* @param spawn the action to stage. If {@link Spawn#stripOutputPaths()} is true, paths like
* "bazel-out/k8-fastbuild/bin/foo" are reduced to "bazel-out/bin/foo". Else they're
* unchanged.
* @param outputRoot the root path where outputs are written (e.g. "bazel-out")
*/
static ActionStager create(Spawn spawn, PathFragment outputRoot) {
public static PathMapper createForAction(
boolean stripOutputPaths,
@Nullable String starlarkMnemonic,
@Nullable PathFragment outputRoot) {
if (stripOutputPaths) {
Preconditions.checkNotNull(outputRoot);
Preconditions.checkState(outputRoot.isSingleSegment());
Preconditions.checkState(!outputRoot.getPathString().contains("\\"));
return spawn.stripOutputPaths() ? actionStripper(outputRoot) : NOOP;
}
return stripOutputPaths ? commandStripper(starlarkMnemonic, outputRoot) : PathMapper.NOOP;
}

/** An {@link ActionStager} that doesn't change paths. */
ActionStager NOOP = execPath -> execPath;
/**
* Creates a new {@link PathMapper} for executor implementation logic to use.
*
* @param spawn the action to stage. If {@link Spawn#stripOutputPaths()} is true, paths like
* "bazel-out/k8-fastbuild/bin/foo" are reduced to "bazel-out/bin/foo". Else they're
* unchanged.
* @param outputRoot the root path where outputs are written (e.g. "bazel-out")
*/
public static PathMapper createForExecutor(Spawn spawn, PathFragment outputRoot) {
Preconditions.checkState(outputRoot.isSingleSegment());
Preconditions.checkState(!outputRoot.getPathString().contains("\\"));
return spawn.stripOutputPaths() ? actionStripper(outputRoot) : PathMapper.NOOP;
}

/** Instantiates an {@link ActionStager} that strips config prefixes from output paths. */
private static ActionStager actionStripper(PathFragment outputRoot) {
return execPath ->
isOutputPath(execPath, outputRoot) ? PathStripper.strip(execPath) : execPath;
}
/** Instantiates a {@link PathMapper} that strips config prefixes from output paths. */
private static PathMapper actionStripper(PathFragment outputRoot) {
return execPath -> isOutputPath(execPath, outputRoot) ? PathStripper.strip(execPath) : execPath;
}

/** Instantiates a {@link PathMapper} that strips config prefixes from output paths. */
private static PathMapper commandStripper(
@Nullable String starlarkMnemonic, PathFragment outputRoot) {
final StringStripper argStripper =
starlarkMnemonic != null ? new StringStripper(outputRoot.getPathString()) : null;
return new PathMapper() {
@Override
public String getMappedExecPathString(ActionInput artifact) {
if (artifact instanceof DerivedArtifact) {
return PathStripper.strip(artifact);
} else {
return artifact.getExecPathString();
}
}

@Override
public PathFragment map(PathFragment execPath) {
return PathStripper.isOutputPath(execPath, outputRoot)
? PathStripper.strip(execPath)
: execPath;
}

@Override
public List<String> mapCustomStarlarkArgs(List<String> args) {
// Add your favorite Starlark mnemonic that needs custom arg processing here.
if (!starlarkMnemonic.contains("Android")
&& !starlarkMnemonic.equals("MergeManifests")
&& !starlarkMnemonic.equals("StarlarkRClassGenerator")) {
return args;
}
// Add your favorite arg to custom-process here. When Bazel finds one of these in the
// argument list (an argument name), it strips output path prefixes from the following
// argument (the argument value).
ImmutableList<String> starlarkArgsToStrip =
ImmutableList.of(
"--primaryData",
"--directData",
"--data",
"--resources",
"--mergeeManifests",
"--library");
for (int i = 1; i < args.size(); i++) {
if (starlarkArgsToStrip.contains(args.get(i - 1))) {
args.set(i, argStripper.strip(args.get(i)));
}
}
return args;
}
};
}

/**
* Support for mapping config parts of exec paths in an action's command line.
* Support for mapping config parts of exec paths in an action's command line as well as when
* staging its inputs and outputs for execution.
*
* <p>Action implementation logic should use this to correctly set an action's command line.
* <p>Action implementation logic should use this to correctly set an action's command line. The
* executor should use this to correctly stage an action for execution.
*/
public interface CommandAdjuster {
public interface PathMapper {
/**
* Returns the exec path to refer to an input or output by.
* Returns the exec path of the input with the config part replaced if necessary.
*
* <p>If the action should be config-stripped ({@link PathStripper}), removes "k8-fastbuild"
* from paths like "bazel-out/k8-fastbuild/foo/bar".
Expand All @@ -149,79 +198,8 @@ default List<String> mapCustomStarlarkArgs(List<String> args) {
return args;
}

/**
* Creates a new command adjuster for action implementation logic to use.
*
* @param stripOutputPaths should this action strip config prefixes?
* @param starlarkMnemonic this action's mnemonic if it's a Starlark action, else null
* @param outputRoot the root path where outputs are written (e.g. "bazel-out"). Actions that
* don't strip outputs can set this to null.
*/
static CommandAdjuster create(
boolean stripOutputPaths,
@Nullable String starlarkMnemonic,
@Nullable PathFragment outputRoot) {
if (stripOutputPaths) {
Preconditions.checkNotNull(outputRoot);
Preconditions.checkState(outputRoot.isSingleSegment());
Preconditions.checkState(!outputRoot.getPathString().contains("\\"));
}
return stripOutputPaths ? commandStripper(starlarkMnemonic, outputRoot) : NOOP;
}

/** Instantiates a {@link CommandAdjuster} that doesn't change paths. */
CommandAdjuster NOOP = execPath -> execPath;

/** Instantiates a {@link CommandAdjuster} that strips config prefixes from output paths. */
private static CommandAdjuster commandStripper(
@Nullable String starlarkMnemonic, PathFragment outputRoot) {
final StringStripper argStripper =
starlarkMnemonic != null ? new StringStripper(outputRoot.getPathString()) : null;
return new CommandAdjuster() {
@Override
public String getMappedExecPathString(ActionInput artifact) {
if (artifact instanceof DerivedArtifact) {
return PathStripper.strip(artifact);
} else {
return artifact.getExecPathString();
}
}

@Override
public PathFragment map(PathFragment execPath) {
return PathStripper.isOutputPath(execPath, outputRoot)
? PathStripper.strip(execPath)
: execPath;
}

@Override
public List<String> mapCustomStarlarkArgs(List<String> args) {
// Add your favorite Starlark mnemonic that needs custom arg processing here.
if (!starlarkMnemonic.contains("Android")
&& !starlarkMnemonic.equals("MergeManifests")
&& !starlarkMnemonic.equals("StarlarkRClassGenerator")) {
return args;
}
// Add your favorite arg to custom-process here. When Bazel finds one of these in the
// argument list (an argument name), it strips output path prefixes from the following
// argument (the argument value).
ImmutableList<String> starlarkArgsToStrip =
ImmutableList.of(
"--primaryData",
"--directData",
"--data",
"--resources",
"--mergeeManifests",
"--library");
for (int i = 1; i < args.size(); i++) {
if (starlarkArgsToStrip.contains(args.get(i - 1))) {
args.set(i, argStripper.strip(args.get(i)));
}
}
return args;
}
};
}
/** Instantiates a {@link PathMapper} that doesn't change paths. */
PathMapper NOOP = execPath -> execPath;
}

/**
Expand Down
Loading

0 comments on commit acec6c0

Please sign in to comment.