gnodet-bot commented on code in PR #12694:
URL: https://github.com/apache/maven/pull/12694#discussion_r4357501436
##########
impl/maven-core/src/main/java/org/apache/maven/logging/SimpleBuildEventListener.java:
##########
@@ -38,8 +39,8 @@ public void sessionStarted(ExecutionEvent event) {}
public void projectStarted(String projectId) {}
@Override
- public void projectLogMessage(String projectId, String event) {
- log(event);
+ public void projectLogMessage(String projectId, LogEvent event) {
+ log(event.message());
Review Comment:
**[critical] Raw `event.message()` drops all log-level formatting.**
Before this commit, `MavenSimpleLogger` produced a fully formatted console
line (e.g. `[INFO] Building my-project 1.0.0`) and that string was passed as
`formattedMessage` to `SimpleBuildEventListener`. Now `formattedMessage()` is
removed from the API and `needsFormattedMessage()` is hardcoded to `false`, so
only the raw substituted text is available here. All standard Maven terminal
output will lose its `[INFO]`/`[WARN]`/`[ERROR]` prefix, thread name, and
timestamp.
Fix options:
1. Reconstruct from structured fields: `log("[" + event.level().name() + "]
" + event.message())`
2. Inject a `LogEventFormatter` (the renderer concern stays outside the
event, which is the right direction)
3. Make `MavenSimpleLogger.write()` call `super.write(buf, t)`
unconditionally for console output and route the structured event separately to
the sink
Option 2 is the cleanest long-term design — pass-through renderers should
carry their own formatter, not rely on a pre-formatted field in the data
structure.
```suggestion
public void projectLogMessage(String projectId, LogEvent event) {
log("[" + event.level().name() + "] " + event.message());
}
```
##########
impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/LookupInvoker.java:
##########
@@ -314,6 +331,23 @@ protected void configureLogging(C context) throws
Exception {
} else if (context.options().quiet().orElse(false)) {
context.loggerLevel = Slf4jConfiguration.Level.ERROR;
context.slf4jConfiguration.setRootLoggerLevel(context.loggerLevel);
+ // Install the JUL handler early and clamp the JUL root to SEVERE
+ // so that JLine terminal-init DEBUG events (emitted via
java.util.logging
+ // before activateLogging() runs) are suppressed at source.
+ // createTerminal() executes after configureLogging() but before
+ // activateLogging(), so without this guard those DEBUG lines leak
+ // into the build output even when -q is in effect, causing
+ // MavenITmng4387QuietLoggingTest to fail on Windows.
+ // Setting SEVERE here is safe: it only blocks events, so it
+ // cannot trigger the SLF4J-bootstrap reentrancy flood that
+ // Level.ALL would cause.
+ if (LogManager.getLogManager().getClass() == LogManager.class
+ && !MavenJulHandler.isInstalled()) {
+ MavenJulHandler.install();
+ }
+ if (LogManager.getLogManager().getClass() == LogManager.class) {
+
LogManager.getLogManager().getLogger("").setLevel(Level.SEVERE);
Review Comment:
**[low] `LogManager.getLogManager()` called redundantly** — 3 times in this
method alone (lines 344, 348, 349), and 7 times total across `invoke()`,
`configureLogging()`, and `activateLogging()`.
The `LogManager` singleton doesn't change at runtime; cache it in a local
variable at the top of each method:
```java
LogManager logManager = LogManager.getLogManager();
if (logManager.getClass() == LogManager.class &&
!MavenJulHandler.isInstalled()) {
MavenJulHandler.install();
}
if (logManager.getClass() == LogManager.class) {
logManager.getLogger("").setLevel(Level.SEVERE);
}
```
--
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]
