You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by "Tarun4201 (via GitHub)" <gi...@apache.org> on 2026/10/21 07:24:02 UTC
[PR] [MNG-6402] Retain imported POM in DependencyManagement in effective m… [maven]
Tarun4201 opened a new pull request, #13663:
URL: https://github.com/apache/maven/pull/13663
…odel
Preserve import-scoped dependencies in dependencyManagement during model building so that plugins can inspect the imported BOM coordinates and versions.
<!--
Please open pull requests against one of the following branches:
- master — default target for all changes (new features + bug fixes).
If the change should also go to maven-4.0.x, request a backport in the PR.
- maven-3.10.x — target for 3.x changes (new features + bug fixes).
If the change should also go to maven-3.9.x, request a backport in the PR.
maven-3.9.x should receive only serious bug fixes, as it is approaching EOL.
Backports are typically handled by maintainers;
however, after agreement you may open a separate backport PR to the target maintenance branch.
-->
<!-- Add your description here... what, why and how you resolve the issue -->
---
To make clear that you license your contribution under
the [Apache License Version 2.0, January 2004](https://www.apache.org/licenses/LICENSE-2.0)
check one of the following:
- [ ] I hereby declare this contribution to be licensed under the [Apache License Version 2.0, January 2004](https://www.apache.org/licenses/LICENSE-2.0)
- [ ] I hereby declare that I have filed an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-6402] Retain imported POM in DependencyManagement in effective model [maven]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #13663:
URL: https://github.com/apache/maven/pull/13663#issuecomment-6426974280
<!-- license-declaration-check -->
### ⚠️ License declaration missing
The pull request description must contain at least one of the following lines with a **checked** box (`[x]`):
```markdown
- [ ] I hereby declare this contribution to be licensed under the [Apache License Version 2.0, January 2004](https://www.apache.org/licenses/LICENSE-2.0)
- [ ] I hereby declare that I have filed an [Apache Individual Contributor License Agreement](https://www.apache.org/licenses/icla.pdf).
```
This comment will be hidden automatically once the description is fixed.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-6402] Retain imported POM in DependencyManagement in effective model [maven]
Posted by "gnodet (via GitHub) via issues" <is...@maven.apache.org>.
gnodet closed pull request #13663: [MNG-6402] Retain imported POM in DependencyManagement in effective model
URL: https://github.com/apache/maven/pull/13663
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-6402] Retain imported POM in DependencyManagement in effective model [maven]
Posted by "gnodet (via GitHub) via issues" <is...@maven.apache.org>.
gnodet commented on PR #13663:
URL: https://github.com/apache/maven/pull/13663#issuecomment-6662196511
Thanks for the contribution and for digging into this long-standing issue.
After discussion with @cstamas, I believe this approach is not the right direction. The fundamental problem is that `import` scope is not a "real" dependency scope — it is a build-time directive that tells the model builder to expand a BOM's `dependencyManagement` in-place. The effective model is by definition the **resolved** model: keeping `import`-scoped entries in it creates a semantic invariant violation that every consumer of the effective model (resolver, plugins, tooling) would have to guard against.
The use case described — detecting which BOMs were imported and at which version — is real and valid. However the right tools for this are already available:
1. **The raw/file model** still contains the original `import`-scoped entries untouched. Any tool that needs to know which BOMs were imported can read the raw model alongside the effective model. This is exactly the approach taken by [maveniverse/toolbox's `doCollectDmRecursive`](https://github.com/maveniverse/toolbox/blob/main/shared/src/main/java/eu/maveniverse/maven/toolbox/shared/internal/ToolboxResolverImpl.java#L716-L745), which uses [`MavenModelReader`](https://github.com/maveniverse/mima/blob/main/extensions/mmr/src/main/java/eu/maveniverse/maven/mima/extensions/mmr/ModelResponse.java) to get both `getRawModel()` and `getEffectiveModel()`, then walks the lineage to reconstruct BOM provenance without touching the effective model.
2. **Location API** — each managed dependency in the effective model carries location information tracing it back to its source, which can be used to infer BOM origin.
For the release-guard use case specifically: a plugin can call `project.getModel()` (file model, pre-import-expansion) to find the `import`-scoped entries, then validate their versions from there — no changes to Maven core needed.
Closing this PR. I appreciate the effort and hope the alternative approach proves useful.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-6402] Retain imported POM in DependencyManagement in effective m… [maven]
Posted by "gnodet-bot (via GitHub)" <gi...@apache.org>.
gnodet-bot commented on code in PR #13663:
URL: https://github.com/apache/maven/pull/13663#discussion_r4334189889
##########
compat/maven-model-builder/src/test/java/org/apache/maven/model/building/DefaultModelBuilderTest.java:
##########
@@ -291,4 +292,84 @@ public ModelResolver newCopy() {
return this;
}
}
+
+ @Test
+ public void testMng6402ImportedPomRetainedInDependencyManagement() throws Exception {
+ String bomPom = "<project>\n"
+ + " <modelVersion>4.0.0</modelVersion>\n"
+ + " <groupId>thegroup</groupId>\n"
+ + " <artifactId>bom</artifactId>\n"
+ + " <version>1.0.0</version>\n"
+ + " <packaging>pom</packaging>\n"
+ + " <dependencyManagement>\n"
+ + " <dependencies>\n"
+ + " <dependency>\n"
+ + " <groupId>thegroup</groupId>\n"
+ + " <artifactId>managed-dep</artifactId>\n"
+ + " <version>2.0.0</version>\n"
+ + " </dependency>\n"
+ + " </dependencies>\n"
+ + " </dependencyManagement>\n"
+ + "</project>\n";
+
+ String projectPom = "<project>\n"
+ + " <modelVersion>4.0.0</modelVersion>\n"
+ + " <groupId>thegroup</groupId>\n"
+ + " <artifactId>project</artifactId>\n"
+ + " <version>1.0.0</version>\n"
+ + " <dependencyManagement>\n"
+ + " <dependencies>\n"
+ + " <dependency>\n"
+ + " <groupId>thegroup</groupId>\n"
+ + " <artifactId>bom</artifactId>\n"
+ + " <version>1.0.0</version>\n"
+ + " <type>pom</type>\n"
+ + " <scope>import</scope>\n"
+ + " </dependency>\n"
+ + " </dependencies>\n"
+ + " </dependencyManagement>\n"
+ + "</project>\n";
+
+ ModelBuilder builder = new DefaultModelBuilderFactory().newInstance();
+ DefaultModelBuildingRequest request = new DefaultModelBuildingRequest();
+ request.setModelSource(new StringModelSource(projectPom));
+ request.setModelResolver(new BaseModelResolver() {
+ @Override
+ public ModelSource resolveModel(String groupId, String artifactId, String version)
+ throws UnresolvableModelException {
+ if ("thegroup".equals(groupId) && "bom".equals(artifactId) && "1.0.0".equals(version)) {
+ return new StringModelSource(bomPom);
+ }
+ return super.resolveModel(groupId, artifactId, version);
+ }
+
+ @Override
+ public ModelSource resolveModel(Dependency dependency) throws UnresolvableModelException {
+ return resolveModel(dependency.getGroupId(), dependency.getArtifactId(), dependency.getVersion());
+ }
+ });
+
+ ModelBuildingResult result = builder.build(request);
+ Model effectiveModel = result.getEffectiveModel();
+ assertNotNull(effectiveModel.getDependencyManagement());
+ List<Dependency> dependencies = effectiveModel.getDependencyManagement().getDependencies();
+
+ // Verify the managed dependency declared inside the BOM is present
+ Dependency managedDep = dependencies.stream()
+ .filter(d -> "managed-dep".equals(d.getArtifactId()))
+ .findFirst()
+ .orElse(null);
+ assertNotNull(managedDep, "Managed dependency from imported BOM must be present");
+ assertEquals("2.0.0", managedDep.getVersion());
+
+ // Verify the imported BOM dependency itself is retained in dependencyManagement (MNG-6402)
+ Dependency bomDep = dependencies.stream()
+ .filter(d -> "bom".equals(d.getArtifactId()))
+ .findFirst()
+ .orElse(null);
+ assertNotNull(bomDep, "The imported BOM dependency itself must be retained in dependencyManagement");
+ assertEquals("1.0.0", bomDep.getVersion());
+ assertEquals("pom", bomDep.getType());
+ assertEquals("import", bomDep.getScope());
+ }
}
Review Comment:
💡 This test verifies the BOM entry is *retained* — which is the new compat behaviour. If the compat change is reverted (as suggested above), this test will fail. Please remove it together with the compat revert.
##########
compat/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java:
##########
@@ -1235,15 +1234,11 @@ private void importDependencyManagement(
List<DependencyManagement> importMgmts = null;
- for (Iterator<Dependency> it = depMgmt.getDependencies().iterator(); it.hasNext(); ) {
- Dependency dependency = it.next();
-
+ for (Dependency dependency : depMgmt.getDependencies()) {
if (!"pom".equals(dependency.getType()) || !"import".equals(dependency.getScope())) {
continue;
}
Review Comment:
⚠️ **Compat rule violation:** The `compat/` module must mirror Maven 3.9.x exactly. Maven 3.9.x still calls `it.remove()` here — removing the import-scope entry from the live list after processing. Dropping `it.remove()` in compat without a corresponding 3.9.x change introduces a behaviour divergence. Please revert this hunk; the fix belongs only in `impl/`.
##########
impl/maven-impl/src/test/java/org/apache/maven/impl/model/DefaultModelBuilderTest.java:
##########
@@ -1883,6 +1891,40 @@ private static DefaultProfileActivationContext newProfileActivationContext(
null, null, activeIds, inactiveIds, Map.of(), Map.of(), Model.newInstance());
}
+ @Test
Review Comment:
🔍 Minor: the test method is named `testMng6402…` but loads fixtures from `poms/factory/mng-8450/`. Consider adding dedicated `mng-6402` fixtures (or renaming the reference) so that the test name and resource path align — makes future refactoring easier.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [MNG-6402] Retain imported POM in DependencyManagement in effective model [maven]
Posted by "Tarun4201 (via GitHub)" <gi...@apache.org>.
Tarun4201 commented on PR #13663:
URL: https://github.com/apache/maven/pull/13663#issuecomment-6422280180
Thanks for the review! I have addressed all the feedback in the latest commit:
Reverted compat/ changes: Reverted DefaultModelBuilder.java and removed the accompanying test in compat/ so that the module continues to strictly mirror Maven 3.9.x behavior without divergence.
Dedicated test fixtures: Added dedicated mng-6402 fixtures under poms/factory/mng-6402/ and updated testMng6402ImportedPomRetainedInDependencyManagement in maven-impl to use them.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org