gnodet commented on issue #10851:
URL: https://github.com/apache/maven/issues/10851#issuecomment-6654015990

   ## Analysis
   
   The bug is real: running `mvn install -Dmaven.test.skip` fails when a module 
depends on a `test-jar` from a sibling module, because Maven attempts to 
resolve the `test-jar` from the remote repository even though it was never 
installed (the `jar:test-jar` goal was skipped).
   
   ### Root cause
   
   The resolution is triggered by `LifecycleDependencyResolver`, which runs 
before each mojo to populate `MavenProject.getArtifacts()` — the legacy Maven 3 
artifact API. When any plugin in the lifecycle declares 
`@requiresDependencyResolution test` (e.g. `maven-compiler-plugin`), Maven 
resolves *all* test-scoped dependencies, including `test-jar` artifacts. At 
this point in the build, the sibling `test-jar` has not been produced (because 
`jar:test-jar` was skipped), and it doesn't exist in the remote repository 
either, so resolution fails.
   
   The `ReactorReader` has logic to serve reactor artifacts from the project 
local repository, but a `test-jar` that was never built is simply absent.
   
   ### Why PR #13774 is not the right fix
   
   PR #13774 addresses the symptom by filtering out `test-jar` artifacts inside 
`LifecycleDependencyResolver` and `DefaultProjectDependenciesResolver` when 
`maven.test.skip` is detected. This approach has several fundamental problems:
   
   **1. Hard-coded heuristic in a generic resolver**
   
   `DefaultProjectDependenciesResolver` is a general-purpose service — plugins 
can call it directly to build any dependency graph for any purpose (reporting, 
analysis, etc.). Silently filtering nodes based on a build property breaks 
those use cases. The resolver should not know about `maven.test.skip`.
   
   **2. Cache poisoning / cache key mismatch**
   
   The `ProjectArtifactsCache` key is based on `{groupId, artifactId, version, 
dependencyArtifacts, workspace, localRepo, repos, scopesToCollect, 
scopesToResolve, aggregating}`. The `maven.test.skip` flag is not part of the 
key. If the resolution runs once without the flag and once with it in the same 
session (or across sessions sharing a workspace), the cached result will be 
incorrect.
   
   **3. `-DskipTests` is not handled**
   
   `-DskipTests` only skips test *execution*, not compilation — `test-jar` 
artifacts may still be needed. `-Dmaven.test.skip` skips both compilation and 
execution. The PR only handles `maven.test.skip`. Any other plugin-specific 
skip flag (`skipTests`, `skipITs`, custom flags) is ignored. This is an 
inherently incomplete enumeration that cannot be maintained in Maven core.
   
   **4. Maven 4 lazy dependency injection**
   
   In Maven 4, plugins using `@Resolution` get their dependencies injected 
directly in `DefaultMavenPluginManager.configureMojo()`, bypassing 
`LifecycleDependencyResolver` entirely. The injected `DependencyResolverResult` 
is computed eagerly at injection time — before the mojo has a chance to check 
its own skip flags. Intercepting this at the resolver level is the wrong 
abstraction boundary.
   
   ### The real solution
   
   The correct fix belongs at the **plugin level**, not in Maven core:
   
   - **`maven-jar-plugin`**: when `maven.test.skip=true`, the `test-jar` goal 
is skipped but should still install a placeholder (or attach the artifact with 
a marker) so the `ReactorReader` can serve it. This is similar to how the main 
jar gets served from `target/classes` before packaging.
   - **`maven-compiler-plugin`**: when skipping test compilation, it should not 
declare `requiresDependencyResolution=test`, or should explicitly not request 
test-scoped resolution.
   
   ### Maven 4: a better path forward
   
   For Maven 4 plugins, the right design is to make `@Resolution` lazy. Instead 
of resolving eagerly in `configureMojo()`, Maven could inject a **lazy proxy** 
(JDK dynamic proxy for `DependencyResolverResult`, which is an interface) that 
defers the actual `DependencyResolver.resolve()` call until the mojo first 
accesses the result. This would allow a plugin to:
   
   ```java
   @Resolution(pathScope = "test")
   private DependencyResolverResult testDependencies;
   
   public void execute() {
       if (skipTests) return; // testDependencies.getNodes() never called → 
resolution never triggered
       testDependencies.getNodes().forEach(...);
   }
   ```
   
   This is a clean, correct, and extensible solution — the plugin owns the skip 
decision, Maven core stays generic. It would require a small addition to the 
`@Resolution` annotation (e.g. a `lazy = true` attribute) and a corresponding 
change in `DefaultMavenPluginManager.configureMojo()` to wrap the resolver call 
in a proxy.
   
   For Maven 3 / legacy mojos, the problem is structural and the best 
mitigation remains ensuring that reactor `test-jar` artifacts are always 
servable by the `ReactorReader` regardless of skip flags.


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