sanitizeVersion breaks helm lint because it's not semver compliant

[matteomazza91]

use case:
build and release docker images and helm charts

the issue:
Because docker tags won't accept the + symbol, I added the sanitizeVersion property

-Prelease.sanitizeVersion=true 

This replaces the + symbol with the . symbol creating a new pre-release identifier.
example from the readme:

This will generate a version string similar to: 0.1.0-dev.2.e1c43c7

The last pre-release component ( e1c43c7 ) is the shortened hash of the revision being built.

According to https://semver.org/#backusnaur-form-grammar-for-valid-semver-versions, each of the dot-separated pre-release components should be an alphanumeric string or a number starting with a positive-digit.

So, in the event of a revision hash that has digits only and it is starting with a 0, the version created by this plugin is not SemVer compliant. ( the probability of this happening is circa 0.4%)

And apparently Helm lint is very strict about this.

[ERROR] Chart.yaml: version '1.351.0-dev.21.pr.489.0303690' is not a valid SemVer

the solution:
prepending or appending a character to the shortened revision hash could be very confusing (as it could easily be mistaken for a character of the hash).
Maybe prepending something like rev- to have 1.351.0-dev.21.pr.489.rev-0303690 could work?
(note that the symbol - is a valid non-digit in the grammar)

However I'm not sure if this could break some other use cases, so maybe we need a breaking-change or a new property to adopt this behaviour?

[matteomazza91]

if anyone is hitting this issue as well, I'm using this workaround.

right after plugin application,

 void sanitizeDevSnapshotVersion(Project project){
    // fix for https://github.com/nebula-plugins/nebula-release-plugin/issues/258
    if(project.gradle.startParameter.taskNames.contains('devSnapshot')){
      if((project.findProperty("release.sanitizeVersion") as Boolean)){
        def inferredVersion = project.version.toString()
        def semVerParts = inferredVersion.split(/-/,2)
        def preReleaseComponents = semVerParts[1].split(/\./).toList()
        def commitHash = preReleaseComponents.removeLast()
        project.allprojects {
          it.setVersion("${semVerParts[0]}-${preReleaseComponents.join(".")}.rev-${commitHash}")
        }
        project.logger.lifecycle("sanitized version: ${project.version}")
      }
    }
  }

[DanielThomas]

Is there a specific reason you choose to do this for the entire project, instead of just varying it for the task(s) that need a sanitized version? This doesn't feel like something that should be a concern of the plugin, and happen only when necessary.

Arguably if you follow the semantic versioning meaning of + you'd strip everything after + for publishing to the registry, because it's intended to be ignored for the purposes of version precedence, so essentially latest wins:

https://semver.org/#spec-item-10

[matteomazza91]

Is there a specific reason you choose to do this for the entire project, instead of just varying it for the task(s) that need a sanitized version?

maybe is yet another opinion, but I prefer to have all the artifacts built by one gradle-build with the same version. To avoid any kind of confusion.

This doesn't feel like something that should be a concern of the plugin, and happen only when necessary.

I think it should be concern of the plugin to maintain compliancy with semantic versioning also with the release.sanitizeVersion property enabled (or to avoid to break compatibility, to offer a new property that would allow so)

regarding the proposed solution to strip everything from the + symbol, I'm afraid it could affect some development workflows.

For example, in the affected pipeline that caused me to create this issue, we build a docker image, a helm chart (and some other artifacts) on a pr with a version that at the moment looks like this 1.351.0-dev.1.pr.489.0303690
If we strip build metadata for all published artifacts, we would have 1.351.0-dev.1.pr.489.

If we do this, if/when the pipeline (or a developer) will try to install the helm chart, it's going to be more difficult to understand which commit is installed in a dev/test cluster because developer can amend their commits on the same pr, causing the pre-release section dev.1.pr.489 to not be altered.

While if would keep the unsanitized version 1.351.0-dev.1+pr.489.0303690 for the helm chart and strip the build metada-data for the docker image 1.351.0-dev.1, we will have 2 pr builds (if the latest commit was amended) publishing the same docker tag in the registry (with the 2nd build overriding the image published by the 1st build). But there will be 2 different helm charts (because the version contains the build metadata) referencing the same docker image tag.

It's just a bit confusing and increases complexity when the time for debugging comes.