[PATCH] git-rm: don't remove newly added file without -f

From: Jeff King <peff@peff.net>
Date: Mar 26, 2007, 18:55 UTC
Message-ID: <20070326185539.GA1650@coredump.intra.peff.net>
In-Reply-To: <8aa486160703260759v438d445ev82161600d8e8bf02@mail.gmail.com>

Given this set of commands:

  $ echo "newly added file" >new
  $ git add new
  $ git rm new

the file "new" was previously removed from the working directory and the index. Because it was not in HEAD, it is available only by searching for unreachable objects.

Instead, we now err on the safe side and refuse to remove a file which is not referenced by HEAD.

Signed-off-by: Jeff King <peff@peff.net>
---

> I think that the "git rm new" should remove "new" from the index or
> should fail, maybe with:

Something like this?

I think there are still some issues with the safety valve, though; this triggers on 'git rm --cached new' which should be a perfectly safe operation. However, that is not new to this change; we already erroneously trigger the valve on a --cached removal of a file with matching index and working directory, but mismatch with HEAD. Example:

  git-add foo
  git-commit -m 'added foo'
  echo changes >>foo
  git-add foo
  git-rm --cached foo ;# should be OK because we have working copy

I think the logic for "safe to remove" and "safe to remove --cached" needs to be separate. I will take a look.

Also, this just implements "error, try -f" when it would be safe to drop back to --cached for that file. Is there any interest in trying to make git-rm more clever in this way, or is simple and predictable preferred?

 builtin-rm.c |   18 ++++--------------
 1 files changed, 4 insertions(+), 14 deletions(-)

diff --git a/builtin-rm.c b/builtin-rm.c
index 00dbe39..bf42003 100644
--- a/builtin-rm.c
+++ b/builtin-rm.c
@@ -89,20 +89,10 @@ static int check_local_mod(unsigned char *head)
 		if (ce_match_stat(ce, &st, 0))
 			errs = error("'%s' has local modifications "
 				     "(hint: try -f)", ce->name);
-		if (no_head)
-			continue;
-		/*
-		 * It is Ok to remove a newly added path, as long as
-		 * it is cache-clean.
-		 */
-		if (get_tree_entry(head, name, sha1, &mode))
-			continue;
-		/*
-		 * Otherwise make sure the version from the HEAD
-		 * matches the index.
-		 */
-		if (ce->ce_mode != create_ce_mode(mode) ||
-		    hashcmp(ce->sha1, sha1))
+		if (no_head
+		     || get_tree_entry(head, name, sha1, &mode)
+		     || ce->ce_mode != create_ce_mode(mode)
+		     || hashcmp(ce->sha1, sha1))
 			errs = error("'%s' has changes staged in the index "
 				     "(hint: try -f)", name);
 	}

-- 
1.5.1.rc2.615.g992d-dirty