ajoberstar / reckon

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

When using SnapshotPreReleaseStrategy the Reckoner always returns the current version and never the target version #69

Closed cmuchinsky closed 6 years ago

cmuchinsky commented 6 years ago

The following test added to BaseCompatTest.groovy demonstrates the issue:

  def 'if prerelease strategy is snapshot, version should increment and end with -SNAPSHOT'() {
    given:
    def local = Grgit.clone(dir: projectDir, uri: remote.repository.rootDir)
    local.tag.add(name: '1.1.0', message: '1.1.0')

    buildFile << """
plugins {
  id 'org.ajoberstar.grgit'
  id 'org.ajoberstar.reckon'
}

reckon {
  normal = scopeFromProp()
  preRelease = snapshotFromProp()
}

task printVersion {
  doLast  {
    println project.version
  }
}
"""
    when:
    def result = build('printVersion')
    then:
    result.output.contains('Reckoned version: 1.2.0-SNAPSHOT')
  }
ajoberstar commented 6 years ago

The documentation is lacking in describing this, this is intended behavior. When building from a HEAD that is already tagged with a version, the assumption is that you are rebuilding that version. If you want to release that already tagged commit with a new version, you'll need to explicitly provide the scope or snapshot property:

./gradlew printVersion -Preckon.scope=minor
cmuchinsky commented 6 years ago

I wasn't able to find any combination of properties that allowed me to get it to "reckon" a -SNAPSHOT version. I am explicitly setting both reckon.scope and reckon.snapshot but couldn't get it to ever create a -SNAPSHOT version.

cmuchinsky commented 6 years ago

As an example, here is what I tried (edited for brevity):

$ ./gradlew 
Reckoned version: 1.0.100
BUILD SUCCESSFUL in 0s
1 actionable task: 1 executed

$ ./gradlew -Preckon.scope=patch -Preckon.snapshot=true
Reckoned version: 1.0.100
BUILD SUCCESSFUL in 0s
1 actionable task: 1 executed
ajoberstar commented 6 years ago

This is where it decides to consider something a "rebuild". The logic's not the easiest to read, but effectively:

  1. If the HEAD is tagged with a "pre-release" version (e.g. 1.0.0-beta.1), you can release it as a higher stage (e.g. 1.0.0-rc.1 or 1.0.0).
  2. If the HEAD is tagged with a "final" version (e.g. 1.2.3), you can release it as an incremented "final" version only (e.g. 1.3.0).

Use cases being:

  1. You may have released a beta, not found any major bugs and decide, "this is ready to be a release candidate" or "this is ready to be the final version".
  2. You may have mistakenly released something as a patch version and then realized it contained a new feature (or a breaking change), so you want to re-release it with the correct increment to meet SemVer spec. The assumption at that time is you wouldn't even need to consider a "final" version a "pre-release" of anything else.

These were some (probably pretty non-obvious, hence #46) hueristics in trying to determine if you "really" mean to have an incremented version or if you're just trying to reproduce an existing version.

Any feedback you have on this would be helpful.

ajoberstar commented 6 years ago

Given that, I would expect that you could do:

$ ./gradlew -Preckon.scope=patch -Preckon.snapshot=false

And get 1.0.101.

cmuchinsky commented 6 years ago

For my use case every build done by our build system is considered "final" and should bump the patch. For developers working locally, if they have a dirty repository we want to append "-SNAPSHOT" so it locally takes precedence over the final version and its obvious that it didn't come from our official build system.

cmuchinsky commented 6 years ago

There is also a behavior in the deprecated release-base plugin whereby calling release on a dirty repository blows up. This is a behavior we want to preserve going forward as well, although perhaps with a better error message because what shows up now is somewhat cryptic (something about no version strategies). I suppose I could simply add the check and failure condition in a reckonTagPush.doFirst {} override if this isn't something that reckon will provide.

ajoberstar commented 6 years ago

I do plan to add enforcing clean repos for a final or significant version. See #67

cmuchinsky commented 6 years ago

@ajoberstar any thoughts on the use case I mentioned above, do you see a way I could do that with the current implementation?

ajoberstar commented 6 years ago

That's the same use case from #70, right? If so, I think the workaround you posted there is probably all you can do for now. Longer term, there will be a couple changes that will help:

  1. The configurable snapshotFrom that takes a closure.
  2. Support for dirty repos will include not treating a dirty repo as a rebuild, so it will always increment (and thus have a higher precedence) than a tagged version that you released from the CI.
cmuchinsky commented 6 years ago

The workaround mentioned in #70 is just to automatically set the reckon.snapshot property, but even with that hard coded to true I haven't figured out a way to get it to ever return a -SNAPSHOT version. I don't think its possible given the way the Reckoner class currently works, but was hoping maybe I was missing something.

ajoberstar commented 6 years ago

When you've tried this has it only been on a dirty repo whose HEAD is tagged with a version? Or have you tried making a commit (without a tag) and running with snapshot=true?

You may be getting bit by the rebuild logic, but it's definitely possible to get a snapshot. There do need to be improvements here though.

ajoberstar commented 6 years ago

I think 0.5.0 provides the behavior to return snapshots when the repo is dirty. You'll still have to use your workaround from #70 to always have clean repos treated as a final.