gnodet-bot commented on code in PR #13860:
URL: https://github.com/apache/maven/pull/13860#discussion_r4487664750
##########
compat/maven-embedder/src/main/java/org/apache/maven/cli/MavenCli.java:
##########
@@ -615,9 +620,19 @@ void logging(CliRequest cliRequest) throws ExitException {
// Ignore
//
}
- }
- slf4jConfiguration.activate();
+ // Logging caches System.out; with -l the wait above already ran,
so activate now.
+ slf4jConfiguration.activate();
+ } else {
+ // Logging caches System.out, so wait for terminal installation to
complete before
+ // activating, except when color is explicitly forced. With forced
color the user expects
+ // raw ANSI to flow through the original stdout; waiting causes
AnsiConsole to wrap that
+ // stream before the logger caches it, which may strip the escape
codes.
+ if (!"always".equals(styleColor) && !"yes".equals(styleColor) &&
!"force".equals(styleColor)) {
+ MessageUtils.awaitTerminalInitialization();
+ }
+ slf4jConfiguration.activate();
Review Comment:
This is a bugfix for a subtle race condition — it needs regression tests.
The compat layer uses SLF4J Simple (not `MavenLoggerFactory`), so the 3.10.x
`MavenCliLoggingTest` is directly portable. `FastTerminal` is already on the
compat test classpath (`compat/maven-embedder` depends on `maven-jline`) and
`MavenCliTest.java` already imports `MessageUtils`.
Please add
`compat/maven-embedder/src/test/java/org/apache/maven/cli/MavenCliLoggingTest.java`
covering at minimum:
1. forced-color (`-Dstyle.color=always`) skips the await — `logging()`
returns without blocking
2. non-forced-color awaits before `activate()` — logger caches the terminal
stream, not the early stream
3. `LOG_FILE` branch awaits before stream redirection — prevents #12188
A follow-up is not appropriate for a bugfix's own regression guard.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
