From 47404c921e5b6c82cb3652d7547c568289bfbfbe Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Sat, 19 Feb 2022 11:25:06 -0800 Subject: [PATCH 1/3] Set some gc.* git config In particular, this sets `gc.autoDetach` to "false". --- cmd/git-sync/main.go | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/cmd/git-sync/main.go b/cmd/git-sync/main.go index 2b629d4..56283dc 100644 --- a/cmd/git-sync/main.go +++ b/cmd/git-sync/main.go @@ -408,6 +408,12 @@ func main() { askpassCount.WithLabelValues(metricKeySuccess).Inc() } + // Set additional configs we want, but users might override. + if err := setupDefaultGitConfigs(ctx); err != nil { + fmt.Fprintf(os.Stderr, "ERROR: can't set default git configs: %v\n", err) + os.Exit(1) + } + // This needs to be after all other git-related config flags. if *flGitConfig != "" { if err := setupExtraGitConfigs(ctx, *flGitConfig); err != nil { @@ -755,8 +761,8 @@ func addWorktreeAndSwap(ctx context.Context, gitRoot, dest, branch, rev string, return err } - // GC clone - if _, err := cmdRunner.Run(ctx, gitRoot, nil, *flGitCmd, "gc", "--prune=all"); err != nil { + // Run GC if needed. + if _, err := cmdRunner.Run(ctx, gitRoot, nil, *flGitCmd, "gc", "--auto"); err != nil { return err } @@ -1170,6 +1176,24 @@ func callGitAskPassURL(ctx context.Context, url string) error { return nil } +func setupDefaultGitConfigs(ctx context.Context) error { + configs := []keyVal{{ + // Never auto-detach GC runs. + key: "gc.autoDetach", + val: "false", + }, { + // Fairly aggressive GC. + key: "gc.pruneExpire", + val: "now", + }} + for _, kv := range configs { + if _, err := cmdRunner.Run(ctx, "", nil, *flGitCmd, "config", "--global", kv.key, kv.val); err != nil { + return fmt.Errorf("error configuring git %q %q: %v", kv.key, kv.val, err) + } + } + return nil +} + func setupExtraGitConfigs(ctx context.Context, configsFlag string) error { log.V(1).Info("setting additional git configs") From ad3955c0fa44b31c5e4bd59dfbf948c30fb09f54 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Sat, 19 Feb 2022 16:38:16 -0800 Subject: [PATCH 2/3] Add --git-gc flag to control GC on each sync Values: * "auto" - run `git gc --auto` (default, respects git gc.* configs) * "always" - run `git gc` * "aggressive" - run `git gc --aggressive` (may require a longer timeout) * "off" - do not run `git gc` on each sync (good for --one-time use) --- cmd/git-sync/main.go | 60 ++++++++++++++++++++++++++----- test_e2e.sh | 84 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 135 insertions(+), 9 deletions(-) diff --git a/cmd/git-sync/main.go b/cmd/git-sync/main.go index 56283dc..12211b9 100644 --- a/cmd/git-sync/main.go +++ b/cmd/git-sync/main.go @@ -126,6 +126,8 @@ var flGitCmd = flag.String("git", envString("GIT_SYNC_GIT", "git"), "the git command to run (subject to PATH search, mostly for testing)") var flGitConfig = flag.String("git-config", envString("GIT_SYNC_GIT_CONFIG", ""), "additional git config options in 'key1:val1,key2:val2' format") +var flGitGC = flag.String("git-gc", envString("GIT_SYNC_GIT_GC", "auto"), + "git garbage collection behavior: one of 'auto', 'always', 'aggressive', or 'off'") var flHTTPBind = flag.String("http-bind", envString("GIT_SYNC_HTTP_BIND", ""), "the bind address (including port) for git-sync's HTTP endpoint") @@ -169,6 +171,11 @@ const ( submodulesRecursive = "recursive" submodulesShallow = "shallow" submodulesOff = "off" + + gcAuto = "auto" + gcAlways = "always" + gcAggressive = "aggressive" + gcOff = "off" ) func init() { @@ -278,6 +285,12 @@ func main() { handleError(true, "ERROR: --submodules must be one of %q, %q, or %q", submodulesRecursive, submodulesShallow, submodulesOff) } + switch *flGitGC { + case gcAuto, gcAlways, gcAggressive, gcOff: + default: + handleError(true, "ERROR: --git-gc must be one of %q, %q, %q, or %q", gcAuto, gcAlways, gcAggressive, gcOff) + } + if *flRoot == "" { handleError(true, "ERROR: --root must be specified") } @@ -761,11 +774,6 @@ func addWorktreeAndSwap(ctx context.Context, gitRoot, dest, branch, rev string, return err } - // Run GC if needed. - if _, err := cmdRunner.Run(ctx, gitRoot, nil, *flGitCmd, "gc", "--auto"); err != nil { - return err - } - // The .git file in the worktree directory holds a reference to // /git/.git/worktrees/. Replace it with a reference // using relative paths, so that other containers can use a different volume @@ -859,19 +867,53 @@ func addWorktreeAndSwap(ctx context.Context, gitRoot, dest, branch, rev string, setRepoReady() // From here on we have to save errors until the end. + var cleanupErrs multiError // Clean up previous worktree(s). - var cleanupErr error if oldWorktree != "" { - cleanupErr = cleanupWorkTree(ctx, gitRoot, oldWorktree) + if err := cleanupWorkTree(ctx, gitRoot, oldWorktree); err != nil { + cleanupErrs = append(cleanupErrs, err) + } } - if cleanupErr != nil { - return cleanupErr + // Run GC if needed. + if *flGitGC != gcOff { + args := []string{"gc"} + switch *flGitGC { + case gcAuto: + args = append(args, "--auto") + case gcAlways: + // no extra flags + case gcAggressive: + args = append(args, "--aggressive") + } + if _, err := cmdRunner.Run(ctx, gitRoot, nil, *flGitCmd, args...); err != nil { + cleanupErrs = append(cleanupErrs, err) + } + } + + if len(cleanupErrs) > 0 { + return cleanupErrs } return nil } +type multiError []error + +func (m multiError) Error() string { + if len(m) == 0 { + return "" + } + if len(m) == 1 { + return m[0].Error() + } + strs := make([]string, 0, len(m)) + for _, e := range m { + strs = append(strs, e.Error()) + } + return strings.Join(strs, "; ") +} + func cloneRepo(ctx context.Context, repo, branch, rev string, depth int, gitRoot string) error { args := []string{"clone", "-v", "--no-checkout", "-b", branch} if depth != 0 { diff --git a/test_e2e.sh b/test_e2e.sh index d656da4..ac93739 100755 --- a/test_e2e.sh +++ b/test_e2e.sh @@ -1729,6 +1729,90 @@ function e2e::github_https() { assert_file_exists "$ROOT"/link/LICENSE } +############################################## +# Test git-gc=auto +############################################## +function e2e::gc_auto() { + echo "$FUNCNAME" > "$REPO"/file + git -C "$REPO" commit -qam "$FUNCNAME" + + GIT_SYNC \ + --one-time \ + --repo="file://$REPO" \ + --branch="$MAIN_BRANCH" \ + --rev=HEAD \ + --root="$ROOT" \ + --dest="link" \ + --git-gc="auto" \ + >> "$1" 2>&1 + assert_link_exists "$ROOT"/link + assert_file_exists "$ROOT"/link/file + assert_file_eq "$ROOT"/link/file "$FUNCNAME" +} + +############################################## +# Test git-gc=always +############################################## +function e2e::gc_always() { + echo "$FUNCNAME" > "$REPO"/file + git -C "$REPO" commit -qam "$FUNCNAME" + + GIT_SYNC \ + --one-time \ + --repo="file://$REPO" \ + --branch="$MAIN_BRANCH" \ + --rev=HEAD \ + --root="$ROOT" \ + --dest="link" \ + --git-gc="always" \ + >> "$1" 2>&1 + assert_link_exists "$ROOT"/link + assert_file_exists "$ROOT"/link/file + assert_file_eq "$ROOT"/link/file "$FUNCNAME" +} + +############################################## +# Test git-gc=aggressive +############################################## +function e2e::gc_aggressive() { + echo "$FUNCNAME" > "$REPO"/file + git -C "$REPO" commit -qam "$FUNCNAME" + + GIT_SYNC \ + --one-time \ + --repo="file://$REPO" \ + --branch="$MAIN_BRANCH" \ + --rev=HEAD \ + --root="$ROOT" \ + --dest="link" \ + --git-gc="aggressive" \ + >> "$1" 2>&1 + assert_link_exists "$ROOT"/link + assert_file_exists "$ROOT"/link/file + assert_file_eq "$ROOT"/link/file "$FUNCNAME" +} + +############################################## +# Test git-gc=off +############################################## +function e2e::gc_off() { + echo "$FUNCNAME" > "$REPO"/file + git -C "$REPO" commit -qam "$FUNCNAME" + + GIT_SYNC \ + --one-time \ + --repo="file://$REPO" \ + --branch="$MAIN_BRANCH" \ + --rev=HEAD \ + --root="$ROOT" \ + --dest="link" \ + --git-gc="off" \ + >> "$1" 2>&1 + assert_link_exists "$ROOT"/link + assert_file_exists "$ROOT"/link/file + assert_file_eq "$ROOT"/link/file "$FUNCNAME" +} + function list_tests() { ( shopt -s extdebug From 5707870ecc4e9ba657002d2487c39c3d7be2b60a Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Thu, 24 Feb 2022 09:04:24 -0800 Subject: [PATCH 3/3] Make e2e treat args as tests-name regexes --- test_e2e.sh | 46 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 32 insertions(+), 14 deletions(-) diff --git a/test_e2e.sh b/test_e2e.sh index ac93739..4c5ea5f 100755 --- a/test_e2e.sh +++ b/test_e2e.sh @@ -1827,35 +1827,53 @@ function list_tests() { } # Figure out which, if any, tests to run. -tests=($(list_tests)) +all_tests=($(list_tests)) +tests_to_run=() function print_tests() { echo "available tests:" - for t in "${tests[@]}"; do + for t in "${all_tests[@]}"; do echo " $t" done } -for t; do +# Validate and accumulate tests to run if args are specified. +for arg; do # Use -? to list known tests. - if [[ "${t}" == "-?" ]]; then + if [[ "${arg}" == "-?" ]]; then print_tests exit 0 fi - # Make sure we know this test. - if [[ " ${tests[*]} " =~ " ${t} " ]]; then - continue + if [[ "${arg}" =~ ^- ]]; then + echo "ERROR: unknown flag '${arg}'" + exit 1 fi - # Not a known test or flag. - echo "ERROR: unknown test or flag: '${t}'" - echo - print_tests - exit 1 + # Make sure each non-flag arg matches at least one test. + nmatches=0 + for t in "${all_tests[@]}"; do + if [[ "${t}" =~ ${arg} ]]; then + nmatches=$((nmatches+1)) + # Don't run tests twice, just keep the first match. + if [[ " ${tests_to_run[*]} " =~ " ${t} " ]]; then + continue + fi + tests_to_run+=("${t}") + continue + fi + done + if [[ ${nmatches} == 0 ]]; then + echo "ERROR: no tests match pattern '${arg}'" + echo + print_tests + exit 1 + fi + tests_to_run+=("${matches[@]}") done +set -- "${tests_to_run[@]}" -# If no tests specified, run them all. +# If no tests were specified, run them all. if [[ "$#" == 0 ]]; then - set -- "${tests[@]}" + set -- "${all_tests[@]}" fi # Build it