Use same SemVer logic in both Git implementations

Signed-off-by: Hidde Beydals <[email protected]>

Author: hiddeco

go.mod

@@ -6,7 +6,6 @@ replace github.com/fluxcd/source-controller/api => ./api
 
 require (
 	github.com/Masterminds/semver/v3 v3.1.1
-	github.com/blang/semver/v4 v4.0.0
 	github.com/cyphar/filepath-securejoin v0.2.2
 	github.com/fluxcd/pkg/apis/meta v0.10.0
 	github.com/fluxcd/pkg/gittestserver v0.3.0

go.sum

@@ -113,10 +113,7 @@ github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6r
 github.com/bgentry/speakeasy v0.1.0/go.mod h1:+zsyZBPWlz7T6j88CTgSN5bM796AkVf0kBD4zp0CCIs=
 github.com/bitly/go-simplejson v0.5.0/go.mod h1:cXHtHw4XUPsvGaxgjIAn8PhEWG9NfngEKAMDJEczWVA=
 github.com/bketelsen/crypt v0.0.3-0.20200106085610-5cbc8cc4026c/go.mod h1:MKsuJmJgSg28kpZDP6UIiPt0e0Oz0kqKNGyRaWEPv84=
-github.com/blang/semver v3.5.1+incompatible h1:cQNTCjp13qL8KC3Nbxr/y2Bqb63oX6wdnnjpJbkM4JQ=
 github.com/blang/semver v3.5.1+incompatible/go.mod h1:kRBLl5iJ+tD4TcOOxsy/0fnwebNt5EWlYSAyrTnjyyk=
-github.com/blang/semver/v4 v4.0.0 h1:1PFHFE6yCCTv8C1TeyNNarDzntLi7wMI5i/pzqYIsAM=
-github.com/blang/semver/v4 v4.0.0/go.mod h1:IbckMUScFkM3pff0VJDNKRiT6TG/YpiHIM2yvyW5YoQ=
 github.com/bmizerany/assert v0.0.0-20160611221934-b7ed37b82869/go.mod h1:Ekp36dRnpXw/yCqJaO+ZrUyxD+3VXMFFr56k5XYrpB4=
 github.com/bshuster-repo/logrus-logstash-hook v0.4.1 h1:pgAtgj+A31JBVtEHu2uHuEx0n+2ukqUJnS2vVe5pQNA=
 github.com/bshuster-repo/logrus-logstash-hook v0.4.1/go.mod h1:zsTqEiSzDgAa/8GZR7E1qaXrhYNDKBYy5/dWPTIflbk=

pkg/git/gogit/checkout.go

@@ -193,7 +193,7 @@ func (c *CheckoutSemVer) Checkout(ctx context.Context, path, url string, auth *g
 
 	tags := make(map[string]string)
 	tagTimestamps := make(map[string]time.Time)
-	_ = repoTags.ForEach(func(t *plumbing.Reference) error {
+	if err = repoTags.ForEach(func(t *plumbing.Reference) error {
 		revision := plumbing.Revision(t.Name().String())
 		hash, err := repo.ResolveRevision(revision)
 		if err != nil {
@@ -207,7 +207,9 @@ func (c *CheckoutSemVer) Checkout(ctx context.Context, path, url string, auth *g
 
 		tags[t.Name().Short()] = t.Strings()[1]
 		return nil
-	})
+	}); err != nil {
+		return nil, "", err
+	}
 
 	var matchedVersions semver.Collection
 	for tag, _ := range tags {

pkg/git/libgit2/checkout.go

@@ -19,8 +19,11 @@ package libgit2
 import (
 	"context"
 	"fmt"
+	"sort"
+	"time"
 
-	"github.com/blang/semver/v4"
+	"github.com/Masterminds/semver/v3"
+	"github.com/fluxcd/pkg/version"
 	git2go "github.com/libgit2/git2go/v31"
 
 	"github.com/fluxcd/pkg/gitutil"
@@ -168,7 +171,7 @@ type CheckoutSemVer struct {
 }
 
 func (c *CheckoutSemVer) Checkout(ctx context.Context, path, url string, auth *git.Auth) (git.Commit, string, error) {
-	rng, err := semver.ParseRange(c.semVer)
+	verConstraint, err := semver.NewConstraint(c.semVer)
 	if err != nil {
 		return nil, "", fmt.Errorf("semver parse range error: %w", err)
 	}
@@ -186,28 +189,61 @@ func (c *CheckoutSemVer) Checkout(ctx context.Context, path, url string, auth *g
 		return nil, "", fmt.Errorf("unable to clone '%s', error: %w", url, err)
 	}
 
-	repoTags, err := repo.Tags.List()
-	if err != nil {
-		return nil, "", fmt.Errorf("git list tags error: %w", err)
-	}
+	tags := make(map[string]string)
+	tagTimestamps := make(map[string]time.Time)
+	if err := repo.Tags.Foreach(func(name string, id *git2go.Oid) error {
+		tag, err := repo.LookupTag(id)
+		if err != nil {
+			return nil
+		}
 
-	svTags := make(map[string]string)
-	var svers []semver.Version
-	for _, tag := range repoTags {
-		v, _ := semver.ParseTolerant(tag)
-		if rng(v) {
-			svers = append(svers, v)
-			svTags[v.String()] = tag
+		commit, err := tag.Peel(git2go.ObjectCommit)
+		if err != nil {
+			return fmt.Errorf("can't get commit for tag %s: %w", name, err)
 		}
+		c, err := commit.AsCommit()
+		if err != nil {
+			return err
+		}
+		tagTimestamps[tag.Name()] = c.Committer().When
+		tags[tag.Name()] = name
+		return nil
+	}); err != nil {
+		return nil, "", err
 	}
 
-	if len(svers) == 0 {
+	var matchedVersions semver.Collection
+	for tag, _ := range tags {
+		v, err := version.ParseVersion(tag)
+		if err != nil {
+			continue
+		}
+		if !verConstraint.Check(v) {
+			continue
+		}
+		matchedVersions = append(matchedVersions, v)
+	}
+	if len(matchedVersions) == 0 {
 		return nil, "", fmt.Errorf("no match found for semver: %s", c.semVer)
 	}
 
-	semver.Sort(svers)
-	v := svers[len(svers)-1]
-	t := svTags[v.String()]
+	// Sort versions
+	sort.SliceStable(matchedVersions, func(i, j int) bool {
+		left := matchedVersions[i]
+		right := matchedVersions[j]
+
+		if !left.Equal(right) {
+			return left.LessThan(right)
+		}
+
+		// Having tag target timestamps at our disposal, we further try to sort
+		// versions into a chronological order. This is especially important for
+		// versions that differ only by build metadata, because it is not considered
+		// a part of the comparable version in Semver
+		return tagTimestamps[left.String()].Before(tagTimestamps[right.String()])
+	})
+	v := matchedVersions[len(matchedVersions)-1]
+	t := v.Original()
 
 	ref, err := repo.References.Dwim(t)
 	if err != nil {