diff options
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib')
6 files changed, 29 insertions, 135 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelWorkspaceStatusModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelWorkspaceStatusModule.java index 2434bff695..ef7ecdf4cb 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/BazelWorkspaceStatusModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelWorkspaceStatusModule.java @@ -109,14 +109,12 @@ public class BazelWorkspaceStatusModule extends BlazeModule { this.getWorkspaceStatusCommand = options.workspaceStatusCommand.equals(PathFragment.EMPTY_FRAGMENT) ? null - : new CommandBuilder() - .addArgs(options.workspaceStatusCommand.toString()) + : new CommandBuilder(workspace) + .addArg(options.workspaceStatusCommand.toString()) // Pass client env, because certain SCM client(like // perforce, git) relies on environment variables to work // correctly. .setEnv(clientEnv) - .setWorkingDir(workspace) - .useShell(true) .build(); this.workspace = workspace; } diff --git a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java index 5f231e4fd0..2eaaf499c5 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java +++ b/src/main/java/com/google/devtools/build/lib/exec/SymlinkTreeHelper.java @@ -17,7 +17,6 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.Lists; import com.google.devtools.build.lib.actions.ActionExecutionContext; import com.google.devtools.build.lib.actions.ActionExecutionMetadata; import com.google.devtools.build.lib.actions.ActionInput; @@ -37,6 +36,7 @@ import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import java.io.IOException; +import java.util.ArrayList; import java.util.List; /** @@ -90,10 +90,7 @@ public final class SymlinkTreeHelper { Path execRoot, BuildConfiguration config, BinTools binTools) throws CommandException { List<String> argv = getSpawnArgumentList(execRoot, binTools.getExecPath(BUILD_RUNFILES)); - CommandBuilder builder = new CommandBuilder(); - builder.addArgs(argv); - builder.setWorkingDir(execRoot); - builder.build().execute(); + new CommandBuilder(execRoot).addArgs(argv).build().execute(); } /** @@ -148,7 +145,7 @@ public final class SymlinkTreeHelper { ActionInput buildRunfiles = binTools.getActionInput(BUILD_RUNFILES); return new SimpleSpawn( owner, - getSpawnArgumentList(execRoot, buildRunfiles.getExecPath()), + ImmutableList.copyOf(getSpawnArgumentList(execRoot, buildRunfiles.getExecPath())), environment, ImmutableMap.of( ExecutionRequirements.LOCAL, "", @@ -159,11 +156,9 @@ public final class SymlinkTreeHelper { RESOURCE_SET); } - /** - * Returns the complete argument list build-runfiles has to be called with. - */ - private ImmutableList<String> getSpawnArgumentList(Path execRoot, PathFragment buildRunfiles) { - List<String> args = Lists.newArrayList(); + /** Returns the complete argument list build-runfiles has to be called with. */ + private ArrayList<String> getSpawnArgumentList(Path execRoot, PathFragment buildRunfiles) { + ArrayList<String> args = new ArrayList<>(filesetTree ? 5 : 3); args.add(buildRunfiles.getPathString()); if (filesetTree) { @@ -174,6 +169,6 @@ public final class SymlinkTreeHelper { args.add(inputManifest.relativeTo(execRoot).getPathString()); args.add(symlinkTreeRoot.relativeTo(execRoot).getPathString()); - return ImmutableList.copyOf(args); + return args; } } diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/CleanCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/CleanCommand.java index d848d47469..c9c91f3f48 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/commands/CleanCommand.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/CleanCommand.java @@ -194,10 +194,10 @@ public final class CleanCommand implements BlazeCommand { logger.info("Executing shell commmand " + ShellEscaper.escapeString(command)); // Doesn't throw iff command exited and was successful. - new CommandBuilder() - .addArg(command) - .useShell(true) - .setWorkingDir(tempPath.getParentDirectory()) + new CommandBuilder(tempPath.getParentDirectory()) + // TODO(laszlocsomor): implement the async directory tree deleter as a native, embedded + // binary, and stop relying on the shell. See GitHub issue #4319. + .addArgs("/bin/sh", "-c", command) .build() .execute(); } diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java index 8aa3566041..adcae97ff5 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java @@ -237,8 +237,8 @@ public class RunCommand implements BlazeCommand { null, "Running command line: " + ShellEscaper.escapeJoinAll(prettyCmdLine))); try { - com.google.devtools.build.lib.shell.Command command = new CommandBuilder() - .addArgs(cmdLine).setEnv(env.getClientEnv()).setWorkingDir(workingDir).build(); + com.google.devtools.build.lib.shell.Command command = + new CommandBuilder(workingDir).addArgs(cmdLine).setEnv(env.getClientEnv()).build(); // Restore a raw EventHandler if it is registered. This allows for blaze run to produce the // actual output of the command being run even if --color=no is specified. diff --git a/src/main/java/com/google/devtools/build/lib/runtime/mobileinstall/MobileInstallCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/mobileinstall/MobileInstallCommand.java index 6484bbe3f2..0d43d8f95a 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/mobileinstall/MobileInstallCommand.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/mobileinstall/MobileInstallCommand.java @@ -285,11 +285,7 @@ public class MobileInstallCommand implements BlazeCommand { Path workingDir = env.getBlazeWorkspace().getOutputPath().getParentDirectory(); com.google.devtools.build.lib.shell.Command command = - new CommandBuilder() - .addArgs(cmdLine) - .setEnv(env.getClientEnv()) - .setWorkingDir(workingDir) - .build(); + new CommandBuilder(workingDir).addArgs(cmdLine).setEnv(env.getClientEnv()).build(); try { // Restore a raw EventHandler if it is registered. This allows for blaze run to produce the diff --git a/src/main/java/com/google/devtools/build/lib/util/CommandBuilder.java b/src/main/java/com/google/devtools/build/lib/util/CommandBuilder.java index d60fd94dc4..9bff531521 100644 --- a/src/main/java/com/google/devtools/build/lib/util/CommandBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/util/CommandBuilder.java @@ -14,65 +14,44 @@ package com.google.devtools.build.lib.util; -import static com.google.common.base.StandardSystemProperty.JAVA_IO_TMPDIR; - -import com.google.common.annotations.VisibleForTesting; -import com.google.common.base.CharMatcher; -import com.google.common.base.Joiner; import com.google.common.base.Preconditions; -import com.google.common.base.Splitter; -import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; -import com.google.common.collect.Lists; import com.google.devtools.build.lib.shell.Command; import com.google.devtools.build.lib.vfs.Path; import java.io.File; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; -import java.util.List; import java.util.Map; /** - * Implements OS aware {@link Command} builder. At this point only Linux, Mac - * and Windows XP are supported. - * - * <p>Builder will also apply heuristic to identify trivial cases where - * unix-like command lines could be automatically converted into the - * Windows-compatible form. + * Implements {@link Command} builder. * - * <p>TODO(bazel-team): (2010) Some of the code here is very similar to the - * {@link com.google.devtools.build.lib.shell.Shell} class. This should be looked at. + * <p>TODO(bazel-team): (2010) Some of the code here is very similar to the {@link + * com.google.devtools.build.lib.shell.Shell} class. This should be looked at. */ public final class CommandBuilder { - private static final ImmutableList<String> SHELLS = ImmutableList.of("/bin/sh", "/bin/bash"); - - private static final Splitter ARGV_SPLITTER = Splitter.on(CharMatcher.anyOf(" \t")); + private final ArrayList<String> argv = new ArrayList<>(); + private final HashMap<String, String> env = new HashMap<>(); + private final File workingDir; - private final OS system; - private final List<String> argv = new ArrayList<>(); - private final Map<String, String> env = new HashMap<>(); - private File workingDir = null; - private boolean useShell = false; - - public CommandBuilder() { - this(OS.getCurrent()); + public CommandBuilder(Path workingDir) { + this(workingDir.getPathFile()); } - @VisibleForTesting - CommandBuilder(OS system) { - this.system = system; + public CommandBuilder(File workingDir) { + this.workingDir = Preconditions.checkNotNull(workingDir); } public CommandBuilder addArg(String arg) { - Preconditions.checkNotNull(arg, "Argument must not be null"); + Preconditions.checkNotNull(arg); argv.add(arg); return this; } public CommandBuilder addArgs(Iterable<String> args) { - Preconditions.checkArgument(!Iterables.contains(args, null), "Arguments must not be null"); + Preconditions.checkArgument(!Iterables.contains(args, null)); Iterables.addAll(argv, args); return this; } @@ -98,82 +77,8 @@ public final class CommandBuilder { return this; } - public CommandBuilder setWorkingDir(Path path) { - Preconditions.checkNotNull(path); - workingDir = path.getPathFile(); - return this; - } - - public CommandBuilder useTempDir() { - workingDir = new File(JAVA_IO_TMPDIR.value()); - return this; - } - - public CommandBuilder useShell(boolean useShell) { - this.useShell = useShell; - return this; - } - - private boolean argvStartsWithSh() { - return argv.size() >= 2 && SHELLS.contains(argv.get(0)) && "-c".equals(argv.get(1)); - } - - private String[] transformArgvForLinux() { - // If command line already starts with "/bin/sh -c", ignore useShell attribute. - if (useShell && !argvStartsWithSh()) { - // c.g.io.base.shell.Shell.shellify() actually concatenates argv into the space-separated - // string here. Not sure why, but we will do the same. - return new String[] { "/bin/sh", "-c", Joiner.on(' ').join(argv) }; - } - return argv.toArray(new String[argv.size()]); - } - - private String[] transformArgvForWindows() { - List<String> modifiedArgv; - // Heuristic: replace "/bin/sh -c" with something more appropriate for Windows. - if (argvStartsWithSh()) { - useShell = true; - modifiedArgv = Lists.newArrayList(argv.subList(2, argv.size())); - } else { - modifiedArgv = Lists.newArrayList(argv); - } - - if (!modifiedArgv.isEmpty()) { - // args can contain whitespace, so figure out the first word - String argv0 = modifiedArgv.get(0); - String command = ARGV_SPLITTER.split(argv0).iterator().next(); - - // Automatically enable CMD.EXE use if we are executing something else besides "*.exe" file. - // When use CMD.EXE to invoke a bat/cmd file, the file path must have '\' instead of '/' - if (!command.toLowerCase().endsWith(".exe")) { - useShell = true; - modifiedArgv.set(0, argv0.replace('/', '\\')); - } - } else { - // This is degenerate "/bin/sh -c" case. We ensure that Windows behavior is identical - // to the Linux - call shell that will do nothing. - useShell = true; - } - if (useShell) { - // /S - strip first and last quotes and execute everything else as is. - // /E:ON - enable extended command set. - // /V:ON - enable delayed variable expansion - // /D - ignore AutoRun registry entries. - // /C - execute command. This must be the last option before the command itself. - return new String[] { "CMD.EXE", "/S", "/E:ON", "/V:ON", "/D", "/C", - Joiner.on(' ').join(modifiedArgv) }; - } else { - return modifiedArgv.toArray(new String[argv.size()]); - } - } - public Command build() { - Preconditions.checkState(system != OS.UNKNOWN, "Unidentified operating system"); - Preconditions.checkNotNull(workingDir, "Working directory must be set"); Preconditions.checkState(!argv.isEmpty(), "At least one argument is expected"); - - return new Command( - system == OS.WINDOWS ? transformArgvForWindows() : transformArgvForLinux(), - env, workingDir); + return new Command(argv.toArray(new String[0]), env, workingDir); } } |