Hiteshsai007 commented on PR #12745: URL: https://github.com/apache/maven/pull/12745#issuecomment-5314194613
Thanks again for the re-review, @gnodet! I've pushed a new commit addressing all the remaining feedback points: ### 1. No tests (Fixed) Added comprehensive unit tests covering the new behavior: - Added `testConsumerPomRetainsCompileApiRuntimeDeps` and `testConsumerPomStripsProvidedTestSystemDeps` in `ConsumerPomBuilderTest` to verify that `compile`, `api`, and `runtime` scopes are correctly retained in consumer POMs while `provided`, `test`, and `system` are stripped. - Added `testApiScopeDependencyRequires410`, `testApiScopeInDependencyManagementRequires410`, and `testCompileScopeDependencyRemains400` in `MavenModelVersionTest` to ensure that model validation and feature detection accurately force `4.1.0` when the `api` scope is present. ### 2. Per-dependency MavenModelVersion instantiation (Fixed) Refactored `DefaultArtifactDescriptorReader.populateResult()` to compute the model version check *once* using `model.getModelVersion()`. It now stores the `remapCompileToApi` boolean upfront and passes it down into the `convert()` method, completely avoiding the $O(N)$ object allocations and redundant model traversals. ### 3. Feature detection vs declared version (Fixed) As part of the fix above, `DefaultArtifactDescriptorReader` now uses the explicit `model.getModelVersion()` declaration to determine if a downgrade happened, rather than relying on feature detection. This ensures that if a developer explicitly opts into `4.1.0`, their `compile` non-transitive intent is fully respected during resolution. ### 4. Compile-non-transitive is itself a 4.1.0 feature (Documented) Good catch on the design limitation! Since we cannot force `4.1.0` on every POM that has a `compile` dependency without breaking the ecosystem, I have explicitly documented this behavior in the Javadoc for `DependencyScope.COMPILE`. It now contains a warning stating that if a project relies on non-transitive compile semantics but doesn't otherwise opt-in to `4.1.0` (e.g. via `preserveModelVersion=true`), the consumer POM may downgrade to `4.0.0` and downstream resolvers will remap the scope to `api` (transitive) for backward compatibility. ### 5. Formatting (Fixed) Added the missing blank line in `model-version.vm`. -- 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]
