Skip to content

Commit 5f24cba

Browse files
authored
Bug: when raw-streams are used, ensure system streams are set up (#11303) (#11310)
When `--raw-streams` is used (especially when combined with options like `--quiet` and `-DforceStdout`) there is no guarantee that anything touches terminal. Hence, in case of embedded executor it is quite possible that cached/warm code arrives quickly at the place that would do system out **before** the thread with `FastTerminal` finishes system install. In other words, when `--raw-streams` are used, we cannot guarantee that system streams are already properly set up. This PR changes that, and makes sure (by triggering a dummy call to terminal), at the cost of "jline3 install lag" for CLI invocation. OTOH, this lag in case of embedded executors does not exists (it exists only on first invocation). My suspicion that this is the cause of IT instability issues, when Verifier used Toolbox output ends up on Surefire stdout and not on "grabbed" output, as simply streams are not yet set up properly. Also, this usually happens "around the second half" of ITs, when cached and warmed up embedded instance is already uber optimized. Backport of bb94de0
1 parent 67756e5 commit 5f24cba

2 files changed

Lines changed: 12 additions & 3 deletions

File tree

impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/LookupInvoker.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,13 @@ protected final void createTerminal(C context) {
323323
context.terminal = MessageUtils.getTerminal();
324324
context.closeables.add(MessageUtils::systemUninstall);
325325
MessageUtils.registerShutdownHook(); // safety belt
326+
327+
// when we use embedded executor AND --raw-streams, we must ENSURE streams are properly set up
328+
if (context.invokerRequest.embedded()
329+
&& context.options().rawStreams().orElse(false)) {
330+
// to trigger FastTerminal; with raw-streams we must do this ASAP (to have system in/out/err set up)
331+
context.terminal.getName();
332+
}
326333
} else {
327334
doConfigureWithTerminal(context, context.terminal);
328335
}

impl/maven-executor/src/main/java/org/apache/maven/cling/executor/internal/ToolboxTool.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -159,9 +159,11 @@ private String validateOutput(boolean shave, ByteArrayOutputStream stdout, ByteA
159159
result = result.replace("\n", "").replace("\r", "");
160160
}
161161
// sanity checks: stderr has any OR result is empty string (no method should emit empty string)
162-
if (stderr.size() > 0 || result.trim().isEmpty()) {
163-
System.err.println(
164-
"Unexpected stdout[" + stdout.size() + "]=" + stdout + "; stderr[" + stderr.size() + "]=" + stderr);
162+
if (result.trim().isEmpty()) {
163+
// see bug https://github.com/apache/maven/pull/11303 Fail in this case
164+
// tl;dr We NEVER expect empty string as output from this tool; so fail here instead to chase ghosts
165+
throw new IllegalStateException("Empty output from Toolbox; stdout[" + stdout.size() + "]=" + stdout
166+
+ "; stderr[" + stderr.size() + "]=" + stderr);
165167
}
166168
return result;
167169
}

0 commit comments

Comments
 (0)