From efb0b34395516e8ac6e068d471b8f4d595e0f7d0 Mon Sep 17 00:00:00 2001 From: Mitrahsoft Date: Sat, 17 Oct 2026 12:18:48 +0530 Subject: [PATCH] fix(packages): list highest CRAN version in PACKAGES index (#39793) Fixes https://github.com/go-gitea/gitea/issues/35728 --------- Co-authored-by: wxiaoguang --- models/packages/cran/search.go | 26 +++++++--- modules/base/natural_sort.go | 8 ++-- modules/base/natural_sort_test.go | 4 +- modules/packages/cran/metadata.go | 7 +++ modules/packages/cran/metadata_test.go | 17 +++++++ tests/integration/api_packages_cran_test.go | 53 ++++++++++----------- 6 files changed, 74 insertions(+), 41 deletions(-) diff --git a/models/packages/cran/search.go b/models/packages/cran/search.go index 0ff1c31edef..e6c9b658725 100644 --- a/models/packages/cran/search.go +++ b/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) { diff --git a/modules/base/natural_sort.go b/modules/base/natural_sort.go index 6a821f4d78a..170741faf9c 100644 --- a/modules/base/natural_sort.go +++ b/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) } diff --git a/modules/base/natural_sort_test.go b/modules/base/natural_sort_test.go index 7e0815eee13..31f7ab1ccbe 100644 --- a/modules/base/natural_sort_test.go +++ b/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 } diff --git a/modules/packages/cran/metadata.go b/modules/packages/cran/metadata.go index 36bb38096f6..83f075889fe 100644 --- a/modules/packages/cran/metadata.go +++ b/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 { diff --git a/modules/packages/cran/metadata_test.go b/modules/packages/cran/metadata_test.go index 1d652a4a058..c7b09ba4670 100644 --- a/modules/packages/cran/metadata_test.go +++ b/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) + } +} diff --git a/tests/integration/api_packages_cran_test.go b/tests/integration/api_packages_cran_test.go index f1c2c63ce36..0615a7a1e9d 100644 --- a/tests/integration/api_packages_cran_test.go +++ 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) + } + }) }