gnodet-bot commented on PR #13081: URL: https://github.com/apache/maven/pull/13081#issuecomment-5601623386
Good observations @renechoi. Two distinct points here: **1. Relationship with `MavenArtifactRelocationSource` / `DefaultArtifactDescriptorReader.loadPom`** These operate at different layers: - **`DefaultArtifactDescriptorReader.loadPom`** (resolver layer) — handles relocations during **dependency resolution**, when the resolver reads an artifact descriptor. It walks relocations via `MavenArtifactRelocationSource` (which includes `DistributionManagementArtifactRelocationSource` and `UserPropertiesArtifactRelocationSource`). This path is used when resolving regular dependencies. - **`DefaultModelBuilder.importDependencyManagement`** (model builder layer) — handles BOM imports (`<scope>import</scope>`) during **model building**. This path resolves the imported POM directly via `ModelResolver`, builds its effective model, and extracts `<dependencyManagement>`. It never goes through `DefaultArtifactDescriptorReader`, so the relocation SPI is never consulted. Before this PR, if a BOM declared `<distributionManagement><relocation>`, the model builder would silently ignore it and import nothing (or the wrong thing). This PR adds relocation-following specifically to the model builder's import path. Regarding `maven.relocations.entries` (user-properties-based relocations): correct, those cannot currently reach an imported BOM through this path. That's by design for this fix — the scope is limited to `distributionManagement` relocations, which is the mechanism actually used in published POMs. Extending user-properties relocations to BOM imports would be a separate enhancement. **2. Error message wording for relocation-only cycles** Valid point. When a cycle involves only relocation hops (no actual `scope=import` dependencies form the cycle), the message "The dependencies of type=pom and with scope=import form a cycle" is misleading. The PR does distinguish between relocation cycles and import cycles internally (see `cycleIncludesRelocation` in the Maven 4 model builder), but the error message template is reused. This could be improved with a more specific message like "The import POM relocation chain forms a cycle" when the cycle includes a relocation hop. _This review was generated by an AI agent, Hermès on behalf of @gnodet._ -- 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]
