Best practices for local development with the Reckon-Plugin

klu2 commented 2 years ago

First and foremost, thanks for this great plugin, we are using it in some of our projects and have even included it in our own gradle autoconfigure-plugin.

Still, when doing local development, one question came up. It's not a bug per se, it's more a question of a best practice to be added to the documentation, the only thing I found in the github issues was this comment in #60.

Problem Statement

If you have the Reckon-Plugin applied using the Stage Version Scheme then during local development you will usually end up with insignificant versions.

if your repo is clean, ending with the git hash (won't be the case too often during development)
if it is dirty, ending with a timestamp (this is the usual case)

If you use the version in any of your artifacts (i.e. as Implementation-Version in the JAR Manifest) then this will lead to dirty builds, i.e. Gradle's UP-TO-DATE logic will retrigger a full build always for each module even if no sources have changed.

Even worse, once your repo is dirty, you will have an almost full build (at least repackaging JARs) on every build run (as the version includes the current timestamp).

Especially in projects with a lot of modules this leads to really long build times locally.

Possible Solutions

In order to solve this, I see 3 different approaches:

Only use snapshots()

One possibility would be to switch to the Snapshot Version Scheme, this would produce versions like 1.0.0-SNAPSHOT and you are not dependent on the git hash or the timestamp anymore.

Problem: You lose the possibility to work with stages like milestone and rc.

Use snapshots() locally

Another option would be to configure snapshots() only for local builds, and use stages("rc", "final") only on the CI server. That way you would see nice build numbers on your CI server (using the stage version scheme) but still have a more stable version locally.

Problem: If you once create a significant version (i.e. rc.1), then attempting to do snapshots() results in an Exception:

Reckoned version 1.0.0-SNAPSHOT is (and cannot be) less than base version 1.0.0-rc.1

So this is also no option.

Do not use the Reckon plugin locally

The last option is to simple never apply the Reckon plugin locally for development but only only on your CI server. For local development you could then use a hardcoded version (i.e. 1.0.0-SNAPSHOT).

This will then force you though to never do releases from your local environment (which you shouldn't do anyways). What would bother me a bit more is the fact that you then locally don't see the version you're current contributing to anymore (especially in multi-branch-environments).

Summary

Did I miss something? What would be your recommendations here? Could we add that to the docs? Thanks!

ajoberstar commented 2 years ago

A variant of "don't use reckon locally" would be "don't use the version in artifacts locally". So instead of conditionally applying reckon, conditionally set the Implementation-Version in your manifest. If you have more extensive usage of the version in other places I could see that being a pain, but if it's just the manifest, that feels like a better place to do the conditional.

klu2 commented 2 years ago

That wouldn't be sufficient, as in typical gradle builds the version number is also part of the generated JAR file. I could turn that off as well of course but if I'm not using the version locally anywhere then I'm questioning why I need the plugin at all (besides a println of the reckoned version to the gradle log at the beginning of each build)

ajoberstar commented 2 years ago

Sure, that makes sense. Will keep this open as a doc task for when I make my next updates.

klu2 commented 2 years ago

Hm, maybe my second option up there ("use snapshots locally") could still be an option. As mentioned above, currently that breaks because of

Reckoned version 1.0.0-SNAPSHOT is (and cannot be) less than base version 1.0.0-rc.1

Your doc also says:

This approach is tuned to ensure it sorts correctly both with SemVer rules and Gradle's built in version sorting (which are subtly different).

But I just did a local test now using Maven Artifact on the one side and the internal Gradle mechanism:

import org.apache.maven.artifact.versioning.ComparableVersion
import org.gradle.util.internal.VersionNumber
import org.junit.jupiter.api.Test

class VersionUtilTest {

    private val versions = listOf(
        "1.0.0",
        "1.0.0-SNAPSHOT",
        "1.0.0-rc.1.8+3bb4161",
        "1.0.0-rc.1",
        "1.0.0-milestone.1.0+2cc3321",
        "1.0.0-rc.1.9+4cc4322",
    )

    @Test
    fun sortVersionsWithMaven() {
        println(
            versions.map { ComparableVersion(it) }.sorted().joinToString(separator = System.lineSeparator())
        )
    }

    @Test
    fun sortVersionsWithGradle() {
        println(
            versions.map { VersionNumber.parse(it) }.sorted().joinToString(separator = System.lineSeparator())
        )
    }
}

Both tests print the following:

1.0.0-milestone.1.0+2cc3321
1.0.0-rc.1
1.0.0-rc.1.8+3bb4161
1.0.0-rc.1.9+4cc4322
1.0.0-SNAPSHOT
1.0.0

The means, according to both the official Maven-Logic and the internal Gradle one, 1.0.0-SNAPSHOT comes after 1.0.0-rc.1 but reckon tells the opposite.

Why? Because reckon uses neither of those two libraries, but instead uses Java Semver.

And if I run the same tests here again:

import com.github.zafarkhaja.semver.Version

@Test
fun sortVersionsWithJavaSemver() {
    println(
        versions.map { Version.valueOf(it) }.sorted().joinToString(separator = System.lineSeparator())
    )
}

then I get this output:

1.0.0-SNAPSHOT
1.0.0-milestone.1.0+2cc3321
1.0.0-rc.1
1.0.0-rc.1.8+3bb4161
1.0.0-rc.1.9+4cc4322
1.0.0

And this explains the exception with Reckon.

Don't know who's right or wrong here (Java Semver is on 0.9 though, and Apache Maven and Gradle are quite popular), but if you'd change that in Reckon, it would most likely be a breaking change.

But if you do this, then we could go with choice 2, i.e. using snapshots locally and let the CI server create insignificant versions (which I'd favor a lot)

klu2 commented 2 years ago

Puh, the more I dig into that topic the more confusing it gets. After filing https://github.com/zafarkhaja/jsemver/issues/62 I had a look at some more (conflicting) docus.

The SemVer spec says

Identifiers with letters or hyphens are compared lexically in ASCII sort order.

Gradle says:

The strings rc, snapshot, final, ga, release and sp are considered higher than any other string part (sorted in this order): 1.0-zeta < 1.0-rc < 1.0-snapshot < 1.0-final < 1.0-ga < 1.0-release < 1.0-sp < 1.0.

So, I enhanced my test a bit:

package io.cloudflight.gradle.autoconfigure.util

import com.github.zafarkhaja.semver.Version
import org.apache.maven.artifact.versioning.ComparableVersion
import org.gradle.util.internal.VersionNumber
import org.junit.jupiter.api.Test

class VersionUtilTest {

    private val versions = listOf(
        "1.0.0",
        "1.0.0-sp",
        "1.0.0-SNAPSHOT",
        "1.0.0-snapshot",
        "1.0.0-release",
        "1.0.0-final",
        "1.0.0-rc.1.8+3bb4161",
        "1.0.0-rc.1",
        "1.0.0-ga",
        "1.0.0-milestone.1.0+2cc3321",
        "1.0.0-rc.1.9+4cc4322",
    )

    @Test
    fun sortVersionsWithMaven() {
        sortAndPrintVersions { ComparableVersion(it) }
    }

    @Test
    fun sortVersionsWithGradle() {
        sortAndPrintVersions(VersionNumber::parse)
    }

    @Test
    fun sortVersionsWithJavaSemver() {
        sortAndPrintVersions(Version::valueOf)
    }

    private fun <R : Comparable<R>> sortAndPrintVersions(transform: (String) -> R) {
        println(transform)
        println(
            versions.map { transform.invoke(it) }.sorted().joinToString(separator = System.lineSeparator())
        )
        println("")
    }
}

And the suprising output is:

(kotlin.String) -> org.apache.maven.artifact.versioning.ComparableVersion
1.0.0-milestone.1.0+2cc3321
1.0.0-rc.1
1.0.0-rc.1.8+3bb4161
1.0.0-rc.1.9+4cc4322
1.0.0-SNAPSHOT
1.0.0-snapshot
1.0.0
1.0.0-final
1.0.0-ga
1.0.0-sp
1.0.0-release

fun parse(kotlin.String!): org.gradle.util.internal.VersionNumber!
1.0.0-final
1.0.0-ga
1.0.0-milestone.1.0+2cc3321
1.0.0-rc.1
1.0.0-rc.1.8+3bb4161
1.0.0-rc.1.9+4cc4322
1.0.0-release
1.0.0-SNAPSHOT
1.0.0-snapshot
1.0.0-sp
1.0.0

fun valueOf(kotlin.String!): com.github.zafarkhaja.semver.Version!
1.0.0-SNAPSHOT
1.0.0-final
1.0.0-ga
1.0.0-milestone.1.0+2cc3321
1.0.0-rc.1
1.0.0-rc.1.8+3bb4161
1.0.0-rc.1.9+4cc4322
1.0.0-release
1.0.0-snapshot
1.0.0-sp
1.0.0

So it seems that when it comes to the details, everyone has his own truth (not sticking to any standards too much).

Interestingly, as the SemVer-Plugin seems to really follow the spec, 1.0.0-snapshot comes far after 1.0.0-SNAPSHOT (as it sorts by ASCII char), and as s for snapshot comes after r for rc one possibly solution just for this plugin could be to change the version from -SNAPSHOT to -snapshot when applying the Snapshot Versioning Scheme. But also not too nice....

Still, this plugin here is a Gradle plugin and it may make sense to stick to the Gradle-Versioning scheme here (or at least make it configurable). What do you think?

ajoberstar commented 2 years ago

Reckon was specifically designed for SemVer, so following the spec is core to the plugin's goal. The stage scheme was designed to sort correctly under SemVer spec and Gradle's sorting.

Snapshots were added as an alternative for people who prefer that model, but were meant to be mutually exclusive from the stage scheme. So now that I say that, option 2 wouldn't be something I'd recommend.

klu2 commented 2 years ago

As one of the Gradle Core Devs sent me an info on twitter that there is even another implementation inside Gradle, I created that repository for your reference: https://github.com/cloudflightio/semantic-versioning

big-guy commented 2 years ago

If you use the version in any of your artifacts (i.e. as Implementation-Version in the JAR Manifest) then this will lead to dirty builds, i.e. Gradle's UP-TO-DATE logic will retrigger a full build always for each module even if no sources have changed.

A different way of addressing the symptom is...

Don't include version number in the jar or the version number should only ever be the final release
Don't include version number in the name of the jar file

If you do both of those things, it doesn't matter what the versioning scheme is.

As one of the Gradle Core Devs sent me an info on twitter that there is even another implementation inside Gradle, I created that repository for your reference: cloudflightio/semantic-versioning

And just to be clear, the VersionInfo implementation is the implementation used by Gradle when sorting versions. VersionNumber was only ever intended to be used to parse version-like strings. Its sorting order isn't meaningful. It's also going to be removed in 9.0.

klu2 commented 2 years ago

If you do both of those things, it doesn't matter what the versioning scheme is.

@ajoberstar already suggested that above but my point was that if the plugin doesn't do anything locally anymore (i.e. it's only output, the version number, is not being used anywhere, then why should I apply it at all?

For our plugin on top ot fht reckon-plugin we already found a solution, but I also understand @ajoberstar 's points to leave things as they are here, so we should just make that clear to developers in the documentation.

ajoberstar / reckon