fridrich commented on PR #1189: URL: https://github.com/apache/maven-plugin-tools/pull/1189#issuecomment-5743234249
> Read through the module. The problem it solves (a distro cannot build maven-plugin-plugin without a prior maven-plugin-plugin) is real, but this shape is hard to keep alive: it injects into four private fields of `JavaAnnotationsMojoDescriptorExtractor` by reflection, so any rename breaks it at runtime and only under `-Pbootstrap`, which CI does not run; it re-implements POM parent walking, property interpolation and dependency resolution with placeholder `dummy.jar` files; and it compiles all of `maven-plugin-plugin` a second time through `compileSourceRoots`. Before this can be considered I would want: a CI job that runs the profile, package-private constructors instead of reflection, and ideally a much smaller approach that boots the real mojo through a minimal Sisu container instead of duplicating its wiring. Converting to draft until then. So, this fix addresses your comments: 1) *CI job for -Pbootstrap*: I added a bootstrap job in maven-verify.yml. It runs mvn -B -Pbootstrap verify on every push/PR, so now we test this profile. 2) *Reflection into private fields*: Removed completely, not just replaced with a constructor. `StandaloneDescriptorGenerator` does not create `JavaAnnotationsMojoDescriptorExtractor` manually anymore. Now it starts a small `DefaultPlexusContainer` and calls `container.lookup(MojoScanner.class)`. This way Sisu connects the extractor like a normal Maven build (real @Inject fields, no reflection, no manual setup). 3) Small Sisu container instead of fake code: The same container lookup also replaces the custom fake `RepositorySystem` (which was a dynamic proxy that only worked for `newLocalRepositoryManager`) and the custom `ArchiverManager` implementation. Both now come from the real container. What I did not change on purpose: the local, offline parent POM reader (property interpolation and fake-jar placeholders) that creates the descriptor for `maven-plugin-plugin-bootstrap`. I tried to change it to use real `ProjectBuilder`, but that tries to download and resolve the whole parent POM chain (like junit-bom, which we don't need here). It fails when local repo is empty because our manual container doesn't have the HTTP connector configured like Maven CLI. So I reverted this change. This module's descriptor only needs to work enough for the plugin to run; nobody else uses it. The real descriptor for `maven-plugin-plugin` is generated later by normal Maven process after `maven-plugin-plugin-bootstrap` is ready. I checked it, and it has the correct description, correct `requiredJavaVersion`/`requiredMavenVersion`, and correct dependencies. The extra `compileSourceRoot` for `maven-plugin-plugin` sources (compiling them twice) is still there. This is needed because the bootstrap module makes a copy of the mojo classes to avoid circular dependency, and I couldn't find a smaller way to fix it. I tested with empty local repo (-Dmaven.repo.local): full build passes, and it doesn't download any artifacts from this project (maven-plugin-tools-*, maven-plugin-plugin*). -- 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]
