From 3883daf27c3cb7bb0e21b18edeb163f9fc98191e Mon Sep 17 00:00:00 2001 From: yut23 Date: Fri, 20 Oct 2023 00:38:57 -0400 Subject: [PATCH] Fix pull not symlinking new and renamed files pull creates a temporary tag in each castle before running git pull, and uses it to determine which files have been added (and renamed) in symlink_new_files, which also deletes it. --- bin/homeshick | 4 +- lib/commands/pull.sh | 19 ++- test/fixtures/pull-renamed.sh | 32 +++++ test/fixtures/pull-test.sh | 36 ++++++ test/suites/pull.bats | 233 ++++++++++++++++++++++++++++++++++ 5 files changed, 319 insertions(+), 5 deletions(-) create mode 100644 test/fixtures/pull-renamed.sh create mode 100644 test/fixtures/pull-test.sh diff --git a/bin/homeshick b/bin/homeshick index 0a182154..82f73227 100755 --- a/bin/homeshick +++ b/bin/homeshick @@ -182,7 +182,7 @@ case $cmd in refresh) (refresh $threshhold "$param") ;; pull) - (pull "$param") ;; + (pull "$param") && pull_completed+=("$param") ;; symlink|link) symlink "$param" ;; track) @@ -202,7 +202,7 @@ case $cmd in refresh) pull_outdated $threshhold "${params[@]}" ;; pull) - symlink_new_files "${params[@]}" ;; + symlink_new_files "${pull_completed[@]}" ;; esac result=$? if [[ $exit_status == 0 && $result != 0 ]]; then diff --git a/lib/commands/pull.sh b/lib/commands/pull.sh index 00e49df1..736a0b35 100644 --- a/lib/commands/pull.sh +++ b/lib/commands/pull.sh @@ -1,5 +1,6 @@ #!/usr/bin/env bash +BEFORE_PULL_TAG=__homeshick-before-pull__ pull() { [[ ! $1 ]] && help_err pull local castle=$1 @@ -13,6 +14,15 @@ pull() { return "$EX_SUCCESS" fi + # this tag is exceedingly unlikely to already exist, but if it does, error + # out and let the user resolve it + (cd "$repo" && git rev-parse --verify "refs/tags/$BEFORE_PULL_TAG" &>/dev/null) && \ + err "$EX_DATAERR" "Pull marker tag ($BEFORE_PULL_TAG) already exists in $repo. Please resolve this before pulling." + # make a tag at the current commit, so we can compare against it below + (cd "$repo" && git tag "$BEFORE_PULL_TAG" 2>&1) + # remove the tag if one of the git operations fails + trap '{ cd "$repo" && git tag -d "$BEFORE_PULL_TAG" &>/dev/null; }' EXIT + local git_out git_out=$(cd "$repo" && git pull 2>&1) || \ err "$EX_SOFTWARE" "Unable to pull $repo. Git says:" "$git_out" @@ -26,6 +36,7 @@ pull() { err "$EX_SOFTWARE" "Unable update submodules for $repo. Git says:" "$git_out" fi success + trap - EXIT return "$EX_SUCCESS" } @@ -35,13 +46,15 @@ symlink_new_files() { local castle=$1 shift local repo="$repos/$castle" + local before_pull + if ! before_pull=$(cd "$repo" && git rev-parse "refs/tags/$BEFORE_PULL_TAG" && git tag -d "$BEFORE_PULL_TAG" >/dev/null); then + continue + fi if [[ ! -d $repo/home ]]; then continue; fi local git_out - local now - now=$(date +%s) - if ! git_out=$(cd "$repo" && git diff --name-only --diff-filter=A "HEAD@{(($now-$T_START+1)).seconds.ago}" HEAD -- home 2>/dev/null | wc -l 2>&1); then + if ! git_out=$(cd "$repo" && git diff --name-only --diff-filter=AR "$before_pull" HEAD -- home 2>/dev/null | wc -l 2>&1); then continue # Ignore errors, this operation is not mission critical fi if [[ $git_out -gt 0 ]]; then diff --git a/test/fixtures/pull-renamed.sh b/test/fixtures/pull-renamed.sh new file mode 100644 index 00000000..d9d2a0ca --- /dev/null +++ b/test/fixtures/pull-renamed.sh @@ -0,0 +1,32 @@ +#!/usr/bin/env bash + +# shellcheck disable=2164 +fixture_pull_renamed() { + local git_username="Homeshick user" + local git_useremail="homeshick@example.com" + local repo="$REPO_FIXTURES/pull-renamed" + git init "$repo" + cd "$repo" + git config user.name "$git_username" + git config user.email "$git_useremail" + mkdir home + cd home + + cat > .bashrc-wrong-name <> .bashrc < /dev/null diff --git a/test/fixtures/pull-test.sh b/test/fixtures/pull-test.sh new file mode 100644 index 00000000..2c58845b --- /dev/null +++ b/test/fixtures/pull-test.sh @@ -0,0 +1,36 @@ +#!/usr/bin/env bash + +# shellcheck disable=2164 +fixture_pull_test() { + local git_username="Homeshick user" + local git_useremail="homeshick@example.com" + local repo="$REPO_FIXTURES/pull-test" + git init "$repo" + cd "$repo" + git config user.name "$git_username" + git config user.email "$git_useremail" + mkdir home + cd home + + cat > .bashrc < .gitignore <> .bashrc < /dev/null diff --git a/test/suites/pull.bats b/test/suites/pull.bats index 7efd528b..d1eb8ece 100755 --- a/test/suites/pull.bats +++ b/test/suites/pull.bats @@ -12,6 +12,92 @@ teardown() { delete_test_dir } +BEFORE_PULL_TAG=__homeshick-before-pull__ +assert_tag_is_removed() { + local castle + for castle in "$@"; do + ( + cd "$HOME/.homesick/repos/$castle" || return $? + # show all the tags if the test fails + { + echo "all tags in $castle:" + git show-ref --tags || true + echo + } >&2 + # this tag should not exist + run git rev-parse --verify "refs/tags/$BEFORE_PULL_TAG" >&2 2>&- + assert_failure + ) + done +} + +assert_tag_points_to() { + ( + castle="$1" + tag_before="$2" + cd "$HOME/.homesick/repos/$castle" || return $? + # show all the tags if the test fails + { + echo "all tags in $castle:" + git show-ref --tags || true + echo + } >&2 + tag_after=$(git rev-parse "refs/tags/$BEFORE_PULL_TAG") + [ "$tag_before" == "$tag_after" ] + ) +} + +reset_and_add_new_file() { + # takes castle name as first argument and commit to reset to as second + cd "$HOME/.homesick/repos/$1" || return $? + git reset --hard "$2" >/dev/null + + git config user.name "Homeshick user" + git config user.email "homeshick@example.com" + + cat > home/.ignore </dev/null + git commit -m 'Added .ignore file' >/dev/null +} + +expect_new_files() { + # takes castle name as first argument, and new files as remaining arguments + local castle="$1" + shift + local green='\e[1;32m' + local cyan='\e[1;36m' + local white='\e[1;37m' + local reset='\e[0m' + # these variables are intended to be parsed by printf + # shellcheck disable=SC2059 + { + printf "$cyan pull$reset %s\r" "$castle" + printf "$green pull$reset %s\n" "$castle" + printf "$white updates$reset The castle %s has new files.\n" "$castle" + printf "$cyan symlink?$reset [yN] y\r" + printf "$green symlink?$reset [yN] \n" + for file in "$@"; do + printf "$cyan symlink$reset %s\r" "$file" + printf "$green symlink$reset %s\n" "$file" + done + } | assert_output - +} + +expect_no_new_files() { + # takes castle name as first argument + local castle="$1" + local green='\e[1;32m' + local cyan='\e[1;36m' + local reset='\e[0m' + { + printf "$cyan pull$reset %s\r" "$castle" + printf "$green pull$reset %s\n" "$castle" + } | assert_output - +} + @test 'pull skips castles with no upstream remote' { castle 'rc-files' castle 'dotfiles' @@ -20,6 +106,153 @@ teardown() { (cd "$HOME/.homesick/repos/rc-files" && git remote rm origin) run homeshick pull rc-files dotfiles [ $status -eq 0 ] # EX_SUCCESS + assert_tag_is_removed rc-files dotfiles # dotfiles FETCH_HEAD should exist if the castle was pulled [ -e "$HOME/.homesick/repos/dotfiles/.git/FETCH_HEAD" ] } + +@test 'pull prompts for symlinking if new files are present' { + castle 'rc-files' + (cd "$HOME/.homesick/repos/rc-files" && git reset --hard HEAD~1 >/dev/null) + homeshick link --batch --quiet rc-files + + [ ! -e "$HOME/.gitignore" ] + run homeshick pull rc-files <</dev/null) + + [ ! -e "$HOME/.bashrc" ] + run homeshick pull pull-renamed <</dev/null) + + run homeshick pull --batch pull-test + assert_success + assert_tag_is_removed pull-test + expect_no_new_files pull-test +} + +@test 'pull a recently-pulled castle again' { + # this checks that we don't try to link files again if the last operation was + # a pull + castle 'rc-files' + (cd "$HOME/.homesick/repos/rc-files" && git reset --hard HEAD~1 >/dev/null) + homeshick pull --batch --force rc-files + + run homeshick pull --batch rc-files + assert_success + assert_tag_is_removed rc-files + expect_no_new_files rc-files +} + +@test 'pull a castle with a git conflict' { + local castle=pull-test + castle 'pull-test' + (reset_and_add_new_file pull-test HEAD~2) + (cd "$HOME/.homesick/repos/pull-test" && git config pull.rebase false && git config pull.ff only) + + [ ! -e "$HOME/.gitignore" ] + run homeshick pull --batch pull-test + assert_failure 70 # EX_SOFTWARE + assert_tag_is_removed pull-test + [ ! -e "$HOME/.gitignore" ] + local red='\e[1;31m' + local cyan='\e[1;36m' + local reset='\e[0m' + { + echo -ne "$cyan pull$reset pull-test\r" + echo -ne "$red pull$reset pull-test\n" + echo -ne "$red error$reset Unable to pull $HOME/.homesick/repos/pull-test. Git says:\n" + echo "fatal: Not possible to fast-forward, aborting." + } | assert_output - +} + +@test 'pull cleans up any marker tags after a git error' { + castle 'dotfiles' + castle 'pull-test' + castle 'nodirs' + (reset_and_add_new_file pull-test HEAD~2) + (cd "$HOME/.homesick/repos/pull-test" && git config pull.rebase false && git config pull.ff only) + + run homeshick pull --batch dotfiles pull-test nodirs + assert_failure 70 # EX_SOFTWARE + # tags in all castles should be cleaned up + assert_tag_is_removed dotfiles pull-test nodirs + local green='\e[1;32m' + local red='\e[1;31m' + local cyan='\e[1;36m' + local reset='\e[0m' + { + echo -ne "$cyan pull$reset dotfiles\r" + echo -ne "$green pull$reset dotfiles\n" + echo -ne "$cyan pull$reset pull-test\r" + echo -ne "$red pull$reset pull-test\n" + echo -ne "$red error$reset Unable to pull $HOME/.homesick/repos/pull-test. Git says:\n" + echo "fatal: Not possible to fast-forward, aborting." + echo -ne "$cyan pull$reset nodirs\r" + echo -ne "$green pull$reset nodirs\n" + } | assert_output - +} + +@test 'pull a castle where the marker tag already exists' { + castle 'rc-files' + local tag_before + tag_before=$( + cd "$HOME/.homesick/repos/rc-files" && + git reset --hard HEAD~1 >/dev/null && + git tag "$BEFORE_PULL_TAG" HEAD^ && + git rev-parse "$BEFORE_PULL_TAG" + ) + + [ ! -e "$HOME/.gitignore" ] + run homeshick pull --batch rc-files + assert_failure 65 # EX_DATAERR + # tag should not be touched + assert_tag_points_to rc-files "$tag_before" + [ ! -e "$HOME/.gitignore" ] + + local red='\e[1;31m' + local cyan='\e[1;36m' + local reset='\e[0m' + { + echo -ne "$cyan pull$reset rc-files\r" + echo -ne "$red pull$reset rc-files\n" + echo -ne "$red error$reset Pull marker tag ($BEFORE_PULL_TAG) already exists in $HOME/.homesick/repos/rc-files. Please resolve this before pulling." + } | assert_output - +} + +@test 'pull only cleans up the marker tag if we created it' { + castle 'dotfiles' + castle 'rc-files' + local tag_before + tag_before=$( + cd "$HOME/.homesick/repos/rc-files" && + git reset --hard HEAD~1 >/dev/null && + git tag "$BEFORE_PULL_TAG" HEAD^ && + git rev-parse "refs/tags/$BEFORE_PULL_TAG" + ) + + [ ! -e "$HOME/.gitignore" ] + run homeshick pull --batch dotfiles rc-files + assert_failure 65 # EX_DATAERR + # tag in dotfiles should be cleaned up + assert_tag_is_removed dotfiles + # tag in rc-files should not be touched + assert_tag_points_to rc-files "$tag_before" + [ ! -e "$HOME/.gitignore" ] +}