gnodet-bot commented on code in PR #12694:
URL: https://github.com/apache/maven/pull/12694#discussion_r4046224223
##########
api/maven-api-core/src/main/java/org/apache/maven/api/plugin/Log.java:
##########
@@ -97,9 +95,6 @@ default void trace(Supplier<String> content) {}
* The supplier is only evaluated if trace is enabled.
* <p>
* The default implementation is a no-op for backward compatibility.
- *
- * @param content the message supplier
- * @param error the error that caused this log
*/
default void trace(Supplier<String> content, Throwable error) {}
Review Comment:
**[medium] `@param content` and `@param error` tags dropped — regression
from `caeeeb4`**
Same as the `trace(Supplier<String>)` overload above: both `@param` tags
were added in `caeeeb4` and confirmed by the prior APPROVE. The squash dropped
them. Every other supplier+throwable method (`debug`, `info`, `warn`, `error`)
documents both parameters.
```suggestion
* The default implementation is a no-op for backward compatibility.
*
* @param content the message supplier
* @param error the error that caused this log
*/
default void trace(Supplier<String> content, Throwable error) {}
```
##########
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;
+ } 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] Quiet-mode JUL root level should be `SEVERE`, not `WARNING`**
(carried over — still open)
In quiet mode Maven sets SLF4J to `ERROR` level. The JUL equivalent of ERROR
is `Level.SEVERE`. Using `Level.WARNING` lets JUL `WARNING`-level events pass
the at-source gate and reach `MavenJulHandler`, where they are then filtered by
`isLevelEnabled()`. The comment says the goal is to suppress events "at source"
— but `SEVERE` is more precise: it matches the SLF4J ERROR threshold exactly.
`Level.SEVERE` closes the race-condition window for INFO/DEBUG just as
effectively, while also being semantically correct for quiet mode. Update the
comment from "keep JUL root at WARNING" to "keep JUL root at SEVERE" if adopted.
--
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]
