slachiewicz opened a new pull request, #12705:
URL: https://github.com/apache/maven/pull/12705

   Fixes #12704.
   
   ### 1. The event
   
   `BuildPlanExecutor`'s `TEARDOWN` decided between `ProjectSucceeded` and 
`ProjectSkipped` on `allStepsExecuted`, which required *every* step of the 
project's plan to have reached `EXECUTED`. The plan holds a step for every 
phase of the lifecycle regardless of the requested tasks, so for `mvn compile` 
a project keeps empty `package`, `install` and `deploy` steps that normally run 
as no-ops. As soon as the reactor is halted those get skipped, 
`allStepsExecuted` turns false, and a project that had already finished 
everything asked of it is reported as skipped.
   
   The outcome is now based on the steps that actually carry mojo executions. A 
project that ran all of its work is reported as succeeded; one that still had 
mojo-bearing steps pending stays skipped. A project whose `SETUP` step never 
ran was never started at all, so it stays skipped too — this keeps aggregators 
and other modules with no work of their own from being reported as succeeded 
when they were genuinely never built.
   
   `BuildStep.hasExecutions()` is added for that predicate. (`BuildStep.skip()` 
looks like it was meant for this, but it has no callers, so out-of-scope phases 
are distinguishable only by having no mojos.)
   
   ### 2. The message
   
   `ExecutionEventLogger.projectSkipped` always printed:
   
   ```
   X was not built because a module it depends on failed to build.
   ```
   
   That is the only reason the legacy builder ever produced, because 
`LifecycleModuleBuilder` fires `ProjectSkipped` before `ProjectStarted` and 
only for projects that never started. The concurrent builder also fires it for 
projects stopped by an unrelated failure, where the sentence is simply untrue. 
The logger now checks whether any upstream project actually failed and says 
which of the two happened; when the answer cannot be established the existing 
wording is kept.
   
   ### Before / after
   
   Two sibling modules with no dependency between them, plus one that does 
depend on the failing module. `slow` has 4000 generated sources so it is still 
compiling when `fails` breaks:
   
   ```
   $ mvn compile -b concurrent -T5
   ```
   
   before:
   
   ```
   [INFO] Skipping dependent
   [INFO] dependent was not built because a module it depends on failed to 
build.
   [INFO] Skipping slow
   [INFO] slow was not built because a module it depends on failed to build.
   ```
   
   after:
   
   ```
   [INFO] Skipping dependent
   [INFO] dependent was not built because a module it depends on failed to 
build.
   ```
   
   `slow` compiles its 4001 sources, writes its 4000 class files and is 
reported as succeeded; `dependent` is still skipped, with the reason that is 
true for it. Reproduced 5 runs out of 5 in both directions.
   
   ### Testing
   
   - Two new cases in `ExecutionEventLoggerTest` covering both skip reasons.
   - `MavenITmng8648ProjectEventsTest` passes against the patched distribution 
— `subproject-a` and `subproject-b` succeed, `subproject-c` fails, 
`subproject-d` is skipped and keeps the dependency message.
   - Unit tests of `impl/maven-core` and `impl/maven-cli` are green (635 tests).
   
   ### Not addressed here
   
   `MavenITmng8648ProjectEventsTest` remains timing-sensitive: if 
`subproject-c` fails before `subproject-b`'s steps are scheduled at all, `b` is 
legitimately skipped and the assertion on `ProjectSucceeded` still fails. That 
needs an ordering guarantee in the test rather than a change in core, so I left 
it out of this PR. It is the flake described in #12704.
   


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