[MRELEASE-1154] [REGRESSION] MRELEASE-1109 breaks release of Maven Su…

[michael-o]

…refire

This reverts commit 9e0713b (MRELEASE-1109).

This closes #229

Following this checklist to help us incorporate your
contribution quickly and easily:

• Make sure there is a JIRA issue filed
  for the change (usually before you start working on it). Trivial changes like typos do not
  require a JIRA issue. Your pull request should address just this issue, without
  pulling in other changes.
• Each commit in the pull request should have a meaningful subject line and body.
• Format the pull request title like [MRELEASE-XXX] - Fixes bug in ApproximateQuantiles,
  where you replace MRELEASE-XXX with the appropriate JIRA issue. Best practice
  is to use the JIRA issue title in the pull request title and in the first line of the
  commit message.
• Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
• Run mvn clean verify -Prun-its to make sure basic checks pass. A more thorough check will
  be performed on your pull request automatically.

If your pull request is about ~20 lines of code you don't need to sign an
Individual Contributor License Agreement if you are unsure
please ask on the developers list.

To make clear that you license your contribution under
the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.

• I hereby declare this contribution to be licenced under the Apache License Version 2.0, January 2004

• In any other case, please file an Apache Individual Contributor License Agreement.

[laeubi]

Be aware that the list of supported properties is extensible

[laeubi]

More than one property is supported here!

[michael-o]

@laeubi You know that this is a revert?

[laeubi]

@laeubi You know that this is a revert?

Yes I just wanted to point out problematic things in the current (now reverted) implementation that shows it is either incomplete i not even wrong in its current form.

[kwin]

Any further insights what exactly causes the issue? https://issues.apache.org/jira/browse/MRELEASE-1154 does not even contain the full stack trace....

[michael-o]

Any further insights what exactly causes the issue? https://issues.apache.org/jira/browse/MRELEASE-1154 does not even contain the full stack trace....

Just try to release Surefire, it will immediately fail.

[michael-o]

@gnodet Since you are also on the CI friendy track. Which PR do you prefer? This one or #230? I believe that what we have in Release now is incomplete/broken, I'd prefer to revert it until we have something better. WDYT?

[gnodet]

@gnodet Since you are also on the CI friendy track. Which PR do you prefer? This one or #230? I believe that what we have in Release now is incomplete/broken, I'd prefer to revert it until we have something better. WDYT?

This is definitely open to discussion, but I'm thinking right now is that friendly should be static after the file models have been read. So CI-friendly versions can be handled by a transformer that would be called between the file model being read and the file model being validated.
In addition, I think the default implementation should limit to user properties, but not use project properties. The problem with project properties is that you only have the real values from the effective model, but the file model. And that's really what causes all kind of problems in Maven, as the CI version is actually handled during the file -> raw transformation.
If we go that way, that would mean the best location for those properties would be [root]/.mvn/maven.properties. The release plugin would have to be adapted, but given the model validation would ensure the versions are constants in the file model, I would not necessarily restrict the release plugin to any kind of key for substitution. However, don't really see how it could cope with something like the Jenkins one: ${revision}${changelist}... which placeholder should be updated ?

@michael-o could you briefly describe the two alternatives here ?

[michael-o]

@michael-o could you briefly describe the two alternatives here ?

It all started with: https://issues.apache.org/jira/browse/MRELEASE-1109 which causes https://issues.apache.org/jira/browse/MRELEASE-1151 and led to https://issues.apache.org/jira/browse/MRELEASE-1153. Now, I have discovered yet another regression https://issues.apache.org/jira/browse/MRELEASE-1154 which requires https://issues.apache.org/jira/browse/MRELEASE-1109 to reverted altogether because I was not able to release Maven Surefire. The alternative in #230 provided by @kwin simply warns about non-resolvable properties. I am not really sure whether this is the right way. I know for sure that the revert will restore the previous behavior, but will also the incomplete CI friendly support. I prefer no support rather than incomplete one.

[laeubi]

however, don't really see how it could cope with something like the Jenkins one: ${revision}${changelist}... which placeholder should be updated ?

Tycho version plugin supports updating such properties with a little restriction that only one of them can be updated (usually the last part) that has to be a Number by default.

e.g. if ${revision}${changelist} maps to <revision>1.0</revision> and <changelist>-SNAPSHOT</changelist> and one updates the version to 1.1-SNAPSHOT it first checks the changelist is a suffix of the new version then leave this alone, now check revision what is not a suffix anymore so change revision -> 1.1, this already covers the most usual cases.

[gnodet]

however, don't really see how it could cope with something like the Jenkins one: ${revision}${changelist}... which placeholder should be updated ?

Tycho version plugin supports updating such properties with a little restriction that only one of them can be updated (usually the last part) that has to be a Number by default.

e.g. if ${revision}${changelist} maps to <revision>1.0</revision> and <changelist>-SNAPSHOT</changelist> and one updates the version to 1.1-SNAPSHOT it first checks the changelist is a suffix of the new version then leave this alone, now check revision what is not a suffix anymore so change revision -> 1.1, this already covers the most usual cases.

What's the use case for using multiple placeholders such as ${revision}${changelist}, especially when/if we open those properties a bit more to any user defined properties. If I look at the Jenkins use case, revision is the usual version, and changelist is -SNAPSHOT, unless it's an automatic deploy in which case changelist is computed from the git revision.
So the real use case afaik, and I think Tycho has a similar requirement, is that the version for a given component is computed at runtime and not hardcoded in the POM.
I don't think there's a real need to support two components here. What's the benefit here ?

[laeubi]

What's the use case for using multiple placeholders such as ${revision}${changelist}

If you want to have multi part version where you can change each part individually see

https://maven.apache.org/maven-ci-friendly.html --> "f you like to have more flexibility you can use a combination of the different properties like this"

And project properties itself are also not allowed to contain any other things but need to be defined either in the parent or directly in the pom.

So the real use case afaik, and I think Tycho has a similar requirement, is that the version for a given component is computed at runtime and not hardcoded in the POM

Not really at runtime, but can be set "from the outside" as for Jenkins so if you have a CI/CD-Pipeline that you don't need to change versions all time (in the SCM) but still can make sure they are unique to a build.

I don't think there's a real need to support two components here. What's the benefit here ?

See above the benefit is that one can have multipart versions and each part can be given on the commandline and takes precedence over everything else regardless of lookup-order.

[laeubi]

I found some blogs that describe some usual use cases:

• https://medium.com/outbrain-engineering/faster-release-with-maven-ci-friendly-versions-and-a-customised-flatten-plugin-fe53f0fcc0df
• https://blog.sebastian-daschner.com/entries/cd-friendly-maven-versions
• (a more complex overview of the why and how) https://www.socallinuxexpo.org/sites/default/files/presentations/SCaLE-18x-Maven_Renewed_u2.pdf (starting at page 27)
• https://dev.to/khmarbaise/continuous-delivery-with-apache-maven--4i03

[kwin]

There is lots of issues mixed here in the discussion. Although MRELEASE-1109 only supports single expressions this can be extended innsome follow up PR. However I fail to see how not supporting even simple ci-friendly versions is better than supporting only simple versions. The regression regarding unresolveable properties is fixed by #230. I am happy to process any constructive feedback to #230. Just reverting is IMHO the worst solution.

[michael-o]

I'll get back to this next week.

[michael-o]

The alternative has been merged now.