Fixes eirslett#41 eirslett#343 rewrites ProcessExecutor using commons…

…-exec

By using commons-exec instead of ProcessBuilder we can support killing
the Node process if the Maven build is stopped. An optional timeout can
be specified (not exposed to the task runners yet).

A workaround for eirslett#343 is implemented as well.

frontend-maven-plugin/pom.xml

@@ -98,7 +98,7 @@
             <plugin>
                 <groupId>org.apache.maven.plugins</groupId>
                 <artifactId>maven-invoker-plugin</artifactId>
-                <version>1.8</version>
+                <version>2.0.0</version>
 
                 <configuration>
                     <debug>true</debug>

frontend-plugin-core/pom.xml

@@ -29,6 +29,12 @@
             <version>1.3.2</version>
         </dependency>
 
+        <dependency>
+            <groupId>org.apache.commons</groupId>
+            <artifactId>commons-exec</artifactId>
+            <version>1.3</version>
+        </dependency>
+
         <dependency>
             <groupId>org.apache.httpcomponents</groupId>
             <artifactId>httpclient</artifactId>

frontend-plugin-core/src/main/java/com/github/eirslett/maven/plugins/frontend/lib/ProcessExecutor.java

@@ -1,16 +1,28 @@
 package com.github.eirslett.maven.plugins.frontend.lib;
 
-import java.io.BufferedReader;
+import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.IOException;
-import java.io.InputStream;
-import java.io.InputStreamReader;
+import java.io.OutputStream;
+import java.util.HashMap;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 
+import org.apache.commons.exec.CommandLine;
+import org.apache.commons.exec.DefaultExecutor;
+import org.apache.commons.exec.ExecuteException;
+import org.apache.commons.exec.ExecuteStreamHandler;
+import org.apache.commons.exec.ExecuteWatchdog;
+import org.apache.commons.exec.Executor;
+import org.apache.commons.exec.LogOutputStream;
+import org.apache.commons.exec.PumpStreamHandler;
+import org.apache.commons.exec.ShutdownHookProcessDestroyer;
 import org.slf4j.Logger;
 
 final class ProcessExecutionException extends Exception {
+    private static final long serialVersionUID = 1L;
+
     public ProcessExecutionException(String message) {
         super(message);
     }
@@ -20,67 +32,62 @@ public ProcessExecutionException(Throwable cause) {
 }
 
 final class ProcessExecutor {
-    private final File workingDirectory;
-    private final List<String> localPaths;
-    private final List<String> command;
-    private final ProcessBuilder processBuilder;
-    private final Platform platform;
-
-    public ProcessExecutor(File workingDirectory, List<String> paths, List<String> command, Platform platform){
-        this.workingDirectory = workingDirectory;
-        this.localPaths = paths;
-        this.command = command;
-        this.platform = platform;
-
-        this.processBuilder = createProcessBuilder();
+    private Map<String, String> environment;
+    private CommandLine commandLine;
+    private final Executor executor;
+
+    public ProcessExecutor(File workingDirectory, List<String> paths, List<String> command, Platform platform) {
+        this(workingDirectory, paths, command, platform, 0);
+    }
+
+    public ProcessExecutor(File workingDirectory, List<String> paths, List<String> command, Platform platform,
+            long timeoutInSeconds) {
+        this.environment = createEnvironment(paths, platform);
+        this.commandLine = createCommandLine(command);
+        this.executor = createExecutor(workingDirectory, timeoutInSeconds);
     }
 
     public String executeAndGetResult() throws ProcessExecutionException {
-        try {
-            final Process process = processBuilder.start();
-            final String result = readString(process.getInputStream());
-            final String error = readString(process.getErrorStream());
-            final int exitValue = process.waitFor();
+        ByteArrayOutputStream stdout = new ByteArrayOutputStream();
+        ByteArrayOutputStream stderr = new ByteArrayOutputStream();
 
-            if(exitValue == 0){
-                return result;
-            } else {
-                throw new ProcessExecutionException(result+" "+error);
-            }
-        } catch (IOException e) {
-            throw new ProcessExecutionException(e);
-        } catch (InterruptedException e) {
-            throw new ProcessExecutionException(e);
+        int exitValue = execute(stdout, stderr);
+        if (exitValue == 0) {
+            return stdout.toString().trim();
+        } else {
+            throw new ProcessExecutionException(stdout + " " + stderr);
         }
     }
 
     public int executeAndRedirectOutput(final Logger logger) throws ProcessExecutionException {
-        try {
-            final Process process = processBuilder.start();
+        OutputStream stdout = new LoggerOutputStream(logger, 0);
+        OutputStream stderr = new LoggerOutputStream(logger, 1);
 
-            final Thread infoLogThread = InputStreamHandler.logInfo(process.getInputStream(), logger);
-            infoLogThread.start();
-            final Thread errorLogThread = InputStreamHandler.logError(process.getErrorStream(), logger);
-            errorLogThread.start();
+        return execute(stdout, stderr);
+    }
 
-            int result = process.waitFor();
-            infoLogThread.join();
-            errorLogThread.join();
-            return result;
-        } catch (IOException e) {
+    private int execute(OutputStream stdout, OutputStream stderr) throws ProcessExecutionException {
+        try {
+            ExecuteStreamHandler streamHandler = new PumpStreamHandler(stdout, stderr);
+            executor.setStreamHandler(streamHandler);
+
+            return executor.execute(commandLine, environment);
+        } catch (ExecuteException e) {
+            if (executor.getWatchdog() != null && executor.getWatchdog().killedProcess()) {
+                throw new ProcessExecutionException("Process killed after timeout");
+            }
             throw new ProcessExecutionException(e);
-        } catch (InterruptedException e) {
+        } catch (IOException e) {
             throw new ProcessExecutionException(e);
         }
     }
 
-    private ProcessBuilder createProcessBuilder(){
-        ProcessBuilder pbuilder = new ProcessBuilder(command).directory(workingDirectory);
-        final Map<String, String> environment = pbuilder.environment();
+    private Map<String, String> createEnvironment(List<String> paths, Platform platform) {
+        final Map<String, String> environment = new HashMap<String, String>(System.getenv());
         String pathVarName = "PATH";
         String pathVarValue = environment.get(pathVarName);
         if (platform.isWindows()) {
-            for (String key:environment.keySet()) {
+            for (String key : environment.keySet()) {
                 if ("PATH".equalsIgnoreCase(key)) {
                     pathVarName = key;
                     pathVarValue = environment.get(key);
@@ -92,21 +99,62 @@ private ProcessBuilder createProcessBuilder(){
         if (pathVarValue != null) {
             pathBuilder.append(pathVarValue).append(File.pathSeparator);
         }
-        for (String path : localPaths) {
-        	pathBuilder.insert(0, File.pathSeparator).insert(0, path);
+        for (String path : paths) {
+            pathBuilder.insert(0, File.pathSeparator).insert(0, path);
         }
         environment.put(pathVarName, pathBuilder.toString());
 
-        return pbuilder;
+        return environment;
     }
 
-    private static String readString(InputStream processInputStream) throws IOException {
-        BufferedReader inputStream = new BufferedReader(new InputStreamReader(processInputStream));
-        StringBuilder result = new StringBuilder();
-        String line;
-        while((line = inputStream.readLine()) != null) {
-            result.append(line).append("\n");
+    private CommandLine createCommandLine(List<String> command) {
+        Iterator<String> args = command.iterator();
+        CommandLine commmandLine = new CommandLine(args.next());
+
+        while(args.hasNext()) {
+            commmandLine.addArgument(args.next());
+        }
+
+        return commmandLine;
+    }
+
+    private Executor createExecutor(File workingDirectory, long timeoutInSeconds) {
+        DefaultExecutor executor = new DefaultExecutor();
+        executor.setWorkingDirectory(workingDirectory);
+        executor.setProcessDestroyer(new ShutdownHookProcessDestroyer());   // Fixes #41
+
+        if (timeoutInSeconds > 0) {
+            executor.setWatchdog(new ExecuteWatchdog(timeoutInSeconds * 1000));
+        }
+
+        return executor;
+    }
+
+    private class LoggerOutputStream extends LogOutputStream {
+        private final Logger logger;
+
+        public LoggerOutputStream(Logger logger, int logLevel) {
+            super(logLevel);
+            this.logger = logger;
+        }
+
+        @Override
+        public final void flush() {
+            // buffer processing on close() only
+        }
+
+        @Override
+        protected void processLine(final String line, final int logLevel) {
+            if (logLevel == 0) {
+                logger.info(line);
+            } else {
+                // FIXME: workaround for #343 -> delegate this check (via callback) to specific NodeTaskExecutor runner implementation 
+                if (line.startsWith("npm WARN ")) {
+                    logger.warn(line);
+                } else {
+                    logger.error(line);
+                }
+            }
         }
-        return result.toString().trim();
     }
 }