Skip to content

Commit 885a4b3

Browse files
gnodetgnodet
authored
[MNG-8230] Rewrite CI friendly versions (#1710)
1 parent eefe2c7 commit 885a4b3

File tree

6 files changed

+102
-207
lines changed

6 files changed

+102
-207
lines changed

‎maven-api-impl/src/main/java/org/apache/maven/api/services/model/ModelVersionProcessor.java‎

Lines changed: 0 additions & 44 deletions
This file was deleted.

‎maven-api-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultModelBuilder.java‎

Lines changed: 58 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import java.nio.file.Path;
2727
import java.nio.file.Paths;
2828
import java.util.ArrayList;
29-
import java.util.Arrays;
3029
import java.util.Collection;
3130
import java.util.HashMap;
3231
import java.util.HashSet;
@@ -45,6 +44,8 @@
4544
import java.util.concurrent.atomic.AtomicReference;
4645
import java.util.function.Supplier;
4746
import java.util.function.UnaryOperator;
47+
import java.util.regex.Matcher;
48+
import java.util.regex.Pattern;
4849
import java.util.stream.Collectors;
4950
import java.util.stream.Stream;
5051

@@ -233,6 +234,8 @@ public ModelBuilderResult build(ModelBuilderRequest request) throws ModelBuilder
233234
}
234235

235236
protected class DefaultModelBuilderSession implements ModelProblemCollector {
237+
private static final Pattern REGEX = Pattern.compile("\\$\\{([^}]+)}");
238+
236239
final Session session;
237240
final ModelBuilderRequest request;
238241
final DefaultModelBuilderResult result;
@@ -557,58 +560,15 @@ public void mergeRepositories(List<Repository> toAdd, boolean replace) {
557560
}
558561

559562
//
560-
// Transform raw model to build pom
561-
//
562-
Model transformFileToRaw(Model model) {
563-
Model.Builder builder = Model.newBuilder(model);
564-
builder = handleParent(model, builder);
565-
builder = handleReactorDependencies(model, builder);
566-
builder = handleCiFriendlyVersion(model, builder);
567-
return builder.build();
568-
}
569-
570-
//
571-
// Infer parent information
572-
//
573-
Model.Builder handleParent(Model model, Model.Builder builder) {
574-
Parent parent = model.getParent();
575-
if (parent != null) {
576-
String version = parent.getVersion();
577-
String modVersion = replaceCiFriendlyVersion(version);
578-
if (!Objects.equals(version, modVersion)) {
579-
if (builder == null) {
580-
builder = Model.newBuilder(model);
581-
}
582-
builder.parent(parent.withVersion(modVersion));
583-
}
584-
}
585-
return builder;
586-
}
587-
588-
//
589-
// CI friendly versions
590-
//
591-
Model.Builder handleCiFriendlyVersion(Model model, Model.Builder builder) {
592-
String version = model.getVersion();
593-
String modVersion = replaceCiFriendlyVersion(version);
594-
if (!Objects.equals(version, modVersion)) {
595-
if (builder == null) {
596-
builder = Model.newBuilder(model);
597-
}
598-
builder.version(modVersion);
599-
}
600-
return builder;
601-
}
602-
603-
//
563+
// Transform raw model to build pom.
604564
// Infer inner reactor dependencies version
605565
//
606-
Model.Builder handleReactorDependencies(Model model, Model.Builder builder) {
566+
Model transformFileToRaw(Model model) {
607567
List<Dependency> newDeps = new ArrayList<>();
608568
boolean modified = false;
609569
for (Dependency dep : model.getDependencies()) {
610-
Dependency.Builder depBuilder = null;
611570
if (dep.getVersion() == null) {
571+
Dependency.Builder depBuilder = null;
612572
Model depModel = getRawModel(model.getPomFile(), dep.getGroupId(), dep.getArtifactId());
613573
if (depModel != null) {
614574
String version = depModel.getVersion();
@@ -629,30 +589,35 @@ Model.Builder handleReactorDependencies(Model model, Model.Builder builder) {
629589
depBuilder.groupId(depGroupId).location("groupId", groupIdLocation);
630590
}
631591
}
592+
if (depBuilder != null) {
593+
newDeps.add(depBuilder.build());
594+
modified = true;
595+
} else {
596+
newDeps.add(dep);
597+
}
632598
}
633-
if (depBuilder != null) {
634-
newDeps.add(depBuilder.build());
635-
modified = true;
636-
} else {
637-
newDeps.add(dep);
638-
}
639-
}
640-
if (modified) {
641-
if (builder == null) {
642-
builder = Model.newBuilder(model);
643-
}
644-
builder.dependencies(newDeps);
645599
}
646-
return builder;
600+
return modified ? model.withDependencies(newDeps) : model;
647601
}
648602

649-
String replaceCiFriendlyVersion(String version) {
603+
String replaceCiFriendlyVersion(Map<String, String> properties, String version) {
604+
// TODO: we're using a simple regex here, but we should probably use
605+
// a proper interpolation service to do the replacements
606+
// once one is available in maven-api-impl
607+
// https://issues.apache.org/jira/browse/MNG-8262
650608
if (version != null) {
651-
for (String key : Arrays.asList("changelist", "revision", "sha1")) {
652-
String val = request.getUserProperties().get(key);
653-
if (val != null) {
654-
version = version.replace("${" + key + "}", val);
655-
}
609+
Matcher matcher = REGEX.matcher(version);
610+
if (matcher.find()) {
611+
StringBuilder result = new StringBuilder();
612+
do {
613+
// extract the key inside ${}
614+
String key = matcher.group(1);
615+
// get replacement from the map, or use the original ${xy} if not found
616+
String replacement = properties.getOrDefault(key, "\\" + matcher.group(0));
617+
matcher.appendReplacement(result, replacement);
618+
} while (matcher.find());
619+
matcher.appendTail(result); // Append the remaining part of the string
620+
return result.toString();
656621
}
657622
}
658623
return version;
@@ -733,7 +698,6 @@ Stream<DefaultModelBuilderResult> results(DefaultModelBuilderResult r) {
733698
return Stream.concat(Stream.of(r), r.getChildren().stream().flatMap(this::results));
734699
}
735700

736-
@SuppressWarnings("checkstyle:MethodLength")
737701
private void loadFromRoot(Path root, Path top) {
738702
try (PhasingExecutor executor = createExecutor()) {
739703
DefaultModelBuilderResult r = Objects.equals(top, root) ? result : new DefaultModelBuilderResult();
@@ -1212,16 +1176,18 @@ Model readFileModel() throws ModelBuilderException {
12121176
Model doReadFileModel() throws ModelBuilderException {
12131177
ModelSource modelSource = request.getSource();
12141178
Model model;
1179+
Path rootDirectory;
12151180
setSource(modelSource.getLocation());
12161181
logger.debug("Reading file model from " + modelSource.getLocation());
12171182
try {
12181183
boolean strict = request.getRequestType() == ModelBuilderRequest.RequestType.BUILD_POM;
1219-
// TODO: we do cache, but what if strict does not have the same value?
1220-
Path rootDirectory;
12211184
try {
12221185
rootDirectory = request.getSession().getRootDirectory();
12231186
} catch (IllegalStateException ignore) {
12241187
rootDirectory = modelSource.getPath();
1188+
while (rootDirectory != null && !Files.isDirectory(rootDirectory)) {
1189+
rootDirectory = rootDirectory.getParent();
1190+
}
12251191
}
12261192
try (InputStream is = modelSource.openStream()) {
12271193
model = modelProcessor.read(XmlReaderRequest.builder()
@@ -1355,6 +1321,29 @@ Model doReadFileModel() throws ModelBuilderException {
13551321
add(Severity.FATAL, Version.V41, "Error discovering subprojects", e);
13561322
}
13571323
}
1324+
1325+
// CI friendly version
1326+
// All expressions are interpolated using user properties and properties
1327+
// defined on the root project.
1328+
Map<String, String> properties = new HashMap<>();
1329+
if (!Objects.equals(rootDirectory, model.getProjectDirectory())) {
1330+
Model rootModel = derive(ModelSource.fromPath(modelProcessor.locateExistingPom(rootDirectory)))
1331+
.readFileModel();
1332+
properties.putAll(rootModel.getProperties());
1333+
} else {
1334+
properties.putAll(model.getProperties());
1335+
}
1336+
properties.putAll(session.getUserProperties());
1337+
model = model.with()
1338+
.version(replaceCiFriendlyVersion(properties, model.getVersion()))
1339+
.parent(
1340+
model.getParent() != null
1341+
? model.getParent()
1342+
.withVersion(replaceCiFriendlyVersion(
1343+
properties,
1344+
model.getParent().getVersion()))
1345+
: null)
1346+
.build();
13581347
}
13591348

13601349
for (var transformer : transformers) {

‎maven-api-impl/src/main/java/org/apache/maven/internal/impl/model/DefaultModelValidator.java‎

Lines changed: 42 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,6 @@
7070
import org.apache.maven.api.services.ModelProblem.Version;
7171
import org.apache.maven.api.services.ModelProblemCollector;
7272
import org.apache.maven.api.services.model.ModelValidator;
73-
import org.apache.maven.api.services.model.ModelVersionProcessor;
7473
import org.apache.maven.model.v4.MavenModelVersion;
7574
import org.apache.maven.model.v4.MavenTransformer;
7675

@@ -288,12 +287,8 @@ protected ActivationProperty.Builder transformActivationProperty_Value(
288287

289288
private final Set<String> validProfileIds = new HashSet<>();
290289

291-
private final ModelVersionProcessor versionProcessor;
292-
293290
@Inject
294-
public DefaultModelValidator(ModelVersionProcessor versionProcessor) {
295-
this.versionProcessor = versionProcessor;
296-
}
291+
public DefaultModelValidator() {}
297292

298293
@Override
299294
@SuppressWarnings("checkstyle:MethodLength")
@@ -418,17 +413,42 @@ public void validateFileModel(
418413

419414
Severity errOn30 = getSeverity(validationLevel, ModelValidator.VALIDATION_LEVEL_MAVEN_3_0);
420415

421-
validateStringNoExpression("groupId", problems, Severity.WARNING, Version.V20, m.getGroupId(), m);
422-
if (parent == null) {
423-
validateStringNotEmpty("groupId", problems, Severity.FATAL, Version.V20, m.getGroupId(), m);
416+
// The file pom may not contain the modelVersion yet, as it may be set later by the
417+
// ModelVersionXMLFilter.
418+
if (m.getModelVersion() != null && !m.getModelVersion().isEmpty()) {
419+
validateModelVersion(problems, m.getModelVersion(), m, VALID_MODEL_VERSIONS);
424420
}
425421

426-
validateStringNoExpression("artifactId", problems, Severity.WARNING, Version.V20, m.getArtifactId(), m);
427-
validateStringNotEmpty("artifactId", problems, Severity.FATAL, Version.V20, m.getArtifactId(), m);
422+
boolean isModelVersion41OrMore = !Objects.equals(ModelBuilder.MODEL_VERSION_4_0_0, m.getModelVersion());
423+
if (isModelVersion41OrMore) {
424+
validateStringNoExpression("groupId", problems, Severity.FATAL, Version.V41, m.getGroupId(), m);
425+
426+
validateStringNotEmpty("artifactId", problems, Severity.FATAL, Version.V20, m.getArtifactId(), m);
427+
validateStringNoExpression("artifactId", problems, Severity.FATAL, Version.V20, m.getArtifactId(), m);
428+
429+
validateVersionNoExpression("version", problems, Severity.FATAL, Version.V41, m.getVersion(), m);
428430

429-
validateVersionNoExpression("version", problems, Severity.WARNING, Version.V20, m.getVersion(), m);
430-
if (parent == null) {
431-
validateStringNotEmpty("version", problems, Severity.FATAL, Version.V20, m.getVersion(), m);
431+
if (parent != null) {
432+
validateStringNoExpression(
433+
"groupId", problems, Severity.FATAL, Version.V41, parent.getGroupId(), m);
434+
validateStringNoExpression(
435+
"artifactId", problems, Severity.FATAL, Version.V41, parent.getArtifactId(), m);
436+
validateVersionNoExpression(
437+
"version", problems, Severity.FATAL, Version.V41, parent.getVersion(), m);
438+
}
439+
} else {
440+
validateStringNoExpression("groupId", problems, Severity.WARNING, Version.V20, m.getGroupId(), m);
441+
if (parent == null) {
442+
validateStringNotEmpty("groupId", problems, Severity.FATAL, Version.V20, m.getGroupId(), m);
443+
}
444+
445+
validateStringNoExpression("artifactId", problems, Severity.WARNING, Version.V20, m.getArtifactId(), m);
446+
validateStringNotEmpty("artifactId", problems, Severity.FATAL, Version.V20, m.getArtifactId(), m);
447+
448+
validateVersionNoExpression("version", problems, Severity.WARNING, Version.V20, m.getVersion(), m);
449+
if (parent == null) {
450+
validateStringNotEmpty("version", problems, Severity.FATAL, Version.V20, m.getVersion(), m);
451+
}
432452
}
433453

434454
validate20RawDependencies(
@@ -1634,19 +1654,14 @@ private boolean validateVersionNoExpression(
16341654

16351655
Matcher m = EXPRESSION_NAME_PATTERN.matcher(string.trim());
16361656
while (m.find()) {
1637-
String property = m.group(1);
1638-
if (!versionProcessor.isValidProperty(property)) {
1639-
addViolation(
1640-
problems,
1641-
severity,
1642-
version,
1643-
fieldName,
1644-
null,
1645-
"contains an expression but should be a constant.",
1646-
tracker);
1647-
1648-
return false;
1649-
}
1657+
addViolation(
1658+
problems,
1659+
severity,
1660+
version,
1661+
fieldName,
1662+
null,
1663+
"contains an expression but should be a constant.",
1664+
tracker);
16501665
}
16511666

16521667
return true;

0 commit comments

Comments
 (0)