Skip to content

Add working directory configuration option for Maven plugin run/start #6645

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

Closed
wants to merge 3 commits into from
Closed

Add working directory configuration option for Maven plugin run/start #6645

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7dafcce
Allow clients of RunProcess to specify working directory
Aug 13, 2016
12001df
Add additional Javadoc comments to RunProcess
Aug 15, 2016
0febe25
Add working directory configuration option for Maven plugin run/start
Aug 13, 2016
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
Next Next commit
Allow clients of RunProcess to specify working directory 
In some cases the working directory of a process is important.
  • Loading branch information
Plamen Totev committed Aug 15, 2016
commit 7dafccea097110b48240fb573114d1bd0d3e1266
11 changes: 10 additions & 1 deletion ...ing-boot-loader-tools/src/main/java/org/springframework/boot/loader/tools/RunProcess.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package org.springframework.boot.loader.tools;

import java.io.BufferedReader;
import java.io.File;
import java.io.IOException;
import java.io.InputStreamReader;
import java.lang.reflect.Method;
Expand All @@ -41,23 +42,31 @@ public class RunProcess {

private static final long JUST_ENDED_LIMIT = 500;

private File workingDirectory;

private final String[] command;

private volatile Process process;

private volatile long endTime;

public RunProcess(String... command) {
public RunProcess(File workingDirectory, String... command) {
this.workingDirectory = workingDirectory;
this.command = command;
}

public RunProcess(String... command) {
this(null, command);
}

public int run(boolean waitForProcess, String... args) throws IOException {
return run(waitForProcess, Arrays.asList(args));
}

protected int run(boolean waitForProcess, Collection<String> args)
throws IOException {
ProcessBuilder builder = new ProcessBuilder(this.command);
builder.directory(workingDirectory);
Copy link
Member

@snicoll snicoll Aug 14, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't set it when it's null

Copy link
Author

@plamentotev plamentotev Aug 14, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually null is a valid value. It does nothing but still I feel a bit uncomfortable to just ignore the null value. In general I think in similar cases is better to no accept null values at all(if we consider then as not a valid values) and throw NPE at the constructor. But I guess in this particular case it's enough just to add a note to the javadoc that the null for wokingDirecotry does not change the working directory.
What do you think?

Copy link
Member

@snicoll snicoll Aug 14, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that I've looked at the javadoc of ProcessBuillder#directory I take it back: it is very explicit about the semantic of null there so I'd keep it like that. I'd add a javadoc on the constructor to reflect that information (but I can take care of that myself when I merge).

Feel free to update this PR, just push an additional commit on your branch (or preferably, rebase + push force)

Copy link
Author

@plamentotev plamentotev Aug 15, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated the PR (+rebase). Also I've tried to add some javadoc comments. Writing comments (especially in English) is not my forte, so fell free to edit them 😄

builder.command().addAll(args);
builder.redirectErrorStream(true);
boolean inheritedIO = inheritIO(builder);
Expand Down