From d97e4fa748071635bb7e861ef0c1e7900fcae7c8 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Mon, 19 Mar 2018 23:21:54 +0000 Subject: [PATCH 1/4] stash: fix nonsense pipeline An earlier change bba067d2 ("stash: don't delete untracked files that match pathspec", 2018-01-06) was made by taking a suggestion in a list discussion [1] but did not copy the suggested snippet correctly. And the bug was unnoticed during the review and slipped through. This fixes it. [1] https://public-inbox.org/git/xmqqpo7byjwb.fsf@gitster.mtv.corp.google.com/ Signed-off-by: Junio C Hamano --- git-stash.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/git-stash.sh b/git-stash.sh index b48b164748..4c92ec931f 100755 --- a/git-stash.sh +++ b/git-stash.sh @@ -315,9 +315,9 @@ push_stash () { if test $# != 0 then - git add -u -- "$@" | - git checkout-index -z --force --stdin - git diff-index -p --cached --binary HEAD -- "$@" | git apply --index -R + git add -u -- "$@" + git diff-index -p --cached --binary HEAD -- "$@" | + git apply --index -R else git reset --hard -q fi From 833622a945a677ec690e5ffb05a637e425591f1d Mon Sep 17 00:00:00 2001 From: Thomas Gummerer Date: Mon, 19 Mar 2018 23:21:55 +0000 Subject: [PATCH 2/4] stash push: avoid printing errors 'git stash push -u -- ' prints the following errors if only matches untracked files: fatal: pathspec 'untracked' did not match any files error: unrecognized input This is because we first clean up the untracked files using 'git clean ', and then use a command chain involving 'git add -u ' and 'git apply' to clear the changes to files that are in the index and were stashed. As the only includes untracked files that were already removed by 'git clean', the 'git add' call will barf, and so will 'git apply', as there are no changes that need to be applied. Fix this by avoiding the 'git clean' if a pathspec is given, and use the pipeline that's used for pathspec mode to get rid of the untracked files as well. Reported-by: Marc Strapetz Signed-off-by: Thomas Gummerer Signed-off-by: Junio C Hamano --- git-stash.sh | 6 ++-- t/t3905-stash-include-untracked.sh | 46 ++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/git-stash.sh b/git-stash.sh index 4c92ec931f..5e06f96da5 100755 --- a/git-stash.sh +++ b/git-stash.sh @@ -308,14 +308,16 @@ push_stash () { if test -z "$patch_mode" then test "$untracked" = "all" && CLEAN_X_OPTION=-x || CLEAN_X_OPTION= - if test -n "$untracked" + if test -n "$untracked" && test $# = 0 then git clean --force --quiet -d $CLEAN_X_OPTION -- "$@" fi if test $# != 0 then - git add -u -- "$@" + test -z "$untracked" && UPDATE_OPTION="-u" || UPDATE_OPTION= + test "$untracked" = "all" && FORCE_OPTION="--force" || FORCE_OPTION= + git add $UPDATE_OPTION $FORCE_OPTION -- "$@" git diff-index -p --cached --binary HEAD -- "$@" | git apply --index -R else diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh index bfde4057ad..2f9045553e 100755 --- a/t/t3905-stash-include-untracked.sh +++ b/t/t3905-stash-include-untracked.sh @@ -228,4 +228,50 @@ test_expect_success 'stash previously ignored file' ' test_path_is_file ignored.d/foo ' +test_expect_success 'stash -u -- doesnt print error' ' + >untracked && + git stash push -u -- untracked 2>actual && + test_path_is_missing untracked && + test_line_count = 0 actual +' + +test_expect_success 'stash -u -- leaves rest of working tree in place' ' + >tracked && + git add tracked && + >untracked && + git stash push -u -- untracked && + test_path_is_missing untracked && + test_path_is_file tracked +' + +test_expect_success 'stash -u -- clears changes in both' ' + >tracked && + git add tracked && + >untracked && + git stash push -u -- tracked untracked && + test_path_is_missing tracked && + test_path_is_missing untracked +' + +test_expect_success 'stash --all -- stashes ignored file' ' + >ignored.d/bar && + git stash push --all -- ignored.d/bar && + test_path_is_missing ignored.d/bar +' + +test_expect_success 'stash --all -- clears changes in both' ' + >tracked && + git add tracked && + >ignored.d/bar && + git stash push --all -- tracked ignored.d/bar && + test_path_is_missing tracked && + test_path_is_missing ignored.d/bar +' + +test_expect_success 'stash -u -- leaves ignored file alone' ' + >ignored.d/bar && + git stash push -u -- ignored.d/bar && + test_path_is_file ignored.d/bar +' + test_done From d319bb18b1bd3e0b7269b44b9bae5db99dfbb7dd Mon Sep 17 00:00:00 2001 From: Thomas Gummerer Date: Mon, 19 Mar 2018 23:21:56 +0000 Subject: [PATCH 3/4] stash push -u: don't create empty stash When introducing the stash push feature, and thus allowing users to pass in a pathspec to limit the files that would get stashed in df6bba0937 ("stash: teach 'push' (and 'create_stash') to honor pathspec", 2017-02-28), this developer missed one place where the pathspec should be passed in. Namely in the call to the 'untracked_files()' function in the 'no_changes()' function. This resulted in 'git stash push -u -- ' creating an empty stash when there are untracked files in the repository other that don't match the pathspec. As 'git stash' never creates empty stashes, this behaviour is wrong and confusing for users. Instead it should just show a message "No local changes to save", and not create a stash. Luckily the 'untracked_files()' function already correctly respects pathspecs that are passed to it, so the fix is simply to pass the pathspec along to the function. Reported-by: Marc Strapetz Signed-off-by: Thomas Gummerer Signed-off-by: Junio C Hamano --- git-stash.sh | 2 +- t/t3905-stash-include-untracked.sh | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/git-stash.sh b/git-stash.sh index 5e06f96da5..4e55f278bd 100755 --- a/git-stash.sh +++ b/git-stash.sh @@ -39,7 +39,7 @@ fi no_changes () { git diff-index --quiet --cached HEAD --ignore-submodules -- "$@" && git diff-files --quiet --ignore-submodules -- "$@" && - (test -z "$untracked" || test -z "$(untracked_files)") + (test -z "$untracked" || test -z "$(untracked_files "$@")") } untracked_files () { diff --git a/t/t3905-stash-include-untracked.sh b/t/t3905-stash-include-untracked.sh index 2f9045553e..3ea5b9bb3f 100755 --- a/t/t3905-stash-include-untracked.sh +++ b/t/t3905-stash-include-untracked.sh @@ -274,4 +274,10 @@ test_expect_success 'stash -u -- leaves ignored file alone' ' test_path_is_file ignored.d/bar ' +test_expect_success 'stash -u -- shows no changes when there are none' ' + git stash push -u -- non-existant >actual && + echo "No local changes to save" >expect && + test_i18ncmp expect actual +' + test_done From 353278687e1a1a501c10431bcfe5605b5811d756 Mon Sep 17 00:00:00 2001 From: Thomas Gummerer Date: Wed, 21 Mar 2018 21:53:10 +0000 Subject: [PATCH 4/4] stash: drop superfluos pathspec parameter Since 833622a945 ("stash push: avoid printing errors", 2018-03-19) we don't use the 'git clean' call for the pathspec case anymore. The commit however forgot to remove the pathspec argument to the call. Remove the superfluos argument to make the code a little more obvious. Signed-off-by: Thomas Gummerer Signed-off-by: Junio C Hamano --- git-stash.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-stash.sh b/git-stash.sh index 4e55f278bd..d31924aea3 100755 --- a/git-stash.sh +++ b/git-stash.sh @@ -310,7 +310,7 @@ push_stash () { test "$untracked" = "all" && CLEAN_X_OPTION=-x || CLEAN_X_OPTION= if test -n "$untracked" && test $# = 0 then - git clean --force --quiet -d $CLEAN_X_OPTION -- "$@" + git clean --force --quiet -d $CLEAN_X_OPTION fi if test $# != 0