gnodet commented on code in PR #12695:
URL: https://github.com/apache/maven/pull/12695#discussion_r4059813564


##########
impl/maven-core/src/main/java/org/apache/maven/internal/build/BuildReportCollector.java:
##########
@@ -0,0 +1,667 @@
+/*
+ * 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.internal.build;
+
+import javax.inject.Named;
+import javax.inject.Singleton;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.nio.file.AtomicMoveNotSupportedException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.StandardCopyOption;
+import java.time.Duration;
+import java.time.Instant;
+import java.time.ZoneOffset;
+import java.time.format.DateTimeFormatter;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.TreeMap;
+import java.util.concurrent.ConcurrentHashMap;
+
+import org.apache.maven.api.BuildEnvironment;
+import org.apache.maven.api.MonotonicClock;
+import org.apache.maven.api.build.report.BuildReport;
+import org.apache.maven.api.build.report.BuildStatus;
+import org.apache.maven.api.build.report.FailureReport;
+import org.apache.maven.api.build.report.LogEvent;
+import org.apache.maven.api.build.report.ModuleReport;
+import org.apache.maven.api.build.report.MojoReport;
+import org.apache.maven.eventspy.AbstractEventSpy;
+import org.apache.maven.execution.BuildFailure;
+import org.apache.maven.execution.BuildSuccess;
+import org.apache.maven.execution.BuildSummary;
+import org.apache.maven.execution.ExecutionEvent;
+import org.apache.maven.execution.MavenExecutionResult;
+import org.apache.maven.execution.MavenSession;
+import org.apache.maven.logging.ProjectBuildLogAppender;
+import org.apache.maven.plugin.MojoExecution;
+import org.apache.maven.project.MavenProject;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Collects build lifecycle events and produces a structured {@link 
BuildReport}
+ * at the end of the session.
+ * <p>
+ * Registered as an {@link org.apache.maven.eventspy.EventSpy} via {@code 
@Named}/{@code @Singleton},
+ * following the same pattern as {@code DefaultPluginValidationManager}.
+ * <p>
+ * Thread-safe: concurrent module builds (with {@code -T}) each write to their
+ * own entry in a {@link ConcurrentHashMap}.
+ * <p>
+ * Log capture: registers a callback on {@link ProjectBuildLogAppender} to
+ * receive the already-formed {@link LogEvent} objects produced by the main
+ * logging pipeline. Uses thread-based tracking to associate events with
+ * the currently-executing mojo or module.
+ *
+ * @since 4.1.0
+ */
+@Singleton
+@Named
+public final class BuildReportCollector extends AbstractEventSpy {
+
+    private static final Logger LOGGER = 
LoggerFactory.getLogger(BuildReportCollector.class);
+
+    static final String REPORT_DIR = "build-reports";
+    static final String REPORT_LATEST = "build-report-latest.json";
+
+    private static final int MAX_STACKTRACE_LINES = 30;
+
+    /**
+     * Maximum number of log events captured per scope (mojo, module, or 
build).
+     * Beyond this, events are silently dropped to prevent unbounded memory 
growth.
+     */
+    static final int MAX_LOG_EVENTS_PER_SCOPE = 500;
+
+    // ---- Mutable state, populated during the build ----
+
+    /** Per-project mojo tracking: project key -> list of in-flight/completed 
mojos. */
+    private final Map<String, List<MojoTiming>> mojoTimings = new 
ConcurrentHashMap<>();
+
+    /** Per-project start instants for duration computation. */
+    private final Map<String, Instant> projectStartTimes = new 
ConcurrentHashMap<>();
+
+    /** Per-mojo start instants for duration computation. */
+    private final Map<String, Instant> mojoStartTimes = new 
ConcurrentHashMap<>();
+
+    /** Session-level state - set once on SessionStarted. */
+    private volatile MavenSession session;
+
+    /** Build environment - captured once at SessionStarted, immutable 
thereafter. */
+    private volatile BuildEnvironment buildEnvironment;
+
+    // ---- Log capture state ----
+
+    /**
+     * Maps thread ID -> mojo key for the currently-executing mojo on that 
thread.
+     * Lifecycle events and mojo execution run on the same thread, so this is 
safe
+     * for parallel builds with {@code -T}.
+     */
+    private final Map<Long, String> currentMojoByThread = new 
ConcurrentHashMap<>();
+
+    /** Per-mojo log buffers: mojo key -> captured log events. */
+    private final Map<String, List<LogEvent>> mojoLogBuffers = new 
ConcurrentHashMap<>();
+
+    /**
+     * Maps thread ID -> project key for the currently-building project on 
that thread.
+     * Used to route log events that occur between mojo executions to the 
module-level buffer.
+     */
+    private final Map<Long, String> currentProjectByThread = new 
ConcurrentHashMap<>();
+
+    /** Per-module log buffers: project key -> events captured outside any 
mojo. */
+    private final Map<String, List<LogEvent>> moduleLogBuffers = new 
ConcurrentHashMap<>();
+
+    /** Build-level log buffer: events captured outside any module lifecycle. 
*/
+    private final List<LogEvent> buildLogBuffer = 
Collections.synchronizedList(new ArrayList<>());
+
+    @Override
+    public void onEvent(Object event) {
+        if (event instanceof ExecutionEvent executionEvent) {
+            switch (executionEvent.getType()) {
+                case SessionStarted:
+                    onSessionStarted(executionEvent);
+                    break;
+                case SessionEnded:
+                    onSessionEnded(executionEvent);
+                    break;
+                case ProjectStarted:
+                    onProjectStarted(executionEvent);
+                    break;
+                case ProjectSucceeded:
+                case ProjectFailed:
+                case ProjectSkipped:
+                    onProjectFinished(executionEvent);
+                    break;
+                case MojoStarted:
+                    onMojoStarted(executionEvent);
+                    break;
+                case MojoSucceeded:
+                case MojoFailed:
+                    onMojoFinished(executionEvent);
+                    break;
+                default:
+                    break;
+            }
+        }
+    }
+
+    // ---- Event handlers ----
+
+    private void onSessionStarted(ExecutionEvent event) {
+        this.session = event.getSession();
+        this.buildEnvironment = buildEnvironment(this.session);
+        installLogCapture();
+    }
+
+    private void onSessionEnded(ExecutionEvent event) {
+        removeLogCapture();
+
+        MavenSession endSession = event.getSession();
+        if (endSession == null) {
+            return;
+        }
+
+        try {
+            BuildReport report = buildReport(endSession);
+            writeReport(report, endSession);
+        } catch (Exception e) {
+            // Never let the report collector crash the build
+            LOGGER.debug("Failed to produce build report: {}", e.getMessage(), 
e);
+        }
+    }
+
+    private void onProjectStarted(ExecutionEvent event) {
+        String key = projectKey(event.getProject());
+        projectStartTimes.put(key, MonotonicClock.now());
+        mojoTimings.putIfAbsent(key, Collections.synchronizedList(new 
ArrayList<>()));
+        moduleLogBuffers.put(key, Collections.synchronizedList(new 
ArrayList<>()));
+        currentProjectByThread.put(Thread.currentThread().getId(), key);
+    }
+
+    private void onProjectFinished(ExecutionEvent event) {
+        // Unregister the project from this thread so subsequent log events
+        // fall through to the build-level buffer
+        currentProjectByThread.remove(Thread.currentThread().getId());
+    }
+
+    private void onMojoStarted(ExecutionEvent event) {
+        String mKey = mojoKey(event.getProject(), event.getMojoExecution());
+        mojoStartTimes.put(mKey, MonotonicClock.now());
+
+        // Register the current mojo for this thread so the log event sink
+        // can associate events with this mojo execution
+        currentMojoByThread.put(Thread.currentThread().getId(), mKey);
+        mojoLogBuffers.put(mKey, Collections.synchronizedList(new 
ArrayList<>()));
+    }
+
+    private void onMojoFinished(ExecutionEvent event) {
+        MojoExecution mojo = event.getMojoExecution();
+        MavenProject project = event.getProject();
+        String mKey = mojoKey(project, mojo);
+        String pKey = projectKey(project);
+
+        // Unregister the mojo from this thread
+        currentMojoByThread.remove(Thread.currentThread().getId());
+
+        Instant now = MonotonicClock.now();
+        Instant startInstant = mojoStartTimes.remove(mKey);
+        if (startInstant == null) {
+            startInstant = now;
+        }
+        Duration duration = Duration.between(startInstant, now);
+
+        BuildStatus status =
+                event.getType() == ExecutionEvent.Type.MojoSucceeded ? 
BuildStatus.SUCCESS : BuildStatus.FAILURE;
+
+        // Drain the log buffer for this mojo
+        List<LogEvent> logBuffer = mojoLogBuffers.remove(mKey);
+        List<LogEvent> output = logBuffer != null ? List.copyOf(logBuffer) : 
List.of();
+
+        MojoTiming timing = new MojoTiming(
+                mojo.getGroupId(),
+                mojo.getArtifactId(),
+                mojo.getVersion(),
+                mojo.getGoal(),
+                mojo.getExecutionId(),
+                mojo.getLifecyclePhase(),
+                status,
+                startInstant,
+                duration,
+                output);
+
+        mojoTimings
+                .computeIfAbsent(pKey, k -> Collections.synchronizedList(new 
ArrayList<>()))
+                .add(timing);
+    }
+
+    // ---- Structured log capture ----
+
+    /**
+     * Registers a callback on {@link ProjectBuildLogAppender} to receive the
+     * already-formed {@link LogEvent} objects from the main logging pipeline.
+     * This eliminates the need for a separate capture path and ensures the
+     * build report captures the same enriched events (with sequence number,
+     * source metadata) as the console output.
+     */
+    private void installLogCapture() {
+        ProjectBuildLogAppender.setReportCapture(this::captureLogEvent);
+    }
+
+    private void removeLogCapture() {
+        ProjectBuildLogAppender.setReportCapture(null);
+    }
+
+    /**
+     * Routes a pre-formed {@link LogEvent} to the appropriate buffer
+     * (mojo, module, or build-level) based on the current thread's
+     * lifecycle context.
+     */
+    private void captureLogEvent(LogEvent event) {
+        long threadId = Thread.currentThread().getId();
+
+        // 1. Mojo-level: event belongs to the currently-executing mojo on 
this thread
+        String mKey = currentMojoByThread.get(threadId);
+        if (mKey != null) {
+            List<LogEvent> buffer = mojoLogBuffers.get(mKey);
+            if (buffer != null && buffer.size() < MAX_LOG_EVENTS_PER_SCOPE) {
+                buffer.add(event);
+            }
+            return;
+        }
+
+        // 2. Module-level: project is active but no mojo is running
+        String pKey = currentProjectByThread.get(threadId);
+        if (pKey != null) {
+            List<LogEvent> buffer = moduleLogBuffers.get(pKey);
+            if (buffer != null && buffer.size() < MAX_LOG_EVENTS_PER_SCOPE) {
+                buffer.add(event);
+            }
+            return;
+        }
+
+        // 3. Build-level: no project active (startup, reactor summary, 
post-build)
+        if (buildLogBuffer.size() < MAX_LOG_EVENTS_PER_SCOPE) {
+            buildLogBuffer.add(event);

Review Comment:
   Fixed in `3ba3624085`: wrapped the build-level `size() < MAX` + `add()` in a 
`synchronized(buildLogBuffer)` block to make the check-then-act atomic.



##########
impl/maven-logging/src/main/java/org/apache/maven/slf4j/MavenJulHandler.java:
##########
@@ -0,0 +1,318 @@
+/*
+ * 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.slf4j;
+
+import java.text.MessageFormat;
+import java.util.MissingResourceException;
+import java.util.ResourceBundle;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
+import java.util.logging.Handler;
+import java.util.logging.Level;
+import java.util.logging.LogManager;
+import java.util.logging.LogRecord;
+import java.util.logging.Logger;
+
+import org.slf4j.LoggerFactory;
+import org.slf4j.spi.LocationAwareLogger;
+
+/**
+ * A JUL {@link Handler} that routes {@code java.util.logging} events into
+ * Maven's structured logging pipeline, preserving the rich {@link LogRecord}
+ * metadata that the standard {@code SLF4JBridgeHandler} silently drops
+ * (source class name, source method name, thread ID).
+ * <p>
+ * All JUL events are routed through SLF4J so that {@link MavenSimpleLogger}
+ * produces a consistent {@code formattedMessage} (with timestamp, logger name,
+ * and ANSI styling) regardless of the event's origin.  The JUL metadata is
+ * stashed in a thread-local <em>before</em> the SLF4J call so that downstream
+ * consumers (e.g. {@code ProjectBuildLogAppender}) can read it when
+ * constructing a structured {@code LogEvent}.
+ * <p>
+ * Usage — replace the standard SLF4J bridge in {@code LookupInvoker}:
+ * <pre>
+ *     MavenJulHandler.install();
+ * </pre>
+ *
+ * @since 4.1.0
+ * @see #install()
+ * @see #getJulMetadata()
+ */
+public class MavenJulHandler extends Handler {
+
+    /**
+     * JUL metadata captured from a {@link LogRecord} that would otherwise
+     * be lost when bridging to SLF4J.
+     *
+     * @param sourceClassName  the source class, or {@code null}
+     * @param sourceMethodName the source method, or {@code null}
+     * @param threadId         the originating thread ID
+     */
+    public record JulMetadata(String sourceClassName, String sourceMethodName, 
long threadId) {}
+
+    private static final ThreadLocal<JulMetadata> METADATA = new 
ThreadLocal<>();
+
+    /**
+     * Private SLF4J logger cache using {@link ConcurrentMap#putIfAbsent}
+     * instead of {@link ConcurrentMap#computeIfAbsent}.  This avoids the
+     * {@code ConcurrentHashMap.computeIfAbsent} reentrancy bug
+     * ({@code IllegalStateException("Recursive update")}) that occurs
+     * when a JUL event fires during SLF4J logger initialization: the
+     * handler's {@code publish()} calls {@code LoggerFactory.getLogger()},
+     * which internally uses {@code computeIfAbsent}, and if that triggers
+     * another JUL event whose logger name hashes to the same bucket,
+     * {@code ConcurrentHashMap} throws.  {@code putIfAbsent} is safe
+     * against reentrancy — worst case, two threads create the same
+     * logger and one is discarded.
+     */
+    private static final ConcurrentMap<String, org.slf4j.Logger> LOGGER_CACHE 
= new ConcurrentHashMap<>();
+
+    /**
+     * Re-entrancy guard: set to {@code true} while {@link #publish} is routing
+     * a JUL event through SLF4J on this thread.  Prevents recursive JUL events
+     * (e.g. JLine's {@code StyleResolver} calling {@code 
java.util.logging.Logger}
+     * while inside {@link MavenSimpleLogger#renderLevel} lazy-initialisation,
+     * which in turn is triggered by a JUL event during terminal construction)
+     * from re-entering {@code publish} and crashing with
+     * {@code ConcurrentHashMap.computeIfAbsent 
IllegalStateException("Recursive update")}.
+     */
+    private static final ThreadLocal<Boolean> IN_PUBLISH = new ThreadLocal<>();
+
+    /**
+     * Returns the JUL metadata for the current log event being processed,
+     * or {@code null} if the current log event did not originate from JUL.
+     * <p>
+     * This method is intended to be called from within a
+     * {@link MavenSimpleLogger.LogSink} callback (e.g. in
+     * {@code ProjectBuildLogAppender.accept()}).
+     *
+     * @return the current JUL metadata, or {@code null}
+     */
+    public static JulMetadata getJulMetadata() {
+        return METADATA.get();
+    }
+
+    /**
+     * Package-private test hook: sets or clears the re-entrancy flag on
+     * the current thread without reflection.  Tests in the same package
+     * can call this instead of using {@code getDeclaredField("IN_PUBLISH")}.
+     *
+     * @param inPublish {@code true} to simulate being inside {@link #publish},
+     *                  {@code false} (or pass {@code null} via the {@code 
remove}
+     *                  path) to clear the flag
+     */
+    static void setInPublishForTest(boolean inPublish) {
+        if (inPublish) {
+            IN_PUBLISH.set(Boolean.TRUE);
+        } else {
+            IN_PUBLISH.remove();
+        }
+    }
+
+    /**
+     * Installs this handler on the JUL root logger, removing any
+     * previously installed handlers.  This replaces the standard
+     * {@code SLF4JBridgeHandler.install()} call.
+     */
+    public static void install() {
+        Logger rootLogger = LogManager.getLogManager().getLogger("");
+        // Remove all existing handlers (including any SLF4JBridgeHandler)
+        for (Handler handler : rootLogger.getHandlers()) {
+            rootLogger.removeHandler(handler);
+        }
+        rootLogger.addHandler(new MavenJulHandler());
+        // Note: we intentionally do NOT set rootLogger.setLevel(Level.ALL)
+        // here.  Setting it eagerly floods JUL events during SLF4J bootstrap,
+        // triggering ConcurrentHashMap.computeIfAbsent reentrancy in the
+        // SLF4J logger factory ("Recursive update").  The JUL root default
+        // (INFO) is fine — callers that need FINE/FINEST events (e.g. -X
+        // debug mode) should set the JUL root level after SLF4J is fully
+        // initialized.
+    }
+
+    /**
+     * Returns {@code true} if a {@code MavenJulHandler} is installed
+     * on the JUL root logger.
+     */
+    public static boolean isInstalled() {
+        Logger rootLogger = LogManager.getLogManager().getLogger("");
+        for (Handler handler : rootLogger.getHandlers()) {
+            if (handler instanceof MavenJulHandler) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    @Override
+    public void publish(LogRecord record) {
+        if (record == null) {
+            return;
+        }
+
+        // Honour any Filter registered on this Handler 
(java.util.logging.Handler contract).
+        if (!isLoggable(record)) {
+            return;

Review Comment:
   Fixed in `3ba3624085`: added `publishRespectsHandlerFilter()` to 
`MavenJulHandlerTest` — installs a reject-all `Filter`, verifies it is 
consulted (tracking list), and confirms `publish()` returns cleanly without 
forwarding the record.



-- 
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]

Reply via email to