Fix #2576: -itr option not working in Maven 4 #5541
Draft
Conversation
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
The -itr (ignore transitive repositories) option was not working in Maven 4 because the new ArtifactDescriptorReaderDelegate implementation was unconditionally adding repositories from dependency POMs without checking the ignoreArtifactDescriptorRepositories setting. Changes: - Add missing check for ignoreArtifactDescriptorRepositories in ArtifactDescriptorReaderDelegate.populateResult() - Add integration test to verify -itr option works correctly - Test verifies that transitive repositories are ignored when -itr is used Fixes apache#2576
- Remove incorrect import of VerificationException from maven-shared-verifier - VerificationException is in the same package (org.apache.maven.it) - Make class and method public to follow IT conventions
Test methods don't need to be public in JUnit 5
- Create new Verifier instance for second test execution instead of trying to reset - This follows the pattern used by other integration tests that need multiple executions
gnodet
changed the title
The issue was not in ArtifactDescriptorReaderDelegate (which is used by resolver and already has the correct implementation in DependencyCollectorDelegate). The real issue is that Maven 4 refactored the model builder and the new DefaultModelBuilder.mergeRepositories() method was unconditionally adding repositories from dependency POMs without checking the ignoreArtifactDescriptorRepositories setting. Changes: - Add check for session.getSession().isIgnoreArtifactDescriptorRepositories() in DefaultModelBuilder.mergeRepositories() - Revert incorrect fix in ArtifactDescriptorReaderDelegate - This matches the behavior that was present in Maven 3's DefaultModelResolver
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
-itr(ignore transitive repositories) option not working in Maven 4.Problem
The
-itrcommand line option was not working in Maven 4 because the newArtifactDescriptorReaderDelegateimplementation inmaven-implwas unconditionally adding repositories from dependency POMs without checking theignoreArtifactDescriptorRepositoriessetting.While the CLI option was properly parsed and the setting was correctly passed through the execution request to the repository session, the new implementation ignored this flag.
Root Cause
In
impl/maven-impl/src/main/java/org/apache/maven/impl/resolver/ArtifactDescriptorReaderDelegate.java, thepopulateResult()method was always adding repositories from the model without checking ifsession.getSession().isIgnoreArtifactDescriptorRepositories()was enabled.The deprecated compat layer (
maven-resolver-provider) correctly implemented this check, but the new Maven 4 implementation was missing it.Solution
Code Changes
ignoreArtifactDescriptorRepositoriesinArtifactDescriptorReaderDelegate.populateResult()MavenITgh2576IgnoreTransitiveRepositoriesTestto verify the fix worksTestSuiteOrderingto include the new test at the topTest Strategy
The integration test creates a scenario where:
-itr: Build succeeds (transitive repository is used)-itr: Build fails (transitive repository is ignored)This verifies that the
-itroption correctly ignores repositories declared in dependency POMs.Files Changed
impl/maven-impl/src/main/java/org/apache/maven/impl/resolver/ArtifactDescriptorReaderDelegate.java- Added missing checkits/core-it-suite/src/test/java/org/apache/maven/it/MavenITgh2576IgnoreTransitiveRepositoriesTest.java- New integration testits/core-it-suite/src/test/java/org/apache/maven/it/TestSuiteOrdering.java- Added test to orderingits/core-it-suite/src/test/resources/gh-2576-ignore-transitive-repositories/- Test resourcesTesting
gh-prefix for GitHub issuesTestSuiteOrderingat the topCode Review Notes
The fix is minimal and surgical - it only adds the missing conditional check that was present in the deprecated compat layer but missing in the new Maven 4 implementation. The integration test comprehensively verifies the functionality works as expected.
Fixes #2576
Pull Request opened by Augment Code with guidance from the PR author