git-rm: Fix to properly handle files with spaces, tabs, newlines, etc.

New tests are added to the git-rm test case to cover this as well.

Signed-off-by: Carl Worth <cworth@cworth.org>
Signed-off-by: Junio C Hamano <junkio@cox.net>

diff --git a/git-rm.sh b/git-rm.sh
index 0a3f546683c11b1057209317fff1ab40fe847f81..fda4541c766cb1606402259cc1a6e0281105732f 100755 (executable)
--- a/git-rm.sh
+++ b/git-rm.sh
@@ -4,7 +4,6 @@ USAGE='[-f] [-n] [-v] [--] <file>...'
 SUBDIRECTORY_OK='Yes'
 . git-sh-setup
 
-index_remove_option=--force-remove
 remove_files=
 show_only=
 verbose=
@@ -12,7 +11,6 @@ while : ; do
   case "$1" in
     -f)
        remove_files=true
-       index_remote_option=--force
        ;;
     -n)
        show_only=true
@@ -45,23 +43,28 @@ case "$#" in
        ;;
 esac
 
-files=$(
-    if test -f "$GIT_DIR/info/exclude" ; then
-       git-ls-files \
-           --exclude-from="$GIT_DIR/info/exclude" \
-           --exclude-per-directory=.gitignore -- "$@"
-    else
-       git-ls-files \
+if test -f "$GIT_DIR/info/exclude"
+then
+       git-ls-files -z \
+       --exclude-from="$GIT_DIR/info/exclude" \
        --exclude-per-directory=.gitignore -- "$@"
-    fi | sort | uniq
-)
-
-case "$show_only" in
-true)
-       echo $files
+else
+       git-ls-files -z \
+       --exclude-per-directory=.gitignore -- "$@"
+fi |
+case "$show_only,$remove_files" in
+true,*)
+       xargs -0 echo
+       ;;
+*,true)
+       xargs -0 sh -c "
+               while [ \$# -gt 0 ]; do
+                       file=\$1; shift
+                       rm -- \"\$file\" && git-update-index --remove $verbose \"\$file\"
+               done
+       " inline
        ;;
 *)
-       [[ "$remove_files" = "true" ]] && rm -- $files
-       git-update-index $index_remove_option $verbose $files
+       git-update-index --force-remove $verbose -z --stdin
        ;;
 esac

diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index 841573210a83e0f6c33ef135e7fc86b45af14802..cabfadd56d03d3af26cdf890813945641cd90fad 100755 (executable)
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -7,36 +7,54 @@ test_description='Test of the various options to git-rm.'
 
 . ./test-lib.sh
 
-# Setup some files to be removed
-touch foo bar
-git-add foo bar
-# Need one to test --
-touch -- -q
-git update-index --add -- -q
-git-commit -m "add foo, bar, and -q"
+# Setup some files to be removed, some with funny characters
+touch -- foo bar baz 'space embedded' 'tab     embedded' 'newline
+embedded' -q
+git-add -- foo bar baz 'space embedded' 'tab   embedded' 'newline
+embedded' -q
+git-commit -m "add files"
 
 test_expect_success \
-    'Pre-check that foo is in index before git-rm foo' \
-    'git-ls-files --error-unmatch foo'
+    'Pre-check that foo exists and is in index before git-rm foo' \
+    '[ -f foo ] && git-ls-files --error-unmatch foo'
 
 test_expect_success \
     'Test that git-rm foo succeeds' \
     'git-rm foo'
 
-test_expect_failure \
-    'Post-check that foo is not in index after git-rm foo' \
-    'git-ls-files --error-unmatch foo'
+test_expect_success \
+    'Post-check that foo exists but is not in index after git-rm foo' \
+    '[ -f foo ] && ! git-ls-files --error-unmatch foo'
+
+test_expect_success \
+    'Pre-check that bar exists and is in index before "git-rm -f bar"' \
+    '[ -f bar ] && git-ls-files --error-unmatch bar'
 
 test_expect_success \
-    'Test that "git-rm -f bar" works' \
+    'Test that "git-rm -f bar" succeeds' \
     'git-rm -f bar'
 
-test_expect_failure \
-    'Post-check that bar no longer exists' \
-    '[ -f bar ]'
+test_expect_success \
+    'Post-check that bar does not exist and is not in index after "git-rm -f bar"' \
+    '! [ -f bar ] && ! git-ls-files --error-unmatch bar'
 
 test_expect_success \
-    'Test that "git-rm -- -q" works to delete a file named -q' \
+    'Test that "git-rm -- -q" succeeds (remove a file that looks like an option)' \
     'git-rm -- -q'
 
+test_expect_success \
+    "Test that \"git-rm -f\" succeeds with embedded space, tab, or newline characters." \
+    "git-rm -f 'space embedded' 'tab   embedded' 'newline
+embedded'"
+
+chmod u-w .
+test_expect_failure \
+    'Test that "git-rm -f" fails if its rm fails' \
+    'git-rm -f baz'
+chmod u+w .
+
+test_expect_success \
+    'When the rm in "git-rm -f" fails, it should not remove the file from the index' \
+    'git-ls-files --error-unmatch baz'
+
 test_done