git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Matthieu Moy <Matthieu.Moy@imag.fr>
To: git@vger.kernel.org, Johannes Schindelin <Johannes.Schindelin@gmx.de>
Cc: Matthieu Moy <Matthieu.Moy@imag.fr>
Subject: [PATCH] More permissive "git-rm --cached" behavior without -f.
Date: Fri, 13 Jul 2007 19:41:38 +0200	[thread overview]
Message-ID: <11843484982037-git-send-email-Matthieu.Moy@imag.fr> (raw)
In-Reply-To: <vpq8x9k9peu.fsf@bauges.imag.fr>

In the previous behavior, "git-rm --cached" (without -f) had the same
restriction as "git-rm". This forced the user to use the -f flag in
situations which weren't actually dangerous, like:

$ git add foo           # oops, I didn't want this
$ git rm --cached foo   # back to initial situation

Previously, the index had to match the file *and* the HEAD. With
--cached, the index must now match the file *or* the HEAD. The behavior
without --cached is unchanged, but provides better error messages.

Signed-off-by: Matthieu Moy <Matthieu.Moy@imag.fr>
---
 Documentation/git-rm.txt |    3 ++-
 builtin-rm.c             |   32 ++++++++++++++++++++++++++------
 t/t3600-rm.sh            |   34 ++++++++++++++++++++++++++++++++++
 3 files changed, 62 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt
index 78f45dc..be61a82 100644
--- a/Documentation/git-rm.txt
+++ b/Documentation/git-rm.txt
@@ -14,7 +14,8 @@ DESCRIPTION
 Remove files from the working tree and from the index.  The
 files have to be identical to the tip of the branch, and no
 updates to its contents must have been placed in the staging
-area (aka index).
+area (aka index).  When --cached is given, the staged content has to
+match either the tip of the branch *or* the file on disk.
 
 
 OPTIONS
diff --git a/builtin-rm.c b/builtin-rm.c
index 4a0bd93..9a808c1 100644
--- a/builtin-rm.c
+++ b/builtin-rm.c
@@ -46,7 +46,7 @@ static int remove_file(const char *name)
 	return ret;
 }
 
-static int check_local_mod(unsigned char *head)
+static int check_local_mod(unsigned char *head, int index_only)
 {
 	/* items in list are already sorted in the cache order,
 	 * so we could do this a lot more efficiently by using
@@ -65,6 +65,8 @@ static int check_local_mod(unsigned char *head)
 		const char *name = list.name[i];
 		unsigned char sha1[20];
 		unsigned mode;
+		int local_changes = 0;
+		int staged_changes = 0;
 
 		pos = cache_name_pos(name, strlen(name));
 		if (pos < 0)
@@ -87,14 +89,32 @@ static int check_local_mod(unsigned char *head)
 			continue;
 		}
 		if (ce_match_stat(ce, &st, 0))
-			errs = error("'%s' has local modifications "
-				     "(hint: try -f)", ce->name);
+			local_changes = 1;
 		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);
+			staged_changes = 1;
+
+		if (local_changes && staged_changes)
+			errs = error("'%s' has staged content different "
+				     "from both the file and the HEAD\n"
+				     "(use -f to force removal)", name);
+		else if (!index_only) {
+			/* It's not dangerous to git-rm --cached a
+			 * file if the index matches the file or the
+			 * HEAD, since it means the deleted content is
+			 * still available somewhere.
+			 */
+			if (staged_changes)
+				errs = error("'%s' has changes staged in the index\n"
+					     "(use --cached to keep the file, "
+					     "or -f to force removal)", name);
+			if (local_changes)
+				errs = error("'%s' has local modifications\n"
+					     "(use --cached to keep the file, "
+					     "or -f to force removal)", name);
+		}
 	}
 	return errs;
 }
@@ -192,7 +212,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
 		unsigned char sha1[20];
 		if (get_sha1("HEAD", sha1))
 			hashclr(sha1);
-		if (check_local_mod(sha1))
+		if (check_local_mod(sha1, index_only))
 			exit(1);
 	}
 
diff --git a/t/t3600-rm.sh b/t/t3600-rm.sh
index 13a461f..5c001aa 100755
--- a/t/t3600-rm.sh
+++ b/t/t3600-rm.sh
@@ -46,6 +46,40 @@ test_expect_success \
     'git rm --cached foo'
 
 test_expect_success \
+    'Test that git rm --cached foo succeeds if the index matches the file' \
+    'echo content > foo
+     git add foo
+     git rm --cached foo'
+
+test_expect_success \
+    'Test that git rm --cached foo succeeds if the index matches the file' \
+    'echo content > foo
+     git add foo
+     git commit -m foo
+     echo "other content" > foo
+     git rm --cached foo'
+
+test_expect_failure \
+    'Test that git rm --cached foo fails if the index matches neither the file nor HEAD' \
+    'echo content > foo
+     git add foo
+     git commit -m foo
+     echo "other content" > foo
+     git add foo
+     echo "yet another content" > foo
+     git rm --cached foo'
+
+test_expect_success \
+    'Test that git rm --cached -f foo works in case where --cached only did not' \
+    'echo content > foo
+     git add foo
+     git commit -m foo
+     echo "other content" > foo
+     git add foo
+     echo "yet another content" > foo
+     git rm --cached -f 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'
 
-- 
1.5.3.rc1.4.gaf83-dirty

  reply	other threads:[~2007-07-13 17:41 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-07-02 18:09 git-rm isn't the inverse action of git-add Christian Jaeger
2007-07-02 19:42 ` Yann Dirson
2007-07-02 20:23   ` Christian Jaeger
2007-07-02 20:40     ` Yann Dirson
2007-07-02 20:54       ` Matthieu Moy
2007-07-02 21:05         ` Johannes Schindelin
2007-07-03 10:37           ` Matthieu Moy
2007-07-03 12:09             ` Johannes Schindelin
2007-07-03 13:40               ` Matthieu Moy
2007-07-03 14:21                 ` Johannes Schindelin
2007-07-04 20:08                 ` Jan Hudec
2007-07-05 13:44                   ` Matthieu Moy
2007-07-05 14:00                     ` David Kastrup
2007-07-08 17:36                     ` [RFC][PATCH] " Matthieu Moy
2007-07-08 18:10                       ` Johannes Schindelin
2007-07-08 20:34                         ` Matthieu Moy
2007-07-08 21:49                           ` Johannes Schindelin
2007-07-09  9:45                             ` Matthieu Moy
2007-07-13 17:36                             ` Matthieu Moy
2007-07-13 17:41                               ` Matthieu Moy [this message]
2007-07-13 17:57                                 ` [PATCH] More permissive "git-rm --cached" behavior without -f Jeff King
2007-07-13 18:53                                   ` Matthieu Moy
2007-07-14  3:42                                     ` Jeff King
2007-07-14  0:44                                 ` Jakub Narebski
2007-07-14  6:52                                 ` Junio C Hamano
2007-07-14  7:16                                   ` Junio C Hamano
2007-07-14 10:14                                     ` Matthieu Moy
2007-07-02 21:20       ` git-rm isn't the inverse action of git-add Christian Jaeger
2007-07-03  4:12         ` Jeff King
2007-07-03  4:47           ` Junio C Hamano
2007-07-03  4:59             ` Jeff King
2007-07-03  5:09               ` Junio C Hamano
2007-07-03  5:12                 ` Jeff King
2007-07-03  6:26                   ` Junio C Hamano
2007-07-11 12:20     ` Jakub Narebski
2007-07-11 18:56       ` Jan Hudec
2007-07-11 21:26         ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=11843484982037-git-send-email-Matthieu.Moy@imag.fr \
    --to=matthieu.moy@imag.fr \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).