rm: loosen safety valve for empty files

If a file is different between the working tree copy, the index, and the
HEAD, then we do not allow it to be deleted without --force.

However, this is overly tight in the face of "git add --intent-to-add":

$ git add --intent-to-add file
$ : oops, I don't actually want to stage that yet
$ git rm --cached file
error: 'empty' has staged content different from both the
file and the HEAD (use -f to force removal)
$ git rm -f --cached file

Unfortunately, there is currently no way to distinguish between an empty
file that has been added and an "intent to add" file. The ideal behavior
would be to disallow the former while allowing the latter.

This patch loosens the safety valve to allow the deletion only if we are
deleting the cached entry and the cached content is empty.  This covers
the intent-to-add situation, and assumes there is little harm in not
protecting users who have legitimately added an empty file.  In many
cases, the file will still be empty, in which case the safety valve does
not trigger anyway (since the content remains untouched in the working
tree). Otherwise, we do remove the fact that no content was staged, but
given that the content is by definition empty, it is not terribly
difficult for a user to recreate it.

However, we still document the desired behavior in the form of two
tests. One checks the correct removal of an intent-to-add file. The other
checks that we still disallow removal of empty files, but is marked as
expect_failure to indicate this compromise. If the intent-to-add feature
is ever extended to differentiate between normal empty files and
intent-to-add files, then the safety valve can be re-tightened.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin-rm.c b/builtin-rm.c
index e06640cf8d3418cbbe177b8fdcdccd19e0f3379f..b7126e3e25d4f52ae8c4a120d524a24c11129785 100644 (file)
--- a/builtin-rm.c
+++ b/builtin-rm.c
@@ -79,7 +79,8 @@ static int check_local_mod(unsigned char *head, int index_only)
                     || hashcmp(ce->sha1, sha1))
                        staged_changes = 1;
 
                     || hashcmp(ce->sha1, sha1))
                        staged_changes = 1;
 

-               if (local_changes && staged_changes)
+               if (local_changes && staged_changes &&
+                   !(index_only && is_empty_blob_sha1(ce->sha1)))

                        errs = error("'%s' has staged content different "
                                     "from both the file and the HEAD\n"
                                     "(use -f to force removal)", name);
                        errs = error("'%s' has staged content different "
                                     "from both the file and the HEAD\n"
                                     "(use -f to force removal)", name);

diff --git a/cache.h b/cache.h
index cdbeb48c65f45b4b37ceb767a72866e5a7475cc3..b0edbf9b9f68a98a0052ecb949f20aafb3125e56 100644 (file)
--- a/cache.h
+++ b/cache.h
@@ -519,6 +519,7 @@ static inline void hashclr(unsigned char *hash)
 {
        memset(hash, 0, 20);
 }
 {
        memset(hash, 0, 20);
 }

+extern int is_empty_blob_sha1(const unsigned char *sha1);

 
 int git_mkstemp(char *path, size_t n, const char *template);
 
 
 int git_mkstemp(char *path, size_t n, const char *template);
 

diff --git a/read-cache.c b/read-cache.c
index fdb41b872e22ff8eb09c827416edf39c6031dc1d..2c450866ce92eee2ef484dd798f332cb0047a61d 100644 (file)
--- a/read-cache.c
+++ b/read-cache.c
@@ -160,7 +160,7 @@ static int ce_modified_check_fs(struct cache_entry *ce, struct stat *st)
        return 0;
 }
 
        return 0;
 }
 

-static int is_empty_blob_sha1(const unsigned char *sha1)
+int is_empty_blob_sha1(const unsigned char *sha1)

 {
        static const unsigned char empty_blob_sha1[20] = {
                0xe6,0x9d,0xe2,0x9b,0xb2,0xd1,0xd6,0x43,0x4b,0x8b,
 {
        static const unsigned char empty_blob_sha1[20] = {
                0xe6,0x9d,0xe2,0x9b,0xb2,0xd1,0xd6,0x43,0x4b,0x8b,

diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index 66aca99fd32c6b98f5e6e34a3cf7b096b3e236cf..5b4d6f71387ce7f30cb7ee6b596357aa1942ab6e 100755 (executable)
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -187,6 +187,19 @@ test_expect_success 'but with -f it should work.' '
        test_must_fail git ls-files --error-unmatch baz
 '
 
        test_must_fail git ls-files --error-unmatch baz
 '
 

+test_expect_failure 'refuse to remove cached empty file with modifications' '
+       touch empty &&
+       git add empty &&
+       echo content >empty &&
+       test_must_fail git rm --cached empty
+'
+
+test_expect_success 'remove intent-to-add file without --force' '
+       echo content >intent-to-add &&
+       git add -N intent-to-add
+       git rm --cached intent-to-add
+'
+

 test_expect_success 'Recursive test setup' '
        mkdir -p frotz &&
        echo qfwfq >frotz/nitfol &&
 test_expect_success 'Recursive test setup' '
        mkdir -p frotz &&
        echo qfwfq >frotz/nitfol &&