Unreachable git commits are used for Go module version determination

[BatmanAoD]

I have a branch in which I've created a release rc tag. In a completely separate branch, from which the rc tag is not reachable, I'm getting an "inconsistent versions" error that cites the rc tag, saying Found <rc-tag> which does not match <actual> (where <rc-tag> matches the tag that isn't reachable, and <actual> is the expected version).

I can verify that <actual> is the most recent reachable tag using git describe:

> git describe --match 'v*'
v<actual>-2-ga7d9350

[dbanty]

Oh good, a git issue, those are usually easy to track down 😅. If you’re able to create a reproduction and paste the git commands that set up a problematic repo, that’d be super helpful!

If not, I’ll try to tinker around and find it. I don’t think I have any tests with tags on multiple branches yet, so maybe it’s as simple as doing that 🤔

[BatmanAoD]

Hm, I thought I'd be able to, but I set up a minimal repo with the setup I described above (using 0.0.2 as the "actual" version and 1.0.0-rc.1 as the RC version), and it doesn't have the same behavior; Knope recognizes that 0.0.2 is the current version. I'm not sure what the difference is that's causing the real repository to behave differently, so I guess for now you can just consider this "cannot reproduce."

[BatmanAoD]

(I have added some other oddities to that demo repo try to better match the repo where I saw the problem, including adding a subdir/v0.0.2 tag that doesn't match the v0.0.2 tag. Since I haven't yet reproduced the behavior, I'm assuming that these issues aren't relevant.)

[dbanty]

@BatmanAoD is it possible this is related to #502 and that the perceived, unreachable tag was not the tag being read—but instead there was a tag in the wrong format which knope was using?

[BatmanAoD]

Hm...I will play with the dummy repo a bit more to see if maybe that can cause it, but I'm pretty sure it's definitely the case that I was seeing a version based on an unreachable tag.

[BatmanAoD]

No, this behavior definitely occurs even when there is only one possible tag where it could be getting that specific version. git tag --merged does not include the unreachable tag.

[BatmanAoD]

Okay, I've updated the demo repo and it now demonstrates the issue; on the main branch, the "latest version" is the prerelease tag, even though that tag isn't reachable from the head of the main branch.

Looking at the code, it seems that the determination of the current version doesn't actually use the HEAD commit at all, which means that unreachable commits will always be taken into account (so I'm not really sure why I wasn't able to reproduce this the first time). That doesn't seem like the right behavior to me, since you should be able to make new stable minor and patch releases from any major version; and in particular, you should be able to make minor and patch releases on the current stable version while working on release-candidates for the next major version in a separate long-lived branch.

It also appears that when collecting commit messages, only the latest version is used to stop traversal. Unfortunately, I don't think this will ever work reliably, because if merges are not squashed, there will often be a way to "bypass" a specific release while traversing the parent commits. I believe this is why we've seen Knope decide for some repositories that it is always time for a "major" release.

[BatmanAoD]

Here's a visual demonstration of how there can be a commit graph that permits traversal to "bypass" major tags. The "merge" (branch) commit's latest version is major-tag, because major-tag is reachable from that commit. But "breaking is also reachable, via the other parent. When determining the comments to include when determining the next release from "merge" (trunk), the "fix" commit should be included (since it's not reachable from the last major release), but the "breaking" commit should not, since it was already included in the last major release.

I think the way to reliably determine the correct set of commits must be:

• Collect all tags reachable from the current commit. The highest-version tag (including prereleases) is the most recent release.
• Collect all commits reachable from the current commit, except all commits that are reachable from the most recent release. This should be equivalent to the list you'd see if you ran git rebase -i <current-version>.

[BatmanAoD]

Well... I think I am wrong about the current implementation handling that specific case of "bypassing" the last tag, because I built a repo to check and Knope correctly recognizes the next release as v2.0.1. But that makes it even stranger that in some cases Knope does seem to go much further back in the commit history than it should.

[dbanty]

Yeah… I tried to build a repro test to validate a fix and it worked properly (so I couldn’t validate). But then I manually made a repo and was able to get it to do the wrong thing. So… something is very confusing. I think I can use a rev parse function to do things properly.

[BatmanAoD]

For what it's worth, I started a discussion in the git-oxide repo about constructing a "diff" of commits like this: Byron/gitoxide#1017

[BatmanAoD]

...however, this specific issue was originally just about ignoring unreachable tags when determining the current version. That part, at least, seems fairly straightforwardly solvable.

[dbanty]

Okay, I figured out why it's inconsistent! It's doing a breadth-first search right now, but the inner-sorting of that breadth-first seems to be topological order.. or something equivalent. So if I merge main into breaking before committing the second fix, it just so happens that the order Knope processes the commits means it won't see the the previous tag until after it goes too far down the branch. None that that really matters (beyond giving me a failing test case to test the solution against), but it's very satisfying to know what was happening 😅.

I've created a really bad solution in #574 just to get the bug fixed—it's going to tank performance on really large projects, but I don't have time to build a graph-painting algorithm right now and I don't want this to keep blocking you 😁. It'll be a fun future enhancement to make commit traversal much more efficient (especially for linear histories).

[BatmanAoD]

I think slow but reliable is good for now!

What about the issue of making sure the last-release is actually reachable from the current commit? I only skimmed the changeset, but it looks like the determination of the current version hasn't changed?

[dbanty]

Ah, right. Well versioned files will be preferred, so now that even go.mod has a version in comment it will usually not be an issue. But it’s definitely still a bug that needs fixing!

[dbanty]

Okiedoke, I fixed the other issue also in #574 using a similar solution—iterate all commits that are reachable in order to figure out which tags are relevant. There's probably a better way to do this, but I don't know it 😅. Again, thinking it's better to fix the bug first and worry about performance as needed.

So I think that means that that branch will work for your projects now 🤞