search

ajoberstar / reckon

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

Unable to rebuild final tags #108

Closed norkje closed 5 years ago

norkje commented 5 years ago
  1. Init
  2. Add feature 1
  3. Add feature 2
  4. Tag 0.1.0 from feature 1 commit
  5. Tag 0.2.0-rc.1 from feature 2 commit
  6. Add fix for feature 1 in master
  7. Create release branch release-0.1 from tag 0.1.0
  8. Cherry-pick fix for feature 1 from master into release-0.1 branch
  9. Tag 1.1.1 from release-0.1 branch
  10. git checkout 0.1.0 && ./gradlew build -> OK
  11. git checkout 0.1.1 && ./gradlew build -> OK
  12. Tag 0.2.0 from master
  13. git checkout 0.1.0 && ./gradlew build -> FAIL: Reckoned target normal version 0.2.0 has already been released.
  14. git checkout 0.1.1 && ./gradlew build -> FAIL: Reckoned target normal version 0.2.0 has already been released.

Is this the desired behavior or is this a bug?

Example is located here -> https://github.com/norkje/reckon-rebuild-final-tag-bug

mikaelvik commented 5 years ago

Reckoner::reckon checks all claimed versions. Would it be more correct to only check claimed versions that are reachable from the commit that has been tagged?

The commit with tag 0.2.0 can not be reached by the commit with tag 0.1.1.

https://github.com/ajoberstar/reckon/blob/840b4b4998e444612d84f5011a8ea08fd495fc82/reckon-core/src/main/java/org/ajoberstar/reckon/core/Reckoner.java#L61

ajoberstar commented 5 years ago

@mikaelvik I don't think the claimed version checking is the issue here. The reason we check all of them is to avoid some re-releasing something that's been released elsewhere in the repo. It shouldn't prevent you from rebuilding a tagged version.

ajoberstar commented 5 years ago

There's definitely a bug here. I was able to reproduce this with your repo and by creating a new repo describing your steps. However, I got two very different commit graphs between the two:

Your Repo

* f8637fb (origin/release-0.1, release-0.1) Fix feature 1
| * e95e0c5 (HEAD -> master, origin/master, origin/HEAD) Fix feature 1
| * 32140f1 Add feature 2
|/
* 7b07761 Add feature 1
* c2096ae Init
* 2c422b4 (tag: 0.1.1) Fix for feature 1
| * 236b83d (tag: 0.2.0) Fix for feature 1
| * 0131ee2 (tag: 0.2.0-rc.1) Add feature 2
|/
* ed96d1b (tag: 0.1.0) Add feature 1
* 3ceb4ac Init

Manual Steps

* ac5d996 (tag: 0.1.1, release-0.1) Fix feature 1
| * f6dd0f7 (tag: 0.2.0, master) Fix feature 1
|/
| * 035c80e (tag: 0.2.0-rc.1) feature 2
|/
* b963e53 (HEAD, tag: 0.1.0) feature 1
* db5ccf0 init

Your test repo seems to have some repeated history if you look at the individual branches they don't all line up with the same commits.

The main part that seems problematic in the manual one is that 0.2.0 is tagged off master and not as another commit building on top of 0.2.0-rc.1. Is this intentional? If so, can you explain your workflow more, so I can see if that jives with how I expect reckon to behave?

At a minimum there's something that could be validated or enforced here, but the behavior definitely seems wrong.

ajoberstar commented 5 years ago

Actually, I tracked those manual steps wrong, let me try that again.

ajoberstar commented 5 years ago

Yeah, this looks way more correct:

* d13db36 (tag: 0.1.1, release-0.1) Fix feature 1
| * f356552 (tag: 0.2.0, master) Fix feature 1
| * e0b778e (tag: 0.2.0-rc.1) feature 2
|/
* 589825c (HEAD, tag: 0.1.0) feature 1
* 49631b4 Init

So yeah, this is definitely a bug. It seems to be thinking 0.2.0 is the current version on the 0.1.0 and 0.1.1 tags, which it obviously isn't. May need to add some more logging to reckon to make it more apparent what's going wrong here.

mikaelvik commented 5 years ago

Great that you were able to reproduce the bug.

I work with @norkje and we're eager to start using reckon. Not being able to rebuild previous versions would definetely be a no-go.