From b9b587ccb4c4c05f2045f73cbeb508e60fb4a409 Mon Sep 17 00:00:00 2001 From: yut23 Date: Tue, 24 Sep 2024 22:58:34 -0400 Subject: [PATCH 1/4] Don't use git tags in pull As seen in andsens/homeshick#225, this approach was more complicated than it needed to be and caused confusion for users. --- bin/homeshick | 9 ++++++++- lib/commands/pull.sh | 24 +++++------------------- 2 files changed, 13 insertions(+), 20 deletions(-) diff --git a/bin/homeshick b/bin/homeshick index 82f7322..5e2e905 100755 --- a/bin/homeshick +++ b/bin/homeshick @@ -171,6 +171,7 @@ case $cmd in cd) help cd ;; # cd is implemented in the homeshick.{sh,csh} helper script. help) help $help_cmd ;; *) + declare -a pull_completed for param in "${params[@]}"; do case $cmd in clone) @@ -182,7 +183,13 @@ case $cmd in refresh) (refresh $threshhold "$param") ;; pull) - (pull "$param") && pull_completed+=("$param") ;; + prev_hash=$(cd "$repos/$param" && git rev-parse HEAD) + (pull "$param") + result=$? + curr_hash=$(cd "$repos/$param" && git rev-parse HEAD) + [[ "$prev_hash" != "$curr_hash" ]] && pull_completed+=("$param") + (exit $result) + ;; symlink|link) symlink "$param" ;; track) diff --git a/lib/commands/pull.sh b/lib/commands/pull.sh index cc3b138..5e7360b 100644 --- a/lib/commands/pull.sh +++ b/lib/commands/pull.sh @@ -1,6 +1,5 @@ #!/usr/bin/env bash -BEFORE_PULL_TAG=__homeshick-before-pull__ pull() { [[ ! $1 ]] && help_err pull local castle=$1 @@ -14,15 +13,6 @@ 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 --no-sign "$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" @@ -36,7 +26,6 @@ pull() { err "$EX_SOFTWARE" "Unable update submodules for $repo. Git says:" "$git_out" fi success - trap - EXIT return "$EX_SUCCESS" } @@ -46,17 +35,14 @@ symlink_new_files() { local castle=$1 shift local repo="$repos/$castle" - local git_out - git_out=$(cd "$repo" && git diff --name-only --diff-filter=AR "$BEFORE_PULL_TAG" HEAD -- home 2>/dev/null | wc -l 2>&1) - local result=$? - # Remove the tag before doing anything else - (cd "$repo" && git tag -d "$BEFORE_PULL_TAG" &>/dev/null) - if [[ $result -ne 0 ]]; then - continue # Ignore errors, this operation is not mission critical - fi if [[ ! -d $repo/home ]]; then continue; fi + # @{1} refers to the previous reflog entry on the current branch, which + # will be right before the pull + if ! git_out=$(cd "$repo" && git diff --name-only --diff-filter=AR '@{1}' 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 updated_castles+=("$castle") fi From 7a1519652e634abdabbd6b7df6b4f4dd3b7d6d46 Mon Sep 17 00:00:00 2001 From: yut23 Date: Tue, 24 Sep 2024 23:08:24 -0400 Subject: [PATCH 2/4] Update pull tests --- test/suites/pull.bats | 82 +++---------------------------------------- 1 file changed, 5 insertions(+), 77 deletions(-) diff --git a/test/suites/pull.bats b/test/suites/pull.bats index be03d1a..2709ad9 100755 --- a/test/suites/pull.bats +++ b/test/suites/pull.bats @@ -13,24 +13,6 @@ teardown() { } 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() { # takes castle name as first argument and expected commit hash as second ( @@ -111,7 +93,6 @@ expect_no_new_files() { (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" ] } @@ -124,7 +105,6 @@ expect_no_new_files() { [ ! -e "$HOME/.gitignore" ] run homeshick pull rc-files << Date: Wed, 25 Sep 2024 19:37:13 -0400 Subject: [PATCH 3/4] Don't compare hashes if pull failed Also fix minor style nit. --- bin/homeshick | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/bin/homeshick b/bin/homeshick index 5e2e905..f12ad59 100755 --- a/bin/homeshick +++ b/bin/homeshick @@ -171,7 +171,7 @@ case $cmd in cd) help cd ;; # cd is implemented in the homeshick.{sh,csh} helper script. help) help $help_cmd ;; *) - declare -a pull_completed + pull_completed=() for param in "${params[@]}"; do case $cmd in clone) @@ -183,11 +183,13 @@ case $cmd in refresh) (refresh $threshhold "$param") ;; pull) - prev_hash=$(cd "$repos/$param" && git rev-parse HEAD) + prev_hash=$(cd "$repos/$param" && git rev-parse HEAD --) (pull "$param") result=$? - curr_hash=$(cd "$repos/$param" && git rev-parse HEAD) - [[ "$prev_hash" != "$curr_hash" ]] && pull_completed+=("$param") + if [[ $result -eq 0 ]]; then + curr_hash=$(cd "$repos/$param" && git rev-parse HEAD --) + [[ "$prev_hash" != "$curr_hash" ]] && pull_completed+=("$param") + fi (exit $result) ;; symlink|link) From 30268ff2124bb09bcf48b44cb15de38ed9a517d1 Mon Sep 17 00:00:00 2001 From: yut23 Date: Fri, 27 Sep 2024 12:35:28 -0400 Subject: [PATCH 4/4] Add test for pulling to a pre-initialized castle --- bin/homeshick | 2 +- test/suites/pull.bats | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/bin/homeshick b/bin/homeshick index f12ad59..6304d92 100755 --- a/bin/homeshick +++ b/bin/homeshick @@ -183,7 +183,7 @@ case $cmd in refresh) (refresh $threshhold "$param") ;; pull) - prev_hash=$(cd "$repos/$param" && git rev-parse HEAD --) + prev_hash=$(cd "$repos/$param" && git rev-parse HEAD -- 2>/dev/null) (pull "$param") result=$? if [[ $result -eq 0 ]]; then diff --git a/test/suites/pull.bats b/test/suites/pull.bats index 2709ad9..71f8329 100755 --- a/test/suites/pull.bats +++ b/test/suites/pull.bats @@ -304,3 +304,22 @@ expect_no_new_files() { assert_tag_points_to rc-files "$tag_before" [ ! -e "$HOME/.gitignore" ] } + +@test 'pull a castle with no local commits' { + # note: pull should succeed, but doesn't currently try to link + fixture 'pull-test' + (cd "$HOME" && git init .homesick/repos/pull-test) + ( + cd "$HOME/.homesick/repos/pull-test" && + git remote add origin "$REPO_FIXTURES/pull-test" && + printf '[branch "master"]\n remote = origin\n merge = refs/heads/master' >> .git/config + ) + + [ ! -e "$HOME/.bashrc" ] + [ ! -e "$HOME/.gitignore" ] + run homeshick pull --batch pull-test + assert_success + expect_no_new_files pull-test + [ ! -e "$HOME/.bashrc" ] + [ ! -e "$HOME/.gitignore" ] +}