goutamadwant commented on code in PR #13081:
URL: https://github.com/apache/maven/pull/13081#discussion_r4090309188
##########
impl/maven-impl/src/main/java/org/apache/maven/impl/model/DefaultModelBuilder.java:
##########
@@ -2505,21 +2493,115 @@ private DependencyManagement
loadDependencyManagement(Dependency dependency, Col
.build();
}
+ private Model loadImportModel(Dependency dependency, ImportContext
context) {
+ Collection<String> importIds = context.importIds;
+ String groupId = dependency.getGroupId();
+ String artifactId = dependency.getArtifactId();
+ String version = dependency.getVersion();
+ String imported = groupId + ':' + artifactId + ':' + version;
+
+ if (importIds.contains(imported)) {
+ StringBuilder message = new StringBuilder("The import POMs
form a cycle: ");
+ for (String modelId : importIds) {
+ message.append(modelId).append(" -> ");
+ }
+ message.append(imported);
+ if (context.cycleIncludesRelocation(imported)) {
+ context.reportRelocationProblem(message.toString(),
dependency.getLocation(""), null);
+ } else {
+ add(Severity.ERROR, Version.BASE, message.toString());
+ }
+ return null;
+ }
+
+ ImportModelCacheEntry cached =
+ cache(repositories, groupId, artifactId, version, null,
IMPORT, ImportModelCacheEntry::new);
+ ImportedModel importedModel = cached.model;
+ if (importedModel == null) {
+ boolean locked = cached.lock.tryLock();
+ if (!locked && context.relocationSources.isEmpty()) {
+ cached.lock.lock();
+ locked = true;
+ }
+ try {
+ importedModel = cached.model;
+ if (importedModel == null) {
+ // A relocation may lead back to an import being built
by another thread.
+ // Only these paths avoid waiting; ordinary imports
still share one in-flight build.
+ importedModel = doLoadDependencyManagement(dependency,
groupId, artifactId, version, context);
+ if (locked && importedModel != null) {
+ cached.model = importedModel;
+ }
+ }
+ } finally {
+ if (locked) {
+ cached.lock.unlock();
+ }
+ }
+ }
+ if (importedModel == null) {
+ return null;
+ }
+
+ Model importModel = importedModel.model();
+ Relocation relocation = importModel.getDistributionManagement() !=
null
+ ? importModel.getDistributionManagement().getRelocation()
+ : null;
+ if (relocation != null) {
+ if (!validateRelocationCoordinate(relocation.getGroupId(),
"groupId", dependency, context)
+ ||
!validateRelocationCoordinate(relocation.getArtifactId(), "artifactId",
dependency, context)
+ ||
!validateRelocationCoordinate(relocation.getVersion(), "version", dependency,
context)) {
+ return null;
+ }
+ Dependency.Builder relocated =
Dependency.newBuilder(dependency).version(importedModel.version());
+ if (relocation.getGroupId() != null &&
!relocation.getGroupId().isEmpty()) {
+ relocated.groupId(relocation.getGroupId());
+ }
+ if (relocation.getArtifactId() != null
+ && !relocation.getArtifactId().isEmpty()) {
+ relocated.artifactId(relocation.getArtifactId());
+ }
+ if (relocation.getVersion() != null &&
!relocation.getVersion().isEmpty()) {
+ relocated.version(relocation.getVersion());
+ }
+ Dependency relocatedDependency = relocated.build();
+ String message = "The import POM " + imported + " has been
relocated to "
+ + relocatedDependency.getGroupId() + ':' +
relocatedDependency.getArtifactId() + ':'
+ + relocatedDependency.getVersion();
+ if (relocation.getMessage() != null) {
+ message += ": " + relocation.getMessage();
+ }
+ add(Severity.WARNING, Version.BASE, message,
dependency.getLocation(""));
Review Comment:
Addressed. A direct import relocation remains a WARNING at the declaration
location. Later hops and relocations encountered while building another BOM are
DEBUG. The partial-chain regression asserts that only the direct hop warns.
##########
compat/maven-model-builder/src/main/java/org/apache/maven/model/building/DefaultModelBuilder.java:
##########
@@ -1187,136 +1182,227 @@ private void importDependencyManagement(
it.remove();
- String groupId = dependency.getGroupId();
- String artifactId = dependency.getArtifactId();
- String version = dependency.getVersion();
-
- if (groupId == null || groupId.length() <= 0) {
- problems.add(new ModelProblemCollectorRequest(Severity.ERROR,
Version.BASE)
-
.setMessage("'dependencyManagement.dependencies.dependency.groupId' for "
- + dependency.getManagementKey() + " is
missing.")
- .setLocation(dependency.getLocation("")));
- continue;
- }
- if (artifactId == null || artifactId.length() <= 0) {
- problems.add(new ModelProblemCollectorRequest(Severity.ERROR,
Version.BASE)
-
.setMessage("'dependencyManagement.dependencies.dependency.artifactId' for "
- + dependency.getManagementKey() + " is
missing.")
- .setLocation(dependency.getLocation("")));
- continue;
- }
- if (version == null || version.length() <= 0) {
- problems.add(new ModelProblemCollectorRequest(Severity.ERROR,
Version.BASE)
-
.setMessage("'dependencyManagement.dependencies.dependency.version' for "
- + dependency.getManagementKey() + " is
missing.")
- .setLocation(dependency.getLocation("")));
- continue;
+ DependencyManagement importedManagement =
+ loadDependencyManagement(dependency, model, request,
problems, importIds);
+ if (importedManagement != null) {
+ if (importedManagements == null) {
+ importedManagements = new ArrayList<>();
+ }
+ importedManagements.add(importedManagement);
}
+ }
- String imported = groupId + ':' + artifactId + ':' + version;
+ importIds.remove(importing);
- if (importIds.contains(imported)) {
- StringBuilder message =
- new StringBuilder("The dependencies of type=pom and
with scope=import form a cycle: ");
- for (String modelId : importIds) {
- message.append(modelId);
- message.append(" -> ");
- }
- message.append(imported);
- problems.add(
- new ModelProblemCollectorRequest(Severity.ERROR,
Version.BASE).setMessage(message.toString()));
+ dependencyManagementImporter.importManagement(model,
importedManagements, request, problems);
+ }
- continue;
+ private DependencyManagement loadDependencyManagement(
+ Dependency dependency,
+ Model model,
+ ModelBuildingRequest request,
+ DefaultModelProblemCollector problems,
+ Collection<String> importIds) {
+ String groupId = dependency.getGroupId();
+ String artifactId = dependency.getArtifactId();
+ String version = dependency.getVersion();
+
+ if (groupId == null || groupId.length() <= 0) {
+ problems.add(new ModelProblemCollectorRequest(Severity.ERROR,
Version.BASE)
+
.setMessage("'dependencyManagement.dependencies.dependency.groupId' for "
+ + dependency.getManagementKey() + " is missing.")
+ .setLocation(dependency.getLocation("")));
+ return null;
+ }
+ if (artifactId == null || artifactId.length() <= 0) {
+ problems.add(new ModelProblemCollectorRequest(Severity.ERROR,
Version.BASE)
+
.setMessage("'dependencyManagement.dependencies.dependency.artifactId' for "
+ + dependency.getManagementKey() + " is missing.")
+ .setLocation(dependency.getLocation("")));
+ return null;
+ }
+ if (version == null || version.length() <= 0) {
+ problems.add(new ModelProblemCollectorRequest(Severity.ERROR,
Version.BASE)
+
.setMessage("'dependencyManagement.dependencies.dependency.version' for "
+ + dependency.getManagementKey() + " is missing.")
+ .setLocation(dependency.getLocation("")));
+ return null;
+ }
+
+ String imported = groupId + ':' + artifactId + ':' + version;
+
+ if (importIds.contains(imported)) {
+ StringBuilder message = new StringBuilder("The import POMs form a
cycle: ");
+ for (String modelId : importIds) {
+ message.append(modelId);
+ message.append(" -> ");
}
+ message.append(imported);
+ problems.add(new ModelProblemCollectorRequest(Severity.ERROR,
Version.BASE).setMessage(message.toString()));
+
+ return null;
+ }
- DependencyManagement importMgmt =
- getCache(request.getModelCache(), groupId, artifactId,
version, ModelCacheTag.IMPORT);
+ DependencyManagement importedManagement =
+ getCache(request.getModelCache(), groupId, artifactId,
version, ModelCacheTag.IMPORT);
- if (importMgmt == null) {
- if (workspaceResolver == null && modelResolver == null) {
- throw new NullPointerException(String.format(
- "request.workspaceModelResolver and
request.modelResolver cannot be null"
- + " (parent POM %s and POM %s)",
- ModelProblemUtils.toId(groupId, artifactId,
version),
- ModelProblemUtils.toSourceHint(model)));
- }
+ if (importedManagement == null) {
+ Model importModel = resolveImportModel(dependency, model, request,
problems, importIds);
+ if (importModel == null) {
+ return null;
+ }
- Model importModel = null;
- if (workspaceResolver != null) {
- try {
- importModel =
workspaceResolver.resolveEffectiveModel(groupId, artifactId, version);
- } catch (UnresolvableModelException e) {
- problems.add(new
ModelProblemCollectorRequest(Severity.FATAL, Version.BASE)
- .setMessage(e.getMessage())
- .setException(e));
- continue;
- }
+ Relocation relocation = importModel.getDistributionManagement() !=
null
+ ? importModel.getDistributionManagement().getRelocation()
+ : null;
+ if (relocation != null) {
+ Dependency relocated = dependency.clone();
+ if (!validateRelocationCoordinate(relocation.getGroupId(),
"groupId", dependency, problems)
+ ||
!validateRelocationCoordinate(relocation.getArtifactId(), "artifactId",
dependency, problems)
+ ||
!validateRelocationCoordinate(relocation.getVersion(), "version", dependency,
problems)) {
+ return null;
}
-
- // no workspace resolver or workspace resolver returned null
(i.e. model not in workspace)
- if (importModel == null) {
- final ModelSource importSource;
+ if (relocation.getGroupId() != null &&
!relocation.getGroupId().isEmpty()) {
+ relocated.setGroupId(relocation.getGroupId());
+ }
+ if (relocation.getArtifactId() != null
+ && !relocation.getArtifactId().isEmpty()) {
+ relocated.setArtifactId(relocation.getArtifactId());
+ }
+ if (relocation.getVersion() != null &&
!relocation.getVersion().isEmpty()) {
+ relocated.setVersion(relocation.getVersion());
+ }
+ String message = "The import POM " + imported + " has been
relocated to " + relocated.getGroupId() + ':'
+ + relocated.getArtifactId() + ':' +
relocated.getVersion();
+ if (relocation.getMessage() != null) {
+ message += ": " + relocation.getMessage();
+ }
+ problems.add(new
ModelProblemCollectorRequest(Severity.WARNING, Version.BASE)
+ .setMessage(message)
+ .setLocation(dependency.getLocation("")));
Review Comment:
Addressed by removing relocation handling from the deprecated compat
builder, as requested in the scope review. There is no compat relocation
warning path now.
--
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]
