Add support for indexing submodules without repocache (#985)

* Fix git.Open() fs argument for .git dir instead of worktree

The filesystem argument submitted to git.Open() is of the .git
directory, it should be for the worktree, as in go-git this logic is copied from:

https://github.com/go-git/go-git/blob/145daf2492dd86b1d7e7787555ebd9837b93bff8/repository.go#L373-L375

One side effect of this bug was that go-git would refuse to fetch repo
submodules

* Fix missing RepoURLs and LineFragments for result subrepositories

RepoURLs for subrepositories are currently filtered out when results
are streamed per-repo from shards.

* Add submodule indexing without repo cache

When not using repo cache, index git submodules recursively into

subrepos using the go-git API

* Adjust subrepo naming conventions

- Revert repo-cache behavior to unchanged
- Add support for naming repos and modules with no remote (use the dirname for the
  former and subrepopath for the latter)
- Add test for submodule no-repo-cache behavior

* Fix overallocation in repoURL and lineFragment filtering

* Revert basename treatment from 08f6759

Missed the fact that it is already done in zoekt-git-index
command, the test was improperly configured

https://github.com/sourcegraph/zoekt/blob/4e4a529c3b63c7d4c7897ba736f1cd52cc163134/cmd/zoekt-git-index/main.go#L93-L100

Author: H3mul

gitindex/index.go

@@ -282,6 +282,44 @@ func setTemplatesFromConfig(desc *zoekt.Repository, repoDir string) error {
 	return nil
 }
 
+// This attempts to get a repo URL similar to the main repository template processing as in setTemplatesFromConfig()
+func normalizeSubmoduleRemoteURL(cfg *config.Config) (string, error) {
+	sec := cfg.Raw.Section("zoekt")
+	remoteURL := sec.Options.Get("web-url")
+	if remoteURL == "" {
+		// fall back to "origin" remote
+		remoteURL = configLookupRemoteURL(cfg, "origin")
+		if remoteURL == "" {
+			return "", nil
+		}
+	}
+
+	if sm := sshRelativeURLRegexp.FindStringSubmatch(remoteURL); sm != nil {
+		user := sm[1]
+		host := sm[2]
+		path := sm[3]
+
+		remoteURL = fmt.Sprintf("ssh+git://%s@%s/%s", user, host, path)
+	}
+
+	u, err := url.Parse(remoteURL)
+	if err != nil {
+		return "", fmt.Errorf("unable to parse remote URL %q: %w", remoteURL, err)
+	}
+
+	if u.Scheme == "ssh+git" {
+		u.Scheme = "https"
+		u.User = nil
+	}
+
+	// Assume we cannot build templates for this URL, leave it empty
+	if u.Scheme == "" {
+		return "", nil
+	}
+
+	return u.String(), nil
+}
+
 // SetTemplatesFromOrigin fills in templates based on the origin URL.
 func SetTemplatesFromOrigin(desc *zoekt.Repository, u *url.URL) error {
 	desc.Name = filepath.Join(u.Host, strings.TrimSuffix(u.Path, ".git"))
@@ -495,8 +533,13 @@ func indexGitRepo(opts Options, config gitIndexConfig) (bool, error) {
 		tpl := opts.BuildOptions.RepositoryDescription
 		if path != "" {
 			tpl = zoekt.Repository{URL: info.URL.String()}
-			if err := SetTemplatesFromOrigin(&tpl, info.URL); err != nil {
-				log.Printf("setTemplatesFromOrigin(%s, %s): %s", path, info.URL, err)
+			if info.URL.String() != "" {
+				if err := SetTemplatesFromOrigin(&tpl, info.URL); err != nil {
+					log.Printf("setTemplatesFromOrigin(%s, %s): %s", path, info.URL, err)
+				}
+			}
+			if tpl.Name == "" {
+				tpl.Name = path
 			}
 		}
 		opts.BuildOptions.SubRepositories[path] = &tpl
@@ -569,6 +612,7 @@ func indexGitRepo(opts Options, config gitIndexConfig) (bool, error) {
 // It copies the relevant logic from git.PlainOpen, and tweaks certain filesystem options.
 func openRepo(repoDir string) (*git.Repository, io.Closer, error) {
 	fs := osfs.New(repoDir)
+	wt := fs
 
 	// Check if the root directory exists.
 	if _, err := fs.Stat(""); err != nil {
@@ -591,7 +635,7 @@ func openRepo(repoDir string) (*git.Repository, io.Closer, error) {
 	})
 
 	// Because we're keeping descriptors open, we need to close the storage object when we're done.
-	repo, err := git.Open(s, fs)
+	repo, err := git.Open(s, wt)
 	return repo, s, err
 }
 
@@ -830,10 +874,13 @@ func prepareDeltaBuild(options Options, repository *git.Repository) (repos map[f
 
 func prepareNormalBuild(options Options, repository *git.Repository) (repos map[fileKey]BlobLocation, branchVersions map[string]map[string]plumbing.Hash, err error) {
 	var repoCache *RepoCache
-	if options.Submodules {
+	if options.Submodules && options.RepoCacheDir != "" {
 		repoCache = NewRepoCache(options.RepoCacheDir)
 	}
+	return prepareNormalBuildRecurse(options, repository, repoCache, false)
+}
 
+func prepareNormalBuildRecurse(options Options, repository *git.Repository, repoCache *RepoCache, isSubrepo bool) (repos map[fileKey]BlobLocation, branchVersions map[string]map[string]plumbing.Hash, err error) {
 	// Branch => Repo => SHA1
 	branchVersions = map[string]map[string]plumbing.Hash{}
 
@@ -842,7 +889,22 @@ func prepareNormalBuild(options Options, repository *git.Repository) (repos map[
 		return nil, nil, fmt.Errorf("expandBranches: %w", err)
 	}
 
-	rw := NewRepoWalker(repository, options.BuildOptions.RepositoryDescription.URL, repoCache)
+	repoURL := options.BuildOptions.RepositoryDescription.URL
+
+	if isSubrepo {
+		cfg, err := repository.Config()
+		if err != nil {
+			return nil, nil, fmt.Errorf("unable to get repository config: %w", err)
+		}
+
+		u, err := normalizeSubmoduleRemoteURL(cfg)
+		if err != nil {
+			return nil, nil, fmt.Errorf("failed to identify subrepository URL: %w", err)
+		}
+		repoURL = u
+	}
+
+	rw := NewRepoWalker(repository, repoURL, repoCache)
 	for _, b := range branches {
 		commit, err := getCommit(repository, options.BranchPrefix, b)
 		if err != nil {
@@ -871,6 +933,47 @@ func prepareNormalBuild(options Options, repository *git.Repository) (repos map[
 		branchVersions[b] = subVersions
 	}
 
+	// Index submodules using go-git if we didn't do so using the repo cache
+	if options.Submodules && options.RepoCacheDir == "" {
+		worktree, err := repository.Worktree()
+		if err != nil {
+			return nil, nil, fmt.Errorf("failed to get repository worktree: %w", err)
+		}
+
+		submodules, err := worktree.Submodules()
+		if err != nil {
+			return nil, nil, fmt.Errorf("failed to get submodules: %w", err)
+		}
+
+		for _, submodule := range submodules {
+			subRepository, err := submodule.Repository()
+			if err != nil {
+				log.Printf("failed to open submodule repository: %s, %s", submodule.Config().Name, err)
+				continue
+			}
+
+			sw, subVersions, err := prepareNormalBuildRecurse(options, subRepository, repoCache, true)
+			if err != nil {
+				log.Printf("failed to index submodule repository: %s, %s", submodule.Config().Name, err)
+				continue
+			}
+
+			log.Printf("adding subrepository files from: %s", submodule.Config().Name)
+
+			for k, repo := range sw {
+				rw.Files[fileKey{
+					SubRepoPath: filepath.Join(submodule.Config().Path, k.SubRepoPath),
+					Path:        k.Path,
+					ID:          k.ID,
+				}] = repo
+			}
+
+			for k, v := range subVersions {
+				branchVersions[filepath.Join(submodule.Config().Path, k)] = v
+			}
+		}
+	}
+
 	return rw.Files, branchVersions, nil
 }
 

gitindex/tree.go

@@ -178,9 +178,16 @@ func (rw *RepoWalker) handleSubmodule(p string, id *plumbing.Hash, branch string
 }
 
 func (rw *RepoWalker) handleEntry(p string, e *object.TreeEntry, branch string, subRepoVersions map[string]plumbing.Hash, ig *ignore.Matcher) error {
-	if e.Mode == filemode.Submodule && rw.repoCache != nil {
-		if err := rw.tryHandleSubmodule(p, &e.Hash, branch, subRepoVersions, ig); err != nil {
-			return fmt.Errorf("submodule %s: %v", p, err)
+	if e.Mode == filemode.Submodule {
+		if rw.repoCache != nil {
+			// Index the submodule using repo cache
+			if err := rw.tryHandleSubmodule(p, &e.Hash, branch, subRepoVersions, ig); err != nil {
+				return fmt.Errorf("submodule %s: %v", p, err)
+			}
+		} else {
+			// Record the commit ID for the submodule path
+			// This will be the submodule's commit hash, not the parent's
+			subRepoVersions[p] = e.Hash
 		}
 	}
 

gitindex/tree_test.go

@@ -258,6 +258,70 @@ func TestSubmoduleIndex(t *testing.T) {
 	}
 }
 
+func TestSubmoduleIndexWithoutRepocache(t *testing.T) {
+	dir := t.TempDir()
+
+	if err := createSubmoduleRepo(dir); err != nil {
+		t.Fatalf("createSubmoduleRepo: %v", err)
+	}
+
+	indexDir := t.TempDir()
+
+	buildOpts := index.Options{
+		RepositoryDescription: zoekt.Repository{Name: "adir"},
+		IndexDir:              indexDir,
+	}
+	opts := Options{
+		RepoDir:      filepath.Join(dir, "adir"),
+		BuildOptions: buildOpts,
+		BranchPrefix: "refs/heads/",
+		Branches:     []string{"master"},
+		Submodules:   true,
+		Incremental:  true,
+	}
+	if _, err := IndexGitRepo(opts); err != nil {
+		t.Fatalf("IndexGitRepo: %v", err)
+	}
+
+	searcher, err := search.NewDirectorySearcher(indexDir)
+	if err != nil {
+		t.Fatal("NewDirectorySearcher", err)
+	}
+	defer searcher.Close()
+
+	results, err := searcher.Search(context.Background(),
+		&query.Substring{Pattern: "bcont"},
+		&zoekt.SearchOptions{})
+	if err != nil {
+		t.Fatal("Search", err)
+	}
+
+	if len(results.Files) != 1 {
+		t.Fatalf("got search result %v, want 1 file", results.Files)
+	}
+
+	file := results.Files[0]
+	if got, want := file.SubRepositoryName, "bname"; got != want {
+		t.Errorf("got subrepo name %q, want %q", got, want)
+	}
+	if got, want := file.SubRepositoryPath, "bname"; got != want {
+		t.Errorf("got subrepo path %q, want %q", got, want)
+	}
+
+	subVersion := file.Version
+	if len(subVersion) != 40 {
+		t.Fatalf("got %q, want hex sha1", subVersion)
+	}
+
+	if results, err := searcher.Search(context.Background(), &query.Substring{Pattern: "acont"}, &zoekt.SearchOptions{}); err != nil {
+		t.Fatalf("Search('acont'): %v", err)
+	} else if len(results.Files) != 1 {
+		t.Errorf("got %v, want 1 result", results.Files)
+	} else if f := results.Files[0]; f.Version == subVersion {
+		t.Errorf("version in super repo matched version is subrepo.")
+	}
+}
+
 func createSymlinkRepo(dir string) error {
 	if err := os.MkdirAll(dir, 0o755); err != nil {
 		return err

search/shards.go

@@ -816,15 +816,38 @@ func sendByRepository(result *zoekt.SearchResult, opts *zoekt.SearchOptions, sen
 
 	send := func(repoName string, a, b int, stats zoekt.Stats) {
 		index.SortFiles(result.Files[a:b])
+
+		filteredRepoURLs := map[string]string{repoName: result.RepoURLs[repoName]}
+		filteredLineFragments := map[string]string{repoName: result.LineFragments[repoName]}
+
+		// Filter RepoURLs and LineFragments to only those of repoName and its
+		// subRepositories if there are subRepositories
+		for _, file := range result.Files[a:b] {
+			name := file.SubRepositoryName
+			if name == "" {
+				continue
+			}
+			_, repoSet := filteredRepoURLs[name]
+			url, ok := result.RepoURLs[name]
+			if !repoSet && ok {
+				filteredRepoURLs[name] = url
+			}
+			_, fragSet := filteredLineFragments[name]
+			frag, ok := result.LineFragments[name]
+			if !fragSet && ok {
+				filteredLineFragments[name] = frag
+			}
+		}
+
 		sender.Send(&zoekt.SearchResult{
 			Stats: stats,
 			Progress: zoekt.Progress{
 				Priority:           result.Files[a].RepositoryPriority,
 				MaxPendingPriority: result.MaxPendingPriority,
 			},
 			Files:         result.Files[a:b],
-			RepoURLs:      map[string]string{repoName: result.RepoURLs[repoName]},
-			LineFragments: map[string]string{repoName: result.LineFragments[repoName]},
+			RepoURLs:      filteredRepoURLs,
+			LineFragments: filteredLineFragments,
 		})
 	}