Fix repository adoption on Windows (#21646) (#21651)

Backport #21646

A bug was introduced in #17865 where filepath.Join is used to join
putative unadopted repository owner and names together. This is
incorrect as these names are then used as repository names - which shoud
have the '/' separator. This means that adoption will not work on
Windows servers.

Fix #21632

Signed-off-by: Andrew Thornton <art27@cantab.net>

diff --git a/services/repository/adopt.go b/services/repository/adopt.go
index 48f049cd281174cddaef76f9e748dd4dc97e0ef1..0df0aa049b136e9c322e9e286e6f8f27cf0d8a1d 100644 (file)
--- a/services/repository/adopt.go
+++ b/services/repository/adopt.go
@@ -8,6 +8,7 @@ import (
        "context"
        "fmt"
        "os"
+       "path"
        "path/filepath"
        "strings"
 
@@ -220,21 +221,21 @@ func DeleteUnadoptedRepository(doer, u *user_model.User, repoName string) error
        return util.RemoveAll(repoPath)
 }
 
-type unadoptedRrepositories struct {
+type unadoptedRepositories struct {
        repositories []string
        index        int
        start        int
        end          int
 }
 
-func (unadopted *unadoptedRrepositories) add(repository string) {
+func (unadopted *unadoptedRepositories) add(repository string) {
        if unadopted.index >= unadopted.start && unadopted.index < unadopted.end {
                unadopted.repositories = append(unadopted.repositories, repository)
        }
        unadopted.index++
 }
 
-func checkUnadoptedRepositories(userName string, repoNamesToCheck []string, unadopted *unadoptedRrepositories) error {
+func checkUnadoptedRepositories(userName string, repoNamesToCheck []string, unadopted *unadoptedRepositories) error {
        if len(repoNamesToCheck) == 0 {
                return nil
        }
@@ -266,7 +267,7 @@ func checkUnadoptedRepositories(userName string, repoNamesToCheck []string, unad
        }
        for _, repoName := range repoNamesToCheck {
                if _, ok := repoNames[repoName]; !ok {
-                       unadopted.add(filepath.Join(userName, repoName))
+                       unadopted.add(path.Join(userName, repoName)) // These are not used as filepaths - but as reponames - therefore use path.Join not filepath.Join
                }
        }
        return nil
@@ -294,7 +295,7 @@ func ListUnadoptedRepositories(query string, opts *db.ListOptions) ([]string, in
        var repoNamesToCheck []string
 
        start := (opts.Page - 1) * opts.PageSize
-       unadopted := &unadoptedRrepositories{
+       unadopted := &unadoptedRepositories{
                repositories: make([]string, 0, opts.PageSize),
                start:        start,
                end:          start + opts.PageSize,

diff --git a/services/repository/adopt_test.go b/services/repository/adopt_test.go
index 685bfe9bc4824ec4ca7958d48912ff4484c8df65..b450005f344bc547236a0b5affe6eb7b4d661e85 100644 (file)
--- a/services/repository/adopt_test.go
+++ b/services/repository/adopt_test.go
@@ -19,7 +19,7 @@ import (
 func TestCheckUnadoptedRepositories_Add(t *testing.T) {
        start := 10
        end := 20
-       unadopted := &unadoptedRrepositories{
+       unadopted := &unadoptedRepositories{
                start: start,
                end:   end,
                index: 0,
@@ -39,7 +39,7 @@ func TestCheckUnadoptedRepositories(t *testing.T) {
        //
        // Non existent user
        //
-       unadopted := &unadoptedRrepositories{start: 0, end: 100}
+       unadopted := &unadoptedRepositories{start: 0, end: 100}
        err := checkUnadoptedRepositories("notauser", []string{"repo"}, unadopted)
        assert.NoError(t, err)
        assert.Equal(t, 0, len(unadopted.repositories))
@@ -50,14 +50,14 @@ func TestCheckUnadoptedRepositories(t *testing.T) {
        userName := "user2"
        repoName := "repo2"
        unadoptedRepoName := "unadopted"
-       unadopted = &unadoptedRrepositories{start: 0, end: 100}
+       unadopted = &unadoptedRepositories{start: 0, end: 100}
        err = checkUnadoptedRepositories(userName, []string{repoName, unadoptedRepoName}, unadopted)
        assert.NoError(t, err)
        assert.Equal(t, []string{path.Join(userName, unadoptedRepoName)}, unadopted.repositories)
        //
        // Existing (adopted) repository is not returned
        //
-       unadopted = &unadoptedRrepositories{start: 0, end: 100}
+       unadopted = &unadoptedRepositories{start: 0, end: 100}
        err = checkUnadoptedRepositories(userName, []string{repoName}, unadopted)
        assert.NoError(t, err)
        assert.Equal(t, 0, len(unadopted.repositories))