Skip to content

Commit

Permalink
[MNG-8294] Consistency checks when loading parent
Browse files Browse the repository at this point in the history
  • Loading branch information
@gnodet
gnodet committed Oct 7, 2024
1 parent 954eae7 commit 9dd48c1
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 67 deletions.
18 changes: 7 additions & 11 deletions api/maven-api-model/src/main/mdo/maven.mdo
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1744,20 +1744,16 @@
<name>relativePath</name>
<version>4.0.0+</version>
<description>
The relative path of the parent {@code pom.xml} file within the checkout.
If not specified, it defaults to {@code ../pom.xml}.
The relative path of the parent subproject POM file or directory within the checkout.
If not specified, it defaults to {@code ..}, i.e. the parent directory.
Maven looks for the parent POM first in this location on
the filesystem, then the local repository, and lastly in the remote repo.
{@code relativePath} allows you to select a different location,
for example when your structure is flat, or deeper without an intermediate parent POM.
However, the group ID, artifact ID and version are still required,
and must match the file in the location given, or it will revert to the repository for the POM.
This feature is only for enhancing the development in a local checkout of that project.
Set the value to an empty string in case you want to disable the feature and always resolve
the parent POM from the repositories.
the filesystem if explicitly provided, then in the reactor if groupId and artifactId are provided,
then in the default parent directory, then the local repository, and lastly in the remote repo.
However, if the both relative path and the group ID / artifact ID are provided,
they must match the file in the location given.
Specify either the {@code relativePath} or the {@code groupId}/{@code artifactId}, not both.
</description>
<type>String</type>
<defaultValue>..</defaultValue>
</field>
</fields>
<codeSegments>
Expand Down
128 changes: 72 additions & 56 deletions maven-api-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultModelBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -855,10 +855,7 @@ Model readParent(Model childModel) throws ModelBuilderException {

Parent parent = childModel.getParent();
if (parent != null) {
parentModel = readParentLocally(childModel);
if (parentModel == null) {
parentModel = resolveAndReadParentExternally(childModel);
}
parentModel = resolveParent(childModel);

if (!"pom".equals(parentModel.getPackaging())) {
add(
Expand All @@ -883,43 +880,57 @@ Model readParent(Model childModel) throws ModelBuilderException {
return parentModel;
}

private Model resolveParent(Model childModel) {
Model parentModel = null;
if (request.getRequestType() == ModelBuilderRequest.RequestType.BUILD_POM) {
parentModel = readParentLocally(childModel);
}
if (parentModel == null) {
parentModel = resolveAndReadParentExternally(childModel);
}
return parentModel;
}

private Model readParentLocally(Model childModel) throws ModelBuilderException {
ModelSource candidateSource = getParentPomFile(childModel, request.getSource());
ModelSource candidateSource = null;

Parent parent = childModel.getParent();
String parentPath = parent.getRelativePath();
if (parentPath != null && !parentPath.isEmpty()) {
candidateSource = request.getSource().resolve(modelProcessor::locateExistingPom, parentPath);
if (candidateSource == null) {
wrongParentRelativePath(childModel, parentPath, parent);
return null;
}
}
if (candidateSource == null) {
candidateSource = resolveReactorModel(parent.getGroupId(), parent.getArtifactId(), parent.getVersion());
}
if (candidateSource == null) {
candidateSource = request.getSource().resolve(modelProcessor::locateExistingPom, "..");
}

if (candidateSource == null) {
return null;
}

Model candidateModel = derive(candidateSource).readAsParentModel();

//
// TODO jvz Why isn't all this checking the job of the duty of the workspace resolver, we know that we
// have a model that is suitable, yet more checks are done here and the one for the version is problematic
// before because with parents as ranges it will never work in this scenario.
//

String groupId = getGroupId(candidateModel);
String artifactId = candidateModel.getArtifactId();
String version = getVersion(candidateModel);

Parent parent = childModel.getParent();
if (groupId == null
|| !groupId.equals(parent.getGroupId())
|| artifactId == null
|| !artifactId.equals(parent.getArtifactId())) {
StringBuilder buffer = new StringBuilder(256);
buffer.append("'parent.relativePath'");
if (childModel != getRootModel()) {
buffer.append(" of POM ").append(ModelProblemUtils.toSourceHint(childModel));
}
buffer.append(" points at ").append(groupId).append(':').append(artifactId);
buffer.append(" instead of ").append(parent.getGroupId()).append(':');
buffer.append(parent.getArtifactId()).append(", please verify your project structure");

setSource(childModel);
add(Severity.WARNING, Version.BASE, buffer.toString(), parent.getLocation(""));
// Ensure that relative path and GA match, if both are provided
if (parentPath != null
&& !parentPath.isEmpty()
&& (groupId == null
|| !groupId.equals(parent.getGroupId())
|| artifactId == null
|| !artifactId.equals(parent.getArtifactId()))) {
mismatchRelativePathAndGA(childModel, groupId, artifactId, parent);
return null;
}

String version = getVersion(candidateModel);
if (version != null && parent.getVersion() != null && !version.equals(parent.getVersion())) {
try {
VersionRange parentRange = versionParser.parseVersionRange(parent.getVersion());
Expand Down Expand Up @@ -952,13 +963,35 @@ private Model readParentLocally(Model childModel) throws ModelBuilderException {
return null;
}
}
return candidateModel;
}

//
// Here we just need to know that a version is fine to use but this validation we can do in our workspace
// resolver.
//
private void mismatchRelativePathAndGA(Model childModel, String groupId, String artifactId, Parent parent) {
StringBuilder buffer = new StringBuilder(256);
buffer.append("'parent.relativePath'");
if (childModel != getRootModel()) {
buffer.append(" of POM ").append(ModelProblemUtils.toSourceHint(childModel));
}
buffer.append(" points at ").append(groupId).append(':').append(artifactId);
buffer.append(" instead of ").append(parent.getGroupId()).append(':');
buffer.append(parent.getArtifactId()).append(", please verify your project structure");

return candidateModel;
setSource(childModel);
boolean warn = MODEL_VERSION_4_0_0.equals(childModel.getModelVersion());
add(warn ? Severity.WARNING : Severity.FATAL, Version.BASE, buffer.toString(), parent.getLocation(""));
}

private void wrongParentRelativePath(Model childModel, String parentPath, Parent parent) {
StringBuilder buffer = new StringBuilder(256);
buffer.append("'parent.relativePath'");
if (childModel != getRootModel()) {
buffer.append(" of POM ").append(ModelProblemUtils.toSourceHint(childModel));
}
buffer.append(" points at '").append(parentPath);
buffer.append("' but no POM could be found, please verify your project structure");

setSource(childModel);
add(Severity.FATAL, Version.BASE, buffer.toString(), parent.getLocation(""));
}

Model resolveAndReadParentExternally(Model childModel) throws ModelBuilderException {
Expand Down Expand Up @@ -987,13 +1020,10 @@ Model resolveAndReadParentExternally(Model childModel) throws ModelBuilderExcept

ModelSource modelSource;
try {
modelSource = resolveReactorModel(groupId, artifactId, version);
if (modelSource == null) {
AtomicReference<Parent> modified = new AtomicReference<>();
modelSource = modelResolver.resolveModel(request.getSession(), repositories, parent, modified);
if (modified.get() != null) {
parent = modified.get();
}
AtomicReference<Parent> modified = new AtomicReference<>();
modelSource = modelResolver.resolveModel(request.getSession(), repositories, parent, modified);
if (modified.get() != null) {
parent = modified.get();
}
} catch (ModelResolverException e) {
// Message below is checked for in the MNG-2199 core IT.
Expand All @@ -1006,13 +1036,8 @@ Model resolveAndReadParentExternally(Model childModel) throws ModelBuilderExcept
buffer.append(" for ").append(ModelProblemUtils.toId(childModel));
}
buffer.append(": ").append(e.getMessage());
if (childModel.getProjectDirectory() != null) {
if (parent.getRelativePath() == null
|| parent.getRelativePath().isEmpty()) {
buffer.append(" and 'parent.relativePath' points at no local POM");
} else {
buffer.append(" and 'parent.relativePath' points at wrong local POM");
}
if (request.getRequestType() == ModelBuilderRequest.RequestType.BUILD_POM) {
buffer.append(" and parent could not be found in reactor");
}

add(Severity.FATAL, Version.BASE, buffer.toString(), parent.getLocation(""), e);
Expand Down Expand Up @@ -1831,15 +1856,6 @@ private boolean rawChildVersionReferencesParent(String rawChildModelVersion) {
|| rawChildModelVersion.equals("${project.parent.version}");
}

private ModelSource getParentPomFile(Model childModel, ModelSource source) {
String parentPath = childModel.getParent().getRelativePath();
if (parentPath == null || parentPath.isEmpty()) {
return null;
} else {
return source.resolve(modelProcessor::locateExistingPom, parentPath);
}
}

private Model getSuperModel(String modelVersion) {
return superPomProvider.getSuperPom(modelVersion);
}
Expand Down
18 changes: 18 additions & 0 deletions maven-api-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultModelValidator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,24 @@ public void validateFileModel(
"is either LATEST or RELEASE (both of them are being deprecated)",
parent);
}

if (parent.getRelativePath() != null
&& !parent.getRelativePath().isEmpty()
&& (parent.getGroupId() != null && !parent.getGroupId().isEmpty()
|| parent.getArtifactId() != null
&& !parent.getArtifactId().isEmpty())
&& validationLevel >= ModelValidator.VALIDATION_LEVEL_MAVEN_4_0
&& VALID_MODEL_VERSIONS.contains(m.getModelVersion())
&& !Objects.equals(m.getModelVersion(), ModelBuilder.MODEL_VERSION_4_0_0)) {
addViolation(
problems,
Severity.WARNING,
Version.BASE,
"parent.relativePath",
null,
"only specify relativePath or groupId/artifactId in modelVersion 4.1.0",
parent);
}
}

if (validationLevel == ModelValidator.VALIDATION_LEVEL_MINIMAL) {
Expand Down

0 comments on commit 9dd48c1

Please sign in to comment.