From c3ad93df64c7a3ecaf6b65f20ca02ab80d6ff552 Mon Sep 17 00:00:00 2001 From: Keegan Carruthers-Smith Date: Fri, 20 Mar 2026 09:18:08 +0200 Subject: [PATCH] gitindex: diff config updates for existing clones This supersedes #635 by porting its selective config-sync idea onto current main with a smaller, easier-to-read shape. Clone orchestration stays in gitindex/clone.go, while config argument generation and existing-clone sync logic now live in gitindex/clone_config.go. For existing clones, we now diff each zoekt.* setting before writing. Unchanged values are skipped, changed values are updated, and settings that disappear are removed. CloneRepo returns the repo destination only when a setting change actually happened so the caller can trigger reindexing only when needed. cmd/zoekt-mirror-github was also cleaned up so optional integer metadata keys are only added to the config map when present, which avoids pushing empty config values downstream. Note: On #635 review it mentioned using go-git. This commit initially explored that but it ended up being a _lot_ more code due to missing utilities around easily setting values based on a git config string. --- cmd/zoekt-mirror-github/main.go | 17 +++--- gitindex/clone.go | 48 +++-------------- gitindex/clone_config.go | 93 +++++++++++++++++++++++++++++++++ 3 files changed, 108 insertions(+), 50 deletions(-) create mode 100644 gitindex/clone_config.go diff --git a/cmd/zoekt-mirror-github/main.go b/cmd/zoekt-mirror-github/main.go index 4433fa017..99dd90439 100644 --- a/cmd/zoekt-mirror-github/main.go +++ b/cmd/zoekt-mirror-github/main.go @@ -292,11 +292,10 @@ func getUserRepos(client *github.Client, user string, reposFilters reposFilters) return allRepos, nil } -func itoa(p *int) string { - if p != nil { - return strconv.Itoa(*p) +func setOptionalIntConfig(config map[string]string, key string, value *int) { + if value != nil { + config[key] = strconv.Itoa(*value) } - return "" } func cloneRepos(destDir string, repos []*github.Repository) error { @@ -311,15 +310,15 @@ func cloneRepos(destDir string, repos []*github.Repository) error { "zoekt.web-url": *r.HTMLURL, "zoekt.name": filepath.Join(host.Hostname(), *r.FullName), - "zoekt.github-stars": itoa(r.StargazersCount), - "zoekt.github-watchers": itoa(r.WatchersCount), - "zoekt.github-subscribers": itoa(r.SubscribersCount), - "zoekt.github-forks": itoa(r.ForksCount), - "zoekt.archived": marshalBool(r.Archived != nil && *r.Archived), "zoekt.fork": marshalBool(r.Fork != nil && *r.Fork), "zoekt.public": marshalBool(r.Private == nil || !*r.Private), } + setOptionalIntConfig(config, "zoekt.github-stars", r.StargazersCount) + setOptionalIntConfig(config, "zoekt.github-watchers", r.WatchersCount) + setOptionalIntConfig(config, "zoekt.github-subscribers", r.SubscribersCount) + setOptionalIntConfig(config, "zoekt.github-forks", r.ForksCount) + dest, err := gitindex.CloneRepo(destDir, *r.FullName, *r.CloneURL, config) if err != nil { return err diff --git a/gitindex/clone.go b/gitindex/clone.go index 8bee54de0..f99f2f2d9 100644 --- a/gitindex/clone.go +++ b/gitindex/clone.go @@ -18,37 +18,14 @@ import ( "bytes" "fmt" "log" - "maps" "os" "os/exec" "path/filepath" - "sort" git "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/config" ) -// Updates the zoekt.* git config options after a repo is cloned. -// Once a repo is cloned, we can no longer use the --config flag to update all -// of it's zoekt.* settings at once. `git config` is limited to one option at once. -func updateZoektGitConfig(repoDest string, settings map[string]string) error { - var keys []string - for k := range settings { - keys = append(keys, k) - } - sort.Strings(keys) - - for _, k := range keys { - if settings[k] != "" { - if err := exec.Command("git", "-C", repoDest, "config", k, settings[k]).Run(); err != nil { - return err - } - } - } - - return nil -} - // CloneRepo clones one repository, adding the given config // settings. It returns the bare repo directory. The `name` argument // determines where the repo is stored relative to `destDir`. Returns @@ -61,32 +38,21 @@ func CloneRepo(destDir, name, cloneURL string, settings map[string]string) (stri repoDest := filepath.Join(parent, filepath.Base(name)+".git") if _, err := os.Lstat(repoDest); err == nil { - // Repository exists, ensure settings are in sync including the clone URL - settings := maps.Clone(settings) - settings["remote.origin.url"] = cloneURL - if err := updateZoektGitConfig(repoDest, settings); err != nil { + // Repository exists, ensure zoekt settings are in sync. + hadUpdate, err := updateZoektGitConfig(repoDest, settings) + if err != nil { return "", fmt.Errorf("failed to update repository settings: %w", err) } - return "", nil - } - - var keys []string - for k := range settings { - keys = append(keys, k) - } - sort.Strings(keys) - - var config []string - for _, k := range keys { - if settings[k] != "" { - config = append(config, "--config", k+"="+settings[k]) + if hadUpdate { + return repoDest, nil } + return "", nil } cmd := exec.Command( "git", "clone", "--bare", "--verbose", "--progress", ) - cmd.Args = append(cmd.Args, config...) + cmd.Args = append(cmd.Args, cloneConfigArgs(settings)...) cmd.Args = append(cmd.Args, cloneURL, repoDest) // Prevent prompting diff --git a/gitindex/clone_config.go b/gitindex/clone_config.go new file mode 100644 index 000000000..d178e2f3e --- /dev/null +++ b/gitindex/clone_config.go @@ -0,0 +1,93 @@ +package gitindex + +import ( + "bytes" + "errors" + "fmt" + "maps" + "os/exec" + "slices" + "strings" +) + +func sortedKeys(settings map[string]string) []string { + return slices.Sorted(maps.Keys(settings)) +} + +func cloneConfigArgs(settings map[string]string) []string { + args := make([]string, 0, len(settings)*2) + for _, key := range sortedKeys(settings) { + if value := settings[key]; value != "" { + args = append(args, "--config", key+"="+value) + } + } + return args +} + +// updateZoektGitConfig applies zoekt.* settings to an existing clone. +// It returns whether the repository config changed. +func updateZoektGitConfig(repoDest string, settings map[string]string) (bool, error) { + changed := false + for _, key := range sortedKeys(settings) { + updated, err := syncGitConfigOption(repoDest, key, settings[key]) + if err != nil { + return false, err + } + changed = changed || updated + } + return changed, nil +} + +func syncGitConfigOption(repoDest, key, value string) (bool, error) { + current, ok, err := repoConfigValue(repoDest, key) + if err != nil { + return false, err + } + + if value == "" { + if !ok { + return false, nil + } + if err := unsetRepoConfigValue(repoDest, key); err != nil { + return false, err + } + return true, nil + } + + if ok && current == value { + return false, nil + } + if err := setRepoConfigValue(repoDest, key, value); err != nil { + return false, err + } + return true, nil +} + +func repoConfigValue(repoDest, key string) (string, bool, error) { + cmd := exec.Command("git", "-C", repoDest, "config", "--get", key) + var out bytes.Buffer + cmd.Stdout = &out + if err := cmd.Run(); err == nil { + return strings.TrimSuffix(out.String(), "\n"), true, nil + } else { + var exitErr *exec.ExitError + if errors.As(err, &exitErr) && exitErr.ExitCode() == 1 { + return "", false, nil + } + return "", false, fmt.Errorf("git config --get %q: %w", key, err) + } +} + +func setRepoConfigValue(repoDest, key, value string) error { + if err := exec.Command("git", "-C", repoDest, "config", "--replace-all", key, value).Run(); err != nil { + return fmt.Errorf("git config --replace-all %q: %w", key, err) + } + return nil +} + +func unsetRepoConfigValue(repoDest, key string) error { + if err := exec.Command("git", "-C", repoDest, "config", "--unset-all", key).Run(); err != nil { + return fmt.Errorf("git config --unset-all %q: %w", key, err) + } + return nil +}