Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

In gitlab repo with v-prefixed semver tags, GSG_TAG_PREFIX != "v" fails #11

Open
brhamon opened this issue Feb 14, 2020 · 1 comment
Open

Comments

@brhamon
Copy link

brhamon commented Feb 14, 2020

Suppose that some fork of a repo would like to separately version a project. Upstream commits and tags are synced periodically. Suppose the upstream maintainers are using "v-prefixed" semver tags (which is quite common). For example, "v2.1.0", "v2.0.0", "v1.11.1", etc.

To avoid tag names clashing, the forked repo is versioned with the letter "z". For example, "z0.1.0", "z0.2.1", "z0.2.2", etc. The fork is in GitLab, and go-semrel-gitlab is being used. GSG_TAG_PREFIX is set to "z". When the release next-version --allow-current is run, the output is "2.2.0", which happens to be a major-version bump to "v2.1.0".

I've traced the root cause to github.com/juranki/go-semrel/inspectgit/inspectgit.go. The callback function addIfSemVer strips the prefix, if it is present, then calls semver.ParseTolerant. That function will ignore a leading "v", which makes all of those tags valid, regardless of the setting of GSG_TAG_PREFIX.

My proposal is to filter out tags which don't have GSG_TAG_PREFIX. Here's a patch that will do that:

diff --git a/inspectgit/inspectgit.go b/inspectgit/inspectgit.go
index 6d5e595..461d50b 100644
--- a/inspectgit/inspectgit.go
+++ b/inspectgit/inspectgit.go
@@ -74,6 +74,9 @@ func getVersions(r *git.Repository, prefix string) (map[string]semver.Version, e
        versions := make(map[string]semver.Version)

        addIfSemVer := func(sha string, version string) {
+               if !strings.HasPrefix(version, prefix) {
+                       return
+               }
                s := strings.TrimPrefix(version, prefix)
                sv, err := semver.ParseTolerant(s)
                if err == nil {

With this change, the tests still pass. It is worth noting that the new return line isn't executed, which means that -- in the test data -- all the tags passed to addIfSemVer are prefixed with v. I will experiment with test data that has a mix of "v" and "z" semver tags.

I really appreciate go-semrel, and go-semrel-gitlab. Thank you for this awesome contribution to the community.

@brhamon
Copy link
Author

brhamon commented Feb 14, 2020

After a false alarm, the proposed patch does resolve my issue.

I am still looking at updating the tests to include this use case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant