gnodet-bot commented on code in PR #13700: URL: https://github.com/apache/maven/pull/13700#discussion_r4369981009
########## api/maven-api-log/src/main/java/org/apache/maven/api/logging/LoggerManager.java: ########## @@ -0,0 +1,189 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.api.logging; + +import java.util.ServiceLoader; + +import org.apache.maven.api.annotations.Experimental; +import org.apache.maven.api.annotations.Nonnull; +import org.apache.maven.api.annotations.Nullable; + +/** + * Central entry point for Maven's logging system. + * <p> + * Provides access to loggers, the active {@link LogSink}, level configuration, + * and severity tracking for {@code --fail-on-severity}. + * <p> + * The singleton is resolved lazily on the first {@link #get()} call using + * {@link ServiceLoader}. If no provider is found (e.g. during early classworlds + * bootstrap before the implementation jar is visible), a minimal + * {@link BootstrapLoggerManager} is installed that buffers events. When the + * real implementation is later discovered or installed via {@link #set(LoggerManager)}, + * buffered events are drained in FIFO order. + * + * <h2>Three bootstrap tiers</h2> + * <ul> + * <li><b>Tier 0</b> — before any impl jar: bootstrap manager buffers events</li> + * <li><b>Tier 1</b> — impl jar appears: {@code ServiceLoader} discovers it, + * bootstrap events are drained</li> + * <li><b>Tier 2</b> — normal runtime: fast-path return of the cached manager</li> + * </ul> + * + * @since 4.1.0 + */ +@Experimental +public interface LoggerManager { + + /** + * Returns a {@link Logger} instance for the given name. + */ + @Nonnull + Logger getLogger(@Nonnull String name); + + /** + * Returns a {@link Logger} instance for the given class. + */ + @Nonnull + default Logger getLogger(@Nonnull Class<?> clazz) { + return getLogger(clazz.getName()); + } + + /** + * Returns the active log sink, or {@code null} if none is installed. + */ + @Nullable + LogSink getLogSink(); + + /** + * Installs or removes the active log sink. + */ + void setLogSink(@Nullable LogSink sink); + + /** + * Returns whether logging at {@code level} is enabled for the named logger. + */ + boolean isLevelEnabled(@Nonnull String loggerName, @Nonnull LogLevel level); + + /** + * Sets the root logger level (typically from {@code -X}, {@code -q} CLI flags). + * The new level takes effect immediately. + */ + void setRootLoggerLevel(@Nonnull LogLevel level); + + /** + * Reloads level configuration and resets severity tracking state. + */ + void reconfigure(); + + /** + * Returns {@code true} if any log event at or above {@code level} + * has been emitted since the last {@link #reconfigure()}. + * <p> + * Used by {@code --fail-on-severity}: the CLI stores the threshold + * and checks at build end. + */ + boolean hasReached(@Nonnull LogLevel level); + + /** + * Returns the global {@code LoggerManager} instance. + * <p> + * On the first call, attempts to discover an implementation via + * {@link ServiceLoader}. If none is found, installs a bootstrap + * manager that buffers events until a real implementation is + * {@linkplain #set(LoggerManager) installed}. + * + * @return the global manager, never {@code null} + */ + @Nonnull + static LoggerManager get() { + LoggerManager mgr = Holder.instance; + if (mgr != null && !(mgr instanceof BootstrapLoggerManager)) { + return mgr; + } + return Holder.discover(); + } + + /** + * Installs the global {@code LoggerManager} instance. + * <p> + * If a bootstrap manager was active, its buffered events and pending + * configuration are drained into the new manager. + * + * @param manager the manager to install, or {@code null} to clear + */ + static void set(LoggerManager manager) { + Holder.install(manager); + } + + /** + * @hidden + */ + final class Holder { + private Holder() {} + + static volatile LoggerManager instance; + + private static final ThreadLocal<Boolean> DISCOVERING = new ThreadLocal<>(); + + static synchronized LoggerManager discover() { + LoggerManager mgr = instance; + if (mgr != null && !(mgr instanceof BootstrapLoggerManager)) { + return mgr; + } + + // Re-entrancy guard: if ServiceLoader loading triggers logging, + // return the bootstrap manager to avoid infinite recursion. + if (DISCOVERING.get() == Boolean.TRUE) { + return ensureBootstrap(); + } + + DISCOVERING.set(Boolean.TRUE); + try { + for (LoggerManager discovered : + ServiceLoader.load(LoggerManager.class, LoggerManager.class.getClassLoader())) { + install(discovered); + return discovered; + } + } catch (Exception ignored) { Review Comment: ⚠️ **Regression: `ServiceConfigurationError` no longer caught — bootstrap crash risk** The previous code explicitly caught both `Exception | ServiceConfigurationError`. The import for `ServiceConfigurationError` was removed and the catch narrowed to `Exception` only. `ServiceLoader.load()` iteration can throw `ServiceConfigurationError` (which extends `Error`, not `Exception`) when the provider class listed in `META-INF/services/org.apache.maven.api.logging.LoggerManager` is malformed, missing from the classpath, or cannot be instantiated. This was clearly caught intentionally before. With only `catch (Exception ignored)`, such a failure will escape as an uncaught `Error` and crash the JVM during early bootstrap — before any error reporting is set up. ```suggestion } catch (Exception | java.util.ServiceConfigurationError ignored) { ``` _This review was generated by an AI agent, Hermès on behalf of @gnodet._ <!-- reviewer: gnodet-bot --> -- 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]
