gitindex: preserve explicit zoekt.name repo config (#1146)

2cb1991 regressed caller-supplied repository name preservation by
restoring the fallback name only after template configuration succeeded.
If origin URL template inference partially updated desc.Name and then
failed, IndexGitRepo logged the error and continued indexing under the
inferred remote name instead of the intended caller-provided name.

Author: keegancsmith

gitindex/index.go

@@ -236,19 +236,44 @@ func setTemplatesFromConfig(desc *zoekt.Repository, repoDir string) error {
 }
 
 func setTemplatesFromRepo(desc *zoekt.Repository, repo *git.Repository, repoDir string) error {
-	// A caller-supplied name identifies the repository and its shards, so it
-	// takes precedence over names derived from zoekt config or the origin URL.
-	// Other repository fields are intentionally refreshed from that config.
-	if desc.Name != "" {
-		defer func(name string) { desc.Name = name }(desc.Name)
-	}
+	// The caller-supplied name is a fallback for repos without zoekt.name. It
+	// still identifies the repository and shard names better than an inferred
+	// origin URL, but an explicit repo config should win.
+	name := desc.Name
 
 	cfg, err := repo.Config()
 	if err == nil {
-		return setTemplatesFromRepoConfig(desc, cfg)
+		return setTemplatesFromRepoConfigPreservingName(desc, cfg, name)
+	}
+
+	// Some repositories, notably worktrees with .git files, need the plainOpenRepo
+	// fallback to resolve their common dir before go-git can read config.
+	repo, err = plainOpenRepo(repoDir)
+	if err != nil {
+		return err
 	}
 
-	return setTemplatesFromConfig(desc, repoDir)
+	cfg, err = repo.Config()
+	if err != nil {
+		return err
+	}
+
+	return setTemplatesFromRepoConfigPreservingName(desc, cfg, name)
+}
+
+func setTemplatesFromRepoConfigPreservingName(desc *zoekt.Repository, cfg *config.Config, name string) error {
+	if name != "" && cfg.Raw.Section("zoekt").Options.Get("name") == "" {
+		defer func() {
+			// A caller-supplied name identifies the repository and its shards, so it
+			// takes precedence over names derived from the origin URL. Other repository
+			// fields are intentionally refreshed from config.
+			desc.Name = name
+		}()
+	}
+	if err := setTemplatesFromRepoConfig(desc, cfg); err != nil {
+		return err
+	}
+	return nil
 }
 
 func setTemplatesFromRepoConfig(desc *zoekt.Repository, cfg *config.Config) error {

gitindex/index_test.go

@@ -207,6 +207,83 @@ func TestIndexGitRepoPreservesRepositoryName(t *testing.T) {
 	}
 }
 
+func TestIndexGitRepoPrefersConfiguredRepositoryName(t *testing.T) {
+	t.Parallel()
+
+	configuredName := "github.com/sgtest/go-diff"
+	fallbackName := url.QueryEscape(configuredName)
+	repoDir, _ := initGitWorktree(t, "file1.go", "package main\n\nfunc main() {}\n")
+	runGit(t, repoDir, "config", "zoekt.name", configuredName)
+
+	indexDir := t.TempDir()
+	opts := Options{
+		RepoDir:  repoDir,
+		Branches: []string{"HEAD"},
+		BuildOptions: index.Options{
+			RepositoryDescription: zoekt.Repository{Name: fallbackName},
+			IndexDir:              indexDir,
+			DisableCTags:          true,
+		},
+	}
+
+	if _, err := IndexGitRepo(opts); err != nil {
+		t.Fatal(err)
+	}
+	shards, err := filepath.Glob(filepath.Join(indexDir, "*.zoekt"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(shards) != 1 {
+		t.Fatalf("got %d shards, want 1", len(shards))
+	}
+	repositories, _, err := index.ReadMetadataPath(shards[0])
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got := repositories[0].Name; got != configuredName {
+		t.Fatalf("repository name is %q, want %q", got, configuredName)
+	}
+	if got, wantPrefix := filepath.Base(shards[0]), url.QueryEscape(configuredName)+"_v"; !strings.HasPrefix(got, wantPrefix) {
+		t.Fatalf("shard name is %q, want prefix %q", got, wantPrefix)
+	}
+}
+
+func TestIndexGitRepoPreservesRepositoryNameOnTemplateError(t *testing.T) {
+	t.Parallel()
+
+	repoDir, _ := initGitWorktree(t, "file1.go", "package main\n\nfunc main() {}\n")
+	runGit(t, repoDir, "config", "remote.origin.url", "git@example.com:sourcegraph/zoekt.git")
+
+	indexDir := t.TempDir()
+	opts := Options{
+		RepoDir:  repoDir,
+		Branches: []string{"HEAD"},
+		BuildOptions: index.Options{
+			RepositoryDescription: zoekt.Repository{Name: "local/repo"},
+			IndexDir:              indexDir,
+			DisableCTags:          true,
+		},
+	}
+
+	if _, err := IndexGitRepo(opts); err != nil {
+		t.Fatal(err)
+	}
+	shards, err := filepath.Glob(filepath.Join(indexDir, "*.zoekt"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(shards) != 1 {
+		t.Fatalf("got %d shards, want 1", len(shards))
+	}
+	repositories, _, err := index.ReadMetadataPath(shards[0])
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got, want := repositories[0].Name, opts.BuildOptions.RepositoryDescription.Name; got != want {
+		t.Fatalf("repository name is %q, want %q", got, want)
+	}
+}
+
 func TestOpenRepoVariants(t *testing.T) {
 	t.Parallel()