-
Notifications
You must be signed in to change notification settings - Fork 2.8k
[MNG-8211] Fail the build if CI Friendly revision used without value #1656
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
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
Example failure: https://gist.github.com/cstamas/9dbb79be59ba22b6bc5c823d95593e50 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems currently only one value is assumed but one can combine values e.g.
<version>${revision}${sha1}${changelist}</version>
see
maven/maven-model-builder/src/main/java/org/apache/maven/model/validation/DefaultModelValidator.java
Lines 1231 to 1245 in 76ff89f
Matcher m = CI_FRIENDLY_EXPRESSION.matcher(string.trim()); | |
while (m.find()) { | |
String property = m.group(1); | |
if (!versionProcessor.isValidProperty(property)) { | |
addViolation( | |
problems, | |
severity, | |
version, | |
fieldName, | |
null, | |
"contains an expression but should be a constant.", | |
tracker); | |
return false; | |
} | |
} |
...src/main/java/org/apache/maven/model/interpolation/AbstractStringBasedModelInterpolator.java
Outdated
Show resolved
Hide resolved
This PR does it in wrong place:
|
Example of failure: https://gist.github.com/cstamas/01b1b6152aca56f1383148ef74245583 |
This PR would really help to reduce user confusion with CI Friendly version. |
Last example of failure but also the "escape hatch" in effect: https://gist.github.com/cstamas/1e880b2381bf1c543db052a1298e5074 |
Thanks for keeping the old behaviour with <?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
<groupId>com.example.app</groupId>
<artifactId>app</artifactId>
<version>${revision}</version>
<properties>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<project.reporting.outputEncoding>UTF-8</project.reporting.outputEncoding>
<java.version>21</java.version>
<maven.build.allowExpressionInEffectiveProjectVersion>true</maven.build.allowExpressionInEffectiveProjectVersion>
<maven.compiler.source>${java.version}</maven.compiler.source>
<maven.compiler.target>${java.version}</maven.compiler.target>
</properties>
<build>
<finalName>${project.artifactId}</finalName>
<plugins>
<plugin>
<groupId>io.github.git-commit-id</groupId>
<artifactId>git-commit-id-maven-plugin</artifactId>
<version>9.0.2</version>
<executions>
<execution>
<goals>
<goal>revision</goal>
</goals>
<phase>initialize</phase>
</execution>
</executions>
<configuration>
<abbrevLength>8</abbrevLength>
<commitIdGenerationMode>full</commitIdGenerationMode>
<generateGitPropertiesFile>true</generateGitPropertiesFile>
</configuration>
</plugin>
</plugins>
</build>
<profiles>
<profile>
<id>set-version</id>
<build>
<plugins>
<plugin>
<groupId>org.codehaus.mojo</groupId>
<artifactId>versions-maven-plugin</artifactId>
<version>2.18.0</version>
<executions>
<execution>
<goals>
<goal>set</goal>
</goals>
<phase>initialize</phase>
</execution>
</executions>
<configuration>
<generateBackupPoms>false</generateBackupPoms>
</configuration>
</plugin>
</plugins>
</build>
</profile>
</profiles>
</project> Which I can then use as: if [ $CI_COMMIT_BRANCH == $CI_DEFAULT_BRANCH ]; then
NEW_VERSION='${git.total.commit.count}'
else
NEW_VERSION=${CI_COMMIT_REF_SLUG}-SNAPSHOT
fi
./mvnw --activate-profiles set-version --define newVersion=${NEW_VERSION} initialize
./mvnw source:jar deploy That way, I can let |
@dalbani This becomes way simpler with Maven 4: unlike with Maven 3 that produces invalid POM if installed/deployed without flatten or similar plugin (see page warning about install/deploy), Maven 4 will never deploy invalid/unusable POM (this change is NOT YET released, so is not in last rc-3!). OTOH, for Maven 3 same thing (aka "fixed CI friendly") is done by Nisse: https://github.com/maveniverse/nisse In both cases you can save one of the 2 Maven invocations. |
Thanks for the detailed explanation, @cstamas 👍 |
This change broke our pipeline today (we had We are using eclipse-temurin image to build maven projects, and they just pushed the 3.9.10 images yesterday (which replaced the We were waiting for something similar to change the |
Resolve #9791 |
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}
(or any expression in project.version).Still, to not be disruptive, the old behaviour can be achieved by setting
maven.build.allowExpressionInEffectiveProjectVersion
=true project property.https://issues.apache.org/jira/browse/MNG-8211