# Removing a newly added file 7 messages from 2007-03-26 to 2007-03-26. Participants: Santi Béjar, Jeff King, Junio C Hamano, Johannes Schindelin. Thread: https://gitlist.dev/t/7413 ## Santi Béjar, 2007-03-26 14:59 Subject: Removing a newly added file Message-ID: <8aa486160703260759v438d445ev82161600d8e8bf02@mail.gmail.com> URL: https://gitlist.dev/e/8aa486160703260759v438d445ev82161600d8e8bf02%40mail.gmail.com ``` Hi *, when you try removing a newly added file it success and removes the file from the working directory. So if you do: $ echo "newly added file" > new $ git add new $ git rm new the file "new" is lost, it is not in the index, neither in HEAD. At this moment the only way to recover the file new is searching for unreachable objects. (Am I missing something?) I think that the "git rm new" should remove "new" from the index or should fail, maybe with: $ git rm new error: 'new' is not in HEAD (hint: try --cached) Santi ``` ## Jeff King, 2007-03-26 18:55 Subject: [PATCH] git-rm: don't remove newly added file without -f Message-ID: <20070326185539.GA1650@coredump.intra.peff.net> URL: https://gitlist.dev/e/20070326185539.GA1650%40coredump.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 --- > 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 ``` ## Junio C Hamano, 2007-03-26 21:11 Subject: Re: [PATCH] git-rm: don't remove newly added file without -f Message-ID: <7v1wjbwvzl.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7v1wjbwvzl.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <20070326185539.GA1650@coredump.intra.peff.net> ``` Jeff King writes: > 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. I am not sure if this is a problem, as the user asked it to be removed. It somehow feels to me that the change (I am not convinced your patch should be called a "fix" rather than a "behaviour change") would cause more confusion. Unstaging request would have looked like "git reset HEAD new", wouldn't it? ``` ## Johannes Schindelin, 2007-03-26 21:17 Subject: Re: [PATCH] git-rm: don't remove newly added file without -f Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0703262314090.4045%40wbgn013.biozentrum.uni-wuerzburg.de In-Reply-To: <7v1wjbwvzl.fsf@assigned-by-dhcp.cox.net> ``` Hi, On Mon, 26 Mar 2007, Junio C Hamano wrote: > Jeff King writes: > > > 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. > > I am not sure if this is a problem, as the user asked it to be > removed. It somehow feels to me that the change (I am not > convinced your patch should be called a "fix" rather than a > "behaviour change") would cause more confusion. I agree it would add to confusion. It is well established that "git rm" also removes the file _in the working directory_. If you don't want the file to be deleted, but only the corresponding entry _in the index_, use "git rm --cached". Ciao, Dscho ``` ## Junio C Hamano, 2007-03-26 21:33 Subject: Re: [PATCH] git-rm: don't remove newly added file without -f Message-ID: <7vslbrvgf1.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vslbrvgf1.fsf%40assigned-by-dhcp.cox.net In-Reply-To: ``` Johannes Schindelin writes: > On Mon, 26 Mar 2007, Junio C Hamano wrote: > >> Jeff King writes: >> >> > 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. >> >> I am not sure if this is a problem, as the user asked it to be >> removed. It somehow feels to me that the change (I am not >> convinced your patch should be called a "fix" rather than a >> "behaviour change") would cause more confusion. > > I agree it would add to confusion. > > It is well established that "git rm" also removes the file _in the working > directory_. > > If you don't want the file to be deleted, but only the corresponding > entry _in the index_, use "git rm --cached". Actually, thinking about it a bit more, I think Jeff's patch is in line with the current behaviour. Looking at the cases where we prevent 'git rm' without '-f' succeeding currently, the motivation is to save the user from mistakenly saying "remove" when the user earlier said "this change matters in the next commit" to us. For example, with "edit existing-file; git add existing-file", the user said the new state of the file matters in the next commit. And we refuse to remove, saying "foo has changes staged". By saying "edit new-file; git add new-file", the user expressed the intent to add that content to the upcoming commit. Saying "git rm" later is reverting that intent. Jeff's patch does exactly the same thing for new files what we already do for existing ones --- we ask for a confirmation when "git rm" is given for existing files that has staged changes, saying "you earlier said you want changes to this file in the next commit. are you sure you have changed your mind?" Having said that, we do _not_ ask for confirmation when you do "git add existing-file" after doing "edit ; git add", which is theoretically inconsistent, but rm is special so that is probably Ok. ``` ## Jeff King, 2007-03-26 22:22 Subject: Re: [PATCH] git-rm: don't remove newly added file without -f Message-ID: <20070326222239.GA3498@coredump.intra.peff.net> URL: https://gitlist.dev/e/20070326222239.GA3498%40coredump.intra.peff.net In-Reply-To: <7vslbrvgf1.fsf@assigned-by-dhcp.cox.net> ``` On Mon, Mar 26, 2007 at 02:11:42PM -0700, Junio C Hamano wrote: > Unstaging request would have looked like "git reset HEAD new", > wouldn't it? Yes, I think I am still in the habit of using "git rm" from its old usage. I unstage so rarely that I don't think my fingers have adjusted. > "git rm" later is reverting that intent. Jeff's patch does > exactly the same thing for new files what we already do for > existing ones --- we ask for a confirmation when "git rm" is Yes, regardless of how git-rm is "supposed" to be used, I think this patch is worth it for two reasons: 1. consistency; we should treat newly added files just like existing files (after all, content is content, right?) 2. safety; I work under the assumption that I can do whatever I want with git-rm, and it will _never_ lose my data unless I use -f. Granted, without this patch my data would still be available somewhere in the object database, but it is hard to find and susceptible to pruning. > Having said that, we do _not_ ask for confirmation when you do > "git add existing-file" after doing "edit ; git add", which is > theoretically inconsistent, but rm is special so that is > probably Ok. Hmm, yes, I agree that is inconsistent. However, it's such a common workflow that forcing a '-f' would become meaningless. And you _can_ rescue the blob from the object database in a pinch, though finding it can be tedious. -Peff ``` ## Junio C Hamano, 2007-03-26 22:36 Subject: Re: [PATCH] git-rm: don't remove newly added file without -f Message-ID: <7vk5x3vdhh.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vk5x3vdhh.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <20070326222239.GA3498@coredump.intra.peff.net> ``` Jeff King writes: > Hmm, yes, I agree that is inconsistent. However, it's such a common > workflow that forcing a '-f' would become meaningless. And you _can_ > rescue the blob from the object database in a pinch, though finding it > can be tedious. That's exactly the reason why I mentioned this inconsistency between add vs rm "tongue-in-cheek". We really do not want to require -f for common situations so much that it ends up training people's fingers to type -f without thinking, which would render the safety option meaningless. ```