Conversation
|
One format worth considering for the machine-readable console mode: logfmt. It sits nicely between plain text (human-readable) and JSON lines (fully structured): parseable by Loki, Vector, Grafana out of the box, but still legible with a plain The interesting property: with Could be offered as a |
7eaa538 to
869e497
Compare
gnodet-bot
left a comment
There was a problem hiding this comment.
Console modes review — overall this is solid. Three specific issues worth addressing before merge.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
| MavenProject project = event.getProject(); | ||
| MojoExecution mojo = event.getMojoExecution(); | ||
|
|
||
| String mojoKey = project.getArtifactId() + ":" + mojo.getExecutionId() + ":" + mojo.getGoal(); |
There was a problem hiding this comment.
✖️ Bug: missing — silently collides in multi-module builds
The key is not unique when two modules in the same reactor share the same (different s). in overwrites the prior entry; the subsequent in / returns or the wrong start time — producing zero or negative durations in the JSON output with no error.
correctly includes :
Same fix needed at lines 211 and 293.
| } else { | ||
| logger.info(buffer.toString()); | ||
| } | ||
| logger.info(buffer.toString()); |
There was a problem hiding this comment.
✖️ Behavioral regression: downgraded from ERROR to INFO level
The original used for the status line, producing . This PR changes it to , producing .
The ANSI coloring () still makes the text visually red in terminals, but any tool that scans the log stream by level — CI scripts using , Maven wrappers, IDE build output parsers, test frameworks — will silently miss the failure signal. The level and the color serve different consumers.
|
|
||
| // Reactor state | ||
| private volatile int totalProjects; | ||
| private volatile int currentVisitedProjectCount; |
There was a problem hiding this comment.
nit: with increment — use
is declared but incremented inside at line 155. provides visibility but not atomicity for compound read-modify-write; the block is what makes the increment correct. The modifier is redundant and misleading — it implies the field can be safely read-and-acted-upon outside the lock.
already uses for the same pattern. has the same issue (line 83):
Then in : , and in : — no block needed.
Summary
Part 3 of the logging feature chain. Depends on #12695 (build report).
Adds the
--consoleCLI flag with four output modes:plain— compact one-line-per-module output, ideal for CI (auto-selected in CI environments)rich— JLine status bar with live reactor progress (auto-selected on interactive TTYs)verbose— full mojo-level output, current Maven 4.0 default behaviormachine— JSON lines: one typed JSON object per lifecycle event, designed for piping to external toolsAuto-detection (
--console=auto, the default): CI → plain, interactive TTY → rich, otherwise → verbose.Files changed (16 files, ~4000 insertions)
Options.java(+console())PlainExecutionEventLogger,RichBuildEventListener,RichExecutionEventLogger,MachineBuildEventListener,MachineExecutionEventLoggerCommonsCliOptions,LayeredOptions,LookupInvoker(preliminary interactive detection),MavenInvoker(console mode switch + transfer listener)ExecutionEventLogger(version info on failure)PR chain
mvnlogviewerTest plan
mvn test -pl impl/maven-cli— all 692 tests passmvn test -pl impl/maven-core— all tests pass🤖 Generated with Claude Code