ajoberstar / reckon

Infer a project's version from your Git repository.
Apache License 2.0
191 stars 28 forks source link

`+` in reckoned insignificant version is not docker tag compatible #184

Closed jnehlmeier closed 2 years ago

jnehlmeier commented 2 years ago

When packaging an app into a docker image I want to use the reckoned version as image tag. However when you want to publish nightly builds from your CI/CD environment the reckoned version is typically an insignificant version and thus contains +.

Docker does not allow + in tag names, so something like my-registry:8888/my-webapp:1.3.0-rc.1.8+3bb4161 fails.

Quote from docker tag documentation:

Name components may contain lowercase letters, digits and separators. A separator is defined as a period, one or two underscores, or one or more dashes. A name component may not start or end with a separator.

So any chance to replace + with dash or underscore for insignificant version or provide configuration options for separators?

1.3.0-rc.1.8+3bb4161 vs 1.3.0-rc.1.8-3bb4161 or 1.3.0-rc.1.8_3bb4161

ajoberstar commented 2 years ago

For semver compatibility (which is the core scope of reckon) and proper sorting the plus is needed. For cases like this I think it makes more sense to customize the version going into the docker command.

Unsure exactly how this is configured for Docker, but an illustration of what I mean

tasks.named("dockerBuild")  {
  tag.set(reckon.getVersion().map { v -> v.toString().replace('+', '-') })
}
jnehlmeier commented 2 years ago

Fair enough. I didn't know that semver has the plus sign in its specification. Will stick to manually modifying the version.

But given that docker is pretty dominant you might want to think about something like

reckon.getVersion() // delegates to Compatibility.SEMVER
reckon.getVersion(Compatibility.SEMVER)
reckon.getVersion(Compatibility.DOCKER_TAGS)

And Compatibility.DOCKER_TAGS then contains the replacement code. Then nobody has do to it again and again in their projects.

ajoberstar commented 2 years ago

Can understand the requirement, but I consider this out of scope so will close as won't fix.