From 248a53461dc8aabf51d6aab8e72ab94df8a18662 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Sat, 22 Jan 2022 12:37:07 -0800 Subject: [PATCH] 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 | 5 +++- test_e2e.sh | 62 +++++++++++++++++++++++++------------------- 2 files changed, 40 insertions(+), 27 deletions(-) diff --git a/cmd/git-sync/main.go b/cmd/git-sync/main.go index d0d1d3b..8d9e640 100644 --- a/cmd/git-sync/main.go +++ b/cmd/git-sync/main.go @@ -1068,7 +1068,10 @@ func (git *repoSync) CloneRepo(ctx context.Context) error { if strings.Contains(err.Error(), "already exists and is not an empty directory") { // Maybe a previous run crashed? Git won't use this dir. git.log.V(0).Info("git root exists and is not empty (previous crash?), cleaning up", "path", git.root) - err := os.RemoveAll(git.root) + // 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(git.root, git.log) if err != nil { return err } diff --git a/test_e2e.sh b/test_e2e.sh index 40bb137..c877daf 100755 --- a/test_e2e.sh +++ b/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" @@ -157,7 +167,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 \ @@ -193,19 +205,18 @@ function remove_containers() { function e2e::head_once_root_doesnt_exist() { echo "$FUNCNAME" > "$REPO"/file git -C "$REPO" commit -qam "$FUNCNAME" - rm -rf "$ROOT" # remove the root to test GIT_SYNC \ --one-time \ --repo="file://$REPO" \ --branch="$MAIN_BRANCH" \ --rev=HEAD \ - --root="$ROOT" \ + --root="$ROOT/subdir" \ --link="link" \ >> "$1" 2>&1 - assert_link_exists "$ROOT"/link - assert_file_exists "$ROOT"/link/file - assert_file_eq "$ROOT"/link/file "$FUNCNAME" + assert_link_exists "$ROOT"/subdir/link + assert_file_exists "$ROOT"/subdir/link/file + assert_file_eq "$ROOT"/subdir/link/file "$FUNCNAME" } ############################################## @@ -254,14 +265,14 @@ function e2e::head_once_root_flag_is_weird() { function e2e::head_once_root_flag_has_symlink() { echo "$FUNCNAME" > "$REPO"/file git -C "$REPO" commit -qam "$FUNCNAME" - ln -s "$ROOT" "$DIR/rootlink" # symlink to test + ln -s "$ROOT" "$ROOT/rootlink" # symlink to test GIT_SYNC \ --one-time \ --repo="file://$REPO" \ --branch="$MAIN_BRANCH" \ --rev=HEAD \ - --root="$DIR/rootlink" \ + --root="$ROOT/rootlink" \ --link="link" \ >> "$1" 2>&1 assert_link_exists "$ROOT"/link @@ -275,7 +286,6 @@ function e2e::head_once_root_flag_has_symlink() { 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 @@ -284,7 +294,7 @@ function e2e::non_zero_exit() { --repo="file://$REPO" \ --branch="$MAIN_BRANCH" \ --rev=does-not-exit \ - --root="$DIR/rootlink" \ + --root="$ROOT" \ --link="link" \ >> "$1" 2>&1 RET=$? @@ -330,7 +340,6 @@ function e2e::head_once_root_exists_but_fails_sanity() { SHA=$(git -C "$REPO" rev-parse HEAD) # Make an invalid git repo. - mkdir -p "$ROOT" git -C "$ROOT" init >/dev/null echo "ref: refs/heads/nonexist" > "$ROOT/.git/HEAD" @@ -1136,7 +1145,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" @@ -1183,7 +1192,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" @@ -1232,7 +1241,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" @@ -1269,7 +1278,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" @@ -1305,7 +1314,7 @@ function e2e::webhook_fail_retry_once() { # Test webhook fire-and-forget ############################################## function e2e::webhook_fire_and_forget() { - HITLOG="$DIR/hitlog" + HITLOG="$WORK/hitlog" cat /dev/null > "$HITLOG" CTR=$(docker_run \ @@ -1398,7 +1407,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" @@ -1408,7 +1417,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" @@ -1499,7 +1508,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 -q -b "$MAIN_BRANCH" @@ -1577,7 +1586,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" @@ -1608,7 +1617,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" @@ -1618,7 +1627,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" @@ -1682,9 +1691,9 @@ function e2e::ssh() { # Test sparse-checkout files ############################################## function e2e::sparse_checkout() { - echo "!/*" > "$DIR"/sparseconfig - echo "!/*/" >> "$DIR"/sparseconfig - echo "file2" >> "$DIR"/sparseconfig + echo "!/*" > "$WORK"/sparseconfig + echo "!/*/" >> "$WORK"/sparseconfig + echo "file2" >> "$WORK"/sparseconfig echo "$FUNCNAME" > "$REPO"/file echo "$FUNCNAME" > "$REPO"/file2 mkdir "$REPO"/dir @@ -1700,7 +1709,7 @@ function e2e::sparse_checkout() { --rev=HEAD \ --root="$ROOT" \ --link="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 @@ -1832,6 +1841,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