Merge pull request #418 from thockin/master

Clean up worktree dirs during sync loop

cmd/git-sync/main.go

@@ -881,6 +881,18 @@ func setRepoReady() {
 	repoReady = true
 }
 
+// cleanupWorkTree() is used to remove a worktree and its folder
+func cleanupWorkTree(ctx context.Context, gitRoot, worktree string) error {
+	// Clean up worktree(s)
+	log.V(1).Info("removing worktree", "path", worktree)
+	if err := os.RemoveAll(worktree); err != nil {
+		return fmt.Errorf("error removing directory: %v", err)
+	} else if _, err := runCommand(ctx, gitRoot, *flGitCmd, "worktree", "prune"); err != nil {
+		return err
+	}
+	return nil
+}
+
 // 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, hash string) error {
 	log.V(0).Info("syncing git", "rev", git.rev, "hash", hash)
@@ -903,6 +915,15 @@ func (git *repoSync) AddWorktreeAndSwap(ctx context.Context, hash string) error
 
 	// Make a worktree for this exact git hash.
 	worktreePath := filepath.Join(git.root, hash)
+
+	// Avoid wedge cases where the worktree was created but this function
+	// error'd without cleaning up.  The next time thru the sync loop fails to
+	// create the worktree and bails out. This manifests as:
+	//     "fatal: '/repo/root/rev-nnnn' already exists"
+	if err := cleanupWorkTree(ctx, git.root, worktreePath); err != nil {
+		return err
+	}
+
 	_, err := runCommand(ctx, git.root, git.cmd, "worktree", "add", worktreePath, "origin/"+git.branch, "--no-checkout")
 	log.V(0).Info("adding worktree", "path", worktreePath, "branch", fmt.Sprintf("origin/%s", git.branch))
 	if err != nil {
@@ -1028,12 +1049,7 @@ func (git *repoSync) AddWorktreeAndSwap(ctx context.Context, hash string) error
 	// Clean up previous worktrees.
 	var cleanupErr error
 	if oldWorktree != "" {
-		log.V(1).Info("removing old worktree", "path", oldWorktree)
+		cleanupErr = cleanupWorkTree(ctx, git.root, oldWorktree)
-		if err := os.RemoveAll(oldWorktree); err != nil {
-			cleanupErr = fmt.Errorf("error removing directory: %v", err)
-		} else if _, err := runCommand(ctx, git.root, git.cmd, "worktree", "prune"); err != nil {
-			cleanupErr = err
-		}
 	}
 
 	if cleanupErr != nil {

test_e2e.sh

@@ -424,6 +424,54 @@ assert_file_eq "$ROOT"/link/file "$TESTCASE 1"
 # Wrap up
 pass
 
+##############################################
+# Test worktree-cleanup
+##############################################
+testcase "worktree-cleanup"
+
+echo "$TESTCASE" > "$REPO"/file
+git -C "$REPO" commit -qam "$TESTCASE"
+GIT_SYNC \
+    --period=100ms \
+    --repo="file://$REPO" \
+    --branch=e2e-branch \
+    --rev=HEAD \
+    --root="$ROOT" \
+    --dest="link" \
+    > "$DIR"/log."$TESTCASE" 2>&1 &
+
+# wait for first sync
+sleep 3
+assert_link_exists "$ROOT"/link
+assert_file_exists "$ROOT"/link/file
+assert_file_eq "$ROOT"/link/file "$TESTCASE"
+
+# suspend time so we can fake corruption
+docker ps --filter label="git-sync-e2e=$RUNID" --format="{{.ID}}" \
+    | while read CTR; do
+        docker pause "$CTR" >/dev/null
+    done
+
+# make a second commit
+echo "$TESTCASE-ok" > "$REPO"/file2
+git -C "$REPO" add file2
+git -C "$REPO" commit -qam "$TESTCASE new file"
+# make a worktree to collide with git-sync
+REV=$(git -C "$REPO" rev-list -n1 HEAD)
+git -C "$REPO" worktree add -q "$ROOT"/"$REV" -b e2e --no-checkout
+
+# resume time
+docker ps --filter label="git-sync-e2e=$RUNID" --format="{{.ID}}" \
+    | while read CTR; do
+        docker unpause "$CTR" >/dev/null
+    done
+
+sleep 3
+assert_file_exists "$ROOT"/link/file2
+assert_file_eq "$ROOT"/link/file2 "$TESTCASE-ok"
+# Wrap up
+pass
+
 ##############################################
 # Test readlink
 ##############################################