Don't try to remove the root if it appears corrupt

The `--root` is often a volume and can't be removed.  Instead, remove
the contents of it.  Adjust tests to hit this.

cmd/git-sync/main.go

@@ -566,6 +566,25 @@ func main() {
 	}
 }
 
+func removeDirContents(dir string, log *logging.Logger) error {
+	dirents, err := ioutil.ReadDir(dir)
+	if err != nil {
+		return err
+	}
+
+	for _, fi := range dirents {
+		p := filepath.Join(dir, fi.Name())
+		if log != nil {
+			log.V(2).Info("removing path recursively", "path", p, "isDir", fi.IsDir())
+		}
+		if err := os.RemoveAll(p); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 func updateSyncMetrics(key string, start time.Time) {
 	syncDuration.WithLabelValues(key).Observe(time.Since(start).Seconds())
 	syncCount.WithLabelValues(key).Inc()
@@ -849,7 +868,10 @@ func cloneRepo(ctx context.Context, repo, branch, rev string, depth int, gitRoot
 		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)
+			// We remove the contents rather than the dir itself, because a
+			// common use-case is to have a volume mounted at git.root, which
+			// makes removing it impossible.
+			err := removeDirContents(gitRoot, log)
 			if err != nil {
 				return err
 			}

test_e2e.sh

@@ -98,10 +98,19 @@ function docker_kill() {
     docker kill "$1" >/dev/null
 }
 
+# DIR is the directory in which all this test's state lives.
 RUNID="${RANDOM}${RANDOM}"
 DIR="/tmp/git-sync-e2e.$RUNID"
 mkdir "$DIR"
 
+# WORK is temp space and in reset for each testcase.
+WORK="$DIR/work"
+function clean_work() {
+    rm -rf "$WORK"
+    mkdir -p "$WORK"
+}
+
+# REPO is the source repo under test.
 REPO="$DIR/repo"
 MAIN_BRANCH="e2e-branch"
 function init_repo() {
@@ -113,6 +122,7 @@ function init_repo() {
     git -C "$REPO" commit -aqm "init file"
 }
 
+# ROOT is the volume (usually) used as --root.
 ROOT="$DIR/root"
 function clean_root() {
     rm -rf "$ROOT"
@@ -144,7 +154,9 @@ function GIT_SYNC() {
         --label git-sync-e2e="$RUNID" \
         --network="host" \
         -u $(id -u):$(id -g) \
-        -v "$DIR":"$DIR":rw \
+        -v "$ROOT":"$ROOT":rw \
+        -v "$REPO":"$REPO":ro \
+        -v "$WORK":"$WORK":ro \
         -v "$(pwd)/slow_git_clone.sh":"$SLOW_GIT_CLONE":ro \
         -v "$(pwd)/slow_git_fetch.sh":"$SLOW_GIT_FETCH":ro \
         -v "$(pwd)/askpass_git.sh":"$ASKPASS_GIT":ro \
@@ -200,7 +212,6 @@ function e2e::head_once() {
 function e2e::non_zero_exit() {
     echo "$FUNCNAME" > "$REPO"/file
     git -C "$REPO" commit -qam "$FUNCNAME"
-    ln -s "$ROOT" "$DIR/rootlink" # symlink to test
     (
         set +o errexit
         GIT_SYNC \
@@ -208,7 +219,7 @@ function e2e::non_zero_exit() {
             --repo="file://$REPO" \
             --branch="$MAIN_BRANCH" \
             --rev=does-not-exit \
-            --root="$DIR/rootlink" \
+            --root="$ROOT" \
             --dest="link" \
             >> "$1" 2>&1
         RET=$?
@@ -981,7 +992,7 @@ function e2e::exechook_fail_once() {
 # Test webhook success
 ##############################################
 function e2e::webhook_success() {
-    HITLOG="$DIR/hitlog"
+    HITLOG="$WORK/hitlog"
 
     # First sync
     cat /dev/null > "$HITLOG"
@@ -1028,7 +1039,7 @@ function e2e::webhook_success() {
 # Test webhook fail-retry
 ##############################################
 function e2e::webhook_fail_retry() {
-    HITLOG="$DIR/hitlog"
+    HITLOG="$WORK/hitlog"
 
     # First sync - return a failure to ensure that we try again
     cat /dev/null > "$HITLOG"
@@ -1077,7 +1088,7 @@ function e2e::webhook_fail_retry() {
 # Test webhook success with --one-time
 ##############################################
 function e2e::webhook_success_once() {
-    HITLOG="$DIR/hitlog"
+    HITLOG="$WORK/hitlog"
 
     # First sync
     cat /dev/null > "$HITLOG"
@@ -1114,7 +1125,7 @@ function e2e::webhook_success_once() {
 # Test webhook fail with --one-time
 ##############################################
 function e2e::webhook_fail_retry_once() {
-    HITLOG="$DIR/hitlog"
+    HITLOG="$WORK/hitlog"
 
     # First sync - return a failure to ensure that we try again
     cat /dev/null > "$HITLOG"
@@ -1150,7 +1161,7 @@ function e2e::webhook_fail_retry_once() {
 # Test webhook fire-and-forget
 ##############################################
 function e2e::webhook_fire_and_forget() {
-    HITLOG="$DIR/hitlog"
+    HITLOG="$WORK/hitlog"
 
     # First sync
     cat /dev/null > "$HITLOG"
@@ -1239,7 +1250,7 @@ function e2e::http() {
 function e2e::submodule_sync() {
     # Init submodule repo
     SUBMODULE_REPO_NAME="sub"
-    SUBMODULE="$DIR/$SUBMODULE_REPO_NAME"
+    SUBMODULE="$WORK/$SUBMODULE_REPO_NAME"
     mkdir "$SUBMODULE"
 
     git -C "$SUBMODULE" init -q -b "$MAIN_BRANCH"
@@ -1249,7 +1260,7 @@ function e2e::submodule_sync() {
 
     # Init nested submodule repo
     NESTED_SUBMODULE_REPO_NAME="nested-sub"
-    NESTED_SUBMODULE="$DIR/$NESTED_SUBMODULE_REPO_NAME"
+    NESTED_SUBMODULE="$WORK/$NESTED_SUBMODULE_REPO_NAME"
     mkdir "$NESTED_SUBMODULE"
 
     git -C "$NESTED_SUBMODULE" init -q -b "$MAIN_BRANCH"
@@ -1339,7 +1350,7 @@ function e2e::submodule_sync() {
 function e2e::submodule_sync_depth() {
     # Init submodule repo
     SUBMODULE_REPO_NAME="sub"
-    SUBMODULE="$DIR/$SUBMODULE_REPO_NAME"
+    SUBMODULE="$WORK/$SUBMODULE_REPO_NAME"
     mkdir "$SUBMODULE"
 
     git -C "$SUBMODULE" init -b "$MAIN_BRANCH" > /dev/null
@@ -1417,7 +1428,7 @@ function e2e::submodule_sync_depth() {
 function e2e::submodule_sync_off() {
     # Init submodule repo
     SUBMODULE_REPO_NAME="sub"
-    SUBMODULE="$DIR/$SUBMODULE_REPO_NAME"
+    SUBMODULE="$WORK/$SUBMODULE_REPO_NAME"
     mkdir "$SUBMODULE"
 
     git -C "$SUBMODULE" init -q -b "$MAIN_BRANCH"
@@ -1448,7 +1459,7 @@ function e2e::submodule_sync_off() {
 function e2e::submodule_sync_shallow() {
     # Init submodule repo
     SUBMODULE_REPO_NAME="sub"
-    SUBMODULE="$DIR/$SUBMODULE_REPO_NAME"
+    SUBMODULE="$WORK/$SUBMODULE_REPO_NAME"
     mkdir "$SUBMODULE"
 
     git -C "$SUBMODULE" init -q -b "$MAIN_BRANCH"
@@ -1458,7 +1469,7 @@ function e2e::submodule_sync_shallow() {
 
     # Init nested submodule repo
     NESTED_SUBMODULE_REPO_NAME="nested-sub"
-    NESTED_SUBMODULE="$DIR/$NESTED_SUBMODULE_REPO_NAME"
+    NESTED_SUBMODULE="$WORK/$NESTED_SUBMODULE_REPO_NAME"
     mkdir "$NESTED_SUBMODULE"
 
     git -C "$NESTED_SUBMODULE" init -q -b "$MAIN_BRANCH"
@@ -1522,9 +1533,9 @@ function e2e::ssh() {
 # Test sparse-checkout files
 ##############################################
 function e2e::sparse_checkout() {
-    echo "!/*" > "$DIR"/sparseconfig
+    echo "!/*" > "$WORK"/sparseconfig
-    echo "!/*/" >> "$DIR"/sparseconfig
+    echo "!/*/" >> "$WORK"/sparseconfig
-    echo "file2" >> "$DIR"/sparseconfig
+    echo "file2" >> "$WORK"/sparseconfig
     echo "$FUNCNAME" > "$REPO"/file
     echo "$FUNCNAME" > "$REPO"/file2
     mkdir "$REPO"/dir
@@ -1540,7 +1551,7 @@ function e2e::sparse_checkout() {
         --rev=HEAD \
         --root="$ROOT" \
         --dest="link" \
-        --sparse-checkout-file="$DIR/sparseconfig" \
+        --sparse-checkout-file="$WORK/sparseconfig" \
         >> "$1" 2>&1
     assert_link_exists "$ROOT"/link
     assert_file_exists "$ROOT"/link/file2
@@ -1695,6 +1706,7 @@ echo
 # Iterate over the chosen tests and run them.
 for t; do
     clean_root
+    clean_work
     init_repo
     echo -n "testcase $t: "
     if "e2e::${t}" "${DIR}/log.$t"; then