From 3066e303574ebb097e079cbb7213186e267cca04 Mon Sep 17 00:00:00 2001 From: Ian Kerins Date: Thu, 9 May 2024 20:41:18 -0400 Subject: [PATCH] Make IndexGitRepo signal whether there were changes As described, we'd like this information to be externally available to drive metrics for incremental indexing. Closes #780. --- cmd/zoekt-git-index/main.go | 2 +- gitindex/ignore_test.go | 2 +- gitindex/index.go | 32 ++++++++++++++++++-------------- gitindex/index_test.go | 4 ++-- gitindex/tree_test.go | 16 ++++++++-------- 5 files changed, 30 insertions(+), 26 deletions(-) diff --git a/cmd/zoekt-git-index/main.go b/cmd/zoekt-git-index/main.go index 1d7cfe88..c1aa5d1a 100644 --- a/cmd/zoekt-git-index/main.go +++ b/cmd/zoekt-git-index/main.go @@ -122,7 +122,7 @@ func run() int { DeltaShardNumberFallbackThreshold: *deltaShardNumberFallbackThreshold, } - if err := gitindex.IndexGitRepo(gitOpts); err != nil { + if _, err := gitindex.IndexGitRepo(gitOpts); err != nil { log.Printf("indexGitRepo(%s, delta=%t): %v", dir, gitOpts.BuildOptions.IsDelta, err) exitStatus = 1 } diff --git a/gitindex/ignore_test.go b/gitindex/ignore_test.go index ca19613d..86250775 100644 --- a/gitindex/ignore_test.go +++ b/gitindex/ignore_test.go @@ -73,7 +73,7 @@ func TestIgnore(t *testing.T) { Submodules: true, Incremental: true, } - if err := IndexGitRepo(opts); err != nil { + if _, err := IndexGitRepo(opts); err != nil { t.Fatalf("IndexGitRepo: %v", err) } diff --git a/gitindex/index.go b/gitindex/index.go index eb2588b3..5a295d05 100644 --- a/gitindex/index.go +++ b/gitindex/index.go @@ -388,12 +388,16 @@ func expandBranches(repo *git.Repository, bs []string, prefix string) ([]string, } // IndexGitRepo indexes the git repository as specified by the options. -func IndexGitRepo(opts Options) error { +// The returned bool indicates whether the index was updated as a result. This +// can be informative if doing incremental indexing. +func IndexGitRepo(opts Options) (bool, error) { return indexGitRepo(opts, gitIndexConfig{}) } // indexGitRepo indexes the git repository as specified by the options and the provided gitIndexConfig. -func indexGitRepo(opts Options, config gitIndexConfig) error { +// The returned bool indicates whether the index was updated as a result. This +// can be informative if doing incremental indexing. +func indexGitRepo(opts Options, config gitIndexConfig) (bool, error) { prepareDeltaBuild := prepareDeltaBuild if config.prepareDeltaBuild != nil { prepareDeltaBuild = config.prepareDeltaBuild @@ -407,13 +411,13 @@ func indexGitRepo(opts Options, config gitIndexConfig) error { // Set max thresholds, since we use them in this function. opts.BuildOptions.SetDefaults() if opts.RepoDir == "" { - return fmt.Errorf("gitindex: must set RepoDir") + return false, fmt.Errorf("gitindex: must set RepoDir") } opts.BuildOptions.RepositoryDescription.Source = opts.RepoDir repo, err := git.PlainOpen(opts.RepoDir) if err != nil { - return fmt.Errorf("git.PlainOpen: %w", err) + return false, fmt.Errorf("git.PlainOpen: %w", err) } if err := setTemplatesFromConfig(&opts.BuildOptions.RepositoryDescription, opts.RepoDir); err != nil { @@ -422,7 +426,7 @@ func indexGitRepo(opts Options, config gitIndexConfig) error { branches, err := expandBranches(repo, opts.Branches, opts.BranchPrefix) if err != nil { - return fmt.Errorf("expandBranches: %w", err) + return false, fmt.Errorf("expandBranches: %w", err) } for _, b := range branches { commit, err := getCommit(repo, opts.BranchPrefix, b) @@ -431,7 +435,7 @@ func indexGitRepo(opts Options, config gitIndexConfig) error { continue } - return fmt.Errorf("getCommit(%q, %q): %w", opts.BranchPrefix, b, err) + return false, fmt.Errorf("getCommit(%q, %q): %w", opts.BranchPrefix, b, err) } opts.BuildOptions.RepositoryDescription.Branches = append(opts.BuildOptions.RepositoryDescription.Branches, zoekt.RepositoryBranch{ @@ -445,7 +449,7 @@ func indexGitRepo(opts Options, config gitIndexConfig) error { } if opts.Incremental && opts.BuildOptions.IncrementalSkipIndexing() { - return nil + return false, nil } // branch => (path, sha1) => repo. @@ -474,7 +478,7 @@ func indexGitRepo(opts Options, config gitIndexConfig) error { if !opts.BuildOptions.IsDelta { repos, branchMap, branchVersions, err = prepareNormalBuild(opts, repo) if err != nil { - return fmt.Errorf("preparing normal build: %w", err) + return false, fmt.Errorf("preparing normal build: %w", err) } } @@ -507,7 +511,7 @@ func indexGitRepo(opts Options, config gitIndexConfig) error { builder, err := build.NewBuilder(opts.BuildOptions) if err != nil { - return fmt.Errorf("build.NewBuilder: %w", err) + return false, fmt.Errorf("build.NewBuilder: %w", err) } var ranks repoPathRanks @@ -515,12 +519,12 @@ func indexGitRepo(opts Options, config gitIndexConfig) error { if opts.BuildOptions.DocumentRanksPath != "" { data, err := os.ReadFile(opts.BuildOptions.DocumentRanksPath) if err != nil { - return err + return false, err } err = json.Unmarshal(data, &ranks) if err != nil { - return err + return false, err } // Compute the mean rank for this repository. Note: we overwrite the rank @@ -564,16 +568,16 @@ func indexGitRepo(opts Options, config gitIndexConfig) error { for _, key := range keys { doc, err := createDocument(key, repos, branchMap, ranks, opts.BuildOptions) if err != nil { - return err + return false, err } if err := builder.Add(doc); err != nil { - return fmt.Errorf("error adding document with name %s: %w", key.FullPath(), err) + return false, fmt.Errorf("error adding document with name %s: %w", key.FullPath(), err) } } } - return builder.Finish() + return true, builder.Finish() } type repoPathRanks struct { diff --git a/gitindex/index_test.go b/gitindex/index_test.go index bf599371..63f5899a 100644 --- a/gitindex/index_test.go +++ b/gitindex/index_test.go @@ -56,7 +56,7 @@ func TestIndexEmptyRepo(t *testing.T) { }, } - if err := IndexGitRepo(opts); err != nil { + if _, err := IndexGitRepo(opts); err != nil { t.Fatalf("IndexGitRepo: %v", err) } } @@ -619,7 +619,7 @@ func TestIndexDeltaBasic(t *testing.T) { } // run test - err := indexGitRepo(options, gitIndexConfig{ + _, err := indexGitRepo(options, gitIndexConfig{ prepareDeltaBuild: prepareDeltaSpy, prepareNormalBuild: prepareNormalSpy, }) diff --git a/gitindex/tree_test.go b/gitindex/tree_test.go index 64fe07dc..406c4b17 100644 --- a/gitindex/tree_test.go +++ b/gitindex/tree_test.go @@ -213,7 +213,7 @@ func TestSubmoduleIndex(t *testing.T) { Incremental: true, RepoCacheDir: dir, } - if err := IndexGitRepo(opts); err != nil { + if _, err := IndexGitRepo(opts); err != nil { t.Fatalf("IndexGitRepo: %v", err) } @@ -317,7 +317,7 @@ func TestSearchSymlinkByContent(t *testing.T) { Incremental: true, RepoCacheDir: dir, } - if err := IndexGitRepo(opts); err != nil { + if _, err := IndexGitRepo(opts); err != nil { t.Fatalf("IndexGitRepo: %v", err) } @@ -375,11 +375,11 @@ func TestAllowMissingBranch(t *testing.T) { Incremental: true, RepoCacheDir: dir, } - if err := IndexGitRepo(opts); err == nil { + if _, err := IndexGitRepo(opts); err == nil { t.Fatalf("IndexGitRepo(nonexist) succeeded") } opts.AllowMissingBranch = true - if err := IndexGitRepo(opts); err != nil { + if _, err := IndexGitRepo(opts); err != nil { t.Fatalf("IndexGitRepo(nonexist, allow): %v", err) } } @@ -444,7 +444,7 @@ func TestBranchWildcard(t *testing.T) { Submodules: true, Incremental: true, } - if err := IndexGitRepo(opts); err != nil { + if _, err := IndexGitRepo(opts); err != nil { t.Fatalf("IndexGitRepo: %v", err) } @@ -492,7 +492,7 @@ func TestSkipSubmodules(t *testing.T) { Branches: []string{"master"}, Submodules: false, } - if err := IndexGitRepo(opts); err != nil { + if _, err := IndexGitRepo(opts); err != nil { t.Fatalf("IndexGitRepo: %v", err) } } @@ -523,7 +523,7 @@ func TestFullAndShortRefNames(t *testing.T) { Incremental: false, AllowMissingBranch: false, } - if err := IndexGitRepo(opts); err != nil { + if _, err := IndexGitRepo(opts); err != nil { t.Fatalf("IndexGitRepo: %v", err) } @@ -573,7 +573,7 @@ func TestLatestCommit(t *testing.T) { BranchPrefix: "refs/heads", Branches: []string{"branchdir/a", "branchdir/b"}, } - if err := IndexGitRepo(opts); err != nil { + if _, err := IndexGitRepo(opts); err != nil { t.Fatalf("IndexGitRepo: %v", err) }