diff --git a/CHANGELOG.md b/CHANGELOG.md index a1ea649adc..eba4031fac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,8 +17,13 @@ All notable changes to `src-cli` are documented in this file. ### Fixed +- Fixed a performance issue when serving git repos where it would take an exponentially large amount of time to list the repos. [#810](https://github.com/sourcegraph/src-cli/pull/810) +- Fixed Bare git repo support when serving git repos. [#810](https://github.com/sourcegraph/src-cli/pull/810) + ### Removed +- Removed git sub-repo support when serving git repos as it introduced a huge performance hit. [#810](https://github.com/sourcegraph/src-cli/pull/810) + ## 3.43.0 ### Changed @@ -40,6 +45,7 @@ All notable changes to `src-cli` are documented in this file. - INTERNAL ONLY: Fixed src batch exec not logging errors. + ## 3.42.2 ### Fixed @@ -833,4 +839,4 @@ Re-release of 3.29.3 for Sourcegraph 3.30. ### Changed - The terminal UI has been replaced by the logger-based UI that was previously only visible in verbose-mode (`-v`). [#228](https://github.com/sourcegraph/src-cli/pull/228) -- Deprecated the `-endpoint` flag. Instead, use the `SRC_ENDPOINT` environment variable. [#235](https://github.com/sourcegraph/src-cli/pull/235) +- Deprecated the `-endpoint` flag. Instead, use the `SRC_ENDPOINT` environment variable. [#235](https://github.com/sourcegraph/src-cli/pull/235) \ No newline at end of file diff --git a/internal/servegit/serve.go b/internal/servegit/serve.go index 1ee5c13320..4e3b99a2f1 100644 --- a/internal/servegit/serve.go +++ b/internal/servegit/serve.go @@ -60,8 +60,9 @@ var indexHTML = template.Must(template.New("").Parse(` `)) type Repo struct { - Name string - URI string + Name string + URI string + ClonePath string } func (s *Serve) handler() http.Handler { @@ -178,8 +179,8 @@ func (s *Serve) Repos() ([]Repo, error) { return nil } - // We recurse into bare repositories to find subprojects. Prevent - // recursing into .git + // Previously we recursed into bare repositories which is why this check was here. + // Now we use this as a sanity check to make sure we didn't somehow stumble into a .git dir. if filepath.Base(path) == ".git" { return filepath.SkipDir } @@ -204,20 +205,24 @@ func (s *Serve) Repos() ([]Repo, error) { name := filepath.ToSlash(subpath) reposRootIsRepo = reposRootIsRepo || name == "." - repos = append(repos, Repo{ - Name: name, - URI: pathpkg.Join("/repos", name), - }) + cloneURI := pathpkg.Join("/repos", name) + clonePath := cloneURI - // Check whether a repository is a bare repository or not. - // - // Bare repositories shouldn't have any further child - // repositories. Only regular git worktrees can have children. - if isBare { - return filepath.SkipDir + // Regular git repos won't clone without the full path to the .git directory. + if isGit { + clonePath += "/.git" } - return nil + repos = append(repos, Repo{ + Name: name, + URI: cloneURI, + ClonePath: clonePath, + }) + + // At this point we know the directory is either a git repo or a bare git repo, + // we don't need to recurse further to save time. + // TODO: Look into whether it is useful to support git submodules + return filepath.SkipDir }) if err != nil { diff --git a/internal/servegit/serve_test.go b/internal/servegit/serve_test.go index df80603666..b314b76de4 100644 --- a/internal/servegit/serve_test.go +++ b/internal/servegit/serve_test.go @@ -32,7 +32,7 @@ func TestReposHandler(t *testing.T) { repos: []string{"project1", "project2"}, }, { name: "nested", - repos: []string{"project1", "project1/subproject", "project2", "dir/project3", "dir/project4.bare"}, + repos: []string{"project1", "project2", "dir/project3", "dir/project4.bare"}, }} for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { @@ -47,65 +47,14 @@ func TestReposHandler(t *testing.T) { var want []Repo for _, name := range tc.repos { - want = append(want, Repo{Name: name, URI: path.Join("/repos", name)}) - } - testReposHandler(t, h, want) - }) - - // Now do the same test, but we root it under a repo we serve. This is - // to test we properly serve up the root repo as something other than - // "." - t.Run("rooted-"+tc.name, func(t *testing.T) { - repos := []string{"project-root"} - for _, name := range tc.repos { - repos = append(repos, filepath.Join("project-root", name)) - } - - root := gitInitRepos(t, repos...) - - // This is the difference to above, we point our root at the git repo - root = filepath.Join(root, "project-root") - - h := (&Serve{ - Info: testLogger(t), - Debug: discardLogger, - Addr: testAddress, - Root: root, - }).handler() - - // project-root is served from /repos, etc - want := []Repo{{Name: "project-root", URI: "/repos"}} - for _, name := range tc.repos { - want = append(want, Repo{Name: path.Join("project-root", name), URI: path.Join("/repos", name)}) - } - testReposHandler(t, h, want) - }) - - // Ensure everything still works if root is a symlink - t.Run("rooted-"+tc.name, func(t *testing.T) { - root := gitInitRepos(t, tc.repos...) - - // This is the difference, we create a symlink for root - { - tmp := t.TempDir() - - symlink := filepath.Join(tmp, "symlink-root") - if err := os.Symlink(root, symlink); err != nil { - t.Fatal(err) + isBare := strings.HasSuffix(name, ".bare") + uri := path.Join("/repos", name) + clonePath := uri + if !isBare { + clonePath += "/.git" } - root = symlink - } + want = append(want, Repo{Name: name, URI: uri, ClonePath: clonePath}) - h := (&Serve{ - Info: testLogger(t), - Debug: discardLogger, - Addr: testAddress, - Root: root, - }).handler() - - var want []Repo - for _, name := range tc.repos { - want = append(want, Repo{Name: name, URI: path.Join("/repos", name)}) } testReposHandler(t, h, want) }) @@ -173,6 +122,14 @@ func gitInitBare(t *testing.T, path string) { } } +func gitInit(t *testing.T, path string) { + cmd := exec.Command("git", "init") + cmd.Dir = path + if err := cmd.Run(); err != nil { + t.Fatal(err) + } +} + func gitInitRepos(t *testing.T, names ...string) string { root := t.TempDir() root = filepath.Join(root, "repos-root") @@ -183,10 +140,11 @@ func gitInitRepos(t *testing.T, names ...string) string { t.Fatal(err) } - if !strings.HasSuffix(p, ".bare") { - p = filepath.Join(p, ".git") + if strings.HasSuffix(p, ".bare") { + gitInitBare(t, p) + } else { + gitInit(t, p) } - gitInitBare(t, p) } return root