From 64ed3bb5a7a63385c67f3c4f18e57449f13f6882 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Thu, 23 Feb 2023 09:34:04 -0800 Subject: [PATCH] Capture and simplify a git.Run() method --- cmd/git-sync/main.go | 56 ++++++++++++++++++++++++++------------------ pkg/cmd/cmd.go | 14 +++++++---- pkg/hook/exechook.go | 4 ++-- 3 files changed, 45 insertions(+), 29 deletions(-) diff --git a/cmd/git-sync/main.go b/cmd/git-sync/main.go index 02ef234..08829b4 100644 --- a/cmd/git-sync/main.go +++ b/cmd/git-sync/main.go @@ -409,7 +409,7 @@ type repoSync struct { authURL string // a URL to re-fetch credentials, or "" sparseFile string // path to a sparse-checkout file log *logging.Logger - run *cmd.Runner + run cmd.Runner } func main() { @@ -1127,6 +1127,16 @@ func addUser() error { return err } +// Run runs `git` with the specified args. +func (git *repoSync) Run(ctx context.Context, cwd string, args ...string) (string, error) { + return git.run.WithCallDepth(1).Run(ctx, cwd, nil, git.cmd, args...) +} + +// Run runs `git` with the specified args and stdin. +func (git *repoSync) RunWithStdin(ctx context.Context, cwd string, stdin string, args ...string) (string, error) { + return git.run.WithCallDepth(1).RunWithStdin(ctx, cwd, nil, stdin, git.cmd, args...) +} + // initRepo examines the git repo and determines if it is usable or not. If // not, it will (re)initialize it. After running this function, callers can // assume the repo is valid, though maybe empty. @@ -1163,7 +1173,7 @@ func (git *repoSync) initRepo(ctx context.Context) error { // Running `git init` in an existing repo is safe (according to git docs). git.log.V(0).Info("initializing repo directory", "path", git.root) - if _, err := git.run.Run(ctx, git.root, nil, git.cmd, "init", "-b", "git-sync"); err != nil { + if _, err := git.Run(ctx, git.root, "init", "-b", "git-sync"); err != nil { return err } if !git.sanityCheckRepo(ctx) { @@ -1186,7 +1196,7 @@ func (git *repoSync) sanityCheckRepo(ctx context.Context) bool { } // Check that this is actually the root of the repo. - if root, err := git.run.Run(ctx, git.root, nil, git.cmd, "rev-parse", "--show-toplevel"); err != nil { + if root, err := git.Run(ctx, git.root, "rev-parse", "--show-toplevel"); err != nil { git.log.Error(err, "can't get repo toplevel", "path", git.root) return false } else { @@ -1199,7 +1209,7 @@ func (git *repoSync) sanityCheckRepo(ctx context.Context) bool { // Consistency-check the repo. Don't use --verbose because it can be // REALLY verbose. - if _, err := git.run.Run(ctx, git.root, nil, git.cmd, "fsck", "--no-progress", "--connectivity-only"); err != nil { + if _, err := git.Run(ctx, git.root, "fsck", "--no-progress", "--connectivity-only"); err != nil { git.log.Error(err, "repo fsck failed", "path", git.root) return false } @@ -1225,7 +1235,7 @@ func (git *repoSync) sanityCheckWorktree(ctx context.Context, worktree worktree) // Consistency-check the worktree. Don't use --verbose because it can be // REALLY verbose. - if _, err := git.run.Run(ctx, worktree.Path(), nil, git.cmd, "fsck", "--no-progress", "--connectivity-only"); err != nil { + if _, err := git.Run(ctx, worktree.Path(), "fsck", "--no-progress", "--connectivity-only"); err != nil { git.log.Error(err, "worktree fsck failed", "path", worktree.Path()) return false } @@ -1322,7 +1332,7 @@ func (git *repoSync) cleanupWorktree(ctx context.Context, worktree worktree) err if err := os.RemoveAll(worktree.Path()); err != nil { return fmt.Errorf("error removing directory: %v", err) } - if _, err := git.run.Run(ctx, git.root, nil, git.cmd, "worktree", "prune", "--verbose"); err != nil { + if _, err := git.Run(ctx, git.root, "worktree", "prune", "--verbose"); err != nil { return err } return nil @@ -1343,7 +1353,7 @@ func (git *repoSync) createWorktree(ctx context.Context, hash string) (worktree, } git.log.V(0).Info("adding worktree", "path", worktree.Path(), "hash", hash) - _, err := git.run.Run(ctx, git.root, nil, git.cmd, "worktree", "add", "--force", "--detach", worktree.Path(), hash, "--no-checkout") + _, err := git.Run(ctx, git.root, "worktree", "add", "--force", "--detach", worktree.Path(), hash, "--no-checkout") if err != nil { return "", err } @@ -1409,14 +1419,14 @@ func (git *repoSync) configureWorktree(ctx context.Context, worktree worktree) e } args := []string{"sparse-checkout", "init"} - if _, err = git.run.Run(ctx, worktree.Path(), nil, git.cmd, args...); err != nil { + if _, err = git.Run(ctx, worktree.Path(), args...); err != nil { return err } } // Reset the worktree's working copy to the specific ref. git.log.V(1).Info("setting worktree HEAD", "hash", hash) - if _, err := git.run.Run(ctx, worktree.Path(), nil, git.cmd, "reset", "--hard", hash, "--"); err != nil { + if _, err := git.Run(ctx, worktree.Path(), "reset", "--hard", hash, "--"); err != nil { return err } @@ -1431,7 +1441,7 @@ func (git *repoSync) configureWorktree(ctx context.Context, worktree worktree) e if git.depth != 0 { submodulesArgs = append(submodulesArgs, "--depth", strconv.Itoa(git.depth)) } - if _, err := git.run.Run(ctx, worktree.Path(), nil, git.cmd, submodulesArgs...); err != nil { + if _, err := git.Run(ctx, worktree.Path(), submodulesArgs...); err != nil { return err } } @@ -1458,12 +1468,12 @@ func (git *repoSync) cleanup(ctx context.Context, worktree worktree) error { if err := removeDirContents(git.worktreeFor("").path(), git.log, worktree.hash()); err != nil { cleanupErrs = append(cleanupErrs, err) } - if _, err := git.run.Run(ctx, git.root, nil, git.cmd, "worktree", "prune", "--verbose"); err != nil { + if _, err := git.Run(ctx, git.root, "worktree", "prune", "--verbose"); err != nil { cleanupErrs = append(cleanupErrs, err) } // Expire old refs. - if _, err := git.run.Run(ctx, git.root, nil, git.cmd, "reflog", "expire", "--expire-unreachable=all", "--all"); err != nil { + if _, err := git.Run(ctx, git.root, "reflog", "expire", "--expire-unreachable=all", "--all"); err != nil { cleanupErrs = append(cleanupErrs, err) } @@ -1478,7 +1488,7 @@ func (git *repoSync) cleanup(ctx context.Context, worktree worktree) error { case gcAggressive: args = append(args, "--aggressive") } - if _, err := git.run.Run(ctx, git.root, nil, git.cmd, args...); err != nil { + if _, err := git.Run(ctx, git.root, args...); err != nil { cleanupErrs = append(cleanupErrs, err) } } @@ -1510,7 +1520,7 @@ func (git *repoSync) remoteHashForRef(ctx context.Context, ref string) (string, // Fetch both the bare and dereferenced ref. git sorts the results and // prints the dereferenced result, if present, after the bare result, so we // always want the last result it produces. - output, err := git.run.Run(ctx, git.root, nil, git.cmd, "ls-remote", "-q", git.repo, ref, ref+"^{}") + output, err := git.Run(ctx, git.root, "ls-remote", "-q", git.repo, ref, ref+"^{}") if err != nil { return "", err } @@ -1537,7 +1547,7 @@ func lastNonEmptyLine(text string) string { // is not a hash or is not known to this repo, even if it appears to be a hash, // this will return false. func (git *repoSync) IsKnownHash(ctx context.Context, ref string) (bool, error) { - output, err := git.run.Run(ctx, git.root, nil, git.cmd, "rev-parse", ref+"^{commit}") + output, err := git.Run(ctx, git.root, "rev-parse", ref+"^{commit}") if err != nil { if strings.Contains(err.Error(), "unknown revision") { return false, nil @@ -1608,7 +1618,7 @@ func (git *repoSync) SyncRepo(ctx context.Context, refreshCreds func(context.Con if remoteHash == "" { // If git thinks it tastes like a hash, we just use that and if it // is wrong, we will fail later. - output, err := git.run.Run(ctx, git.root, nil, git.cmd, "rev-parse", git.ref) + output, err := git.Run(ctx, git.root, "rev-parse", git.ref) if err != nil { return false, "", err } @@ -1655,7 +1665,7 @@ func (git *repoSync) SyncRepo(ctx context.Context, refreshCreds func(context.Con // Reset the repo (note: not the worktree - that happens later) to the new // ref. This makes subsequent fetches much less expensive. It uses --soft // so no files are checked out. - if _, err := git.run.Run(ctx, git.root, nil, git.cmd, "reset", "--soft", "FETCH_HEAD"); err != nil { + if _, err := git.Run(ctx, git.root, "reset", "--soft", "FETCH_HEAD"); err != nil { return false, "", err } @@ -1718,7 +1728,7 @@ func (git *repoSync) fetch(ctx context.Context, ref string) error { args = append(args, "--unshallow") } } - if _, err := git.run.Run(ctx, git.root, nil, git.cmd, args...); err != nil { + if _, err := git.Run(ctx, git.root, args...); err != nil { return err } @@ -1726,7 +1736,7 @@ func (git *repoSync) fetch(ctx context.Context, ref string) error { } func (git *repoSync) isShallow(ctx context.Context) (bool, error) { - boolStr, err := git.run.Run(ctx, git.root, nil, git.cmd, "rev-parse", "--is-shallow-repository") + boolStr, err := git.Run(ctx, git.root, "rev-parse", "--is-shallow-repository") if err != nil { return false, fmt.Errorf("can't determine repo shallowness: %w", err) } @@ -1752,7 +1762,7 @@ func (git *repoSync) StoreCredentials(ctx context.Context, username, password st git.log.V(9).Info("md5 of credentials", "username", md5sum(username), "password", md5sum(password)) creds := fmt.Sprintf("url=%v\nusername=%v\npassword=%v\n", git.repo, username, password) - _, err := git.run.RunWithStdin(ctx, "", nil, creds, git.cmd, "credential", "approve") + _, err := git.RunWithStdin(ctx, "", creds, "credential", "approve") if err != nil { return fmt.Errorf("can't configure git credentials: %w", err) } @@ -1801,7 +1811,7 @@ func (git *repoSync) SetupCookieFile(ctx context.Context) error { return fmt.Errorf("can't access git cookiefile: %w", err) } - if _, err = git.run.Run(ctx, "", nil, git.cmd, "config", "--global", "http.cookiefile", pathToCookieFile); err != nil { + if _, err = git.Run(ctx, "", "config", "--global", "http.cookiefile", pathToCookieFile); err != nil { return fmt.Errorf("can't configure git cookiefile: %w", err) } @@ -1887,7 +1897,7 @@ func (git *repoSync) SetupDefaultGitConfigs(ctx context.Context) error { val: "cache --timeout 3600", }} for _, kv := range configs { - if _, err := git.run.Run(ctx, "", nil, git.cmd, "config", "--global", kv.key, kv.val); err != nil { + if _, err := git.Run(ctx, "", "config", "--global", kv.key, kv.val); err != nil { return fmt.Errorf("error configuring git %q %q: %v", kv.key, kv.val, err) } } @@ -1904,7 +1914,7 @@ func (git *repoSync) SetupExtraGitConfigs(ctx context.Context, configsFlag strin return fmt.Errorf("can't parse --git-config flag: %v", err) } for _, kv := range configs { - if _, err := git.run.Run(ctx, "", nil, git.cmd, "config", "--global", kv.key, kv.val); err != nil { + if _, err := git.Run(ctx, "", "config", "--global", kv.key, kv.val); err != nil { return fmt.Errorf("error configuring additional git configs %q %q: %v", kv.key, kv.val, err) } } diff --git a/pkg/cmd/cmd.go b/pkg/cmd/cmd.go index 662b26e..d1a7871 100644 --- a/pkg/cmd/cmd.go +++ b/pkg/cmd/cmd.go @@ -41,18 +41,18 @@ type logintf interface { } // NewRunner returns a new CommandRunner -func NewRunner(log logintf) *Runner { - return &Runner{log: log} +func NewRunner(log logintf) Runner { + return Runner{log: log} } // Run runs given command -func (r *Runner) Run(ctx context.Context, cwd string, env []string, command string, args ...string) (string, error) { +func (r Runner) Run(ctx context.Context, cwd string, env []string, command string, args ...string) (string, error) { // call depth = 2 to erase the runWithStdin frame and this one return runWithStdin(ctx, r.log.WithCallDepth(2), cwd, env, "", command, args...) } // RunWithStdin runs given command with standard input -func (r *Runner) RunWithStdin(ctx context.Context, cwd string, env []string, stdin, command string, args ...string) (string, error) { +func (r Runner) RunWithStdin(ctx context.Context, cwd string, env []string, stdin, command string, args ...string) (string, error) { // call depth = 2 to erase the runWithStdin frame and this one return runWithStdin(ctx, r.log.WithCallDepth(2), cwd, env, stdin, command, args...) } @@ -103,3 +103,9 @@ func cmdForLog(command string, args ...string) string { } return command + " " + strings.Join(argsCopy, " ") } + +func (r Runner) WithCallDepth(depth int) Runner { + return Runner{ + log: r.log.WithCallDepth(depth), + } +} diff --git a/pkg/hook/exechook.go b/pkg/hook/exechook.go index 752ccbd..1eee1f0 100644 --- a/pkg/hook/exechook.go +++ b/pkg/hook/exechook.go @@ -28,7 +28,7 @@ import ( // Exechook structure, implements Hook type Exechook struct { // Runner - cmdrunner *cmd.Runner + cmdrunner cmd.Runner // Command to run command string // Command args @@ -42,7 +42,7 @@ type Exechook struct { } // NewExechook returns a new Exechook -func NewExechook(cmdrunner *cmd.Runner, command string, getWorktree func(string) string, args []string, timeout time.Duration, log logintf) *Exechook { +func NewExechook(cmdrunner cmd.Runner, command string, getWorktree func(string) string, args []string, timeout time.Duration, log logintf) *Exechook { return &Exechook{ cmdrunner: cmdrunner, command: command,