diff --git a/maven-batch-executor/src/main/java/org/apache/maven/executor/batch/internal/InternalStepContext.java b/maven-batch-executor/src/main/java/org/apache/maven/executor/batch/internal/InternalStepContext.java index aa31cb5..a68c9db 100644 --- a/maven-batch-executor/src/main/java/org/apache/maven/executor/batch/internal/InternalStepContext.java +++ b/maven-batch-executor/src/main/java/org/apache/maven/executor/batch/internal/InternalStepContext.java @@ -18,7 +18,6 @@ */ package org.apache.maven.executor.batch.internal; -import java.io.ByteArrayOutputStream; import java.nio.file.Path; import java.util.List; import java.util.Map; @@ -28,6 +27,7 @@ import org.apache.maven.executor.Executor; import org.apache.maven.executor.ExecutorException; import org.apache.maven.executor.ExecutorRequest; +import org.apache.maven.executor.ExecutorResult; import org.apache.maven.executor.ExecutorTool; import org.apache.maven.executor.batch.steps.Environment; import org.apache.maven.executor.batch.steps.Execution; @@ -97,24 +97,22 @@ public Execution.Result execute(Execution.Request execution) throws ExecutorExce execution.environmentVariables().ifPresent(ev -> ev.forEach(builder::environmentVariable)); execution.jvmSystemProperties().ifPresent(sp -> sp.forEach(builder::jvmSystemProperty)); execution.jvmArguments().ifPresent(ja -> ja.forEach(builder::jvmArgument)); - ByteArrayOutputStream out = new ByteArrayOutputStream(); - ByteArrayOutputStream err = new ByteArrayOutputStream(); - int exitCode = executor.execute(builder.stdOut(out).stdErr(err).build()); + ExecutorResult res = executor.execute(builder.build()); Execution.Result result = new Execution.Result() { @Override public int exitCode() { - return exitCode; + return res.exitCode().orElseThrow(); } @Override public String stdOut() { - return out.toString(); + return res.stdOutString().orElseThrow(); } @Override public String stdErr() { - return err.toString(); + return res.stdErrString().orElseThrow(); } }; resultList.add(result); diff --git a/maven-batch-executor/src/test/java/org/apache/maven/executor/batch/BatchExecutorTest.java b/maven-batch-executor/src/test/java/org/apache/maven/executor/batch/BatchExecutorTest.java index ab47f3c..8515f12 100644 --- a/maven-batch-executor/src/test/java/org/apache/maven/executor/batch/BatchExecutorTest.java +++ b/maven-batch-executor/src/test/java/org/apache/maven/executor/batch/BatchExecutorTest.java @@ -21,9 +21,6 @@ import javax.script.ScriptEngine; import javax.script.ScriptEngineManager; -import java.io.IOException; -import java.io.OutputStream; -import java.nio.charset.StandardCharsets; import java.nio.file.Path; import java.util.List; import java.util.Map; @@ -32,11 +29,13 @@ import org.apache.maven.executor.Executor; import org.apache.maven.executor.ExecutorException; import org.apache.maven.executor.ExecutorRequest; +import org.apache.maven.executor.ExecutorResult; import org.apache.maven.executor.batch.steps.ContextStep; import org.apache.maven.executor.batch.steps.Environment; import org.apache.maven.executor.batch.steps.ExecuteStep; import org.apache.maven.executor.batch.steps.Execution; import org.apache.maven.executor.batch.steps.Step; +import org.apache.maven.executor.support.SimpleExecutionResult; import org.junit.jupiter.api.Test; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -48,16 +47,8 @@ public class BatchExecutorTest { void smoke() { Executor executor = new Executor() { @Override - public int execute(ExecutorRequest executorRequest) throws ExecutorException { - try { - executorRequest - .stdOut() - .orElse(OutputStream.nullOutputStream()) - .write("Executed!".getBytes(StandardCharsets.UTF_8)); - return 0; - } catch (IOException e) { - throw new ExecutorException(e); - } + public ExecutorResult execute(ExecutorRequest executorRequest) throws ExecutorException { + return new SimpleExecutionResult(executorRequest, true, 0, "Executed", null); } @Override diff --git a/maven-executor/pom.xml b/maven-executor/pom.xml index 522817a..5a8c8da 100644 --- a/maven-executor/pom.xml +++ b/maven-executor/pom.xml @@ -35,7 +35,8 @@ - 0.15.8 + 8 + 0.15.9 @@ -53,6 +54,25 @@ + + org.apache.maven.plugins + maven-compiler-plugin + + + compile-java17 + + compile + + compile + + 17 + + ${project.basedir}/src/main/java17 + + + + + org.apache.maven.plugins maven-dependency-plugin diff --git a/maven-executor/src/main/java/org/apache/maven/executor/Executor.java b/maven-executor/src/main/java/org/apache/maven/executor/Executor.java index 906d657..5cc9b8d 100644 --- a/maven-executor/src/main/java/org/apache/maven/executor/Executor.java +++ b/maven-executor/src/main/java/org/apache/maven/executor/Executor.java @@ -38,10 +38,10 @@ public interface Executor extends AutoCloseable { * process based on the information contained in the request. * * @param executorRequest the request containing all necessary information for the execution - * @return an integer representing the exit code of the execution (0 typically indicates success) + * @return ExecutorResult carrying the result of the execution * @throws ExecutorException if an error occurs during the execution process */ - int execute(ExecutorRequest executorRequest) throws ExecutorException; + ExecutorResult execute(ExecutorRequest executorRequest) throws ExecutorException; /** * Returns the Maven version that this executor points at (would use). This operation, depending on the underlying diff --git a/maven-executor/src/main/java/org/apache/maven/executor/ExecutorHelper.java b/maven-executor/src/main/java/org/apache/maven/executor/ExecutorHelper.java index 3bd41c3..1d0a1e9 100644 --- a/maven-executor/src/main/java/org/apache/maven/executor/ExecutorHelper.java +++ b/maven-executor/src/main/java/org/apache/maven/executor/ExecutorHelper.java @@ -84,12 +84,12 @@ enum Mode { /** * Executes the request with preferred mode executor. */ - default int execute(ExecutorRequest executorRequest) throws ExecutorException { + default ExecutorResult execute(ExecutorRequest executorRequest) throws ExecutorException { return execute(getDefaultMode(), executorRequest); } /** * Executes the request with passed in mode executor. */ - int execute(Mode mode, ExecutorRequest executorRequest) throws ExecutorException; + ExecutorResult execute(Mode mode, ExecutorRequest executorRequest) throws ExecutorException; } diff --git a/maven-executor/src/main/java/org/apache/maven/executor/ExecutorRequest.java b/maven-executor/src/main/java/org/apache/maven/executor/ExecutorRequest.java index 94435fe..e5a54dd 100644 --- a/maven-executor/src/main/java/org/apache/maven/executor/ExecutorRequest.java +++ b/maven-executor/src/main/java/org/apache/maven/executor/ExecutorRequest.java @@ -22,8 +22,10 @@ import java.io.OutputStream; import java.nio.file.Path; import java.nio.file.Paths; +import java.time.Duration; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -93,9 +95,16 @@ public interface ExecutorRequest { */ Optional> jvmArguments(); + /** + * Whether execution outputs (STDOUT and STDERR) should be captured as plain {@link String}. + * By default, this is {@code true}, unless client manually sets any of {@link #stdOut()} or {@link #stdErr()} + * streams, in which case this value is set to {@code false} and caller must handle these streams manually. + */ + boolean grabOutputAsString(); + /** * Optional provider for STD in of the Maven. If given, this provider will be piped into std input of - * Maven. + * Maven. The stream is closed once tool execution is finished. * * @return an Optional containing the stdin provider, or empty if not specified. */ @@ -105,6 +114,7 @@ public interface ExecutorRequest { * Optional consumer for STD out of the Maven. If given, this consumer will get all output from the std out of * Maven. Note: whether consumer gets to consume anything depends on invocation arguments passed in * {@link #arguments()}, as if log file is set, not much will go to stdout. + * The stream is closed once tool execution is finished. * * @return an Optional containing the stdout consumer, or empty if not specified. */ @@ -114,6 +124,7 @@ public interface ExecutorRequest { * Optional consumer for STD err of the Maven. If given, this consumer will get all output from the std err of * Maven. Note: whether consumer gets to consume anything depends on invocation arguments passed in * {@link #arguments()}, as if log file is set, not much will go to stderr. + * The stream is closed once tool execution is finished. * * @return an Optional containing the stderr consumer, or empty if not specified. */ @@ -126,6 +137,13 @@ public interface ExecutorRequest { */ boolean skipMavenRc(); + /** + * The optional execution time limit. If set, and execution does not finish within the given time, it is considered + * failed and killed. If not set, no time limit is applied. Depending on implementation, the timeout detection may + * be imprecise. + */ + Optional executionTimeout(); + /** * Returns {@link Builder} created from this instance. */ @@ -138,10 +156,12 @@ default Builder toBuilder() { jvmSystemProperties().orElse(null), environmentVariables().orElse(null), jvmArguments().orElse(null), + grabOutputAsString(), stdIn().orElse(null), stdOut().orElse(null), stdErr().orElse(null), - skipMavenRc()); + skipMavenRc(), + executionTimeout().orElse(null)); } /** @@ -157,10 +177,12 @@ static Builder mavenBuilder() { null, null, null, + true, null, null, null, - false); + false, + null); } class Builder { @@ -171,10 +193,12 @@ class Builder { private Map jvmSystemProperties; private Map environmentVariables; private List jvmArguments; + private boolean grabOutputAsString; private InputStream stdIn; private OutputStream stdOut; private OutputStream stdErr; private boolean skipMavenRc; + private Duration executionTimeout; private Builder() {} @@ -187,10 +211,12 @@ private Builder( Map jvmSystemProperties, Map environmentVariables, List jvmArguments, + boolean grabOutputAsString, InputStream stdIn, OutputStream stdOut, OutputStream stdErr, - boolean skipMavenRc) { + boolean skipMavenRc, + Duration executionTimeout) { this.command = command; this.arguments = arguments; this.cwd = cwd; @@ -198,10 +224,12 @@ private Builder( this.jvmSystemProperties = jvmSystemProperties; this.environmentVariables = environmentVariables; this.jvmArguments = jvmArguments; + this.grabOutputAsString = grabOutputAsString; this.stdIn = stdIn; this.stdOut = stdOut; this.stdErr = stdErr; this.skipMavenRc = skipMavenRc; + this.executionTimeout = executionTimeout; } public Builder command(String command) { @@ -279,17 +307,28 @@ public Builder jvmArgument(String jvmArgument) { return this; } + public Builder grabOutputAsString(boolean grabOutputAsString) { + this.grabOutputAsString = grabOutputAsString; + if (grabOutputAsString) { + this.stdOut = null; + this.stdErr = null; + } + return this; + } + public Builder stdIn(InputStream stdIn) { this.stdIn = stdIn; return this; } public Builder stdOut(OutputStream stdOut) { + this.grabOutputAsString = false; this.stdOut = stdOut; return this; } public Builder stdErr(OutputStream stdErr) { + this.grabOutputAsString = false; this.stdErr = stdErr; return this; } @@ -299,6 +338,11 @@ public Builder skipMavenRc(boolean skipMavenRc) { return this; } + public Builder executionTimeout(Duration executionTimeout) { + this.executionTimeout = executionTimeout; + return this; + } + public ExecutorRequest build() { return new Impl( command, @@ -308,10 +352,12 @@ public ExecutorRequest build() { jvmSystemProperties, environmentVariables, jvmArguments, + grabOutputAsString, stdIn, stdOut, stdErr, - skipMavenRc); + skipMavenRc, + executionTimeout); } private static class Impl implements ExecutorRequest { @@ -322,10 +368,12 @@ private static class Impl implements ExecutorRequest { private final Map jvmSystemProperties; private final Map environmentVariables; private final List jvmArguments; + private final boolean grabOutputAsString; private final InputStream stdIn; private final OutputStream stdOut; private final OutputStream stdErr; private final boolean skipMavenRc; + private final Duration executionTimeout; @SuppressWarnings("ParameterNumber") private Impl( @@ -336,25 +384,33 @@ private Impl( Map jvmSystemProperties, Map environmentVariables, List jvmArguments, + boolean grabOutputAsString, InputStream stdIn, OutputStream stdOut, OutputStream stdErr, - boolean skipMavenRc) { + boolean skipMavenRc, + Duration executionTimeout) { this.command = requireNonNull(command); - this.arguments = arguments == null ? List.of() : List.copyOf(arguments); + this.arguments = arguments == null + ? Collections.emptyList() + : Collections.unmodifiableList(new ArrayList<>(arguments)); this.cwd = getCanonicalPath(requireNonNull(cwd)); this.userHomeDirectory = getCanonicalPath(requireNonNull(userHomeDirectory)); this.jvmSystemProperties = jvmSystemProperties != null && !jvmSystemProperties.isEmpty() - ? Map.copyOf(jvmSystemProperties) + ? Collections.unmodifiableMap(new HashMap<>(jvmSystemProperties)) : null; this.environmentVariables = environmentVariables != null && !environmentVariables.isEmpty() - ? Map.copyOf(environmentVariables) + ? Collections.unmodifiableMap(new HashMap<>(environmentVariables)) : null; - this.jvmArguments = jvmArguments != null && !jvmArguments.isEmpty() ? List.copyOf(jvmArguments) : null; + this.jvmArguments = jvmArguments != null && !jvmArguments.isEmpty() + ? Collections.unmodifiableList(new ArrayList<>(jvmArguments)) + : null; + this.grabOutputAsString = grabOutputAsString; this.stdIn = stdIn; this.stdOut = stdOut; this.stdErr = stdErr; this.skipMavenRc = skipMavenRc; + this.executionTimeout = executionTimeout; } @Override @@ -392,6 +448,11 @@ public Optional> jvmArguments() { return Optional.ofNullable(jvmArguments); } + @Override + public boolean grabOutputAsString() { + return grabOutputAsString; + } + @Override public Optional stdIn() { return Optional.ofNullable(stdIn); @@ -412,20 +473,27 @@ public boolean skipMavenRc() { return skipMavenRc; } + @Override + public Optional executionTimeout() { + return Optional.ofNullable(executionTimeout); + } + @Override public String toString() { - return getClass().getSimpleName() + "{" + "command='" + return "Impl{" + "command='" + command + '\'' + ", arguments=" + arguments + ", cwd=" - + cwd + ", installationDirectory=" + + cwd + ", userHomeDirectory=" + userHomeDirectory + ", jvmSystemProperties=" + jvmSystemProperties + ", environmentVariables=" + environmentVariables + ", jvmArguments=" - + jvmArguments + ", stdinProvider=" - + stdIn + ", stdoutConsumer=" - + stdOut + ", stderrConsumer=" + + jvmArguments + ", grabOutputAsString=" + + grabOutputAsString + ", stdIn=" + + stdIn + ", stdOut=" + + stdOut + ", stdErr=" + stdErr + ", skipMavenRc=" - + skipMavenRc + "}"; + + skipMavenRc + ", executionTimeout=" + + executionTimeout + '}'; } } } diff --git a/maven-executor/src/main/java/org/apache/maven/executor/ExecutorResult.java b/maven-executor/src/main/java/org/apache/maven/executor/ExecutorResult.java new file mode 100644 index 0000000..f1fe8a6 --- /dev/null +++ b/maven-executor/src/main/java/org/apache/maven/executor/ExecutorResult.java @@ -0,0 +1,59 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.executor; + +import java.util.Optional; + +/** + * Represents an execution result. + */ +public interface ExecutorResult { + /** + * The {@link ExecutorRequest} this result is for. + */ + ExecutorRequest getRequest(); + + /** + * The outcome of execution. + */ + boolean success(); + + /** + * The exit code, if available (ie running the tool happened in a way it did produce exit code). + */ + default Optional exitCode() { + return Optional.empty(); + } + + /** + * If {@link ExecutorRequest#grabOutputAsString()} was {@code true}, then the {@link String} containing + * STDOUT of tool. Never {@code null}, but maybe empty string. Otherwise, empty. + */ + default Optional stdOutString() { + return Optional.empty(); + } + + /** + * If {@link ExecutorRequest#grabOutputAsString()} was {@code true}, then the {@link String} containing + * STDERR of tool. Never {@code null}, but maybe empty string. Otherwise, empty. + */ + default Optional stdErrString() { + return Optional.empty(); + } +} diff --git a/maven-executor/src/main/java/org/apache/maven/executor/embedded/EmbeddedMavenExecutor.java b/maven-executor/src/main/java/org/apache/maven/executor/embedded/EmbeddedMavenExecutor.java index 3844868..eef1a04 100644 --- a/maven-executor/src/main/java/org/apache/maven/executor/embedded/EmbeddedMavenExecutor.java +++ b/maven-executor/src/main/java/org/apache/maven/executor/embedded/EmbeddedMavenExecutor.java @@ -18,6 +18,7 @@ */ package org.apache.maven.executor.embedded; +import java.io.ByteArrayOutputStream; import java.io.Closeable; import java.io.IOException; import java.io.InputStream; @@ -48,6 +49,8 @@ import org.apache.maven.executor.Executor; import org.apache.maven.executor.ExecutorException; import org.apache.maven.executor.ExecutorRequest; +import org.apache.maven.executor.ExecutorResult; +import org.apache.maven.executor.support.SimpleExecutionResult; import static java.util.Objects.requireNonNull; @@ -60,13 +63,15 @@ public class EmbeddedMavenExecutor implements Executor { /** * Maven4 supports multiple commands from same installation directory. */ - protected static final Map MVN4_MAIN_CLASSES = Map.of( - "mvn", - "org.apache.maven.cling.MavenCling", - "mvnenc", - "org.apache.maven.cling.MavenEncCling", - "mvnsh", - "org.apache.maven.cling.MavenShellCling"); + protected static final Map MVN4_MAIN_CLASSES; + + static { + Map mvn4MavenClasses = new HashMap<>(); + mvn4MavenClasses.put("mvn", "org.apache.maven.cling.MavenCling"); + mvn4MavenClasses.put("mvnenc", "org.apache.maven.cling.MavenEncCling"); + mvn4MavenClasses.put("mvnsh", "org.apache.maven.cling.MavenShellCling"); + MVN4_MAIN_CLASSES = Collections.unmodifiableMap(mvn4MavenClasses); + } /** * Context holds things loaded up from given Maven Installation Directory. @@ -77,7 +82,7 @@ protected static class Context { private final Object classWorld; private final Set originalClassRealmIds; private final ClassLoader tccl; - private final Map> commands; // the commands + private final Map> commands; // the commands private final Collection keepAlive; // refs things to make sure no GC takes it away protected Context( @@ -86,7 +91,7 @@ protected Context( Object classWorld, Set originalClassRealmIds, ClassLoader tccl, - Map> commands, + Map> commands, Collection keepAlive) { this.bootClassLoader = bootClassLoader; this.version = version; @@ -126,7 +131,7 @@ public EmbeddedMavenExecutor(Path installationDirectory, boolean useMavenArgsEnv } @Override - public int execute(ExecutorRequest executorRequest) throws ExecutorException { + public ExecutorResult execute(ExecutorRequest executorRequest) throws ExecutorException { requireNonNull(executorRequest); if (closed.get()) { throw new ExecutorException("Executor is closed"); @@ -135,7 +140,7 @@ public int execute(ExecutorRequest executorRequest) throws ExecutorException { String command = executorRequest.command(); Context context = contextMap.computeIfAbsent( installationDirectory, k -> doCreate(installationDirectory, executorRequest)); - Function exec = context.commands.get(command); + Function exec = context.commands.get(command); if (exec == null) { throw new IllegalArgumentException( "Unknown command: '" + command + "' for '" + installationDirectory + "'"); @@ -193,6 +198,7 @@ protected void disposeRuntimeCreatedRealms(Context context) { } } + @SuppressWarnings("checkstyle:MethodLength") protected Context doCreate(Path mavenHome, ExecutorRequest executorRequest) { if (!Files.isDirectory(mavenHome)) { throw new IllegalArgumentException("Installation directory must point to existing directory: " + mavenHome); @@ -252,7 +258,7 @@ protected Context doCreate(Path mavenHome, ExecutorRequest executorRequest) { Class cliClass = (Class) launcherClass.getMethod("getMainClass").invoke(launcher); String version = getMavenVersion(cliClass); - Map> commands = new HashMap<>(); + Map> commands = new HashMap<>(); ArrayList keepAlive = new ArrayList<>(); if (version.startsWith("3.")) { @@ -271,19 +277,29 @@ protected Context doCreate(Path mavenHome, ExecutorRequest executorRequest) { Class[] parameterTypes = {String[].class, String.class, PrintStream.class, PrintStream.class}; Method doMain = cliClass.getMethod("doMain", parameterTypes); commands.put(ExecutorRequest.MVN, r -> { + OutputStream out; + OutputStream err; + if (r.grabOutputAsString()) { + out = new ByteArrayOutputStream(); + err = new ByteArrayOutputStream(); + } else { + out = r.stdOut().orElse(null); + err = r.stdErr().orElse(null); + } System.setProperties(prepareProperties(r)); try { ArrayList args = new ArrayList<>(mavenArgs); args.addAll(r.arguments()); - PrintStream stdout = r.stdOut().isEmpty() - ? null - : new PrintStream(r.stdOut().orElseThrow(), true); - PrintStream stderr = r.stdErr().isEmpty() - ? null - : new PrintStream(r.stdErr().orElseThrow(), true); - return (int) doMain.invoke(mavenCli, new Object[] { + PrintStream stdout = out == null ? null : new PrintStream(out, true); + PrintStream stderr = err == null ? null : new PrintStream(err, true); + int exitCode = (int) doMain.invoke(mavenCli, new Object[] { args.toArray(new String[0]), r.cwd().toString(), stdout, stderr }); + if (r.grabOutputAsString()) { + return new SimpleExecutionResult(r, exitCode == 0, exitCode, toString(out), toString(err)); + } else { + return new SimpleExecutionResult(r, exitCode == 0, exitCode, null, null); + } } catch (Exception e) { throw new ExecutorException("Failed to execute", e); } @@ -301,17 +317,28 @@ protected Context doCreate(Path mavenHome, ExecutorRequest executorRequest) { OutputStream.class, OutputStream.class); commands.put(cmdEntry.getKey(), r -> { + InputStream in = r.stdIn().orElse(null); + OutputStream out; + OutputStream err; + if (r.grabOutputAsString()) { + out = new ByteArrayOutputStream(); + err = new ByteArrayOutputStream(); + } else { + out = r.stdOut().orElse(null); + err = r.stdErr().orElse(null); + } System.setProperties(prepareProperties(r)); try { ArrayList args = new ArrayList<>(mavenArgs); args.addAll(r.arguments()); - return (int) mainMethod.invoke( - null, - args.toArray(new String[0]), - classWorld, - r.stdIn().orElse(null), - r.stdOut().orElse(null), - r.stdErr().orElse(null)); + int exitCode = (int) + mainMethod.invoke(null, args.toArray(new String[0]), classWorld, in, out, err); + if (r.grabOutputAsString()) { + return new SimpleExecutionResult( + r, exitCode == 0, exitCode, toString(out), toString(err)); + } else { + return new SimpleExecutionResult(r, exitCode == 0, exitCode, null, null); + } } catch (Exception e) { throw new ExecutorException("Failed to execute", e); } @@ -335,6 +362,14 @@ protected Context doCreate(Path mavenHome, ExecutorRequest executorRequest) { } } + private String toString(OutputStream outputStream) { + if (outputStream instanceof ByteArrayOutputStream) { + return outputStream.toString(); + } else { + return null; + } + } + private boolean mayAddToKeepAlive(List keepAlive, Class cliClass, String className) { try { keepAlive.add(cliClass.getClassLoader().loadClass(className)); @@ -375,7 +410,7 @@ protected Properties prepareProperties(ExecutorRequest request) { @Override public void close() throws ExecutorException { - if (closed.compareAndExchange(false, true)) { + if (closed.compareAndSet(false, true)) { try { Context context = contextMap.get(installationDirectory); if (context != null) { diff --git a/maven-executor/src/main/java/org/apache/maven/executor/forked/ForkedMavenExecutor.java b/maven-executor/src/main/java/org/apache/maven/executor/forked/ForkedMavenExecutor.java index 691e042..51d8fbe 100644 --- a/maven-executor/src/main/java/org/apache/maven/executor/forked/ForkedMavenExecutor.java +++ b/maven-executor/src/main/java/org/apache/maven/executor/forked/ForkedMavenExecutor.java @@ -18,7 +18,6 @@ */ package org.apache.maven.executor.forked; -import java.io.ByteArrayOutputStream; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; @@ -26,11 +25,13 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; -import java.util.List; +import java.util.NoSuchElementException; +import java.util.stream.Collectors; import org.apache.maven.executor.Executor; import org.apache.maven.executor.ExecutorException; import org.apache.maven.executor.ExecutorRequest; +import org.apache.maven.executor.ExecutorResult; import org.apache.maven.executor.support.ProcessBuilderExecutorSupport; import static java.util.Objects.requireNonNull; @@ -54,7 +55,7 @@ public ForkedMavenExecutor(Path installationDirectory, boolean useMavenArgsEnv) } @Override - public int execute(ExecutorRequest executorRequest) throws ExecutorException { + public ExecutorResult execute(ExecutorRequest executorRequest) throws ExecutorException { requireNonNull(executorRequest); if (closed.get()) { throw new ExecutorException("Executor is closed"); @@ -72,24 +73,21 @@ public String mavenVersion() throws ExecutorException { try { Path cwd = Files.createTempDirectory("forked-executor-maven-version"); try { - ByteArrayOutputStream stdout = new ByteArrayOutputStream(); - int exitCode = execute(ExecutorRequest.mavenBuilder() + ExecutorResult result = execute(ExecutorRequest.mavenBuilder() .cwd(cwd) .userHomeDirectory(ExecutorRequest.discoverUserHomeDirectory()) - .arguments(List.of("--version", "--quiet")) - .stdOut(stdout) + .arguments(Arrays.asList("--version", "--quiet")) .build()); + int exitCode = result.exitCode().orElseThrow(() -> new NoSuchElementException("No such element")); if (exitCode == 0) { - if (stdout.size() > 0) { - return stdout.toString() - .replace("\n", "") - .replace("\r", "") - .trim(); + String stdout = result.stdOutString().orElse(""); + if (!stdout.isEmpty()) { + return stdout.replace("\n", "").replace("\r", "").trim(); } return UNKNOWN_VERSION; } else { - throw new ExecutorException( - "Maven version query unexpected exitCode=" + exitCode + "\nLog: " + stdout); + throw new ExecutorException("Maven version query unexpected exitCode=" + exitCode + "\nLog: " + + result.stdOutString().orElse("")); } } finally { Files.deleteIfExists(cwd); @@ -101,7 +99,7 @@ public String mavenVersion() throws ExecutorException { protected void validate(ExecutorRequest executorRequest) throws ExecutorException {} - protected int doExecute(ExecutorRequest executorRequest) throws ExecutorException { + protected ExecutorResult doExecute(ExecutorRequest executorRequest) throws ExecutorException { ArrayList cmdAndArguments = new ArrayList<>(); cmdAndArguments.add(installationDirectory .resolve("bin") @@ -127,7 +125,7 @@ protected int doExecute(ExecutorRequest executorRequest) throws ExecutorExceptio if (executorRequest.jvmSystemProperties().isPresent()) { jvmArgs.addAll(executorRequest.jvmSystemProperties().get().entrySet().stream() .map(e -> "-D" + e.getKey() + "=" + e.getValue()) - .toList()); + .collect(Collectors.toList())); } HashMap env = new HashMap<>(); @@ -148,8 +146,19 @@ protected int doExecute(ExecutorRequest executorRequest) throws ExecutorExceptio env.put("MAVEN_SKIP_RC", "true"); } + // Adjust the process invocation to circumvent possible limited buffers + ArrayList command = new ArrayList<>(); + if (IS_WINDOWS) { + command.add("cmd.exe"); + command.add("/c"); + } else { + command.add("sh"); + command.add("-c"); + } + command.add(cmdAndArguments.stream().map(this::mayQuoteAndEscape).collect(Collectors.joining(" "))); + ProcessBuilder pb = - new ProcessBuilder().directory(executorRequest.cwd().toFile()).command(cmdAndArguments); + new ProcessBuilder().directory(executorRequest.cwd().toFile()).command(command); if (!env.isEmpty()) { pb.environment().putAll(env); } diff --git a/maven-executor/src/main/java/org/apache/maven/executor/support/ExecutorHelperImpl.java b/maven-executor/src/main/java/org/apache/maven/executor/support/ExecutorHelperImpl.java index 3eb9925..fd96849 100644 --- a/maven-executor/src/main/java/org/apache/maven/executor/support/ExecutorHelperImpl.java +++ b/maven-executor/src/main/java/org/apache/maven/executor/support/ExecutorHelperImpl.java @@ -27,6 +27,7 @@ import org.apache.maven.executor.ExecutorException; import org.apache.maven.executor.ExecutorHelper; import org.apache.maven.executor.ExecutorRequest; +import org.apache.maven.executor.ExecutorResult; import org.apache.maven.executor.embedded.EmbeddedMavenExecutor; import org.apache.maven.executor.forked.ForkedMavenExecutor; @@ -61,7 +62,7 @@ public Mode getDefaultMode() { } @Override - public int execute(Mode mode, ExecutorRequest executorRequest) throws ExecutorException { + public ExecutorResult execute(Mode mode, ExecutorRequest executorRequest) throws ExecutorException { if (closed.get()) { throw new ExecutorException("Executor is closed"); } @@ -107,15 +108,21 @@ public void close() throws ExecutorException { } protected Executor getExecutor(Mode mode, ExecutorRequest request) throws ExecutorException { - return switch (mode) { - case AUTO -> getExecutorByRequest(request); - case EMBEDDED -> executors.get(Mode.EMBEDDED); - case FORKED -> executors.get(Mode.FORKED); - }; + switch (mode) { + case AUTO: + return getExecutorByRequest(request); + case EMBEDDED: + return executors.get(Mode.EMBEDDED); + case FORKED: + return executors.get(Mode.FORKED); + default: + throw new ExecutorException("Unknown mode: " + mode); + } } protected Executor getExecutorByRequest(ExecutorRequest request) { - if (request.environmentVariables().isEmpty() && request.jvmArguments().isEmpty()) { + if (!request.environmentVariables().isPresent() + && !request.jvmArguments().isPresent()) { return getExecutor(Mode.EMBEDDED, request); } else { return getExecutor(Mode.FORKED, request); diff --git a/maven-executor/src/main/java/org/apache/maven/executor/support/IOTools.java b/maven-executor/src/main/java/org/apache/maven/executor/support/IOTools.java new file mode 100644 index 0000000..cd2b6d8 --- /dev/null +++ b/maven-executor/src/main/java/org/apache/maven/executor/support/IOTools.java @@ -0,0 +1,133 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.executor.support; + +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; + +import static java.util.Objects.requireNonNull; + +/** + * Support class IO. + */ +public final class IOTools { + private IOTools() {} + + private static final int DEFAULT_BUFFER_SIZE = 16384; + + /** + * Copy from Java 9+ + */ + public static long transferTo(InputStream in, OutputStream out) throws IOException { + requireNonNull(in); + requireNonNull(out); + long transferred = 0; + byte[] buffer = new byte[DEFAULT_BUFFER_SIZE]; + int read; + while ((read = in.read(buffer, 0, DEFAULT_BUFFER_SIZE)) >= 0) { + out.write(buffer, 0, read); + if (transferred < Long.MAX_VALUE) { + try { + transferred = Math.addExact(transferred, read); + } catch (ArithmeticException ignore) { + transferred = Long.MAX_VALUE; + } + } + } + return transferred; + } + + /** + * Copy from Java 9+ + */ + public static InputStream nullInputStream() { + return new InputStream() { + private volatile boolean closed; + + private void ensureOpen() throws IOException { + if (closed) { + throw new IOException("Stream closed"); + } + } + + @Override + public int available() throws IOException { + ensureOpen(); + return 0; + } + + @Override + public int read() throws IOException { + ensureOpen(); + return -1; + } + + @Override + public int read(byte[] b, int off, int len) throws IOException { + if (len == 0) { + return 0; + } + ensureOpen(); + return -1; + } + + @Override + public long skip(long n) throws IOException { + ensureOpen(); + return 0L; + } + + @Override + public void close() throws IOException { + closed = true; + } + }; + } + + /** + * Copy from Java 9+ + */ + public static OutputStream nullOutputStream() { + return new OutputStream() { + private volatile boolean closed; + + private void ensureOpen() throws IOException { + if (closed) { + throw new IOException("Stream closed"); + } + } + + @Override + public void write(int b) throws IOException { + ensureOpen(); + } + + @Override + public void write(byte[] b, int off, int len) throws IOException { + ensureOpen(); + } + + @Override + public void close() { + closed = true; + } + }; + } +} diff --git a/maven-executor/src/main/java/org/apache/maven/executor/support/ProcessBuilderExecutorSupport.java b/maven-executor/src/main/java/org/apache/maven/executor/support/ProcessBuilderExecutorSupport.java index 98f9686..16af0a5 100644 --- a/maven-executor/src/main/java/org/apache/maven/executor/support/ProcessBuilderExecutorSupport.java +++ b/maven-executor/src/main/java/org/apache/maven/executor/support/ProcessBuilderExecutorSupport.java @@ -18,16 +18,23 @@ */ package org.apache.maven.executor.support; +import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; import java.io.UncheckedIOException; +import java.util.NoSuchElementException; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ThreadLocalRandom; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import org.apache.maven.executor.Executor; import org.apache.maven.executor.ExecutorException; import org.apache.maven.executor.ExecutorRequest; +import org.apache.maven.executor.ExecutorResult; + +import static java.util.Objects.requireNonNull; /** * Support class for executor implementations using {@link ProcessBuilder}. @@ -41,32 +48,92 @@ protected ProcessBuilderExecutorSupport() { @Override public void close() throws ExecutorException { - if (closed.compareAndExchange(false, true)) { + if (closed.compareAndSet(false, true)) { doClose(); } } protected void doClose() throws ExecutorException {} - protected int doExecuteProcess(ExecutorRequest executorRequest, ProcessBuilder processBuilder) { + protected ExecutorResult doExecuteProcess(ExecutorRequest execution, ProcessBuilder processBuilder) { + requireNonNull(execution); + if (execution.executionTimeout().isPresent() + && execution.executionTimeout().get().isNegative()) { + throw new IllegalArgumentException("Timeout must be greater than zero"); + } + + Process process = null; try { - Process process = processBuilder.start(); - pump(process, executorRequest).await(); - return process.waitFor(); + process = processBuilder.start(); + InputStream stdIn = execution.stdIn().orElse(IOTools.nullInputStream()); + OutputStream stdOut; + OutputStream stdErr; + if (execution.grabOutputAsString()) { + stdOut = new ByteArrayOutputStream(); + stdErr = new ByteArrayOutputStream(); + } else { + stdOut = execution.stdOut().orElse(IOTools.nullOutputStream()); + stdErr = execution.stdErr().orElse(IOTools.nullOutputStream()); + } + if (execution.executionTimeout().isPresent()) { + long timeoutMillis = execution + .executionTimeout() + .orElseThrow(() -> new NoSuchElementException("No such element")) + .toMillis(); + if (pump(process, stdIn, stdOut, stdErr).await(timeoutMillis, TimeUnit.MILLISECONDS)) { + int exitCode = process.waitFor(); + String stdOutString = null; + String stdErrString = null; + if (execution.grabOutputAsString()) { + // they are ByteArrayOutputStreams + stdOutString = stdOut.toString(); + stdErrString = stdErr.toString(); + } + return new SimpleExecutionResult(execution, exitCode == 0, exitCode, stdOutString, stdErrString); + } else { + process.destroyForcibly(); + throw new ExecutorException("Process timeout: " + execution); + } + } else { + pump(process, stdIn, stdOut, stdErr).await(); + int exitCode = process.waitFor(); + String stdOutString = null; + String stdErrString = null; + if (execution.grabOutputAsString()) { + // they are ByteArrayOutputStreams + stdOutString = stdOut.toString(); + stdErrString = stdErr.toString(); + } + return new SimpleExecutionResult(execution, exitCode == 0, exitCode, stdOutString, stdErrString); + } } catch (IOException e) { - throw new ExecutorException("IO problem while executing command: " + executorRequest, e); + if (process != null) { + process.destroyForcibly(); + } + throw new ExecutorException("IO problem while executing command: " + execution, e); } catch (InterruptedException e) { - throw new ExecutorException("Interrupted while executing command: " + executorRequest, e); + process.destroyForcibly(); + throw new ExecutorException("Interrupted while executing command: " + execution, e); + } + } + + protected String mayQuoteAndEscape(String command) { + if (command.contains(" ")) { + if (command.contains("\"")) { + return "\"" + command.replace("\"", "\\\"") + "\""; + } else { + return "\"" + command + "\""; + } } + return command; } - protected CountDownLatch pump(Process p, ExecutorRequest executorRequest) { + protected CountDownLatch pump(Process p, InputStream stdIn, OutputStream stdOut, OutputStream stdErr) { CountDownLatch latch = new CountDownLatch(3); - String suffix = "-pump-" + p.pid(); + String suffix = "-pump-" + ThreadLocalRandom.current().nextInt(); Thread stdoutPump = new Thread(() -> { - try { - OutputStream stdout = executorRequest.stdOut().orElse(OutputStream.nullOutputStream()); - p.getInputStream().transferTo(stdout); + try (OutputStream stdout = stdOut) { + IOTools.transferTo(p.getInputStream(), stdout); stdout.flush(); } catch (IOException e) { throw new UncheckedIOException(e); @@ -75,11 +142,11 @@ protected CountDownLatch pump(Process p, ExecutorRequest executorRequest) { } }); stdoutPump.setName("stdout" + suffix); + stdoutPump.setDaemon(true); stdoutPump.start(); Thread stderrPump = new Thread(() -> { - try { - OutputStream stderr = executorRequest.stdErr().orElse(OutputStream.nullOutputStream()); - p.getErrorStream().transferTo(stderr); + try (OutputStream stderr = stdErr) { + IOTools.transferTo(p.getErrorStream(), stderr); stderr.flush(); } catch (IOException e) { throw new UncheckedIOException(e); @@ -88,11 +155,11 @@ protected CountDownLatch pump(Process p, ExecutorRequest executorRequest) { } }); stderrPump.setName("stderr" + suffix); + stderrPump.setDaemon(true); stderrPump.start(); Thread stdinPump = new Thread(() -> { - try { - OutputStream in = p.getOutputStream(); - executorRequest.stdIn().orElse(InputStream.nullInputStream()).transferTo(in); + try (OutputStream in = p.getOutputStream()) { + IOTools.transferTo(stdIn, in); in.flush(); } catch (IOException e) { throw new UncheckedIOException(e); @@ -101,6 +168,7 @@ protected CountDownLatch pump(Process p, ExecutorRequest executorRequest) { } }); stdinPump.setName("stdin" + suffix); + stdinPump.setDaemon(true); stdinPump.start(); return latch; } diff --git a/maven-executor/src/main/java/org/apache/maven/executor/support/SimpleExecutionResult.java b/maven-executor/src/main/java/org/apache/maven/executor/support/SimpleExecutionResult.java new file mode 100644 index 0000000..7bbd3e3 --- /dev/null +++ b/maven-executor/src/main/java/org/apache/maven/executor/support/SimpleExecutionResult.java @@ -0,0 +1,81 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.executor.support; + +import java.util.Optional; + +import org.apache.maven.executor.ExecutorRequest; +import org.apache.maven.executor.ExecutorResult; + +import static java.util.Objects.requireNonNull; + +/** + * Simple execution result. + */ +public class SimpleExecutionResult implements ExecutorResult { + private final ExecutorRequest request; + private final boolean success; + private final Integer exitCode; + private final String stdOut; + private final String stdErr; + + /** + * Constructor. + * + * @param request the request, must not be {@code null} + * @param success the logical outcome + * @param exitCode the exit code, if available, or {@code null} + * @param stdOut the STDOUT as string, if available, or {@code null} + * @param stdErr the STDERR as string, if available, or {@code null} + */ + public SimpleExecutionResult( + ExecutorRequest request, boolean success, Integer exitCode, String stdOut, String stdErr) { + this.request = requireNonNull(request); + this.success = success; + // below all are nullable + this.exitCode = exitCode; + this.stdOut = stdOut; + this.stdErr = stdErr; + } + + @Override + public ExecutorRequest getRequest() { + return request; + } + + @Override + public boolean success() { + return success; + } + + @Override + public Optional exitCode() { + return Optional.ofNullable(exitCode); + } + + @Override + public Optional stdOutString() { + return Optional.ofNullable(stdOut); + } + + @Override + public Optional stdErrString() { + return Optional.ofNullable(stdErr); + } +} diff --git a/maven-executor/src/main/java/org/apache/maven/executor/support/ToolboxExecutorTool.java b/maven-executor/src/main/java/org/apache/maven/executor/support/ToolboxExecutorTool.java index 32d5e29..8828283 100644 --- a/maven-executor/src/main/java/org/apache/maven/executor/support/ToolboxExecutorTool.java +++ b/maven-executor/src/main/java/org/apache/maven/executor/support/ToolboxExecutorTool.java @@ -23,12 +23,14 @@ import java.io.IOException; import java.util.HashMap; import java.util.Map; +import java.util.NoSuchElementException; import java.util.Properties; import java.util.stream.Collectors; import org.apache.maven.executor.Executor; import org.apache.maven.executor.ExecutorException; import org.apache.maven.executor.ExecutorRequest; +import org.apache.maven.executor.ExecutorResult; import org.apache.maven.executor.ExecutorTool; import static java.util.Objects.requireNonNull; @@ -136,7 +138,8 @@ private ExecutorRequest.Builder mojo(ExecutorRequest.Builder builder, String moj private void doExecute(ExecutorRequest.Builder builder) { ExecutorRequest request = builder.build(); - int ec = executor.execute(request); + ExecutorResult result = executor.execute(request); + int ec = result.exitCode().orElseThrow(() -> new NoSuchElementException("No such element")); if (ec != 0) { throw new ExecutorException("Unexpected exit code=" + ec + "; stdout=" + request.stdOut().orElse(null) + "; stderr=" diff --git a/maven-executor/src/main/java/module-info.java b/maven-executor/src/main/java17/module-info.java similarity index 100% rename from maven-executor/src/main/java/module-info.java rename to maven-executor/src/main/java17/module-info.java diff --git a/maven-executor/src/test/java/org/apache/maven/executor/MavenExecutorTestSupport.java b/maven-executor/src/test/java/org/apache/maven/executor/MavenExecutorTestSupport.java index 0cea5b6..2e6a74c 100644 --- a/maven-executor/src/test/java/org/apache/maven/executor/MavenExecutorTestSupport.java +++ b/maven-executor/src/test/java/org/apache/maven/executor/MavenExecutorTestSupport.java @@ -20,11 +20,14 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.Paths; import java.util.Collection; -import java.util.List; +import java.util.Collections; import java.util.Map; +import java.util.NoSuchElementException; import java.util.concurrent.ConcurrentHashMap; import org.junit.jupiter.api.AfterAll; @@ -52,13 +55,21 @@ public abstract class MavenExecutorTestSupport { @BeforeEach void beforeEach(TestInfo testInfo) throws Exception { - cwd = tempDir.resolve(testInfo.getTestMethod().orElseThrow().getName()).resolve("cwd"); + cwd = tempDir.resolve(testInfo.getTestMethod() + .orElseThrow(() -> new NoSuchElementException("No such element")) + .getName()) + .resolve("cwd"); Files.createDirectories(cwd.resolve(".mvn")); - userHome = tempDir.resolve(testInfo.getTestMethod().orElseThrow().getName()) + userHome = tempDir.resolve(testInfo.getTestMethod() + .orElseThrow(() -> new NoSuchElementException("No such element")) + .getName()) .resolve("home"); Files.createDirectories(userHome); - System.out.println("=== " + testInfo.getTestMethod().orElseThrow().getName()); + System.out.println("=== " + + testInfo.getTestMethod() + .orElseThrow(() -> new NoSuchElementException("No such element")) + .getName()); } private static final Map EXECUTORS = new ConcurrentHashMap<>(); @@ -81,9 +92,9 @@ static void afterAll() { void mvnenc4() throws Exception { String logfile = "m4.log"; execute( - Path.of(Environment.MAVEN4_HOME), + Paths.get(Environment.MAVEN4_HOME), cwd.resolve(logfile), - List.of(customizedRequest() + Collections.singletonList(customizedRequest() .command("mvnenc") .cwd(cwd) .userHomeDirectory(userHome) @@ -91,7 +102,7 @@ void mvnenc4() throws Exception { .argument("-l") .argument(logfile) .build())); - System.out.println(Files.readString(cwd.resolve(logfile))); + System.out.println(readString(cwd.resolve(logfile))); } @DisabledOnOs( @@ -101,32 +112,32 @@ void mvnenc4() throws Exception { void dump3() throws Exception { String logfile = "m3.log"; execute( - Path.of(Environment.MAVEN3_HOME), + Paths.get(Environment.MAVEN3_HOME), cwd.resolve(logfile), - List.of(customizedRequest() + Collections.singletonList(customizedRequest() .cwd(cwd) .userHomeDirectory(userHome) .argument("eu.maveniverse.maven.plugins:toolbox:" + Environment.TOOLBOX_VERSION + ":gav-dump") .argument("-l") .argument(logfile) .build())); - System.out.println(Files.readString(cwd.resolve(logfile))); + System.out.println(readString(cwd.resolve(logfile))); } @Test void dump4() throws Exception { String logfile = "m4.log"; execute( - Path.of(Environment.MAVEN4_HOME), + Paths.get(Environment.MAVEN4_HOME), cwd.resolve(logfile), - List.of(customizedRequest() + Collections.singletonList(customizedRequest() .cwd(cwd) .userHomeDirectory(userHome) .argument("eu.maveniverse.maven.plugins:toolbox:" + Environment.TOOLBOX_VERSION + ":gav-dump") .argument("-l") .argument(logfile) .build())); - System.out.println(Files.readString(cwd.resolve(logfile))); + System.out.println(readString(cwd.resolve(logfile))); } @DisabledOnOs( @@ -137,16 +148,16 @@ void defaultFs3() throws Exception { layDownFiles(cwd); String logfile = "m3.log"; execute( - Path.of(Environment.MAVEN3_HOME), + Paths.get(Environment.MAVEN3_HOME), cwd.resolve(logfile), - List.of(customizedRequest() + Collections.singletonList(customizedRequest() .cwd(cwd) .argument("-V") .argument("verify") .argument("-l") .argument(logfile) .build())); - System.out.println(Files.readString(cwd.resolve(logfile))); + System.out.println(readString(cwd.resolve(logfile))); } @Test @@ -154,26 +165,26 @@ void defaultFs4() throws Exception { layDownFiles(cwd); String logfile = "m4.log"; execute( - Path.of(Environment.MAVEN4_HOME), + Paths.get(Environment.MAVEN4_HOME), cwd.resolve(logfile), - List.of(customizedRequest() + Collections.singletonList(customizedRequest() .cwd(cwd) .argument("-V") .argument("verify") .argument("-l") .argument(logfile) .build())); - System.out.println(Files.readString(cwd.resolve(logfile))); + System.out.println(readString(cwd.resolve(logfile))); } @Test void version3() throws Exception { - assertEquals(System.getProperty("maven3version"), mavenVersion(Path.of(Environment.MAVEN3_HOME))); + assertEquals(System.getProperty("maven3version"), mavenVersion(Paths.get(Environment.MAVEN3_HOME))); } @Test void version4() throws Exception { - assertEquals(System.getProperty("maven4version"), mavenVersion(Path.of(Environment.MAVEN4_HOME))); + assertEquals(System.getProperty("maven4version"), mavenVersion(Paths.get(Environment.MAVEN4_HOME))); } @Test @@ -181,9 +192,9 @@ void defaultFs4CaptureOutput() throws Exception { layDownFiles(cwd); ByteArrayOutputStream stdout = new ByteArrayOutputStream(); execute( - Path.of(Environment.MAVEN4_HOME), + Paths.get(Environment.MAVEN4_HOME), null, - List.of(customizedRequest() + Collections.singletonList(customizedRequest() .cwd(cwd) .argument("-V") .argument("verify") @@ -199,9 +210,9 @@ void defaultFs4CaptureOutputWithForcedColor() throws Exception { layDownFiles(cwd); ByteArrayOutputStream stdout = new ByteArrayOutputStream(); execute( - Path.of(Environment.MAVEN4_HOME), + Paths.get(Environment.MAVEN4_HOME), null, - List.of(customizedRequest() + Collections.singletonList(customizedRequest() .cwd(cwd) .argument("-V") .argument("verify") @@ -218,9 +229,9 @@ void defaultFs4CaptureOutputWithForcedOffColor() throws Exception { layDownFiles(cwd); ByteArrayOutputStream stdout = new ByteArrayOutputStream(); execute( - Path.of(Environment.MAVEN4_HOME), + Paths.get(Environment.MAVEN4_HOME), null, - List.of(customizedRequest() + Collections.singletonList(customizedRequest() .cwd(cwd) .argument("-V") .argument("verify") @@ -237,9 +248,9 @@ void defaultFs3CaptureOutput() throws Exception { layDownFiles(cwd); ByteArrayOutputStream stdout = new ByteArrayOutputStream(); execute( - Path.of(Environment.MAVEN3_HOME), + Paths.get(Environment.MAVEN3_HOME), null, - List.of(customizedRequest() + Collections.singletonList(customizedRequest() .cwd(cwd) .argument("-V") .argument("verify") @@ -256,9 +267,9 @@ void defaultFs3CaptureOutputWithForcedColor() throws Exception { layDownFiles(cwd); ByteArrayOutputStream stdout = new ByteArrayOutputStream(); execute( - Path.of(Environment.MAVEN3_HOME), + Paths.get(Environment.MAVEN3_HOME), null, - List.of(customizedRequest() + Collections.singletonList(customizedRequest() .cwd(cwd) .argument("-V") .argument("verify") @@ -275,9 +286,9 @@ void defaultFs3CaptureOutputWithForcedOffColor() throws Exception { layDownFiles(cwd); ByteArrayOutputStream stdout = new ByteArrayOutputStream(); execute( - Path.of(Environment.MAVEN3_HOME), + Paths.get(Environment.MAVEN3_HOME), null, - List.of(customizedRequest() + Collections.singletonList(customizedRequest() .cwd(cwd) .argument("-V") .argument("verify") @@ -289,57 +300,53 @@ void defaultFs3CaptureOutputWithForcedOffColor() throws Exception { assertTrue(stdout.toString().contains("INFO"), "No INFO found"); } - public static final String POM_STRING = """ - - - - 4.0.0 - - org.apache.maven.samples - sample - 1.0.0 - - - - - org.junit - junit-bom - 5.11.1 - pom - import - - - - - - - org.junit.jupiter - junit-jupiter-api - test - - - - - """; - - public static final String APP_JAVA_STRING = """ - package org.apache.maven.samples.sample; - - public class App { - public static void main(String... args) { - System.out.println("Hello World!"); - } - } - """; + public static final String POM_STRING = "\n" + + " \n" + + "\n" + + " 4.0.0\n" + + "\n" + + " org.apache.maven.samples\n" + + " sample\n" + + " 1.0.0\n" + + "\n" + + " \n" + + " \n" + + " \n" + + " org.junit\n" + + " junit-bom\n" + + " 5.11.1\n" + + " pom\n" + + " import\n" + + " \n" + + " \n" + + " \n" + + "\n" + + " \n" + + " \n" + + " org.junit.jupiter\n" + + " junit-jupiter-api\n" + + " test\n" + + " \n" + + " \n" + + "\n" + + " "; + + public static final String APP_JAVA_STRING = " package org.apache.maven.samples.sample;\n" + "\n" + + " public class App {\n" + + " public static void main(String... args) {\n" + + " System.out.println(\"Hello World!\");\n" + + " }\n" + + " }"; protected void execute(Path installationDirectory, Path logFile, Collection requests) throws Exception { Executor invoker = createAndMemoizeExecutor(installationDirectory); for (ExecutorRequest request : requests) { - int exitCode = invoker.execute(request); + ExecutorResult result = invoker.execute(request); + int exitCode = result.exitCode().orElseThrow(() -> new NoSuchElementException("No such element")); if (exitCode != 0) { - throw new FailedExecution(request, exitCode, logFile == null ? "" : Files.readString(logFile)); + throw new FailedExecution(request, exitCode, logFile == null ? "" : readString(logFile)); } } } @@ -363,10 +370,10 @@ protected ExecutorRequest.Builder customizedRequest(ExecutorRequest.Builder buil protected void layDownFiles(Path cwd) throws IOException { Path pom = cwd.resolve("pom.xml").toAbsolutePath(); - Files.writeString(pom, POM_STRING); + writeString(pom, POM_STRING); Path appJava = cwd.resolve("src/main/java/org/apache/maven/samples/sample/App.java"); Files.createDirectories(appJava.getParent()); - Files.writeString(appJava, APP_JAVA_STRING); + writeString(appJava, APP_JAVA_STRING); } protected static class FailedExecution extends Exception { @@ -393,4 +400,12 @@ public String getLog() { return log; } } + + private static String readString(Path path) throws IOException { + return new String(Files.readAllBytes(path), StandardCharsets.UTF_8); + } + + private static void writeString(Path path, String content) throws IOException { + Files.write(path, content.getBytes(StandardCharsets.UTF_8)); + } } diff --git a/maven-executor/src/test/java/org/apache/maven/executor/ToolboxExecutorToolTest.java b/maven-executor/src/test/java/org/apache/maven/executor/ToolboxExecutorToolTest.java index 88fb51d..0e8c9e5 100644 --- a/maven-executor/src/test/java/org/apache/maven/executor/ToolboxExecutorToolTest.java +++ b/maven-executor/src/test/java/org/apache/maven/executor/ToolboxExecutorToolTest.java @@ -23,6 +23,7 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.Map; +import java.util.NoSuchElementException; import org.apache.maven.executor.support.ToolboxExecutorTool; import org.junit.jupiter.api.BeforeEach; @@ -46,11 +47,16 @@ public class ToolboxExecutorToolTest { @BeforeEach void beforeEach(TestInfo testInfo) throws Exception { - String testName = testInfo.getTestMethod().orElseThrow().getName(); + String testName = testInfo.getTestMethod() + .orElseThrow(() -> new NoSuchElementException("No such element")) + .getName(); userHome = tempDir.resolve(testName); cwd = userHome.resolve("cwd"); Files.createDirectories(cwd.resolve(".mvn")); - System.out.println("=== " + testInfo.getTestMethod().orElseThrow().getName()); + System.out.println("=== " + + testInfo.getTestMethod() + .orElseThrow(() -> new NoSuchElementException("No such element")) + .getName()); } private ExecutorRequest.Builder getExecutorRequest() { @@ -65,11 +71,11 @@ private ExecutorRequest.Builder getExecutorRequest() { } private Path mvn3Home() { - return Path.of(Environment.MAVEN3_HOME); + return Paths.get(Environment.MAVEN3_HOME); } private Path mvn4Home() { - return Path.of(Environment.MAVEN4_HOME); + return Paths.get(Environment.MAVEN4_HOME); } @ParameterizedTest diff --git a/pom.xml b/pom.xml index 89a51f4..abef31e 100644 --- a/pom.xml +++ b/pom.xml @@ -76,7 +76,7 @@ 4.0.0-rc-5 2.0.17 - 6.0.3 + 5.14.4 5.23.0 diff --git a/providers/docker-exe/src/main/java/org/apache/maven/executor/providers/dockerexe/DockerExeExecutor.java b/providers/docker-exe/src/main/java/org/apache/maven/executor/providers/dockerexe/DockerExeExecutor.java index 76fa645..085c103 100644 --- a/providers/docker-exe/src/main/java/org/apache/maven/executor/providers/dockerexe/DockerExeExecutor.java +++ b/providers/docker-exe/src/main/java/org/apache/maven/executor/providers/dockerexe/DockerExeExecutor.java @@ -18,7 +18,6 @@ */ package org.apache.maven.executor.providers.dockerexe; -import java.io.ByteArrayOutputStream; import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; @@ -30,6 +29,7 @@ import org.apache.maven.executor.Executor; import org.apache.maven.executor.ExecutorException; import org.apache.maven.executor.ExecutorRequest; +import org.apache.maven.executor.ExecutorResult; import org.apache.maven.executor.support.ProcessBuilderExecutorSupport; import static java.util.Objects.requireNonNull; @@ -69,7 +69,7 @@ protected DockerExeExecutor(String imageName, String imageTag) { } @Override - public int execute(ExecutorRequest request) throws ExecutorException { + public ExecutorResult execute(ExecutorRequest request) throws ExecutorException { requireNonNull(request); try { @@ -112,20 +112,18 @@ public int execute(ExecutorRequest request) throws ExecutorException { @Override public String mavenVersion() throws ExecutorException { return cache.computeIfAbsent("maven.version", k -> { - ByteArrayOutputStream stdOut = new ByteArrayOutputStream(); - ByteArrayOutputStream stdErr = new ByteArrayOutputStream(); - int exitCode = execute(ExecutorRequest.mavenBuilder() + ExecutorResult result = execute(ExecutorRequest.mavenBuilder() .userHomeDirectory(ExecutorRequest.discoverUserHomeDirectory()) .command(ExecutorRequest.MVN) .arguments("-q", "-v") - .stdOut(stdOut) - .stdErr(stdErr) .build()); + int exitCode = result.exitCode().orElseThrow(); if (exitCode == 0) { - return stdOut.toString().trim(); + return result.stdOutString().orElse("").trim(); } else { - throw new ExecutorException( - "Unexpected exit code: " + exitCode + "; stdout = " + stdOut + "; stderr = " + stdErr); + throw new ExecutorException("Unexpected exit code: " + exitCode + "; stdout = " + + result.stdOutString().orElse("").trim() + "; stderr = " + + result.stdErrString().orElse("").trim()); } }); } diff --git a/providers/docker-exe/src/test/java/org/apache/maven/executor/providers/dockerexe/DockerExeExecutorTest.java b/providers/docker-exe/src/test/java/org/apache/maven/executor/providers/dockerexe/DockerExeExecutorTest.java index 17b8028..d4f011f 100644 --- a/providers/docker-exe/src/test/java/org/apache/maven/executor/providers/dockerexe/DockerExeExecutorTest.java +++ b/providers/docker-exe/src/test/java/org/apache/maven/executor/providers/dockerexe/DockerExeExecutorTest.java @@ -22,6 +22,7 @@ import java.nio.file.Path; import org.apache.maven.executor.ExecutorRequest; +import org.apache.maven.executor.ExecutorResult; import org.apache.maven.executor.test.TestProjects; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.DisabledOnOs; @@ -49,8 +50,9 @@ void smoke() throws Exception { .stdOut(stdOut) .build(); try (DockerExeExecutor executor = DockerExeExecutor.withMavenImageVersion(MAVEN_VERSION)) { - int exitCode = executor.execute(request); - assertEquals(0, exitCode); + ExecutorResult result = executor.execute(request); + assertTrue(result.success()); + assertEquals(0, result.exitCode().orElse(-1)); assertTrue(stdOut.toString().contains("[INFO] BUILD SUCCESS")); assertEquals(MAVEN_VERSION, executor.mavenVersion()); } diff --git a/providers/testcontainers/src/main/java/org/apache/maven/executor/providers/testcontainers/TestContainersExecutor.java b/providers/testcontainers/src/main/java/org/apache/maven/executor/providers/testcontainers/TestContainersExecutor.java index 2f6a15e..589621d 100644 --- a/providers/testcontainers/src/main/java/org/apache/maven/executor/providers/testcontainers/TestContainersExecutor.java +++ b/providers/testcontainers/src/main/java/org/apache/maven/executor/providers/testcontainers/TestContainersExecutor.java @@ -19,7 +19,6 @@ package org.apache.maven.executor.providers.testcontainers; import java.io.ByteArrayInputStream; -import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.OutputStream; import java.io.UncheckedIOException; @@ -35,6 +34,8 @@ import org.apache.maven.executor.Executor; import org.apache.maven.executor.ExecutorException; import org.apache.maven.executor.ExecutorRequest; +import org.apache.maven.executor.ExecutorResult; +import org.apache.maven.executor.support.SimpleExecutionResult; import org.testcontainers.containers.GenericContainer; import org.testcontainers.containers.output.OutputFrame; import org.testcontainers.containers.startupcheck.OneShotStartupCheckStrategy; @@ -81,7 +82,7 @@ protected TestContainersExecutor(String imageName, String imageTag) { * Note: Testcontainers uses Lombok {@code @Sneakythrows}. */ @Override - public int execute(ExecutorRequest request) throws ExecutorException { + public ExecutorResult execute(ExecutorRequest request) throws ExecutorException { requireNonNull(request); HashMap env = new HashMap<>(); @@ -108,13 +109,25 @@ public int execute(ExecutorRequest request) throws ExecutorException { .start(); try { - new ByteArrayInputStream( - container.getLogs(OutputFrame.OutputType.STDOUT).getBytes(StandardCharsets.UTF_8)) - .transferTo(request.stdOut().orElse(OutputStream.nullOutputStream())); - new ByteArrayInputStream( - container.getLogs(OutputFrame.OutputType.STDERR).getBytes(StandardCharsets.UTF_8)) - .transferTo(request.stdErr().orElse(OutputStream.nullOutputStream())); - return startupCheckStrategy.lastStatus.get() == StartupCheckStrategy.StartupStatus.SUCCESSFUL ? 0 : 1; + boolean success = + startupCheckStrategy.lastStatus.get() == StartupCheckStrategy.StartupStatus.SUCCESSFUL; + int exitCode = success ? 0 : 1; + String out = null; + String err = null; + if (request.grabOutputAsString()) { + out = container.getLogs(OutputFrame.OutputType.STDOUT); + err = container.getLogs(OutputFrame.OutputType.STDERR); + } else { + new ByteArrayInputStream(container + .getLogs(OutputFrame.OutputType.STDOUT) + .getBytes(StandardCharsets.UTF_8)) + .transferTo(request.stdOut().orElse(OutputStream.nullOutputStream())); + new ByteArrayInputStream(container + .getLogs(OutputFrame.OutputType.STDERR) + .getBytes(StandardCharsets.UTF_8)) + .transferTo(request.stdErr().orElse(OutputStream.nullOutputStream())); + } + return new SimpleExecutionResult(request, success, exitCode, out, err); } catch (IOException e) { throw new ExecutorException(e); } @@ -124,20 +137,18 @@ public int execute(ExecutorRequest request) throws ExecutorException { @Override public String mavenVersion() throws ExecutorException { return cache.computeIfAbsent("maven.version", k -> { - ByteArrayOutputStream stdOut = new ByteArrayOutputStream(); - ByteArrayOutputStream stdErr = new ByteArrayOutputStream(); - int exitCode = execute(ExecutorRequest.mavenBuilder() + ExecutorResult result = execute(ExecutorRequest.mavenBuilder() .userHomeDirectory(ExecutorRequest.discoverUserHomeDirectory()) .command(ExecutorRequest.MVN) .arguments("-q", "-v") - .stdOut(stdOut) - .stdErr(stdErr) .build()); + int exitCode = result.exitCode().orElseThrow(); if (exitCode == 0) { - return stdOut.toString().trim(); + return result.stdOutString().orElseThrow().trim(); } else { - throw new ExecutorException( - "Unexpected exit code: " + exitCode + "; stdout = " + stdOut + "; stderr = " + stdErr); + throw new ExecutorException("Unexpected exit code: " + exitCode + "; stdout = " + + result.stdOutString().orElse("").trim() + "; stderr = " + + result.stdErrString().orElse("").trim()); } }); } diff --git a/providers/testcontainers/src/test/java/org/apache/maven/executor/providers/testcontainers/TestContainersExecutorTest.java b/providers/testcontainers/src/test/java/org/apache/maven/executor/providers/testcontainers/TestContainersExecutorTest.java index af6b7e1..c03dcce 100644 --- a/providers/testcontainers/src/test/java/org/apache/maven/executor/providers/testcontainers/TestContainersExecutorTest.java +++ b/providers/testcontainers/src/test/java/org/apache/maven/executor/providers/testcontainers/TestContainersExecutorTest.java @@ -22,6 +22,7 @@ import java.nio.file.Path; import org.apache.maven.executor.ExecutorRequest; +import org.apache.maven.executor.ExecutorResult; import org.apache.maven.executor.test.TestProjects; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.DisabledOnOs; @@ -49,8 +50,9 @@ void smoke() throws Exception { .stdOut(stdOut) .build(); try (TestContainersExecutor executor = TestContainersExecutor.withMavenImageVersion(MAVEN_VERSION)) { - int exitCode = executor.execute(request); - assertEquals(0, exitCode); + ExecutorResult result = executor.execute(request); + assertTrue(result.success()); + assertEquals(0, result.exitCode().orElse(-1)); assertTrue(stdOut.toString().contains("[INFO] BUILD SUCCESS")); assertEquals(MAVEN_VERSION, executor.mavenVersion()); }