ajoberstar / reckon

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

distinguish between prerelease strategies again #85

Closed esz closed 6 years ago

esz commented 6 years ago

When building from a dirty repo on a commit tagged with a final release, the behavior of the reckoner is inconsistent when using snapshotFromProp().

The following tests (https://github.com/esz/reckon/commit/08ab81d6f092f08e094f203cf149f46c1e067926) reproduce this behavior:

  def 'if building from a dirty repo on a commit tagged with a final version without supplying a stage, the next snapshot is built'() {
    given:
    VcsInventory inventory = new VcsInventory(
      'abcdef',
      false,
      Version.valueOf('2.0.0'),
      Version.valueOf('2.0.0'),
      Version.valueOf('2.0.0'),
      0,
      [] as Set,
      [Version.valueOf('1.0.0'), Version.valueOf('2.0.0')] as Set
    )
    expect:
    reckonSnapshot(inventory, null, null) == '2.1.0-SNAPSHOT'
  }

  def 'if building from a dirty repo on a commit tagged with a final version supplying a "snapshot" stage, the next snapshot is built'() {
    given:
    VcsInventory inventory = new VcsInventory(
      'abcdef',
      false,
      Version.valueOf('2.0.0'),
      Version.valueOf('2.0.0'),
      Version.valueOf('2.0.0'),
      0,
      [] as Set,
      [Version.valueOf('1.0.0'), Version.valueOf('2.0.0')] as Set
    )
    expect:
    reckonSnapshot(inventory, null, 'snapshot') == '2.1.0-SNAPSHOT'
  }

Both tests expect the same result, once supplying the -Preckon.stage=snapshot property and once not. IMHO both tests should yield 2.1.0-SNAPSHOT however the second test fails with "Cannot release a final or significant stage without a clean repo."

This behavior is due to

    if (stage != null && !inventory.isClean()) {
      throw new IllegalStateException("Cannot release a final or significant stage without a clean repo.");
    }

in org.ajoberstar.reckon.core.Reckoner.reckonTargetVersion(VcsInventory, Version)

This makes perfectly sense when using stageFromProp(), since when the user is setting a stage on the CLI this should yield a significant (rc, milestone, beta, whatever..) Version. However, it also causes the inconsistent behavior from above, when using snapshotFromProp()

Fixing this by simply excluding "snapshot" in the condition above is not sufficient, since one never knows if we have stageFromProp('snapshot', ...). Please have a look at the changes in https://github.com/esz/reckon/tree/distinguish_between_prerelease_strategies where I attempted to solve this issue.

I introduced org.ajoberstar.reckon.core.Stages and two implementation which are (or will be) responsible for:

ajoberstar commented 6 years ago

Thanks for all of the detail and implementation ideas!

I agree with changing the behavior you see in the tests. As for how to do it, I think there are smaller changes that would get us there: