From a88fca4ba096f5f681d94dff400652b096082bf3 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Thu, 15 Aug 2024 16:12:15 +0200 Subject: [PATCH 1/6] [MNG-8211] Fail the build if CI Friendly revision used without value As this is almost always source of confusion. If feature is used and there is no proper value set, fail the build, as users for sure does not plan to deploy artifacts with version `${revision}`. --- https://issues.apache.org/jira/browse/MNG-8211 --- .../AbstractStringBasedModelInterpolator.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/interpolation/AbstractStringBasedModelInterpolator.java b/maven-model-builder/src/main/java/org/apache/maven/model/interpolation/AbstractStringBasedModelInterpolator.java index 9b1fae7d007f..ffd1a1b7cb4d 100644 --- a/maven-model-builder/src/main/java/org/apache/maven/model/interpolation/AbstractStringBasedModelInterpolator.java +++ b/maven-model-builder/src/main/java/org/apache/maven/model/interpolation/AbstractStringBasedModelInterpolator.java @@ -30,7 +30,9 @@ import org.apache.maven.model.Model; import org.apache.maven.model.building.ModelBuildingRequest; +import org.apache.maven.model.building.ModelProblem; import org.apache.maven.model.building.ModelProblemCollector; +import org.apache.maven.model.building.ModelProblemCollectorRequest; import org.apache.maven.model.path.PathTranslator; import org.apache.maven.model.path.UrlNormalizer; import org.codehaus.plexus.interpolation.AbstractValueSource; @@ -158,6 +160,16 @@ public Object getValue(String expression) { // Overwrite existing values in model properties. Otherwise it's not possible // to define them via command line e.g.: mvn -Drevision=6.5.7 ... versionProcessor.overwriteModelProperties(modelProperties, config); + String version = model.getVersion(); + if (version != null && version.startsWith("${") && version.endsWith("}")) { + String property = version.substring(2, version.length() - 1); + if (!modelProperties.containsKey(property)) { + problems.add(new ModelProblemCollectorRequest(ModelProblem.Severity.FATAL, ModelProblem.Version.BASE) + .setMessage("Model '" + model.getId() + "' uses CI Friendly Versions expression '${" + property + + "}' without value being set") + .setLocation(model.getLocation(""))); + } + } valueSources.add(new MapBasedValueSource(modelProperties)); valueSources.add(new MapBasedValueSource(config.getSystemProperties())); From 5c595e13f25a29ff027190b6a98ff7101a288ff5 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Tue, 20 Aug 2024 23:07:39 +0200 Subject: [PATCH 2/6] Move to proper place --- .../AbstractStringBasedModelInterpolator.java | 12 ------------ .../model/validation/DefaultModelValidator.java | 3 +++ 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/interpolation/AbstractStringBasedModelInterpolator.java b/maven-model-builder/src/main/java/org/apache/maven/model/interpolation/AbstractStringBasedModelInterpolator.java index ffd1a1b7cb4d..9b1fae7d007f 100644 --- a/maven-model-builder/src/main/java/org/apache/maven/model/interpolation/AbstractStringBasedModelInterpolator.java +++ b/maven-model-builder/src/main/java/org/apache/maven/model/interpolation/AbstractStringBasedModelInterpolator.java @@ -30,9 +30,7 @@ import org.apache.maven.model.Model; import org.apache.maven.model.building.ModelBuildingRequest; -import org.apache.maven.model.building.ModelProblem; import org.apache.maven.model.building.ModelProblemCollector; -import org.apache.maven.model.building.ModelProblemCollectorRequest; import org.apache.maven.model.path.PathTranslator; import org.apache.maven.model.path.UrlNormalizer; import org.codehaus.plexus.interpolation.AbstractValueSource; @@ -160,16 +158,6 @@ public Object getValue(String expression) { // Overwrite existing values in model properties. Otherwise it's not possible // to define them via command line e.g.: mvn -Drevision=6.5.7 ... versionProcessor.overwriteModelProperties(modelProperties, config); - String version = model.getVersion(); - if (version != null && version.startsWith("${") && version.endsWith("}")) { - String property = version.substring(2, version.length() - 1); - if (!modelProperties.containsKey(property)) { - problems.add(new ModelProblemCollectorRequest(ModelProblem.Severity.FATAL, ModelProblem.Version.BASE) - .setMessage("Model '" + model.getId() + "' uses CI Friendly Versions expression '${" + property - + "}' without value being set") - .setLocation(model.getLocation(""))); - } - } valueSources.add(new MapBasedValueSource(modelProperties)); valueSources.add(new MapBasedValueSource(config.getSystemProperties())); diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java b/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java index 1f1a7ab57242..78cd09d5e220 100644 --- a/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java +++ b/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java @@ -475,6 +475,9 @@ public void validateEffectiveModel(Model m, ModelBuildingRequest request, ModelP } validateStringNotEmpty("version", problems, Severity.ERROR, Version.BASE, m.getVersion(), m); + if (hasExpression(m.getVersion())) { + addViolation(problems, Severity.ERROR, Version.BASE, "version", null, "must be a constant version but is '" + m.getVersion() + "'.", m); + } Severity errOn30 = getSeverity(request, ModelBuildingRequest.VALIDATION_LEVEL_MAVEN_3_0); From 531941e627f6944475b8155072f4c8bb559b3a3c Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Tue, 20 Aug 2024 23:11:08 +0200 Subject: [PATCH 3/6] Reformat --- .../maven/model/validation/DefaultModelValidator.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java b/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java index 78cd09d5e220..77968ef5dce4 100644 --- a/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java +++ b/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java @@ -476,7 +476,14 @@ public void validateEffectiveModel(Model m, ModelBuildingRequest request, ModelP validateStringNotEmpty("version", problems, Severity.ERROR, Version.BASE, m.getVersion(), m); if (hasExpression(m.getVersion())) { - addViolation(problems, Severity.ERROR, Version.BASE, "version", null, "must be a constant version but is '" + m.getVersion() + "'.", m); + addViolation( + problems, + Severity.ERROR, + Version.BASE, + "version", + null, + "must be a constant version but is '" + m.getVersion() + "'.", + m); } Severity errOn30 = getSeverity(request, ModelBuildingRequest.VALIDATION_LEVEL_MAVEN_3_0); From 329422d6878fb689453239acdc9f28a3dfdaebe3 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Thu, 22 Aug 2024 11:26:28 +0200 Subject: [PATCH 4/6] More to proper place --- .../validation/DefaultModelValidator.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java b/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java index 77968ef5dce4..f1274b58cd52 100644 --- a/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java +++ b/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java @@ -475,16 +475,6 @@ public void validateEffectiveModel(Model m, ModelBuildingRequest request, ModelP } validateStringNotEmpty("version", problems, Severity.ERROR, Version.BASE, m.getVersion(), m); - if (hasExpression(m.getVersion())) { - addViolation( - problems, - Severity.ERROR, - Version.BASE, - "version", - null, - "must be a constant version but is '" + m.getVersion() + "'.", - m); - } Severity errOn30 = getSeverity(request, ModelBuildingRequest.VALIDATION_LEVEL_MAVEN_3_0); @@ -516,6 +506,16 @@ public void validateEffectiveModel(Model m, ModelBuildingRequest request, ModelP validateBannedCharacters( EMPTY, "version", problems, errOn31, Version.V20, m.getVersion(), null, m, ILLEGAL_VERSION_CHARS); validate20ProperSnapshotVersion("version", problems, errOn31, Version.V20, m.getVersion(), null, m); + if (hasExpression(m.getVersion())) { + addViolation( + problems, + Severity.ERROR, + Version.V20, + "version", + null, + "must be a constant version but is '" + m.getVersion() + "'.", + m); + } Build build = m.getBuild(); if (build != null) { From ed082920c68847b883c224c28bb2e0131d375103 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Thu, 22 Aug 2024 12:08:14 +0200 Subject: [PATCH 5/6] Add "escape hatch" property that makes violation a warning only. --- .../maven/model/validation/DefaultModelValidator.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java b/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java index f1274b58cd52..b5e0e2373476 100644 --- a/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java +++ b/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java @@ -74,6 +74,8 @@ @Named @Singleton public class DefaultModelValidator implements ModelValidator { + public static final String BUILD_ALLOW_EXPRESSION_IN_EFFECTIVE_PROJECT_VERSION = + "maven.build.allowExpressionInEffectiveProjectVersion"; private static final Pattern CI_FRIENDLY_EXPRESSION = Pattern.compile("\\$\\{(.+?)}"); private static final Pattern EXPRESSION_PROJECT_NAME_PATTERN = Pattern.compile("\\$\\{(project.+?)}"); @@ -507,9 +509,14 @@ public void validateEffectiveModel(Model m, ModelBuildingRequest request, ModelP EMPTY, "version", problems, errOn31, Version.V20, m.getVersion(), null, m, ILLEGAL_VERSION_CHARS); validate20ProperSnapshotVersion("version", problems, errOn31, Version.V20, m.getVersion(), null, m); if (hasExpression(m.getVersion())) { + Severity versionExpressionSeverity = Severity.ERROR; + if (Boolean.parseBoolean(m.getProperties() + .getProperty(BUILD_ALLOW_EXPRESSION_IN_EFFECTIVE_PROJECT_VERSION, Boolean.FALSE.toString()))) { + versionExpressionSeverity = Severity.WARNING; + } addViolation( problems, - Severity.ERROR, + versionExpressionSeverity, Version.V20, "version", null, From fc331d552618ea41194add7c3c4762d5300ec4f5 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Thu, 22 Aug 2024 12:20:11 +0200 Subject: [PATCH 6/6] Simplify --- .../apache/maven/model/validation/DefaultModelValidator.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java b/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java index b5e0e2373476..f4d7369c88d2 100644 --- a/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java +++ b/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java @@ -510,8 +510,8 @@ public void validateEffectiveModel(Model m, ModelBuildingRequest request, ModelP validate20ProperSnapshotVersion("version", problems, errOn31, Version.V20, m.getVersion(), null, m); if (hasExpression(m.getVersion())) { Severity versionExpressionSeverity = Severity.ERROR; - if (Boolean.parseBoolean(m.getProperties() - .getProperty(BUILD_ALLOW_EXPRESSION_IN_EFFECTIVE_PROJECT_VERSION, Boolean.FALSE.toString()))) { + if (Boolean.parseBoolean( + m.getProperties().getProperty(BUILD_ALLOW_EXPRESSION_IN_EFFECTIVE_PROJECT_VERSION))) { versionExpressionSeverity = Severity.WARNING; } addViolation(