gnodet commented on code in PR #12697: URL: https://github.com/apache/maven/pull/12697#discussion_r3743159447
########## impl/maven-cli/src/main/java/org/apache/maven/cling/event/RichBuildEventListener.java: ########## @@ -0,0 +1,785 @@ +/* + * 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.cling.event; + +import java.io.PrintWriter; +import java.time.Duration; +import java.time.Instant; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ScheduledExecutorService; +import java.util.concurrent.ScheduledFuture; +import java.util.concurrent.ScheduledThreadPoolExecutor; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; + +import org.apache.maven.api.MonotonicClock; +import org.apache.maven.api.build.report.LogEvent; +import org.apache.maven.api.build.report.LogLevel; +import org.apache.maven.execution.ExecutionEvent; +import org.apache.maven.execution.MavenSession; +import org.apache.maven.logging.BuildEventListener; +import org.apache.maven.project.MavenProject; +import org.eclipse.aether.transfer.TransferEvent; +import org.jline.terminal.Terminal; +import org.jline.utils.Display; + +/** + * A rich terminal build event listener using JLine's {@link Display} in + * non-fullscreen mode — the same approach as mvnd. + * <p> + * The status area is rendered at the current cursor position using + * {@link Display#updateAnsi}. When log output arrives, the display is + * cleared (updated with empty lines), the log line is printed normally, + * and then the status is redrawn below it. JLine handles all the cursor + * math (moving up, erasing changed lines, etc.) and only repaints what + * actually changed. + * <p> + * At the end of the build, the display is cleared and nothing remains + * on screen — the summary then prints as normal scrolling text. + * <p> + * The status area has a fixed height based on the degree of concurrency, + * so the separator and summary line stay anchored at the bottom. Active + * projects are packed to the top of the slot area; empty lines fill the + * gap between the last active project and the separator. + * <p> + * Falls back to simple log passthrough on dumb terminals. + * + * @since 4.1.0 + * @see PlainExecutionEventLogger + * @see ExecutionEventLogger + */ +public class RichBuildEventListener implements BuildEventListener { + + // ---- ANSI colors ---- + + private static final String ESC = "\033["; + private static final String CYAN = ESC + "36m"; + private static final String YELLOW = ESC + "33m"; + private static final String BLUE = ESC + "34m"; + private static final String GREEN = ESC + "32m"; + private static final String RED = ESC + "31m"; + private static final String BOLD = ESC + "1m"; + private static final String DIM = ESC + "2m"; + private static final String RESET = ESC + "0m"; + + // ---- Terminal & output ---- + + private final Terminal terminal; + private final PrintWriter writer; + private final boolean supported; + + // ---- JLine Display ---- + + /** JLine display in non-fullscreen mode — handles cursor math. */ + private volatile Display display; + /** Whether the display is currently active. */ + private volatile boolean displayActive; + /** Fixed number of lines in the status area (set once in initReactor). */ + private volatile int statusHeight; + + // ---- Reactor state ---- + + private volatile int totalProjects; + private volatile int completedProjects; + private volatile Instant buildStartTime; + /** One-line header shown at the top of the status area. */ + private volatile String headerLine; + + // ---- Project display ---- + + private final Map<String, ProjectState> activeProjects = new ConcurrentHashMap<>(); + private final List<String> projectOrder = new ArrayList<>(); + private final Map<String, String> projectNames = new ConcurrentHashMap<>(); + + // ---- Active downloads ---- + + private final Map<String, TransferInfo> activeTransfers = new ConcurrentHashMap<>(); + + // ---- Synchronization ---- + + /** Guards all terminal output and slot mutations. */ + private final Object outputLock = new Object(); + + // ---- Periodic refresh ---- + + /** Scheduler for 1-second display refresh so elapsed timers stay live. */ + private volatile ScheduledExecutorService refreshScheduler; + /** Handle for the periodic refresh task. */ + private volatile ScheduledFuture<?> refreshFuture; + + // ---- Warning tracking ---- + + /** Number of WARN-level messages seen during the build. */ + private final AtomicInteger warningCount = new AtomicInteger(); + + /** Number of ERROR-level messages seen during the build. */ + private final AtomicInteger errorCount = new AtomicInteger(); + + // ---- Constructor ---- + + /** + * Creates a new RichBuildEventListener. + * + * @param terminal the JLine terminal for output + * @param output fallback output consumer (unused — kept for API compat) + */ + public RichBuildEventListener(Terminal terminal, java.util.function.Consumer<String> output) { + this.terminal = terminal; + this.writer = terminal.writer(); + // Support ANSI if terminal type is not "dumb" and has reasonable size + String type = terminal.getType(); + this.supported = type != null && !Terminal.TYPE_DUMB.equals(type) && terminal.getWidth() > 0; + } + + // ---- Reactor lifecycle ---- + + /** + * Initialize reactor state from the session. Called by {@link RichExecutionEventLogger} + * during {@code sessionStarted}. + */ + public void initReactor(MavenSession session) { + List<MavenProject> allProjects = session.getAllProjects(); + List<MavenProject> projects = session.getProjects(); + + this.totalProjects = allProjects.size(); + this.completedProjects = allProjects.size() - projects.size(); + this.buildStartTime = MonotonicClock.now(); + + for (MavenProject project : allProjects) { + projectOrder.add(project.getArtifactId()); + projectNames.put(project.getArtifactId(), project.getName()); + } + + // Build header line + this.headerLine = buildHeaderLine(session); + + // Slot count = degree of concurrency (capped for sanity) + int concurrency = 1; + try { + concurrency = Math.max(1, session.getRequest().getDegreeOfConcurrency()); + } catch (Exception e) { + // fallback to 1 + } + int slotCount = Math.min(concurrency, 8); + // Fixed height: 1 header + N project slots + 1 separator + 1 summary + this.statusHeight = slotCount + 3; + + if (supported) { + setupDisplay(); + } + } + + private String buildHeaderLine(MavenSession session) { + StringBuilder h = new StringBuilder(); + h.append(' ').append(BOLD); + + // Maven version + String mavenVersion = null; + if (session.getSystemProperties() != null) { + mavenVersion = session.getSystemProperties().getProperty("maven.version"); + } + if (mavenVersion != null) { + h.append("Maven ").append(mavenVersion); + } else { + h.append("Maven"); + } + h.append(RESET); + + // Project name + MavenProject top = session.getTopLevelProject(); + if (top != null) { + h.append(DIM).append(" ─ ").append(RESET); + h.append("building "); + h.append(CYAN).append(top.getName()).append(RESET); + if (top.getVersion() != null) { + h.append(' ').append(DIM).append(top.getVersion()).append(RESET); + } + } + + // Goals + List<String> goals = session.getGoals(); + if (goals != null && !goals.isEmpty()) { + h.append(DIM).append(" ─ ").append(RESET); + h.append(YELLOW).append(String.join(" ", goals)).append(RESET); + } + + return h.toString(); + } + + /** + * Set up the JLine Display in non-fullscreen mode. + */ + private void setupDisplay() { + synchronized (outputLock) { + display = new Display(terminal, false); + display.resize(statusHeight, terminal.getWidth()); + displayActive = true; + display.updateAnsi(buildStatusLines(), 0); + } + + // Start a 1-second periodic refresh so that elapsed-time counters + // stay live even when no build events are arriving (e.g. during + // a slow mojo execution with no log output). + ScheduledThreadPoolExecutor executor = new ScheduledThreadPoolExecutor(1, r -> { + Thread t = new Thread(r, "maven-rich-display-refresh"); + t.setDaemon(true); + return t; + }); + executor.setRemoveOnCancelPolicy(true); + refreshScheduler = executor; + refreshFuture = refreshScheduler.scheduleAtFixedRate(this::redraw, 1, 1, TimeUnit.SECONDS); + } + + /** + * Tear down the status display. Called by {@link RichExecutionEventLogger} + * during {@code sessionEnded} before printing the summary. + * <p> + * The flush at the end is critical: {@link Display} writes through + * {@link Terminal#writer()} (a {@code PrintWriter} that does not auto-flush), + * while subsequent log output from SLF4J goes through {@code System.out} + * (which <em>does</em> auto-flush on {@code println}). Without the flush, + * the clear sequences sit in the writer's buffer while the summary text + * reaches the terminal first via {@code System.out} — then the belated + * clear erases the summary the user was supposed to see. + */ + public void tearDown() { + // Stop the periodic refresh first (outside outputLock to avoid deadlock) + if (refreshFuture != null) { + refreshFuture.cancel(false); + refreshFuture = null; + } + if (refreshScheduler != null) { + refreshScheduler.shutdownNow(); + refreshScheduler = null; + } + + synchronized (outputLock) { + if (!displayActive) { + return; + } + displayActive = false; + + // Clear the display area: update with empty lines, cursor at top + display.updateAnsi(Collections.nCopies(statusHeight, ""), 0); + // Erase from cursor to end of screen — removes any leftover artifacts + writer.print("\033[J"); + // Flush immediately so the clear reaches the terminal BEFORE + // any subsequent log output that goes through System.out + writer.flush(); + } + } + + // ---- BuildEventListener interface ---- + + @Override + public void sessionStarted(ExecutionEvent event) { + // Reactor init is handled via initReactor() called from RichExecutionEventLogger + } + + @Override + public void projectStarted(String projectId) { + activeProjects.put(projectId, new ProjectState(projectId, MonotonicClock.now())); + redraw(); + } + + @Override + public void projectFinished(String projectId) { + activeProjects.remove(projectId); + completedProjects++; Review Comment: Race condition: `completedProjects++` is not atomic. The field is `volatile int` (line 103) but `++` is a non-atomic read-increment-write compound operation. In parallel builds (`-T N`), concurrent calls to `projectFinished()` from worker threads can lose increments, causing the progress counter to under-report. The same class already uses `AtomicInteger` for `warningCount` and `errorCount` (lines 133-136). ```suggestion completedProjects.incrementAndGet(); ``` (with `completedProjects` changed to `AtomicInteger` at line 103) ########## impl/maven-cli/src/main/java/org/apache/maven/cling/invoker/mvn/MavenInvoker.java: ########## @@ -343,21 +349,102 @@ protected String determineGlobalChecksumPolicy(MavenContext context) { } protected ExecutionListener determineExecutionListener(MavenContext context) { - ExecutionListener listener = new ExecutionEventLogger(context.invokerRequest.messageBuilderFactory()); + ExecutionListener listener; + String consoleMode = determineConsoleMode(context); + switch (consoleMode) { + case "machine": + BuildEventListener machineBel = determineBuildEventListener(context); + if (machineBel instanceof MachineBuildEventListener machineListener) { + listener = new MachineExecutionEventLogger(machineListener); + } else { + // Fallback if machine listener couldn't be created + listener = new PlainExecutionEventLogger(context.invokerRequest.messageBuilderFactory()); + } + break; + case "rich": + BuildEventListener richBel = determineBuildEventListener(context); + if (richBel instanceof RichBuildEventListener richListener) { + listener = + new RichExecutionEventLogger(context.invokerRequest.messageBuilderFactory(), richListener); + } else { + // Fallback if status bar couldn't be created + listener = new PlainExecutionEventLogger(context.invokerRequest.messageBuilderFactory()); + } + break; + case "plain": + listener = new PlainExecutionEventLogger(context.invokerRequest.messageBuilderFactory()); + break; + default: + listener = new ExecutionEventLogger(context.invokerRequest.messageBuilderFactory()); + break; + } if (context.eventSpyDispatcher != null) { listener = context.eventSpyDispatcher.chainListener(listener); } return new LoggingExecutionListener(listener, determineBuildEventListener(context)); } + @Override + protected BuildEventListener doDetermineBuildEventListener(MavenContext context) { + String consoleMode = determineConsoleMode(context); + if ("machine".equals(consoleMode)) { + return new MachineBuildEventListener(determineWriter(context)); + } + if ("rich".equals(consoleMode) && context.terminal != null) { + return new RichBuildEventListener(context.terminal, determineWriter(context)); + } + return super.doDetermineBuildEventListener(context); + } + + /** + * Resolves the effective console mode from the {@code --console} flag and CI/TTY detection. + * <p> + * Resolution order: + * <ol> + * <li>Explicit {@code --console=plain}, {@code --console=verbose}, {@code --console=rich}, + * or {@code --console=machine} — always honored</li> + * <li>{@code --console=auto} (or unset) — selects mode based on environment: + * <ul> + * <li>CI detected → "plain"</li> + * <li>Interactive TTY → "rich"</li> + * <li>Otherwise → "verbose"</li> + * </ul> + * </li> + * </ol> + */ + String determineConsoleMode(MavenContext context) { Review Comment: Unrecognized `--console` values (including typos like `--console=plian`) silently fall through to auto-detection. This is inconsistent with the existing `--color` option in `LookupInvoker.java` (lines 274-276) which throws `IllegalArgumentException` for invalid values. Adding validation for consistency would help users catch configuration mistakes. -- 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]
