From 036627edb39ed7cf9a524d47925fdbba792f3a36 Mon Sep 17 00:00:00 2001 From: Rodrigo Silva Mendoza Date: Tue, 20 Jun 2023 12:00:46 -0700 Subject: [PATCH 1/2] diff old/new zoekt settings before applying them. OnUpdate have CloneRepo return the repo so it gets re-indexed --- gitindex/clone.go | 77 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 72 insertions(+), 5 deletions(-) diff --git a/gitindex/clone.go b/gitindex/clone.go index 2399e29fa..2031d2abd 100644 --- a/gitindex/clone.go +++ b/gitindex/clone.go @@ -22,30 +22,89 @@ import ( "os/exec" "path/filepath" "sort" + "strings" git "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/config" ) +// returns a list of all the git zoekt settings that have changed or are +// new +func getZoektSettingsToUpdate(repoDest string, newSettings map[string]string, newSettingsKeys []string) ([]string, error) { + // get the existing config for this repo - and turn it into a map. Used to check + // diff between old/new config + cmd := exec.Command("git", "-C", repoDest, "config", "--local", "--get-regexp", "zoekt") + outBuf := &bytes.Buffer{} + errBuf := &bytes.Buffer{} + cmd.Stdout = outBuf + cmd.Stderr = errBuf + + if err := cmd.Run(); err != nil { + log.Printf("error getting settings\n") + return nil, err + } + + // turn it into a settings map + oldSettings := make(map[string]string) + for _, cl := range bytes.Split(outBuf.Bytes(), []byte{'\n'}) { + if len(cl) == 0 { + continue + } + + parts := bytes.SplitN(cl, []byte{' '}, 2) + if len(parts) != 2 { + return nil, fmt.Errorf("more parts than expected in git config key/v line") + } + oldSettings[string(parts[0])] = strings.TrimSpace(string(parts[1])) + } + + var oldSettingsKeys []string + for k := range oldSettings { + oldSettingsKeys = append(oldSettingsKeys, k) + } + sort.Strings(oldSettingsKeys) + + // get the list of settings that have changed, or are new + var settingsToUpdate []string + for _, k := range newSettingsKeys { + oldVal, oldHasSetting := oldSettings[k] + if (!oldHasSetting && newSettings[k] != "") || oldVal != newSettings[k] { + settingsToUpdate = append(settingsToUpdate, k) + } + } + + return settingsToUpdate, nil +} + // 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 { +// Settings are brute updated +func updateZoektGitConfig(repoDest string, settings map[string]string) (hasChange bool, err error) { var keys []string for k := range settings { keys = append(keys, k) } sort.Strings(keys) - for _, k := range keys { + settingsToUpdate, err := getZoektSettingsToUpdate(repoDest, settings, keys) + if err != nil { + return false, err + } + // if there is nothing to update, return + if len(settingsToUpdate) == 0 { + return false, nil + } + + for _, k := range settingsToUpdate { if settings[k] != "" { if err := exec.Command("git", "-C", repoDest, "config", k, settings[k]).Run(); err != nil { - return err + return true, err } } } - return nil + return true, nil } // CloneRepo clones one repository, adding the given config @@ -61,9 +120,17 @@ 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 - if err := updateZoektGitConfig(repoDest, settings); err != nil { + hadUpdate, err := updateZoektGitConfig(repoDest, settings) + if err != nil { return "", fmt.Errorf("failed to update repository settings: %w", err) } + // For xvandish/zoekt only, we trigger a re-index here, since we have logic + // to only brute-reindex on an interval. In that case, after a zoektConfig update, + // if may take bruteReindexInterval hours for the repo-to be re-index and that config + // to actually be reflected. Unfortunately we have no easy way of knowing whether + if hadUpdate { + return repoDest, nil + } return "", nil } From 23f501adcc08ea7d9073acbdba7ff31669fca7eb Mon Sep 17 00:00:00 2001 From: Rodrigo Silva Mendoza Date: Tue, 20 Jun 2023 12:23:25 -0700 Subject: [PATCH 2/2] Cleanup the new gitindex zoekt settings diff code --- gitindex/clone.go | 24 ++++++------------------ 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/gitindex/clone.go b/gitindex/clone.go index 2031d2abd..493abef64 100644 --- a/gitindex/clone.go +++ b/gitindex/clone.go @@ -29,10 +29,9 @@ import ( ) // returns a list of all the git zoekt settings that have changed or are -// new +// new. To do this it gets the current config, turns into into a map and diffs +// against the new settings map func getZoektSettingsToUpdate(repoDest string, newSettings map[string]string, newSettingsKeys []string) ([]string, error) { - // get the existing config for this repo - and turn it into a map. Used to check - // diff between old/new config cmd := exec.Command("git", "-C", repoDest, "config", "--local", "--get-regexp", "zoekt") outBuf := &bytes.Buffer{} errBuf := &bytes.Buffer{} @@ -44,7 +43,7 @@ func getZoektSettingsToUpdate(repoDest string, newSettings map[string]string, ne return nil, err } - // turn it into a settings map + // collect every current setting and put it into a map oldSettings := make(map[string]string) for _, cl := range bytes.Split(outBuf.Bytes(), []byte{'\n'}) { if len(cl) == 0 { @@ -58,12 +57,6 @@ func getZoektSettingsToUpdate(repoDest string, newSettings map[string]string, ne oldSettings[string(parts[0])] = strings.TrimSpace(string(parts[1])) } - var oldSettingsKeys []string - for k := range oldSettings { - oldSettingsKeys = append(oldSettingsKeys, k) - } - sort.Strings(oldSettingsKeys) - // get the list of settings that have changed, or are new var settingsToUpdate []string for _, k := range newSettingsKeys { @@ -79,8 +72,7 @@ func getZoektSettingsToUpdate(repoDest string, newSettings map[string]string, ne // 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. -// Settings are brute updated -func updateZoektGitConfig(repoDest string, settings map[string]string) (hasChange bool, err error) { +func updateZoektGitConfig(repoDest string, settings map[string]string) (bool, error) { var keys []string for k := range settings { keys = append(keys, k) @@ -91,7 +83,7 @@ func updateZoektGitConfig(repoDest string, settings map[string]string) (hasChang if err != nil { return false, err } - // if there is nothing to update, return + if len(settingsToUpdate) == 0 { return false, nil } @@ -99,7 +91,7 @@ func updateZoektGitConfig(repoDest string, settings map[string]string) (hasChang for _, k := range settingsToUpdate { if settings[k] != "" { if err := exec.Command("git", "-C", repoDest, "config", k, settings[k]).Run(); err != nil { - return true, err + return false, err } } } @@ -124,10 +116,6 @@ func CloneRepo(destDir, name, cloneURL string, settings map[string]string) (stri if err != nil { return "", fmt.Errorf("failed to update repository settings: %w", err) } - // For xvandish/zoekt only, we trigger a re-index here, since we have logic - // to only brute-reindex on an interval. In that case, after a zoektConfig update, - // if may take bruteReindexInterval hours for the repo-to be re-index and that config - // to actually be reflected. Unfortunately we have no easy way of knowing whether if hadUpdate { return repoDest, nil }