gnodet-bot commented on code in PR #12694:
URL: https://github.com/apache/maven/pull/12694#discussion_r4009614691
##########
impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/LookupInvoker.java:
##########
@@ -447,12 +447,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] JUL quiet level should be `SEVERE`, not `WARNING`**
In quiet mode, SLF4J is configured at `ERROR` level. The stated goal of the
JUL root level assignment is to suppress events *at source* so that the brief
window where newly-created SLF4J loggers are still at their default `INFO`
level doesn't leak output. Using `Level.WARNING` only blocks
`INFO`/`DEBUG`/`FINE` JUL events — `WARNING` events (JUL integer value 900)
still pass the root filter and reach `MavenJulHandler.publish()`. Inside
`publish()`, they hit `isLevelEnabled()`, which during that brief race window
may see the SLF4J logger at `INFO` (not yet propagated to `ERROR`), allowing
the `WARNING` event through to the console — exactly the flakiness pattern this
fix is trying to prevent.
`Level.SEVERE` (integer value 1000) is the JUL equivalent of SLF4J `ERROR`
and matches the quiet-mode intent precisely: only `SEVERE`-and-above JUL events
pass the root filter.
```suggestion
julRootLevel = java.util.logging.Level.SEVERE;
```
##########
impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/LookupInvoker.java:
##########
@@ -447,12 +447,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;
+ } else if (context.invokerRequest.effectiveVerbose()) {
+ julRootLevel = java.util.logging.Level.ALL;
+ } else {
+ julRootLevel = java.util.logging.Level.INFO;
+ }
+
java.util.logging.LogManager.getLogManager().getLogger("").setLevel(julRootLevel);
Review Comment:
**[low] Update comment to reference `SEVERE` instead of `WARNING`**
If `Level.SEVERE` is adopted above, the comment on line 460 ("keep JUL root
at WARNING") should be updated to say `SEVERE` to stay accurate.
--
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]
