fix(packages): list highest CRAN version in PACKAGES index (#39793)

Fixes https://github.com/go-gitea/gitea/issues/35728

---------

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

models/packages/cran/search.go

@@ -58,18 +58,30 @@ func (opts *SearchOptions) toConds() builder.Cond {
 	return cond
 }
 
-func SearchLatestVersions(ctx context.Context, opts *SearchOptions) ([]*packages.PackageVersion, error) {
-	sess := db.GetEngine(ctx).
+func SearchLatestVersions(ctx context.Context, opts *SearchOptions) (latest []*packages.PackageVersion, err error) {
+	var allPkgVers []*packages.PackageVersion
+	err = db.GetEngine(ctx).
 		Table("package_version").
 		Select("package_version.*").
-		Join("LEFT", "package_version pv2", builder.Expr("package_version.package_id = pv2.package_id AND pv2.is_internal = ? AND (package_version.created_unix < pv2.created_unix OR (package_version.created_unix = pv2.created_unix AND package_version.id < pv2.id))", false)).
 		Join("INNER", "package", "package.id = package_version.package_id").
 		Join("INNER", "package_file", "package_file.version_id = package_version.id").
-		Where(opts.toConds().And(builder.Expr("pv2.id IS NULL"))).
-		Asc("package.name")
+		Where(opts.toConds()).
+		Asc("package.name", "package.id").
+		Find(&allPkgVers)
+	if err != nil {
+		return nil, err
+	}
 
-	pvs := make([]*packages.PackageVersion, 0, 10)
-	return pvs, sess.Find(&pvs)
+	// SQL can't order R version strings, so pick the highest version per package here
+	for _, pv := range allPkgVers {
+		curIdx := len(latest) - 1
+		if curIdx == -1 || latest[curIdx].PackageID != pv.PackageID {
+			latest = append(latest, pv) // start a new package version
+		} else if cran_module.CompareVersions(pv.Version, latest[curIdx].Version) > 0 {
+			latest[curIdx] = pv // replace with a higher version
+		}
+	}
+	return latest, nil
 }
 
 func SearchFile(ctx context.Context, opts *SearchOptions) (*packages.PackageFile, error) {

modules/base/natural_sort.go

@@ -52,7 +52,7 @@ func naturalSortTrimZeros(num string) string {
 }
 
 // NaturalSortCompare compares two strings so that they could be sorted in natural order
-func NaturalSortCompare(s1, s2 string) int {
+func NaturalSortCompare(s1, s2 string) (ret int) {
 	// There is a bug in Golang's collate package: https://github.com/golang/go/issues/67997
 	// text/collate: CompareString(collate.Numeric) returns wrong result for "0.0" vs "1.0" #67997
 	// So we need to handle the number parts by ourselves
@@ -70,8 +70,7 @@ func NaturalSortCompare(s1, s2 string) int {
 				}
 				if cmp := strings.Compare(num1, num2); cmp != 0 {
 					return cmp // equal digit count, so byte order is numeric order
-				}
-				return len(part1) - len(part2) // equal value, fewer leading zeros sorts first
+				} // else: naturally equal value, leave it to the outer return
 			}
 		} else {
 			if cmp := c.CompareString(part1, part2); cmp != 0 {
@@ -80,5 +79,6 @@ func NaturalSortCompare(s1, s2 string) int {
 		}
 		pos1, pos2 = end1, end2
 	}
-	return len(s1) - len(s2)
+	// each field is naturally equal, just use string comparison: "v1.02.3" < "v1.2.03"
+	return strings.Compare(s1, s2)
 }

modules/base/natural_sort_test.go

@@ -21,6 +21,8 @@ func TestNaturalSortLess(t *testing.T) {
 	testLess("", "a")
 	testLess("", "1")
 
+	testLess("v1.02.3", "v1.2.03")
+	testLess("1.01.0", "1.1.1")
 	testLess("v1.2", "v1.2.0")
 	testLess("v1.2.0", "v1.10.0")
 	testLess("v1.20.0", "v1.29.0")
@@ -42,7 +44,7 @@ func TestNaturalSortLess(t *testing.T) {
 	testLess("0.txt", "1.txt")
 
 	testLess("file0001", "file2")
-	testLess("a8", "a08")
+	testLess("a08", "a8")
 	testLess("00", "1")
 	testLess("0", "00") // equal value, fewer leading zeros sorts first
 }

modules/packages/cran/metadata.go

@@ -7,12 +7,14 @@ import (
 	"archive/tar"
 	"archive/zip"
 	"bufio"
+	"cmp"
 	"compress/gzip"
 	"io"
 	"path"
 	"regexp"
 	"strings"
 
+	"gitea.dev/modules/base"
 	"gitea.dev/modules/packages"
 	"gitea.dev/modules/util"
 )
@@ -234,6 +236,11 @@ func setField(p *Package, data string) error {
 	return nil
 }
 
+// CompareVersions returns -1, 0 or 1, R treats "." and "-" as equivalent separators
+func CompareVersions(a, b string) int {
+	return cmp.Compare(base.NaturalSortCompare(strings.ReplaceAll(a, "-", "."), strings.ReplaceAll(b, "-", ".")), 0)
+}
+
 func splitAndTrim(s string) []string {
 	items := strings.Split(s, ", ")
 	for i := range items {

modules/packages/cran/metadata_test.go

@@ -159,3 +159,20 @@ func TestParseDescription(t *testing.T) {
 		assert.True(t, p.Metadata.NeedsCompilation)
 	})
 }
+
+func TestCompareVersions(t *testing.T) {
+	cases := []struct {
+		a, b     string
+		expected int
+	}{
+		{"1.0.3", "1.0.2", 1},
+		{"0.9.10", "0.9.9", 1},
+		{"2.5.2", "3.0.2", -1},
+		{"1.0-1", "1.0.0", 1},
+		{"1.0-2", "1.0.2", 0},
+		{"1.0", "1.0.0", -1},
+	}
+	for _, c := range cases {
+		assert.Equal(t, c.expected, CompareVersions(c.a, c.b), "%s vs %s", c.a, c.b)
+	}
+}

tests/integration/api_packages_cran_test.go

@@ -4,7 +4,6 @@
 package integration
 
 import (
-	"archive/tar"
 	"bytes"
 	"compress/gzip"
 	"fmt"
@@ -46,22 +45,6 @@ func TestPackageCran(t *testing.T) {
 	url := fmt.Sprintf("/api/packages/%s/cran", user.Name)
 
 	t.Run("Source", func(t *testing.T) {
-		createArchive := func(filename string, content []byte) *bytes.Buffer {
-			var buf bytes.Buffer
-			gw := gzip.NewWriter(&buf)
-			tw := tar.NewWriter(gw)
-			hdr := &tar.Header{
-				Name: filename,
-				Mode: 0o600,
-				Size: int64(len(content)),
-			}
-			tw.WriteHeader(hdr)
-			tw.Write(content)
-			tw.Close()
-			gw.Close()
-			return &buf
-		}
-
 		t.Run("Upload", func(t *testing.T) {
 			defer tests.PrintCurrentTest(t)()
 
@@ -70,16 +53,14 @@ func TestPackageCran(t *testing.T) {
 			req := NewRequestWithBody(t, "PUT", uploadURL, bytes.NewReader([]byte{}))
 			MakeRequest(t, req, http.StatusUnauthorized)
 
-			req = NewRequestWithBody(t, "PUT", uploadURL, createArchive(
-				"dummy.txt",
-				[]byte{},
-			)).AddBasicAuth(user.Name)
+			req = NewRequestWithBody(t, "PUT", uploadURL, test.WriteTarCompression(gzip.NewWriter, map[string]string{
+				"dummy.txt": "",
+			})).AddBasicAuth(user.Name)
 			MakeRequest(t, req, http.StatusBadRequest)
 
-			req = NewRequestWithBody(t, "PUT", uploadURL, createArchive(
-				"package/DESCRIPTION",
-				createDescription(packageName, packageVersion),
-			)).AddBasicAuth(user.Name)
+			req = NewRequestWithBody(t, "PUT", uploadURL, test.WriteTarCompression(gzip.NewWriter, map[string]string{
+				"package/DESCRIPTION": string(createDescription(packageName, packageVersion)),
+			})).AddBasicAuth(user.Name)
 			MakeRequest(t, req, http.StatusCreated)
 
 			pvs, err := packages.GetVersionsByPackageType(t.Context(), user.ID, packages.TypeCran)
@@ -99,10 +80,9 @@ func TestPackageCran(t *testing.T) {
 			assert.Equal(t, fmt.Sprintf("%s_%s.tar.gz", packageName, packageVersion), pfs[0].Name)
 			assert.True(t, pfs[0].IsLead)
 
-			req = NewRequestWithBody(t, "PUT", uploadURL, createArchive(
-				"package/DESCRIPTION",
-				createDescription(packageName, packageVersion),
-			)).AddBasicAuth(user.Name)
+			req = NewRequestWithBody(t, "PUT", uploadURL, test.WriteTarCompression(gzip.NewWriter, map[string]string{
+				"package/DESCRIPTION": string(createDescription(packageName, packageVersion)),
+			})).AddBasicAuth(user.Name)
 			MakeRequest(t, req, http.StatusConflict)
 		})
 
@@ -226,4 +206,19 @@ func TestPackageCran(t *testing.T) {
 			assert.Contains(t, resp.Header().Get("Content-Type"), "application/x-gzip")
 		})
 	})
+
+	t.Run("EnumerateAfterOlderUpload", func(t *testing.T) {
+		defer tests.PrintCurrentTest(t)()
+
+		req := NewRequestWithBody(t, "PUT", url+"/src", test.WriteTarCompression(gzip.NewWriter, map[string]string{
+			"package/DESCRIPTION": string(createDescription(packageName, "1.0.2")),
+		})).AddBasicAuth(user.Name)
+		MakeRequest(t, req, http.StatusCreated)
+
+		for _, indexURL := range []string{url + "/src/contrib/PACKAGES", url + "/bin/windows/contrib/4.2/PACKAGES"} {
+			req = NewRequest(t, "GET", indexURL).AddBasicAuth(user.Name)
+			resp := MakeRequest(t, req, http.StatusOK)
+			assert.Contains(t, resp.Body.String(), "Version: "+packageVersion)
+		}
+	})
 }