Merge pull request #208 from thockin/code-move-cleanups

Code move cleanups

cmd/git-sync/main.go

@@ -380,50 +380,36 @@ func sleepForever() {
 	os.Exit(0)
 }
 
-// updateSymlink atomically swaps the symlink to point at the specified directory and cleans up the previous worktree.
+// updateSymlink atomically swaps the symlink to point at the specified
-func updateSymlink(ctx context.Context, gitRoot, link, newDir string) error {
+// 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) {
 	// Get currently-linked repo directory (to be removed), unless it doesn't exist
 	fullpath := path.Join(gitRoot, link)
 	currentDir, err := filepath.EvalSymlinks(fullpath)
 	if err != nil && !os.IsNotExist(err) {
-		return fmt.Errorf("error accessing symlink: %v", err)
+		return "", fmt.Errorf("error accessing symlink: %v", err)
 	}
 
 	// newDir is /git/rev-..., we need to change it to relative path.
 	// Volume in other container may not be mounted at /git, so the symlink can't point to /git.
 	newDirRelative, err := filepath.Rel(gitRoot, newDir)
 	if err != nil {
-		return fmt.Errorf("error converting to relative path: %v", err)
+		return "", fmt.Errorf("error converting to relative path: %v", err)
 	}
 
 	const tmplink = "tmp-link"
 	if _, err := runCommand(ctx, gitRoot, "ln", "-snf", newDirRelative, tmplink); err != nil {
-		return fmt.Errorf("error creating symlink: %v", err)
+		return "", fmt.Errorf("error creating symlink: %v", err)
 	}
 	log.V(1).Info("created tmp symlink", "root", gitRoot, "dst", newDirRelative, "src", tmplink)
 
 	if _, err := runCommand(ctx, gitRoot, "mv", "-T", tmplink, link); err != nil {
-		return fmt.Errorf("error replacing symlink: %v", err)
+		return "", fmt.Errorf("error replacing symlink: %v", err)
 	}
 	log.V(1).Info("renamed symlink", "root", gitRoot, "old_name", tmplink, "new_name", link)
 
-	// Clean up previous worktree
+	return currentDir, nil
-	if len(currentDir) > 0 {
-		if err = os.RemoveAll(currentDir); err != nil {
-			return fmt.Errorf("error removing directory: %v", err)
-		}
-
-		log.V(1).Info("removed previous worktree", "path", currentDir)
-
-		_, err := runCommand(ctx, gitRoot, *flGitCmd, "worktree", "prune")
-		if err != nil {
-			return err
-		}
-
-		log.V(1).Info("pruned old worktrees")
-	}
-
-	return nil
 }
 
 // addWorktreeAndSwap creates a new worktree and calls updateSymlink to swap the symlink to point to the new worktree
@@ -494,7 +480,21 @@ func addWorktreeAndSwap(ctx context.Context, gitRoot, dest, branch, rev string,
 		}
 	}
 
-	return updateSymlink(ctx, gitRoot, dest, worktreePath)
+	// Flip the symlink.
+	if oldWorktree, err := updateSymlink(ctx, gitRoot, dest, worktreePath); err != nil {
+		return err
+	} else if oldWorktree != "" {
+		// Clean up previous worktree
+		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 {
+			return err
+		}
+		log.V(1).Info("pruned old worktrees")
+	}
+
+	return nil
 }
 
 func cloneRepo(ctx context.Context, repo, branch, rev string, depth int, gitRoot string) error {
@@ -533,6 +533,16 @@ func hashForRev(ctx context.Context, rev, gitRoot string) (string, error) {
 	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)
+	if err != nil {
+		return "", err
+	}
+	parts := strings.Split(string(output), "\t")
+	return parts[0], nil
+}
+
 func 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)
@@ -616,15 +626,6 @@ func getRevs(ctx context.Context, localDir, branch, rev string) (string, string,
 	return local, remote, nil
 }
 
-func remoteHashForRef(ctx context.Context, ref, gitRoot string) (string, error) {
-	output, err := runCommand(ctx, gitRoot, *flGitCmd, "ls-remote", "-q", "origin", ref)
-	if err != nil {
-		return "", err
-	}
-	parts := strings.Split(string(output), "\t")
-	return parts[0], nil
-}
-
 func cmdForLog(command string, args ...string) string {
 	if strings.ContainsAny(command, " \t\n") {
 		command = fmt.Sprintf("%q", command)