zoekt-index: with a shared repo name, later positional directories silently delete earlier ones from the index

[evanmarshall]

What's wrong

If you run zoekt-index with more than one directory in a single command and the directories share one repository name (for example when using -meta, which is how branches are configured), only the last directory's files end up searchable. Every earlier directory is indexed, then silently deleted when the next directory finishes. There is no error or warning, so the data loss is easy to miss.

Where

main() loops over every positional argument and calls indexArg once per directory:

zoekt/cmd/zoekt-index/main.go

Lines 123 to 139 in f186498

	if *metaFile != "" {
	// Read and parse the .meta JSON file into opts.RepositoryDescription
	data, err := os.ReadFile(*metaFile)
	if err != nil {
	log.Fatalf("failed to read .meta file %s: %v", *metaFile, err)
	}
	if err := json.Unmarshal(data, &opts.RepositoryDescription); err != nil {
	log.Fatalf("failed to decode .meta file %s: %v", *metaFile, err)
	}
	}
	for _, arg := range flag.Args() {
	opts.RepositoryDescription.Source = arg
	if err := indexArg(arg, *opts, ignoreDirMap); err != nil {
	log.Fatal(err)
	}
	}

	if *metaFile != "" {
		// Read and parse the .meta JSON file into opts.RepositoryDescription
		...
		if err := json.Unmarshal(data, &opts.RepositoryDescription); err != nil {
			log.Fatalf("failed to decode .meta file %s: %v", *metaFile, err)
		}
	}

	for _, arg := range flag.Args() {
		opts.RepositoryDescription.Source = arg
		if err := indexArg(arg, *opts, ignoreDirMap); err != nil {
			log.Fatal(err)
		}
	}

Each indexArg call creates its own fresh non-delta builder and finishes it:

zoekt/cmd/zoekt-index/main.go

Lines 152 to 161 in f186498

	if opts.RepositoryDescription.Name == "" {
	opts.RepositoryDescription.Name = filepath.Base(dir)
	}
	builder, err := index.NewBuilder(opts)
	if err != nil {
	return err
	}
	// we don't need to check error, since we either already have an error, or
	// we returning the first call to builder.Finish.
	defer builder.Finish() // nolint:errcheck

	if opts.RepositoryDescription.Name == "" {
		opts.RepositoryDescription.Name = filepath.Base(dir)
	}
	builder, err := index.NewBuilder(opts)
	...
	defer builder.Finish() // nolint:errcheck

The shard file prefix defaults to RepositoryDescription.Name (index/builder.go L345-L362), so when the name is shared, every per-directory builder writes to the same shard namespace. A non-delta Builder.Finish then treats all existing shards for that prefix as stale: it collects them via FindAllShards and removes them after installing the new build (index/builder.go L759-L803):

	var toDelete map[string]struct{}
	if !b.opts.IsDelta {
		// Non-delta shard builds delete all existing shards before they write out
		// new ones.
		...
	}
	...
	for p := range toDelete {
		...
		log.Printf("removing old shard file: %s", p)
		if err := os.Remove(p); err != nil {

So directory two's Finish deletes directory one's shards.

Reproduction

1. Create two folders with one file each: alpha/alpha.txt and beta/beta.txt.
2. Create repo.meta with a shared name and a branch:

   {"Name": "repo", "Branches": [{"Name": "main", "Version": "0123456789abcdef0123456789abcdef01234567"}]}

3. Index both in one command:

   zoekt-index -index /tmp/idx -meta repo.meta alpha beta

4. Search the index. beta.txt is found, alpha.txt is gone. The log even shows removing old shard file: .../repo_v16.00000.zoekt between the two builds.

The same thing happens without -meta when two directories share a base name (for example zoekt-index /a/src /b/src), since the default name is filepath.Base(dir).

Expected behavior

All positional directories passed in one command should be searchable afterward. One way to get there: create a single builder for the whole run and feed each directory's files into it, instead of one builder plus Finish per directory. Alternatively, indexArg could refuse to run (or warn loudly) when a later argument would reuse the shard prefix of an earlier one.

Additional context

The per-directory builder loop predates #1117, but #1117 ("zoekt-index: attach configured branches to documents") makes the -meta plus multiple-directories combination the natural way to index with branches, which is exactly the case that loses data. Verified against HEAD f186498.

Found while running Ito (AI code review, free for open source) against recently merged PRs. Full analysis: https://app.ito.ai/share/d0120462-5bd1-4a84-9eb4-ad13b4fd7b77.

[linear]

CU-3216