gnodet-bot commented on code in PR #12694:
URL: https://github.com/apache/maven/pull/12694#discussion_r4012352892
##########
impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/LookupInvoker.java:
##########
@@ -447,12 +457,31 @@ protected Consumer<String> doDetermineWriter(C context) {
}
protected void activateLogging(C context) throws Exception {
- if (!SLF4JBridgeHandler.isInstalled()) {
- SLF4JBridgeHandler.removeHandlersForRootLogger();
- SLF4JBridgeHandler.install();
+ if (!MavenJulHandler.isInstalled()) {
+ MavenJulHandler.install();
}
context.slf4jConfiguration.activate();
+
+ // Now that SLF4J is fully initialized, set the JUL root logger level
+ // to match the effective log level. This must happen AFTER install()
+ // + activate() to avoid flooding JUL events during SLF4J bootstrap
+ // (ConcurrentHashMap.computeIfAbsent reentrancy).
+ // In quiet mode keep the JUL root at WARNING so that INFO/DEBUG JUL
+ // events are suppressed at source — relying solely on the SLF4J-level
+ // check in MavenJulHandler.isLevelEnabled() is racy: newly created
+ // SLF4J loggers may briefly see the default INFO level before
+ // quiet-mode propagation completes, leaking output that
+ // MavenITmng4387QuietLoggingTest detects as a flaky failure.
+ java.util.logging.Level julRootLevel;
+ if (context.options().quiet().orElse(false)) {
+ julRootLevel = java.util.logging.Level.WARNING;
Review Comment:
**[medium] Quiet-mode JUL root level should be `SEVERE`, not `WARNING`**
(carried over from previous review — still unaddressed)
In quiet mode Maven sets SLF4J to `ERROR` level
(`Slf4jConfiguration.Level.ERROR`, line 296). The stated goal of this JUL root
level assignment is to suppress events **at source** so that newly-created
SLF4J loggers (briefly at `INFO` before propagation) don't leak output. But
`Level.WARNING` (integer 900) blocks only `INFO`/`DEBUG`/`FINE` JUL events —
`WARNING`-and-above events still pass the root filter and reach
`MavenJulHandler.publish()`. During the race window where a new SLF4J logger
hasn't had `ERROR` propagated yet, a JUL `WARNING` event will slip through
`isLevelEnabled()` (which sees `INFO`) and appear on console — exactly the
flakiness pattern this is trying to fix.
`Level.SEVERE` (integer 1000) is the correct JUL equivalent of SLF4J
`ERROR`. Update the code and the comment above:
```suggestion
julRootLevel = java.util.logging.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]
