Skip to content

Commit

Permalink
Use absolute paths in ProcessBuilder invocations.
Browse files Browse the repository at this point in the history
Needed for #276.

--
MOS_MIGRATED_REVID=114838538
  • Loading branch information
dslomov authored and damienmg committed Feb 17, 2016
1 parent 2fc8f97 commit 8efc3ef
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 6 deletions.
28 changes: 23 additions & 5 deletions src/main/java/com/google/devtools/build/lib/shell/Command.java
Original file line number Diff line number Diff line change
Expand Up @@ -178,23 +178,41 @@ public Command(final String[] commandLineElements) {

/**
* Creates a new {@link Command} for the given command line elements. The
* command line is executed exactly as given, without a shell. The given
* environment variables and working directory are used in subsequent
* command line is executed without a shell.
*
* The given environment variables and working directory are used in subsequent
* calls to {@link #execute()}.
*
* This command treats the 0-th element of {@code commandLineElement}
* (the name of an executable to run) specially.
* <ul>
* <li>If it is an absolute path, it is used as it</li>
* <li>If it is a single file name, the PATH lookup is performed</li>
* <li>If it is a relative path that is not a single file name, the command will attempt to
* execute the the binary at that path relative to {@code workingDirectory}.</li>
* </ul>
*
* @param commandLineElements elements of raw command line to execute
* @param environmentVariables environment variables to replace JVM's
* environment variables; may be null
* @param workingDirectory working directory for execution; if null, current
* working directory is used
* @throws IllegalArgumentException if commandLine is null or empty
*/
public Command(final String[] commandLineElements,
final Map<String, String> environmentVariables,
final File workingDirectory) {
public Command(
String[] commandLineElements,
final Map<String, String> environmentVariables,
final File workingDirectory) {
if (commandLineElements == null || commandLineElements.length == 0) {
throw new IllegalArgumentException("command line is null or empty");
}

File executable = new File(commandLineElements[0]);
if (!executable.isAbsolute() && executable.getParent() != null) {
commandLineElements = commandLineElements.clone();
commandLineElements[0] = new File(workingDirectory, commandLineElements[0]).getAbsolutePath();
}

this.processBuilder =
new ProcessBuilder(commandLineElements);
if (environmentVariables != null) {
Expand Down
10 changes: 9 additions & 1 deletion src/main/java/com/google/devtools/build/lib/worker/Worker.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import com.google.devtools.build.lib.util.Preconditions;
import com.google.devtools.build.lib.vfs.Path;

import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
Expand Down Expand Up @@ -58,8 +59,15 @@ static Worker create(WorkerKey key, Path logDir, Reporter reporter, boolean verb
int workerId = pidCounter.getAndIncrement();
Path logFile = logDir.getRelative("worker-" + workerId + "-" + key.getMnemonic() + ".log");

String[] command = key.getArgs().toArray(new String[0]);

// Follows the logic of {@link com.google.devtools.build.lib.shell.Command}.
File executable = new File(command[0]);
if (!executable.isAbsolute() && executable.getParent() != null) {
command[0] = new File(key.getWorkDir().getPathFile(), command[0]).getAbsolutePath();
}
ProcessBuilder processBuilder =
new ProcessBuilder(key.getArgs().toArray(new String[0]))
new ProcessBuilder(command)
.directory(key.getWorkDir().getPathFile())
.redirectError(Redirect.appendTo(logFile.getPathFile()));
processBuilder.environment().putAll(key.getEnv());
Expand Down
10 changes: 10 additions & 0 deletions src/test/java/com/google/devtools/build/lib/shell/CommandTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.testutil.BlazeTestUtils;
import com.google.devtools.build.lib.testutil.TestConstants;

Expand Down Expand Up @@ -682,4 +683,13 @@ private static void checkSuccess(final CommandResult result,
assertEquals(0, result.getStderr().length);
assertEquals(expectedOutput, new String(result.getStdout()));
}

@Test
public void testRelativePath() throws Exception {
Command command = new Command(new String[]{"relative/path/to/binary"},
ImmutableMap.<String, String>of(),
new File("/working/directory"));
assertThat(command.getCommandLineElements()[0])
.isEqualTo("/working/directory/relative/path/to/binary");
}
}

0 comments on commit 8efc3ef

Please sign in to comment.