From 2dd4705c1b23065c4ccb5d3183ede0635810e24c Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Sat, 7 Nov 2020 14:48:36 -0800 Subject: [PATCH 1/8] Add a main struct, part 1 Start the process of encapsulating most of the flags and not using them as global variables. This commit JUST does the git command flag, which is now only accessed from main() --- cmd/git-sync/main.go | 106 +++++++++++++++++++++++++------------------ 1 file changed, 61 insertions(+), 45 deletions(-) diff --git a/cmd/git-sync/main.go b/cmd/git-sync/main.go index 55a3c11..ffbb604 100644 --- a/cmd/git-sync/main.go +++ b/cmd/git-sync/main.go @@ -249,6 +249,11 @@ func setGlogFlags() { vFlag.Value.Set(strconv.Itoa(*flVerbose)) } +// repoSync represents the remote repo and the local sync of it. +type repoSync struct { + cmd string // the git command to run +} + func main() { // In case we come up as pid 1, act as init. if os.Getpid() == 1 { @@ -425,12 +430,17 @@ func main() { } } + // Capture the various git parameters. + git := &repoSync{ + cmd: *flGitCmd, + } + // This context is used only for git credentials initialization. There are no long-running operations like // `git clone`, so hopefully 30 seconds will be enough. ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) if *flUsername != "" && *flPassword != "" { - if err := setupGitAuth(ctx, *flUsername, *flPassword, *flRepo); err != nil { + if err := git.SetupAuth(ctx, *flUsername, *flPassword, *flRepo); err != nil { log.Error(err, "ERROR: can't set up git auth") os.Exit(1) } @@ -444,14 +454,14 @@ func main() { } if *flCookieFile { - if err := setupGitCookieFile(ctx); err != nil { + if err := git.SetupCookieFile(ctx); err != nil { log.Error(err, "ERROR: can't set up git cookie file") os.Exit(1) } } if *flAskPassURL != "" { - if err := callGitAskPassURL(ctx, *flAskPassURL); err != nil { + if err := git.CallAskPassURL(ctx, *flAskPassURL); err != nil { askpassCount.WithLabelValues(metricKeyError).Inc() log.Error(err, "ERROR: failed to call ASKPASS callback URL", "url", *flAskPassURL) os.Exit(1) @@ -516,7 +526,7 @@ func main() { for { start := time.Now() ctx, cancel := context.WithTimeout(context.Background(), *flSyncTimeout) - if changed, hash, err := syncRepo(ctx, *flRepo, *flBranch, *flRev, *flDepth, absRoot, *flLink, *flAskPassURL, *flSubmodules); err != nil { + if changed, hash, err := git.SyncRepo(ctx, *flRepo, *flBranch, *flRev, *flDepth, absRoot, *flLink, *flAskPassURL, *flSubmodules); err != nil { updateSyncMetrics(metricKeyError, start) if *flMaxSyncFailures != -1 && failCount >= *flMaxSyncFailures { // Exit after too many retries, maybe the error is not recoverable. @@ -543,7 +553,7 @@ func main() { if *flOneTime { os.Exit(0) } - if isHash, err := revIsHash(ctx, *flRev, absRoot); err != nil { + if isHash, err := git.RevIsHash(ctx, *flRev, absRoot); err != nil { log.Error(err, "can't tell if rev is a git hash, exiting", "rev", *flRev) os.Exit(1) } else if isHash { @@ -657,8 +667,8 @@ func setRepoReady() { repoReady = true } -// addWorktreeAndSwap creates a new worktree and calls updateSymlink to swap the symlink to point to the new worktree -func addWorktreeAndSwap(ctx context.Context, gitRoot, link, branch, rev string, depth int, hash string, submoduleMode string) error { +// AddWorktreeAndSwap creates a new worktree and calls updateSymlink to swap the symlink to point to the new worktree +func (git *repoSync) AddWorktreeAndSwap(ctx context.Context, gitRoot, link, branch, rev string, depth int, hash string, submoduleMode string) error { log.V(0).Info("syncing git", "rev", rev, "hash", hash) args := []string{"fetch", "-f", "--tags"} @@ -668,18 +678,18 @@ func addWorktreeAndSwap(ctx context.Context, gitRoot, link, branch, rev string, args = append(args, "origin", branch) // Update from the remote. - if _, err := runCommand(ctx, gitRoot, *flGitCmd, args...); err != nil { + if _, err := runCommand(ctx, gitRoot, git.cmd, args...); err != nil { return err } // GC clone - if _, err := runCommand(ctx, gitRoot, *flGitCmd, "gc", "--prune=all"); err != nil { + if _, err := runCommand(ctx, gitRoot, git.cmd, "gc", "--prune=all"); err != nil { return err } // Make a worktree for this exact git hash. worktreePath := filepath.Join(gitRoot, "rev-"+hash) - _, err := runCommand(ctx, gitRoot, *flGitCmd, "worktree", "add", worktreePath, "origin/"+branch) + _, err := runCommand(ctx, gitRoot, git.cmd, "worktree", "add", worktreePath, "origin/"+branch) log.V(0).Info("adding worktree", "path", worktreePath, "branch", fmt.Sprintf("origin/%s", branch)) if err != nil { return err @@ -699,7 +709,7 @@ func addWorktreeAndSwap(ctx context.Context, gitRoot, link, branch, rev string, } // Reset the worktree's working copy to the specific rev. - _, err = runCommand(ctx, worktreePath, *flGitCmd, "reset", "--hard", hash) + _, err = runCommand(ctx, worktreePath, git.cmd, "reset", "--hard", hash) if err != nil { return err } @@ -716,7 +726,7 @@ func addWorktreeAndSwap(ctx context.Context, gitRoot, link, branch, rev string, if depth != 0 { submodulesArgs = append(submodulesArgs, "--depth", strconv.Itoa(depth)) } - _, err = runCommand(ctx, worktreePath, *flGitCmd, submodulesArgs...) + _, err = runCommand(ctx, worktreePath, git.cmd, submodulesArgs...) if err != nil { return err } @@ -753,7 +763,7 @@ func addWorktreeAndSwap(ctx context.Context, gitRoot, link, branch, rev string, if err := os.RemoveAll(oldWorktree); err != nil { return fmt.Errorf("error removing directory: %v", err) } - if _, err := runCommand(ctx, gitRoot, *flGitCmd, "worktree", "prune"); err != nil { + if _, err := runCommand(ctx, gitRoot, git.cmd, "worktree", "prune"); err != nil { return err } } @@ -761,7 +771,8 @@ func addWorktreeAndSwap(ctx context.Context, gitRoot, link, branch, rev string, return nil } -func cloneRepo(ctx context.Context, repo, branch, rev string, depth int, gitRoot string) error { +// CloneRepo does an initial clone of the git repo. +func (git *repoSync) CloneRepo(ctx context.Context, repo, branch, rev string, depth int, gitRoot string) error { args := []string{"clone", "--no-checkout", "-b", branch} if depth != 0 { args = append(args, "--depth", strconv.Itoa(depth)) @@ -769,7 +780,7 @@ func cloneRepo(ctx context.Context, repo, branch, rev string, depth int, gitRoot args = append(args, repo, gitRoot) log.V(0).Info("cloning repo", "origin", repo, "path", gitRoot) - _, err := runCommand(ctx, "", *flGitCmd, args...) + _, err := runCommand(ctx, "", git.cmd, args...) if err != nil { if strings.Contains(err.Error(), "already exists and is not an empty directory") { // Maybe a previous run crashed? Git won't use this dir. @@ -778,7 +789,7 @@ func cloneRepo(ctx context.Context, repo, branch, rev string, depth int, gitRoot if err != nil { return err } - _, err = runCommand(ctx, "", *flGitCmd, args...) + _, err = runCommand(ctx, "", git.cmd, args...) if err != nil { return err } @@ -790,18 +801,18 @@ func cloneRepo(ctx context.Context, repo, branch, rev string, depth int, gitRoot return nil } -// localHashForRev returns the locally known hash for a given rev. -func localHashForRev(ctx context.Context, rev, gitRoot string) (string, error) { - output, err := runCommand(ctx, gitRoot, *flGitCmd, "rev-parse", rev) +// LocalHashForRev returns the locally known hash for a given rev. +func (git *repoSync) LocalHashForRev(ctx context.Context, rev, gitRoot string) (string, error) { + output, err := runCommand(ctx, gitRoot, git.cmd, "rev-parse", rev) if err != nil { return "", err } return strings.Trim(string(output), "\n"), nil } -// remoteHashForRef returns the upstream hash for a given ref. -func remoteHashForRef(ctx context.Context, ref, gitRoot string) (string, error) { - output, err := runCommand(ctx, gitRoot, *flGitCmd, "ls-remote", "-q", "origin", ref) +// RemoteHashForRef returns the upstream hash for a given ref. +func (git *repoSync) RemoteHashForRef(ctx context.Context, ref, gitRoot string) (string, error) { + output, err := runCommand(ctx, gitRoot, git.cmd, "ls-remote", "-q", "origin", ref) if err != nil { return "", err } @@ -809,9 +820,9 @@ func remoteHashForRef(ctx context.Context, ref, gitRoot string) (string, error) return parts[0], nil } -func revIsHash(ctx context.Context, rev, gitRoot string) (bool, error) { +func (git *repoSync) RevIsHash(ctx context.Context, rev, gitRoot string) (bool, error) { // If git doesn't identify rev as a commit, we're done. - output, err := runCommand(ctx, gitRoot, *flGitCmd, "cat-file", "-t", rev) + output, err := runCommand(ctx, gitRoot, git.cmd, "cat-file", "-t", rev) if err != nil { return false, err } @@ -824,20 +835,20 @@ func revIsHash(ctx context.Context, rev, gitRoot string) (bool, error) { // hash, the output will be the same hash as the input. Of course, a user // could specify "abc" and match "abcdef12345678", so we just do a prefix // match. - output, err = localHashForRev(ctx, rev, gitRoot) + output, err = git.LocalHashForRev(ctx, rev, gitRoot) if err != nil { return false, err } return strings.HasPrefix(output, rev), nil } -// syncRepo syncs the branch of a given repository to the link at the given rev. +// SyncRepo syncs the branch of a given repository to the link at the given rev. // returns (1) whether a change occured, (2) the new hash, and (3) an error if one happened -func syncRepo(ctx context.Context, repo, branch, rev string, depth int, gitRoot, link string, authURL string, submoduleMode string) (bool, string, error) { +func (git *repoSync) SyncRepo(ctx context.Context, repo, branch, rev string, depth int, gitRoot, link string, authURL string, submoduleMode string) (bool, string, error) { if authURL != "" { // For ASKPASS Callback URL, the credentials behind is dynamic, it needs to be // re-fetched each time. - if err := callGitAskPassURL(ctx, authURL); err != nil { + if err := git.CallAskPassURL(ctx, authURL); err != nil { askpassCount.WithLabelValues(metricKeyError).Inc() return false, "", fmt.Errorf("failed to call GIT_ASKPASS_URL: %v", err) } @@ -851,11 +862,11 @@ func syncRepo(ctx context.Context, repo, branch, rev string, depth int, gitRoot, switch { case os.IsNotExist(err): // First time. Just clone it and get the hash. - err = cloneRepo(ctx, repo, branch, rev, depth, gitRoot) + err = git.CloneRepo(ctx, repo, branch, rev, depth, gitRoot) if err != nil { return false, "", err } - hash, err = localHashForRev(ctx, rev, gitRoot) + hash, err = git.LocalHashForRev(ctx, rev, gitRoot) if err != nil { return false, "", err } @@ -863,7 +874,7 @@ func syncRepo(ctx context.Context, repo, branch, rev string, depth int, gitRoot, return false, "", fmt.Errorf("error checking if repo exists %q: %v", gitRepoPath, err) default: // Not the first time. Figure out if the ref has changed. - local, remote, err := getRevs(ctx, target, branch, rev) + local, remote, err := git.GetRevs(ctx, target, branch, rev) if err != nil { return false, "", err } @@ -875,13 +886,13 @@ func syncRepo(ctx context.Context, repo, branch, rev string, depth int, gitRoot, hash = remote } - return true, hash, addWorktreeAndSwap(ctx, gitRoot, link, branch, rev, depth, hash, submoduleMode) + return true, hash, git.AddWorktreeAndSwap(ctx, gitRoot, link, branch, rev, depth, hash, submoduleMode) } -// getRevs returns the local and upstream hashes for rev. -func getRevs(ctx context.Context, localDir, branch, rev string) (string, string, error) { +// GetRevs returns the local and upstream hashes for rev. +func (git *repoSync) GetRevs(ctx context.Context, localDir, branch, rev string) (string, string, error) { // Ask git what the exact hash is for rev. - local, err := localHashForRev(ctx, rev, localDir) + local, err := git.LocalHashForRev(ctx, rev, localDir) if err != nil { return "", "", err } @@ -895,7 +906,7 @@ func getRevs(ctx context.Context, localDir, branch, rev string) (string, string, } // Figure out what hash the remote resolves ref to. - remote, err := remoteHashForRef(ctx, ref, localDir) + remote, err := git.RemoteHashForRef(ctx, ref, localDir) if err != nil { return "", "", err } @@ -947,16 +958,18 @@ func runCommandWithStdin(ctx context.Context, cwd, stdin, command string, args . return stdout, nil } -func setupGitAuth(ctx context.Context, username, password, gitURL string) error { +// SetupAuth configures the local git repo to use a username and password when +// accessing the repo at gitURL. +func (git *repoSync) SetupAuth(ctx context.Context, username, password, gitURL string) error { log.V(1).Info("setting up git credential store") - _, err := runCommand(ctx, "", *flGitCmd, "config", "--global", "credential.helper", "store") + _, err := runCommand(ctx, "", git.cmd, "config", "--global", "credential.helper", "store") if err != nil { return fmt.Errorf("can't configure git credential helper: %w", err) } creds := fmt.Sprintf("url=%v\nusername=%v\npassword=%v\n", gitURL, username, password) - _, err = runCommandWithStdin(ctx, "", creds, *flGitCmd, "credential", "approve") + _, err = runCommandWithStdin(ctx, "", creds, git.cmd, "credential", "approve") if err != nil { return fmt.Errorf("can't configure git credentials: %w", err) } @@ -993,7 +1006,7 @@ func setupGitSSH(setupKnownHosts bool) error { return nil } -func setupGitCookieFile(ctx context.Context) error { +func (git *repoSync) SetupCookieFile(ctx context.Context) error { log.V(1).Info("configuring git cookie file") var pathToCookieFile = "/etc/git-secret/cookie_file" @@ -1004,18 +1017,21 @@ func setupGitCookieFile(ctx context.Context) error { } if _, err = runCommand(ctx, "", - *flGitCmd, "config", "--global", "http.cookiefile", pathToCookieFile); err != nil { + git.cmd, "config", "--global", "http.cookiefile", pathToCookieFile); err != nil { return fmt.Errorf("can't configure git cookiefile: %w", err) } return nil } +// CallAskPassURL consults the specified URL looking for git credentials in the +// response. +// // The expected ASKPASS callback output are below, // see https://git-scm.com/docs/gitcredentials for more examples: -// username=xxx@example.com -// password=ya29.xxxyyyzzz -func callGitAskPassURL(ctx context.Context, url string) error { +// username=xxx@example.com +// password=ya29.xxxyyyzzz +func (git *repoSync) CallAskPassURL(ctx context.Context, url string) error { log.V(1).Info("calling GIT_ASKPASS URL to get credentials") var netClient = &http.Client{ @@ -1056,7 +1072,7 @@ func callGitAskPassURL(ctx context.Context, url string) error { } } - if err := setupGitAuth(ctx, username, password, *flRepo); err != nil { + if err := git.SetupAuth(ctx, username, password, *flRepo); err != nil { return err } From a80afb427d05280b829f205d986eeed6b7c5c430 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Sat, 7 Nov 2020 15:21:33 -0800 Subject: [PATCH 2/8] Add a main struct, part 2 This commit encapsulates the root parameter. This exposed a bug where we do not reset the root of the workspace. --- cmd/git-sync/main.go | 93 ++++++++++++++++++++++++-------------------- 1 file changed, 51 insertions(+), 42 deletions(-) diff --git a/cmd/git-sync/main.go b/cmd/git-sync/main.go index ffbb604..cccf60a 100644 --- a/cmd/git-sync/main.go +++ b/cmd/git-sync/main.go @@ -251,7 +251,8 @@ func setGlogFlags() { // repoSync represents the remote repo and the local sync of it. type repoSync struct { - cmd string // the git command to run + cmd string // the git command to run + root string // absolute path to the root directory } func main() { @@ -432,7 +433,8 @@ func main() { // Capture the various git parameters. git := &repoSync{ - cmd: *flGitCmd, + cmd: *flGitCmd, + root: absRoot, } // This context is used only for git credentials initialization. There are no long-running operations like @@ -526,7 +528,7 @@ func main() { for { start := time.Now() ctx, cancel := context.WithTimeout(context.Background(), *flSyncTimeout) - if changed, hash, err := git.SyncRepo(ctx, *flRepo, *flBranch, *flRev, *flDepth, absRoot, *flLink, *flAskPassURL, *flSubmodules); err != nil { + if changed, hash, err := git.SyncRepo(ctx, *flRepo, *flBranch, *flRev, *flDepth, *flLink, *flAskPassURL, *flSubmodules); err != nil { updateSyncMetrics(metricKeyError, start) if *flMaxSyncFailures != -1 && failCount >= *flMaxSyncFailures { // Exit after too many retries, maybe the error is not recoverable. @@ -553,7 +555,7 @@ func main() { if *flOneTime { os.Exit(0) } - if isHash, err := git.RevIsHash(ctx, *flRev, absRoot); err != nil { + if isHash, err := git.RevIsHash(ctx, *flRev); err != nil { log.Error(err, "can't tell if rev is a git hash, exiting", "rev", *flRev) os.Exit(1) } else if isHash { @@ -619,12 +621,12 @@ func addUser() error { return err } -// updateSymlink atomically swaps the symlink to point at the specified +// UpdateSymlink atomically swaps the symlink to point at the specified // directory and cleans up the previous worktree. If there was a previous // worktree, this returns the path to it. -func updateSymlink(ctx context.Context, gitRoot, link, newDir string) (string, error) { +func (git *repoSync) UpdateSymlink(ctx context.Context, link, newDir string) (string, error) { // Get currently-linked repo directory (to be removed), unless it doesn't exist - linkPath := filepath.Join(gitRoot, link) + linkPath := filepath.Join(git.root, link) oldWorktreePath, err := filepath.EvalSymlinks(linkPath) if err != nil && !os.IsNotExist(err) { return "", fmt.Errorf("error accessing current worktree: %v", err) @@ -632,19 +634,19 @@ func updateSymlink(ctx context.Context, gitRoot, link, newDir string) (string, e // newDir is absolute, so we need to change it to a relative path. This is // so it can be volume-mounted at another path and the symlink still works. - newDirRelative, err := filepath.Rel(gitRoot, newDir) + newDirRelative, err := filepath.Rel(git.root, newDir) if err != nil { return "", fmt.Errorf("error converting to relative path: %v", err) } const tmplink = "tmp-link" - log.V(1).Info("creating tmp symlink", "root", gitRoot, "dst", newDirRelative, "src", tmplink) - if _, err := runCommand(ctx, gitRoot, "ln", "-snf", newDirRelative, tmplink); err != nil { + log.V(1).Info("creating tmp symlink", "root", git.root, "dst", newDirRelative, "src", tmplink) + if _, err := runCommand(ctx, git.root, "ln", "-snf", newDirRelative, tmplink); err != nil { return "", fmt.Errorf("error creating symlink: %v", err) } - log.V(1).Info("renaming symlink", "root", gitRoot, "old_name", tmplink, "new_name", link) - if _, err := runCommand(ctx, gitRoot, "mv", "-T", tmplink, link); err != nil { + log.V(1).Info("renaming symlink", "root", git.root, "old_name", tmplink, "new_name", link) + if _, err := runCommand(ctx, git.root, "mv", "-T", tmplink, link); err != nil { return "", fmt.Errorf("error replacing symlink: %v", err) } @@ -667,8 +669,8 @@ func setRepoReady() { repoReady = true } -// AddWorktreeAndSwap creates a new worktree and calls updateSymlink to swap the symlink to point to the new worktree -func (git *repoSync) AddWorktreeAndSwap(ctx context.Context, gitRoot, link, branch, rev string, depth int, hash string, submoduleMode string) error { +// AddWorktreeAndSwap creates a new worktree and calls UpdateSymlink to swap the symlink to point to the new worktree +func (git *repoSync) AddWorktreeAndSwap(ctx context.Context, link, branch, rev string, depth int, hash string, submoduleMode string) error { log.V(0).Info("syncing git", "rev", rev, "hash", hash) args := []string{"fetch", "-f", "--tags"} @@ -678,18 +680,18 @@ func (git *repoSync) AddWorktreeAndSwap(ctx context.Context, gitRoot, link, bran args = append(args, "origin", branch) // Update from the remote. - if _, err := runCommand(ctx, gitRoot, git.cmd, args...); err != nil { + if _, err := runCommand(ctx, git.root, git.cmd, args...); err != nil { return err } // GC clone - if _, err := runCommand(ctx, gitRoot, git.cmd, "gc", "--prune=all"); err != nil { + if _, err := runCommand(ctx, git.root, git.cmd, "gc", "--prune=all"); err != nil { return err } // Make a worktree for this exact git hash. - worktreePath := filepath.Join(gitRoot, "rev-"+hash) - _, err := runCommand(ctx, gitRoot, git.cmd, "worktree", "add", worktreePath, "origin/"+branch) + worktreePath := filepath.Join(git.root, "rev-"+hash) + _, err := runCommand(ctx, git.root, git.cmd, "worktree", "add", worktreePath, "origin/"+branch) log.V(0).Info("adding worktree", "path", worktreePath, "branch", fmt.Sprintf("origin/%s", branch)) if err != nil { return err @@ -699,7 +701,7 @@ func (git *repoSync) AddWorktreeAndSwap(ctx context.Context, gitRoot, link, bran // /git/.git/worktrees/. Replace it with a reference // using relative paths, so that other containers can use a different volume // mount name. - worktreePathRelative, err := filepath.Rel(gitRoot, worktreePath) + worktreePathRelative, err := filepath.Rel(git.root, worktreePath) if err != nil { return err } @@ -751,8 +753,15 @@ func (git *repoSync) AddWorktreeAndSwap(ctx context.Context, gitRoot, link, bran } } + // Reset the root's rev (so we can prune and so we can rely on it later). + _, err = runCommand(ctx, git.root, git.cmd, "reset", "--hard", hash) + if err != nil { + return err + } + log.V(0).Info("reset root to hash", "path", git.root, "hash", hash) + // Flip the symlink. - oldWorktree, err := updateSymlink(ctx, gitRoot, link, worktreePath) + oldWorktree, err := git.UpdateSymlink(ctx, link, worktreePath) if err != nil { return err } @@ -763,7 +772,7 @@ func (git *repoSync) AddWorktreeAndSwap(ctx context.Context, gitRoot, link, bran if err := os.RemoveAll(oldWorktree); err != nil { return fmt.Errorf("error removing directory: %v", err) } - if _, err := runCommand(ctx, gitRoot, git.cmd, "worktree", "prune"); err != nil { + if _, err := runCommand(ctx, git.root, git.cmd, "worktree", "prune"); err != nil { return err } } @@ -772,20 +781,20 @@ func (git *repoSync) AddWorktreeAndSwap(ctx context.Context, gitRoot, link, bran } // CloneRepo does an initial clone of the git repo. -func (git *repoSync) CloneRepo(ctx context.Context, repo, branch, rev string, depth int, gitRoot string) error { +func (git *repoSync) CloneRepo(ctx context.Context, repo, branch, rev string, depth int) error { args := []string{"clone", "--no-checkout", "-b", branch} if depth != 0 { args = append(args, "--depth", strconv.Itoa(depth)) } - args = append(args, repo, gitRoot) - log.V(0).Info("cloning repo", "origin", repo, "path", gitRoot) + args = append(args, repo, git.root) + log.V(0).Info("cloning repo", "origin", repo, "path", git.root) _, err := runCommand(ctx, "", git.cmd, args...) if err != nil { if strings.Contains(err.Error(), "already exists and is not an empty directory") { // Maybe a previous run crashed? Git won't use this dir. - log.V(0).Info("git root exists and is not empty (previous crash?), cleaning up", "path", gitRoot) - err := os.RemoveAll(gitRoot) + log.V(0).Info("git root exists and is not empty (previous crash?), cleaning up", "path", git.root) + err := os.RemoveAll(git.root) if err != nil { return err } @@ -802,8 +811,8 @@ func (git *repoSync) CloneRepo(ctx context.Context, repo, branch, rev string, de } // LocalHashForRev returns the locally known hash for a given rev. -func (git *repoSync) LocalHashForRev(ctx context.Context, rev, gitRoot string) (string, error) { - output, err := runCommand(ctx, gitRoot, git.cmd, "rev-parse", rev) +func (git *repoSync) LocalHashForRev(ctx context.Context, rev string) (string, error) { + output, err := runCommand(ctx, git.root, git.cmd, "rev-parse", rev) if err != nil { return "", err } @@ -811,8 +820,8 @@ func (git *repoSync) LocalHashForRev(ctx context.Context, rev, gitRoot string) ( } // RemoteHashForRef returns the upstream hash for a given ref. -func (git *repoSync) RemoteHashForRef(ctx context.Context, ref, gitRoot string) (string, error) { - output, err := runCommand(ctx, gitRoot, git.cmd, "ls-remote", "-q", "origin", ref) +func (git *repoSync) RemoteHashForRef(ctx context.Context, ref string) (string, error) { + output, err := runCommand(ctx, git.root, git.cmd, "ls-remote", "-q", "origin", ref) if err != nil { return "", err } @@ -820,9 +829,9 @@ func (git *repoSync) RemoteHashForRef(ctx context.Context, ref, gitRoot string) return parts[0], nil } -func (git *repoSync) RevIsHash(ctx context.Context, rev, gitRoot string) (bool, error) { +func (git *repoSync) RevIsHash(ctx context.Context, rev string) (bool, error) { // If git doesn't identify rev as a commit, we're done. - output, err := runCommand(ctx, gitRoot, git.cmd, "cat-file", "-t", rev) + output, err := runCommand(ctx, git.root, git.cmd, "cat-file", "-t", rev) if err != nil { return false, err } @@ -835,7 +844,7 @@ func (git *repoSync) RevIsHash(ctx context.Context, rev, gitRoot string) (bool, // hash, the output will be the same hash as the input. Of course, a user // could specify "abc" and match "abcdef12345678", so we just do a prefix // match. - output, err = git.LocalHashForRev(ctx, rev, gitRoot) + output, err = git.LocalHashForRev(ctx, rev) if err != nil { return false, err } @@ -844,7 +853,7 @@ func (git *repoSync) RevIsHash(ctx context.Context, rev, gitRoot string) (bool, // SyncRepo syncs the branch of a given repository to the link at the given rev. // returns (1) whether a change occured, (2) the new hash, and (3) an error if one happened -func (git *repoSync) SyncRepo(ctx context.Context, repo, branch, rev string, depth int, gitRoot, link string, authURL string, submoduleMode string) (bool, string, error) { +func (git *repoSync) SyncRepo(ctx context.Context, repo, branch, rev string, depth int, link string, authURL string, submoduleMode string) (bool, string, error) { if authURL != "" { // For ASKPASS Callback URL, the credentials behind is dynamic, it needs to be // re-fetched each time. @@ -855,18 +864,18 @@ func (git *repoSync) SyncRepo(ctx context.Context, repo, branch, rev string, dep askpassCount.WithLabelValues(metricKeySuccess).Inc() } - target := filepath.Join(gitRoot, link) + target := filepath.Join(git.root, link) gitRepoPath := filepath.Join(target, ".git") var hash string _, err := os.Stat(gitRepoPath) switch { case os.IsNotExist(err): // First time. Just clone it and get the hash. - err = git.CloneRepo(ctx, repo, branch, rev, depth, gitRoot) + err = git.CloneRepo(ctx, repo, branch, rev, depth) if err != nil { return false, "", err } - hash, err = git.LocalHashForRev(ctx, rev, gitRoot) + hash, err = git.LocalHashForRev(ctx, rev) if err != nil { return false, "", err } @@ -874,7 +883,7 @@ func (git *repoSync) SyncRepo(ctx context.Context, repo, branch, rev string, dep return false, "", fmt.Errorf("error checking if repo exists %q: %v", gitRepoPath, err) default: // Not the first time. Figure out if the ref has changed. - local, remote, err := git.GetRevs(ctx, target, branch, rev) + local, remote, err := git.GetRevs(ctx, branch, rev) if err != nil { return false, "", err } @@ -886,13 +895,13 @@ func (git *repoSync) SyncRepo(ctx context.Context, repo, branch, rev string, dep hash = remote } - return true, hash, git.AddWorktreeAndSwap(ctx, gitRoot, link, branch, rev, depth, hash, submoduleMode) + return true, hash, git.AddWorktreeAndSwap(ctx, link, branch, rev, depth, hash, submoduleMode) } // GetRevs returns the local and upstream hashes for rev. -func (git *repoSync) GetRevs(ctx context.Context, localDir, branch, rev string) (string, string, error) { +func (git *repoSync) GetRevs(ctx context.Context, branch, rev string) (string, string, error) { // Ask git what the exact hash is for rev. - local, err := git.LocalHashForRev(ctx, rev, localDir) + local, err := git.LocalHashForRev(ctx, rev) if err != nil { return "", "", err } @@ -906,7 +915,7 @@ func (git *repoSync) GetRevs(ctx context.Context, localDir, branch, rev string) } // Figure out what hash the remote resolves ref to. - remote, err := git.RemoteHashForRef(ctx, ref, localDir) + remote, err := git.RemoteHashForRef(ctx, ref) if err != nil { return "", "", err } From b44dab6817378fafafecca5cafd930e2b109c8f7 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Sun, 8 Nov 2020 09:41:05 -0800 Subject: [PATCH 3/8] Add a main struct, part 3 This commit encapsulates the repo parameter. --- cmd/git-sync/main.go | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/cmd/git-sync/main.go b/cmd/git-sync/main.go index cccf60a..2e3dcdc 100644 --- a/cmd/git-sync/main.go +++ b/cmd/git-sync/main.go @@ -253,6 +253,7 @@ func setGlogFlags() { type repoSync struct { cmd string // the git command to run root string // absolute path to the root directory + repo string // remote repo to sync } func main() { @@ -435,6 +436,7 @@ func main() { git := &repoSync{ cmd: *flGitCmd, root: absRoot, + repo: *flRepo, } // This context is used only for git credentials initialization. There are no long-running operations like @@ -442,7 +444,7 @@ func main() { ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) if *flUsername != "" && *flPassword != "" { - if err := git.SetupAuth(ctx, *flUsername, *flPassword, *flRepo); err != nil { + if err := git.SetupAuth(ctx, *flUsername, *flPassword); err != nil { log.Error(err, "ERROR: can't set up git auth") os.Exit(1) } @@ -528,7 +530,7 @@ func main() { for { start := time.Now() ctx, cancel := context.WithTimeout(context.Background(), *flSyncTimeout) - if changed, hash, err := git.SyncRepo(ctx, *flRepo, *flBranch, *flRev, *flDepth, *flLink, *flAskPassURL, *flSubmodules); err != nil { + if changed, hash, err := git.SyncRepo(ctx, *flBranch, *flRev, *flDepth, *flLink, *flAskPassURL, *flSubmodules); err != nil { updateSyncMetrics(metricKeyError, start) if *flMaxSyncFailures != -1 && failCount >= *flMaxSyncFailures { // Exit after too many retries, maybe the error is not recoverable. @@ -781,13 +783,13 @@ func (git *repoSync) AddWorktreeAndSwap(ctx context.Context, link, branch, rev s } // CloneRepo does an initial clone of the git repo. -func (git *repoSync) CloneRepo(ctx context.Context, repo, branch, rev string, depth int) error { +func (git *repoSync) CloneRepo(ctx context.Context, branch, rev string, depth int) error { args := []string{"clone", "--no-checkout", "-b", branch} if depth != 0 { args = append(args, "--depth", strconv.Itoa(depth)) } - args = append(args, repo, git.root) - log.V(0).Info("cloning repo", "origin", repo, "path", git.root) + args = append(args, git.repo, git.root) + log.V(0).Info("cloning repo", "origin", git.repo, "path", git.root) _, err := runCommand(ctx, "", git.cmd, args...) if err != nil { @@ -853,7 +855,7 @@ func (git *repoSync) RevIsHash(ctx context.Context, rev string) (bool, error) { // SyncRepo syncs the branch of a given repository to the link at the given rev. // returns (1) whether a change occured, (2) the new hash, and (3) an error if one happened -func (git *repoSync) SyncRepo(ctx context.Context, repo, branch, rev string, depth int, link string, authURL string, submoduleMode string) (bool, string, error) { +func (git *repoSync) SyncRepo(ctx context.Context, branch, rev string, depth int, link string, authURL string, submoduleMode string) (bool, string, error) { if authURL != "" { // For ASKPASS Callback URL, the credentials behind is dynamic, it needs to be // re-fetched each time. @@ -871,7 +873,7 @@ func (git *repoSync) SyncRepo(ctx context.Context, repo, branch, rev string, dep switch { case os.IsNotExist(err): // First time. Just clone it and get the hash. - err = git.CloneRepo(ctx, repo, branch, rev, depth) + err = git.CloneRepo(ctx, branch, rev, depth) if err != nil { return false, "", err } @@ -968,8 +970,8 @@ func runCommandWithStdin(ctx context.Context, cwd, stdin, command string, args . } // SetupAuth configures the local git repo to use a username and password when -// accessing the repo at gitURL. -func (git *repoSync) SetupAuth(ctx context.Context, username, password, gitURL string) error { +// accessing the repo. +func (git *repoSync) SetupAuth(ctx context.Context, username, password string) error { log.V(1).Info("setting up git credential store") _, err := runCommand(ctx, "", git.cmd, "config", "--global", "credential.helper", "store") @@ -977,7 +979,7 @@ func (git *repoSync) SetupAuth(ctx context.Context, username, password, gitURL s return fmt.Errorf("can't configure git credential helper: %w", err) } - creds := fmt.Sprintf("url=%v\nusername=%v\npassword=%v\n", gitURL, username, password) + creds := fmt.Sprintf("url=%v\nusername=%v\npassword=%v\n", git.repo, username, password) _, err = runCommandWithStdin(ctx, "", creds, git.cmd, "credential", "approve") if err != nil { return fmt.Errorf("can't configure git credentials: %w", err) @@ -1081,7 +1083,7 @@ func (git *repoSync) CallAskPassURL(ctx context.Context, url string) error { } } - if err := git.SetupAuth(ctx, username, password, *flRepo); err != nil { + if err := git.SetupAuth(ctx, username, password); err != nil { return err } From 3a1212da364c27ee3fe5de90df24c9d5b3e39ba9 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Sun, 8 Nov 2020 09:52:21 -0800 Subject: [PATCH 4/8] Add a main struct, part 4 This commit encapsulates the branch and rev parameters. --- cmd/git-sync/main.go | 74 +++++++++++++++++++++++--------------------- 1 file changed, 39 insertions(+), 35 deletions(-) diff --git a/cmd/git-sync/main.go b/cmd/git-sync/main.go index 2e3dcdc..9925c88 100644 --- a/cmd/git-sync/main.go +++ b/cmd/git-sync/main.go @@ -251,9 +251,11 @@ func setGlogFlags() { // repoSync represents the remote repo and the local sync of it. type repoSync struct { - cmd string // the git command to run - root string // absolute path to the root directory - repo string // remote repo to sync + cmd string // the git command to run + root string // absolute path to the root directory + repo string // remote repo to sync + branch string // remote branch to sync + rev string // the rev or SHA to sync } func main() { @@ -434,9 +436,11 @@ func main() { // Capture the various git parameters. git := &repoSync{ - cmd: *flGitCmd, - root: absRoot, - repo: *flRepo, + cmd: *flGitCmd, + root: absRoot, + repo: *flRepo, + branch: *flBranch, + rev: *flRev, } // This context is used only for git credentials initialization. There are no long-running operations like @@ -530,7 +534,7 @@ func main() { for { start := time.Now() ctx, cancel := context.WithTimeout(context.Background(), *flSyncTimeout) - if changed, hash, err := git.SyncRepo(ctx, *flBranch, *flRev, *flDepth, *flLink, *flAskPassURL, *flSubmodules); err != nil { + if changed, hash, err := git.SyncRepo(ctx, *flDepth, *flLink, *flAskPassURL, *flSubmodules); err != nil { updateSyncMetrics(metricKeyError, start) if *flMaxSyncFailures != -1 && failCount >= *flMaxSyncFailures { // Exit after too many retries, maybe the error is not recoverable. @@ -557,11 +561,11 @@ func main() { if *flOneTime { os.Exit(0) } - if isHash, err := git.RevIsHash(ctx, *flRev); err != nil { - log.Error(err, "can't tell if rev is a git hash, exiting", "rev", *flRev) + if isHash, err := git.RevIsHash(ctx); err != nil { + log.Error(err, "can't tell if rev is a git hash, exiting", "rev", git.rev) os.Exit(1) } else if isHash { - log.V(0).Info("rev appears to be a git hash, no further sync needed", "rev", *flRev) + log.V(0).Info("rev appears to be a git hash, no further sync needed", "rev", git.rev) sleepForever() } initialSync = false @@ -672,14 +676,14 @@ func setRepoReady() { } // AddWorktreeAndSwap creates a new worktree and calls UpdateSymlink to swap the symlink to point to the new worktree -func (git *repoSync) AddWorktreeAndSwap(ctx context.Context, link, branch, rev string, depth int, hash string, submoduleMode string) error { - log.V(0).Info("syncing git", "rev", rev, "hash", hash) +func (git *repoSync) AddWorktreeAndSwap(ctx context.Context, link string, depth int, hash string, submoduleMode string) error { + log.V(0).Info("syncing git", "rev", git.rev, "hash", hash) args := []string{"fetch", "-f", "--tags"} if depth != 0 { args = append(args, "--depth", strconv.Itoa(depth)) } - args = append(args, "origin", branch) + args = append(args, "origin", git.branch) // Update from the remote. if _, err := runCommand(ctx, git.root, git.cmd, args...); err != nil { @@ -693,8 +697,8 @@ func (git *repoSync) AddWorktreeAndSwap(ctx context.Context, link, branch, rev s // Make a worktree for this exact git hash. worktreePath := filepath.Join(git.root, "rev-"+hash) - _, err := runCommand(ctx, git.root, git.cmd, "worktree", "add", worktreePath, "origin/"+branch) - log.V(0).Info("adding worktree", "path", worktreePath, "branch", fmt.Sprintf("origin/%s", branch)) + _, err := runCommand(ctx, git.root, git.cmd, "worktree", "add", worktreePath, "origin/"+git.branch) + log.V(0).Info("adding worktree", "path", worktreePath, "branch", fmt.Sprintf("origin/%s", git.branch)) if err != nil { return err } @@ -783,8 +787,8 @@ func (git *repoSync) AddWorktreeAndSwap(ctx context.Context, link, branch, rev s } // CloneRepo does an initial clone of the git repo. -func (git *repoSync) CloneRepo(ctx context.Context, branch, rev string, depth int) error { - args := []string{"clone", "--no-checkout", "-b", branch} +func (git *repoSync) CloneRepo(ctx context.Context, depth int) error { + args := []string{"clone", "--no-checkout", "-b", git.branch} if depth != 0 { args = append(args, "--depth", strconv.Itoa(depth)) } @@ -813,8 +817,8 @@ func (git *repoSync) CloneRepo(ctx context.Context, branch, rev string, depth in } // LocalHashForRev returns the locally known hash for a given rev. -func (git *repoSync) LocalHashForRev(ctx context.Context, rev string) (string, error) { - output, err := runCommand(ctx, git.root, git.cmd, "rev-parse", rev) +func (git *repoSync) LocalHashForRev(ctx context.Context) (string, error) { + output, err := runCommand(ctx, git.root, git.cmd, "rev-parse", git.rev) if err != nil { return "", err } @@ -831,9 +835,9 @@ func (git *repoSync) RemoteHashForRef(ctx context.Context, ref string) (string, return parts[0], nil } -func (git *repoSync) RevIsHash(ctx context.Context, rev string) (bool, error) { +func (git *repoSync) RevIsHash(ctx context.Context) (bool, error) { // If git doesn't identify rev as a commit, we're done. - output, err := runCommand(ctx, git.root, git.cmd, "cat-file", "-t", rev) + output, err := runCommand(ctx, git.root, git.cmd, "cat-file", "-t", git.rev) if err != nil { return false, err } @@ -846,16 +850,16 @@ func (git *repoSync) RevIsHash(ctx context.Context, rev string) (bool, error) { // hash, the output will be the same hash as the input. Of course, a user // could specify "abc" and match "abcdef12345678", so we just do a prefix // match. - output, err = git.LocalHashForRev(ctx, rev) + output, err = git.LocalHashForRev(ctx) if err != nil { return false, err } - return strings.HasPrefix(output, rev), nil + return strings.HasPrefix(output, git.rev), nil } // SyncRepo syncs the branch of a given repository to the link at the given rev. // returns (1) whether a change occured, (2) the new hash, and (3) an error if one happened -func (git *repoSync) SyncRepo(ctx context.Context, branch, rev string, depth int, link string, authURL string, submoduleMode string) (bool, string, error) { +func (git *repoSync) SyncRepo(ctx context.Context, depth int, link string, authURL string, submoduleMode string) (bool, string, error) { if authURL != "" { // For ASKPASS Callback URL, the credentials behind is dynamic, it needs to be // re-fetched each time. @@ -873,11 +877,11 @@ func (git *repoSync) SyncRepo(ctx context.Context, branch, rev string, depth int switch { case os.IsNotExist(err): // First time. Just clone it and get the hash. - err = git.CloneRepo(ctx, branch, rev, depth) + err = git.CloneRepo(ctx, depth) if err != nil { return false, "", err } - hash, err = git.LocalHashForRev(ctx, rev) + hash, err = git.LocalHashForRev(ctx) if err != nil { return false, "", err } @@ -885,35 +889,35 @@ func (git *repoSync) SyncRepo(ctx context.Context, branch, rev string, depth int return false, "", fmt.Errorf("error checking if repo exists %q: %v", gitRepoPath, err) default: // Not the first time. Figure out if the ref has changed. - local, remote, err := git.GetRevs(ctx, branch, rev) + local, remote, err := git.GetRevs(ctx) if err != nil { return false, "", err } if local == remote { - log.V(1).Info("no update required", "rev", rev, "local", local, "remote", remote) + log.V(1).Info("no update required", "rev", git.rev, "local", local, "remote", remote) return false, "", nil } - log.V(0).Info("update required", "rev", rev, "local", local, "remote", remote) + log.V(0).Info("update required", "rev", git.rev, "local", local, "remote", remote) hash = remote } - return true, hash, git.AddWorktreeAndSwap(ctx, link, branch, rev, depth, hash, submoduleMode) + return true, hash, git.AddWorktreeAndSwap(ctx, link, depth, hash, submoduleMode) } // GetRevs returns the local and upstream hashes for rev. -func (git *repoSync) GetRevs(ctx context.Context, branch, rev string) (string, string, error) { +func (git *repoSync) GetRevs(ctx context.Context) (string, string, error) { // Ask git what the exact hash is for rev. - local, err := git.LocalHashForRev(ctx, rev) + local, err := git.LocalHashForRev(ctx) if err != nil { return "", "", err } // Build a ref string, depending on whether the user asked to track HEAD or a tag. ref := "" - if rev == "HEAD" { - ref = "refs/heads/" + branch + if git.rev == "HEAD" { + ref = "refs/heads/" + git.branch } else { - ref = "refs/tags/" + rev + ref = "refs/tags/" + git.rev } // Figure out what hash the remote resolves ref to. From 8b321e3940f80f97aa95693704ddca708261b1b6 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Sun, 8 Nov 2020 09:55:57 -0800 Subject: [PATCH 5/8] Add a main struct, part 5 This commit encapsulates the depth parameter. --- cmd/git-sync/main.go | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/cmd/git-sync/main.go b/cmd/git-sync/main.go index 9925c88..a9882ae 100644 --- a/cmd/git-sync/main.go +++ b/cmd/git-sync/main.go @@ -256,6 +256,7 @@ type repoSync struct { repo string // remote repo to sync branch string // remote branch to sync rev string // the rev or SHA to sync + depth int // for shallow sync } func main() { @@ -441,6 +442,7 @@ func main() { repo: *flRepo, branch: *flBranch, rev: *flRev, + depth: *flDepth, } // This context is used only for git credentials initialization. There are no long-running operations like @@ -534,7 +536,7 @@ func main() { for { start := time.Now() ctx, cancel := context.WithTimeout(context.Background(), *flSyncTimeout) - if changed, hash, err := git.SyncRepo(ctx, *flDepth, *flLink, *flAskPassURL, *flSubmodules); err != nil { + if changed, hash, err := git.SyncRepo(ctx, *flLink, *flAskPassURL, *flSubmodules); err != nil { updateSyncMetrics(metricKeyError, start) if *flMaxSyncFailures != -1 && failCount >= *flMaxSyncFailures { // Exit after too many retries, maybe the error is not recoverable. @@ -676,12 +678,12 @@ func setRepoReady() { } // AddWorktreeAndSwap creates a new worktree and calls UpdateSymlink to swap the symlink to point to the new worktree -func (git *repoSync) AddWorktreeAndSwap(ctx context.Context, link string, depth int, hash string, submoduleMode string) error { +func (git *repoSync) AddWorktreeAndSwap(ctx context.Context, link string, hash string, submoduleMode string) error { log.V(0).Info("syncing git", "rev", git.rev, "hash", hash) args := []string{"fetch", "-f", "--tags"} - if depth != 0 { - args = append(args, "--depth", strconv.Itoa(depth)) + if git.depth != 0 { + args = append(args, "--depth", strconv.Itoa(git.depth)) } args = append(args, "origin", git.branch) @@ -731,8 +733,8 @@ func (git *repoSync) AddWorktreeAndSwap(ctx context.Context, link string, depth if submoduleMode == submodulesRecursive { submodulesArgs = append(submodulesArgs, "--recursive") } - if depth != 0 { - submodulesArgs = append(submodulesArgs, "--depth", strconv.Itoa(depth)) + if git.depth != 0 { + submodulesArgs = append(submodulesArgs, "--depth", strconv.Itoa(git.depth)) } _, err = runCommand(ctx, worktreePath, git.cmd, submodulesArgs...) if err != nil { @@ -787,10 +789,10 @@ func (git *repoSync) AddWorktreeAndSwap(ctx context.Context, link string, depth } // CloneRepo does an initial clone of the git repo. -func (git *repoSync) CloneRepo(ctx context.Context, depth int) error { +func (git *repoSync) CloneRepo(ctx context.Context) error { args := []string{"clone", "--no-checkout", "-b", git.branch} - if depth != 0 { - args = append(args, "--depth", strconv.Itoa(depth)) + if git.depth != 0 { + args = append(args, "--depth", strconv.Itoa(git.depth)) } args = append(args, git.repo, git.root) log.V(0).Info("cloning repo", "origin", git.repo, "path", git.root) @@ -859,7 +861,7 @@ func (git *repoSync) RevIsHash(ctx context.Context) (bool, error) { // SyncRepo syncs the branch of a given repository to the link at the given rev. // returns (1) whether a change occured, (2) the new hash, and (3) an error if one happened -func (git *repoSync) SyncRepo(ctx context.Context, depth int, link string, authURL string, submoduleMode string) (bool, string, error) { +func (git *repoSync) SyncRepo(ctx context.Context, link string, authURL string, submoduleMode string) (bool, string, error) { if authURL != "" { // For ASKPASS Callback URL, the credentials behind is dynamic, it needs to be // re-fetched each time. @@ -877,7 +879,7 @@ func (git *repoSync) SyncRepo(ctx context.Context, depth int, link string, authU switch { case os.IsNotExist(err): // First time. Just clone it and get the hash. - err = git.CloneRepo(ctx, depth) + err = git.CloneRepo(ctx) if err != nil { return false, "", err } @@ -901,7 +903,7 @@ func (git *repoSync) SyncRepo(ctx context.Context, depth int, link string, authU hash = remote } - return true, hash, git.AddWorktreeAndSwap(ctx, link, depth, hash, submoduleMode) + return true, hash, git.AddWorktreeAndSwap(ctx, link, hash, submoduleMode) } // GetRevs returns the local and upstream hashes for rev. From 70dd821e7b77f1411c9495aabe79221604766f2f Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Sun, 8 Nov 2020 10:06:35 -0800 Subject: [PATCH 6/8] Add a main struct, part 6 This commit encapsulates the submodules parameter. --- cmd/git-sync/main.go | 48 ++++++++++++++++++++++++-------------------- 1 file changed, 26 insertions(+), 22 deletions(-) diff --git a/cmd/git-sync/main.go b/cmd/git-sync/main.go index a9882ae..23199fd 100644 --- a/cmd/git-sync/main.go +++ b/cmd/git-sync/main.go @@ -164,10 +164,12 @@ const ( metricKeyNoOp = "noop" ) +type submodulesMode string + const ( - submodulesRecursive = "recursive" - submodulesShallow = "shallow" - submodulesOff = "off" + submodulesRecursive submodulesMode = "recursive" + submodulesShallow submodulesMode = "shallow" + submodulesOff submodulesMode = "off" ) func init() { @@ -251,12 +253,13 @@ func setGlogFlags() { // repoSync represents the remote repo and the local sync of it. type repoSync struct { - cmd string // the git command to run - root string // absolute path to the root directory - repo string // remote repo to sync - branch string // remote branch to sync - rev string // the rev or SHA to sync - depth int // for shallow sync + cmd string // the git command to run + root string // absolute path to the root directory + repo string // remote repo to sync + branch string // remote branch to sync + rev string // the rev or SHA to sync + depth int // for shallow sync + submodules submodulesMode // how to handle submodules } func main() { @@ -308,7 +311,7 @@ func main() { os.Exit(1) } - switch *flSubmodules { + switch submodulesMode(*flSubmodules) { case submodulesRecursive, submodulesShallow, submodulesOff: default: fmt.Fprintf(os.Stderr, "ERROR: --submodules must be one of %q, %q, or %q", submodulesRecursive, submodulesShallow, submodulesOff) @@ -437,12 +440,13 @@ func main() { // Capture the various git parameters. git := &repoSync{ - cmd: *flGitCmd, - root: absRoot, - repo: *flRepo, - branch: *flBranch, - rev: *flRev, - depth: *flDepth, + cmd: *flGitCmd, + root: absRoot, + repo: *flRepo, + branch: *flBranch, + rev: *flRev, + depth: *flDepth, + submodules: submodulesMode(*flSubmodules), } // This context is used only for git credentials initialization. There are no long-running operations like @@ -536,7 +540,7 @@ func main() { for { start := time.Now() ctx, cancel := context.WithTimeout(context.Background(), *flSyncTimeout) - if changed, hash, err := git.SyncRepo(ctx, *flLink, *flAskPassURL, *flSubmodules); err != nil { + if changed, hash, err := git.SyncRepo(ctx, *flLink, *flAskPassURL); err != nil { updateSyncMetrics(metricKeyError, start) if *flMaxSyncFailures != -1 && failCount >= *flMaxSyncFailures { // Exit after too many retries, maybe the error is not recoverable. @@ -678,7 +682,7 @@ func setRepoReady() { } // AddWorktreeAndSwap creates a new worktree and calls UpdateSymlink to swap the symlink to point to the new worktree -func (git *repoSync) AddWorktreeAndSwap(ctx context.Context, link string, hash string, submoduleMode string) error { +func (git *repoSync) AddWorktreeAndSwap(ctx context.Context, link string, hash string) error { log.V(0).Info("syncing git", "rev", git.rev, "hash", hash) args := []string{"fetch", "-f", "--tags"} @@ -727,10 +731,10 @@ func (git *repoSync) AddWorktreeAndSwap(ctx context.Context, link string, hash s // Update submodules // NOTE: this works for repo with or without submodules. - if submoduleMode != submodulesOff { + if git.submodules != submodulesOff { log.V(0).Info("updating submodules") submodulesArgs := []string{"submodule", "update", "--init"} - if submoduleMode == submodulesRecursive { + if git.submodules == submodulesRecursive { submodulesArgs = append(submodulesArgs, "--recursive") } if git.depth != 0 { @@ -861,7 +865,7 @@ func (git *repoSync) RevIsHash(ctx context.Context) (bool, error) { // SyncRepo syncs the branch of a given repository to the link at the given rev. // returns (1) whether a change occured, (2) the new hash, and (3) an error if one happened -func (git *repoSync) SyncRepo(ctx context.Context, link string, authURL string, submoduleMode string) (bool, string, error) { +func (git *repoSync) SyncRepo(ctx context.Context, link string, authURL string) (bool, string, error) { if authURL != "" { // For ASKPASS Callback URL, the credentials behind is dynamic, it needs to be // re-fetched each time. @@ -903,7 +907,7 @@ func (git *repoSync) SyncRepo(ctx context.Context, link string, authURL string, hash = remote } - return true, hash, git.AddWorktreeAndSwap(ctx, link, hash, submoduleMode) + return true, hash, git.AddWorktreeAndSwap(ctx, link, hash) } // GetRevs returns the local and upstream hashes for rev. From 4d808d47ed5deb31bf940488d742e89ee5037201 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Sun, 8 Nov 2020 10:34:25 -0800 Subject: [PATCH 7/8] Add a main struct, part 7 This commit encapsulates the chmod and link parameters. --- cmd/git-sync/main.go | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/cmd/git-sync/main.go b/cmd/git-sync/main.go index 23199fd..dbf148c 100644 --- a/cmd/git-sync/main.go +++ b/cmd/git-sync/main.go @@ -260,6 +260,8 @@ type repoSync struct { rev string // the rev or SHA to sync depth int // for shallow sync submodules submodulesMode // how to handle submodules + chmod int // mode to change repo to, or 0 + link string // the name of the symlink to publish under `root` } func main() { @@ -447,6 +449,8 @@ func main() { rev: *flRev, depth: *flDepth, submodules: submodulesMode(*flSubmodules), + chmod: *flChmod, + link: *flLink, } // This context is used only for git credentials initialization. There are no long-running operations like @@ -540,7 +544,7 @@ func main() { for { start := time.Now() ctx, cancel := context.WithTimeout(context.Background(), *flSyncTimeout) - if changed, hash, err := git.SyncRepo(ctx, *flLink, *flAskPassURL); err != nil { + if changed, hash, err := git.SyncRepo(ctx, *flAskPassURL); err != nil { updateSyncMetrics(metricKeyError, start) if *flMaxSyncFailures != -1 && failCount >= *flMaxSyncFailures { // Exit after too many retries, maybe the error is not recoverable. @@ -636,9 +640,9 @@ func addUser() error { // UpdateSymlink atomically swaps the symlink to point at the specified // directory and cleans up the previous worktree. If there was a previous // worktree, this returns the path to it. -func (git *repoSync) UpdateSymlink(ctx context.Context, link, newDir string) (string, error) { +func (git *repoSync) UpdateSymlink(ctx context.Context, newDir string) (string, error) { // Get currently-linked repo directory (to be removed), unless it doesn't exist - linkPath := filepath.Join(git.root, link) + linkPath := filepath.Join(git.root, git.link) oldWorktreePath, err := filepath.EvalSymlinks(linkPath) if err != nil && !os.IsNotExist(err) { return "", fmt.Errorf("error accessing current worktree: %v", err) @@ -657,8 +661,8 @@ func (git *repoSync) UpdateSymlink(ctx context.Context, link, newDir string) (st return "", fmt.Errorf("error creating symlink: %v", err) } - log.V(1).Info("renaming symlink", "root", git.root, "old_name", tmplink, "new_name", link) - if _, err := runCommand(ctx, git.root, "mv", "-T", tmplink, link); err != nil { + log.V(1).Info("renaming symlink", "root", git.root, "oldName", tmplink, "newName", git.link) + if _, err := runCommand(ctx, git.root, "mv", "-T", tmplink, git.link); err != nil { return "", fmt.Errorf("error replacing symlink: %v", err) } @@ -682,7 +686,7 @@ func setRepoReady() { } // AddWorktreeAndSwap creates a new worktree and calls UpdateSymlink to swap the symlink to point to the new worktree -func (git *repoSync) AddWorktreeAndSwap(ctx context.Context, link string, hash string) error { +func (git *repoSync) AddWorktreeAndSwap(ctx context.Context, hash string) error { log.V(0).Info("syncing git", "rev", git.rev, "hash", hash) args := []string{"fetch", "-f", "--tags"} @@ -747,8 +751,8 @@ func (git *repoSync) AddWorktreeAndSwap(ctx context.Context, link string, hash s } // Change the file permissions, if requested. - if *flChmod != 0 { - mode := fmt.Sprintf("%#o", *flChmod) + if git.chmod != 0 { + mode := fmt.Sprintf("%#o", git.chmod) log.V(0).Info("changing file permissions", "mode", mode) _, err = runCommand(ctx, "", "chmod", "-R", mode, worktreePath) if err != nil { @@ -773,7 +777,7 @@ func (git *repoSync) AddWorktreeAndSwap(ctx context.Context, link string, hash s log.V(0).Info("reset root to hash", "path", git.root, "hash", hash) // Flip the symlink. - oldWorktree, err := git.UpdateSymlink(ctx, link, worktreePath) + oldWorktree, err := git.UpdateSymlink(ctx, worktreePath) if err != nil { return err } @@ -865,7 +869,7 @@ func (git *repoSync) RevIsHash(ctx context.Context) (bool, error) { // SyncRepo syncs the branch of a given repository to the link at the given rev. // returns (1) whether a change occured, (2) the new hash, and (3) an error if one happened -func (git *repoSync) SyncRepo(ctx context.Context, link string, authURL string) (bool, string, error) { +func (git *repoSync) SyncRepo(ctx context.Context, authURL string) (bool, string, error) { if authURL != "" { // For ASKPASS Callback URL, the credentials behind is dynamic, it needs to be // re-fetched each time. @@ -876,7 +880,7 @@ func (git *repoSync) SyncRepo(ctx context.Context, link string, authURL string) askpassCount.WithLabelValues(metricKeySuccess).Inc() } - target := filepath.Join(git.root, link) + target := filepath.Join(git.root, git.link) gitRepoPath := filepath.Join(target, ".git") var hash string _, err := os.Stat(gitRepoPath) @@ -907,7 +911,7 @@ func (git *repoSync) SyncRepo(ctx context.Context, link string, authURL string) hash = remote } - return true, hash, git.AddWorktreeAndSwap(ctx, link, hash) + return true, hash, git.AddWorktreeAndSwap(ctx, hash) } // GetRevs returns the local and upstream hashes for rev. From c410bb5610b540faf659daf281cd3a9e94e5491b Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Sun, 8 Nov 2020 10:41:53 -0800 Subject: [PATCH 8/8] Add a main struct, part 8 This commit encapsulates the authURL parameter. --- cmd/git-sync/main.go | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/cmd/git-sync/main.go b/cmd/git-sync/main.go index dbf148c..ba8e062 100644 --- a/cmd/git-sync/main.go +++ b/cmd/git-sync/main.go @@ -262,6 +262,7 @@ type repoSync struct { submodules submodulesMode // how to handle submodules chmod int // mode to change repo to, or 0 link string // the name of the symlink to publish under `root` + authURL string // a URL to re-fetch credentials, or "" } func main() { @@ -451,6 +452,7 @@ func main() { submodules: submodulesMode(*flSubmodules), chmod: *flChmod, link: *flLink, + authURL: *flAskPassURL, } // This context is used only for git credentials initialization. There are no long-running operations like @@ -478,15 +480,6 @@ func main() { } } - if *flAskPassURL != "" { - if err := git.CallAskPassURL(ctx, *flAskPassURL); err != nil { - askpassCount.WithLabelValues(metricKeyError).Inc() - log.Error(err, "ERROR: failed to call ASKPASS callback URL", "url", *flAskPassURL) - os.Exit(1) - } - askpassCount.WithLabelValues(metricKeySuccess).Inc() - } - // The scope of the initialization context ends here, so we call cancel to release resources associated with it. cancel() @@ -544,7 +537,7 @@ func main() { for { start := time.Now() ctx, cancel := context.WithTimeout(context.Background(), *flSyncTimeout) - if changed, hash, err := git.SyncRepo(ctx, *flAskPassURL); err != nil { + if changed, hash, err := git.SyncRepo(ctx); err != nil { updateSyncMetrics(metricKeyError, start) if *flMaxSyncFailures != -1 && failCount >= *flMaxSyncFailures { // Exit after too many retries, maybe the error is not recoverable. @@ -869,11 +862,11 @@ func (git *repoSync) RevIsHash(ctx context.Context) (bool, error) { // SyncRepo syncs the branch of a given repository to the link at the given rev. // returns (1) whether a change occured, (2) the new hash, and (3) an error if one happened -func (git *repoSync) SyncRepo(ctx context.Context, authURL string) (bool, string, error) { - if authURL != "" { +func (git *repoSync) SyncRepo(ctx context.Context) (bool, string, error) { + if git.authURL != "" { // For ASKPASS Callback URL, the credentials behind is dynamic, it needs to be // re-fetched each time. - if err := git.CallAskPassURL(ctx, authURL); err != nil { + if err := git.CallAskPassURL(ctx); err != nil { askpassCount.WithLabelValues(metricKeyError).Inc() return false, "", fmt.Errorf("failed to call GIT_ASKPASS_URL: %v", err) } @@ -1056,7 +1049,7 @@ func (git *repoSync) SetupCookieFile(ctx context.Context) error { // see https://git-scm.com/docs/gitcredentials for more examples: // username=xxx@example.com // password=ya29.xxxyyyzzz -func (git *repoSync) CallAskPassURL(ctx context.Context, url string) error { +func (git *repoSync) CallAskPassURL(ctx context.Context) error { log.V(1).Info("calling GIT_ASKPASS URL to get credentials") var netClient = &http.Client{ @@ -1065,7 +1058,7 @@ func (git *repoSync) CallAskPassURL(ctx context.Context, url string) error { return http.ErrUseLastResponse }, } - httpReq, err := http.NewRequestWithContext(ctx, "GET", url, nil) + httpReq, err := http.NewRequestWithContext(ctx, "GET", git.authURL, nil) if err != nil { return fmt.Errorf("can't create auth request: %w", err) }