fix: preserve processor path types across conflict resolution#11805
Open
mariuszs wants to merge 5 commits intoapache:masterfrom
Open
fix: preserve processor path types across conflict resolution#11805mariuszs wants to merge 5 commits intoapache:masterfrom
mariuszs wants to merge 5 commits intoapache:masterfrom
Conversation
Instead to "smear" this feature across Maven and Resolver classes, for start let's keep it "confined" with single class: the TypeDeriver. Later we can see where to go further with it. Supersedes apache#11373
When a dependency is both a direct dep (e.g., modular-jar) and a transitive dep of a processor, ConflictResolver removes the transitive occurrence before TypeDeriver can assign processor path types. This causes the artifact to appear only on --module-path, not on --processor-module-path (maven-compiler-plugin#1039). Fix: two-phase approach without changing maven-resolver: - TypeCollector runs BEFORE ConflictResolver, records which artifacts are transitive deps of processors in the transformation context - TypeDeriver (after ConflictResolver) reads the context and sets PROCESSOR_TYPE property on surviving nodes that need both paths - DefaultDependencyResolverResult reads PROCESSOR_TYPE and adds the artifact to processor paths as well TypeCollector short-circuits when no processor-type direct deps exist, so the vast majority of builds pay zero cost.
Member
|
Sorry for this, I rebased my PR without thinking and this one is now full of conflicts. |
Contributor
|
Hello @mariuszs . Would it be possible to rebase this pull request? |
Resolve conflicts after PR apache#11380 was merged to master. Our TypeCollector/ProcessorTypeMerger additions preserved on top.
- Replace full-tree DependencyVisitor with simple 2-level loop (only direct processor deps and their children matter) - Replace derive() method with static DERIVE_MAP lookup - Extract PROCESSOR_TYPE_IDS constant, reuse in ProcessorTypeMerger - Lift processorPathTypesFor() sets to static finals - Remove System.out.println from test - Remove unused DependencyGraphDumper import from test
Author
Sure, rebased on current master. |
Open
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes the issue described in maven-compiler-plugin#1042 (comment): when a dependency is both a direct dep (e.g.,
modular-jar) and a transitive dep of a processor,ConflictResolverremoves the transitive occurrence beforeTypeDerivercan assign processor path types. The artifact ends up only on--module-path, missing from--processor-module-path.This builds on PR #11380 which introduced
TypeDeriver.Root cause
ConflictMarkerkeys ongroupId:artifactId:extension:classifier. Sincemodular-jarandmodular-processorboth haveextension=jar, classifier="", they share the same conflict key.ConflictResolver(which runs beforeTypeDeriver) picks one winner and removes the loser entirely (verbosity=NONE).TypeDeriverthen never sees the transitive processor occurrence.Fix: two-phase approach (no maven-resolver changes needed)
TypeCollector{GA:ext:cls → derived processor type}in contextProcessorTypeMerger(inTypeDeriver)PROCESSOR_TYPEproperty on surviving nodes that need both pathsDefaultDependencyResolverResultPROCESSOR_TYPE, adds artifact to processor paths tooTypeCollectorshort-circuits when no processor-type direct deps exist, so builds without processors pay zero cost.Changes
TypeCollectortransformer (impl + compat)PROCESSOR_TYPEartifact property inMavenArtifactPropertiesTypeDeriverenhanced withProcessorTypeMergerinner visitorDefaultDependencyResolverResult.addDependency()handlesPROCESSOR_TYPEMavenSessionBuilderSupplierwiresTypeCollectorbeforeConflictResolverTest plan
TypeDeriverWithConflictResolutionTest.sharedDependencyLosesProcessorType— reproduces the exact scenario and verifies the fixtypeDeriverAloneWorksCorrectly— proves TypeDeriver logic is correct in isolationTypeDeriverTest(2 tests) still passmodular-processor-shared-dep) — 3-module project with shared-lib as bothmodular-jardirect dep and transitive dep ofmodular-processor, verifiesjavaccompiles successfullyRelated
FindExceptionwith modular processors)TypeDeriver(first step, but doesn't handle the conflict resolution case)