git: Support parsing of release-candidate versions

In some places, we need to check whether Git supports a certain feature
or not. As Git doesn't usually provide a direct way to test for a
feature, we instead use the version number as a proxy. This works just
fine for normal releases by just parsing the semantic version number
as a tuple of integers in the format "major.minor.patch". But we do not
correctly cope with release candidates like "2.28.0.rc0", returning an
error.

Fix this by expecting an optional fourth touple, setting a flag inside
the version structure that says that it's a release candidate.
Furthermore, we also need to extend the version comparison code: a
release candidate is always older than its respective release.

Note that we do not parse the actual release candidate number. As
numbering is zero-indexed, we cannot simply use "0" as a placeholder for
a non-release-candidate, and everything else will require additional
logic. As it's quite unlikely that we'll ever need to verify whether
a given version is a specific release candidate (new features aren't
usually introduced in between rc's and they shouldn't run in production
anyway), adding this additional logic doesn't seem to be worth it at
all.

4 files changed, 26 insertions, 2 deletions

diff --git a/changelogs/unreleased/pks-git-version-release-candidates.yml b/changelogs/unreleased/pks-git-version-release-candidates.yml
new file mode 100644
index 000000000..d5735b1ef
--- /dev/null
+++ b/changelogs/unreleased/pks-git-version-release-candidates.yml
@@ -0,0 +1,5 @@
+---
+title: Fix parsing of Git release-candidate versions
+merge_request: 2389
+author:
+type: fixed
diff --git a/internal/git/helper_test.go b/internal/git/helper_test.go
index ffbf64739..f3cd415b1 100644
--- a/internal/git/helper_test.go
+++ b/internal/git/helper_test.go
@@ -48,6 +48,7 @@ func TestSupportsDeltaIslands(t *testing.T) {
 		{version: "2.19.8", delta: false},
 		{version: "1.20.8", delta: false},
 		{version: "1.18.0", delta: false},
+		{version: "2.28.0.rc0", delta: true},
 		{version: "2.20", fail: true},
 		{version: "bla bla", fail: true},
 	}
diff --git a/internal/git/proto.go b/internal/git/proto.go
index 9cd8be66d..053215918 100644
--- a/internal/git/proto.go
+++ b/internal/git/proto.go
@@ -115,6 +115,11 @@ func versionLessThan(v1, v2 version) bool {
 	case v1.patch > v2.patch:
 		return false
 
+	case v1.rc && !v2.rc:
+		return true
+	case !v1.rc && v2.rc:
+		return false
+
 	default:
 		// this should only be reachable when versions are equal
 		return false
@@ -123,10 +128,11 @@ func versionLessThan(v1, v2 version) bool {
 
 type version struct {
 	major, minor, patch uint32
+	rc                  bool
 }
 
 func parseVersion(versionStr string) (version, error) {
-	versionSplit := strings.SplitN(versionStr, ".", 3)
+	versionSplit := strings.SplitN(versionStr, ".", 4)
 	if len(versionSplit) < 3 {
 		return version{}, fmt.Errorf("expected major.minor.patch in %q", versionStr)
 	}
@@ -142,6 +148,14 @@ func parseVersion(versionStr string) (version, error) {
 		*v = uint32(n64)
 	}
 
+	if len(versionSplit) == 4 {
+		if strings.HasPrefix(versionSplit[3], "rc") {
+			ver.rc = true
+		} else {
+			return version{}, fmt.Errorf("unknown pre-release identifier %q", versionSplit[3])
+		}
+	}
+
 	return ver, nil
 }
 
@@ -153,7 +167,7 @@ func SupportsDeltaIslands(versionStr string) (bool, error) {
 		return false, err
 	}
 
-	return !versionLessThan(v, version{2, 20, 0}), nil
+	return !versionLessThan(v, version{2, 20, 0, false}), nil
 }
 
 // NoMissingWantErrMessage checks if the git version is before Git 2.22,
diff --git a/internal/git/proto_test.go b/internal/git/proto_test.go
index 3acb952ae..2dcafbb36 100644
--- a/internal/git/proto_test.go
+++ b/internal/git/proto_test.go
@@ -84,6 +84,10 @@ func TestVersionComparator(t *testing.T) {
 		{"1.1.1", "1.0.1", false},
 		{"1.1.1", "1.1.0", false},
 		{"1.1.1", "1.1.1", false},
+
+		{"1.1.1.rc0", "1.1.1", true},
+		{"1.1.1.rc0", "1.1.1.rc0", false},
+		{"1.1.1.rc0", "1.1.0", false},
 	} {
 		actual, err := git.VersionLessThan(tc.v1, tc.v2)
 		require.NoError(t, err)