{"thread":{"id":"7413","subject":"Removing a newly added file","startedAt":"2007-03-26T14:59:57Z","lastAt":"2007-03-26T22:36:42Z","messageCount":7,"participants":["Santi Béjar","Jeff King","Junio C Hamano","Johannes Schindelin"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"38035","messageId":"8aa486160703260759v438d445ev82161600d8e8bf02@mail.gmail.com","threadId":"7413","inReplyTo":null,"subject":"Removing a newly added file","fromName":"Santi Béjar","fromEmail":"sbejar@gmail.com","sentAt":"2007-03-26T14:59:57Z","receivedAt":"2007-03-26T14:59:57Z","isPatch":false,"sender":{"key":"santi@agolina.net","avatar":null},"body":"Hi *,\n\n when you try removing a newly added file it success and removes the\nfile from the working directory. So if you do:\n\n$ echo \"newly added file\" > new\n$ git add new\n$ git rm new\n\nthe file \"new\" is lost, it is not in the index, neither in HEAD. At\nthis moment the only way to recover the file new is searching for\nunreachable objects. (Am I missing something?)\n\n I think that the \"git rm new\" should remove \"new\" from the index or\nshould fail, maybe with:\n\n$ git rm new\nerror: 'new' is not in HEAD (hint: try --cached)\n\n Santi\n"},{"id":"38054","messageId":"20070326185539.GA1650@coredump.intra.peff.net","threadId":"7413","inReplyTo":"8aa486160703260759v438d445ev82161600d8e8bf02@mail.gmail.com","subject":"[PATCH] git-rm: don't remove newly added file without -f","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-03-26T18:55:39Z","receivedAt":"2007-03-26T18:55:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Given this set of commands:\n\n $ echo \"newly added file\" >new\n $ git add new\n $ git rm new\n\nthe file \"new\" was previously removed from the working\ndirectory and the index. Because it was not in HEAD, it is\navailable only by searching for unreachable objects.\n\nInstead, we now err on the safe side and refuse to remove\na file which is not referenced by HEAD.\n\nSigned-off-by: Jeff King \n---\n> I think that the \"git rm new\" should remove \"new\" from the index or\n> should fail, maybe with:\n\nSomething like this?\n\nI think there are still some issues with the safety valve, though; this\ntriggers on 'git rm --cached new' which should be a perfectly safe\noperation. However, that is not new to this change; we already\nerroneously trigger the valve on a --cached removal of a file with\nmatching index and working directory, but mismatch with HEAD. Example:\n\n git-add foo\n git-commit -m 'added foo'\n echo changes >>foo\n git-add foo\n git-rm --cached foo ;# should be OK because we have working copy\n\nI think the logic for \"safe to remove\" and \"safe to remove --cached\"\nneeds to be separate. I will take a look.\n\nAlso, this just implements \"error, try -f\" when it would be safe to drop\nback to --cached for that file. Is there any interest in trying to make\ngit-rm more clever in this way, or is simple and predictable preferred?\n\n builtin-rm.c | 18 ++++--------------\n 1 files changed, 4 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin-rm.c b/builtin-rm.c\nindex 00dbe39..bf42003 100644\n--- a/builtin-rm.c\n+++ b/builtin-rm.c\n@@ -89,20 +89,10 @@ static int check_local_mod(unsigned char *head)\n \t\tif (ce_match_stat(ce, &st, 0))\n \t\t\terrs = error(\"'%s' has local modifications \"\n \t\t\t\t \"(hint: try -f)\", ce->name);\n-\t\tif (no_head)\n-\t\t\tcontinue;\n-\t\t/*\n-\t\t * It is Ok to remove a newly added path, as long as\n-\t\t * it is cache-clean.\n-\t\t */\n-\t\tif (get_tree_entry(head, name, sha1, &mode))\n-\t\t\tcontinue;\n-\t\t/*\n-\t\t * Otherwise make sure the version from the HEAD\n-\t\t * matches the index.\n-\t\t */\n-\t\tif (ce->ce_mode != create_ce_mode(mode) ||\n-\t\t hashcmp(ce->sha1, sha1))\n+\t\tif (no_head\n+\t\t || get_tree_entry(head, name, sha1, &mode)\n+\t\t || ce->ce_mode != create_ce_mode(mode)\n+\t\t || hashcmp(ce->sha1, sha1))\n \t\t\terrs = error(\"'%s' has changes staged in the index \"\n \t\t\t\t \"(hint: try -f)\", name);\n \t}\n-- \n1.5.1.rc2.615.g992d-dirty\n"},{"id":"38067","messageId":"7v1wjbwvzl.fsf@assigned-by-dhcp.cox.net","threadId":"7413","inReplyTo":"20070326185539.GA1650@coredump.intra.peff.net","subject":"Re: [PATCH] git-rm: don't remove newly added file without -f","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-26T21:11:42Z","receivedAt":"2007-03-26T21:11:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Given this set of commands:\n>\n> $ echo \"newly added file\" >new\n> $ git add new\n> $ git rm new\n>\n> the file \"new\" was previously removed from the working\n> directory and the index. Because it was not in HEAD, it is\n> available only by searching for unreachable objects.\n\nI am not sure if this is a problem, as the user asked it to be\nremoved. It somehow feels to me that the change (I am not\nconvinced your patch should be called a \"fix\" rather than a\n\"behaviour change\") would cause more confusion.\n\nUnstaging request would have looked like \"git reset HEAD new\",\nwouldn't it?\n"},{"id":"38068","messageId":"Pine.LNX.4.63.0703262314090.4045@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"7413","inReplyTo":"7v1wjbwvzl.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] git-rm: don't remove newly added file without -f","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-03-26T21:17:14Z","receivedAt":"2007-03-26T21:17:14Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 26 Mar 2007, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > Given this set of commands:\n> >\n> > $ echo \"newly added file\" >new\n> > $ git add new\n> > $ git rm new\n> >\n> > the file \"new\" was previously removed from the working\n> > directory and the index. Because it was not in HEAD, it is\n> > available only by searching for unreachable objects.\n> \n> I am not sure if this is a problem, as the user asked it to be\n> removed. It somehow feels to me that the change (I am not\n> convinced your patch should be called a \"fix\" rather than a\n> \"behaviour change\") would cause more confusion.\n\nI agree it would add to confusion.\n\nIt is well established that \"git rm\" also removes the file _in the working \ndirectory_.\n\nIf you don't want the file to be deleted, but only the corresponding \nentry _in the index_, use \"git rm --cached\".\n\nCiao,\nDscho\n"},{"id":"38069","messageId":"7vslbrvgf1.fsf@assigned-by-dhcp.cox.net","threadId":"7413","inReplyTo":"Pine.LNX.4.63.0703262314090.4045@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] git-rm: don't remove newly added file without -f","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-26T21:33:22Z","receivedAt":"2007-03-26T21:33:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> On Mon, 26 Mar 2007, Junio C Hamano wrote:\n>\n>> Jeff King writes:\n>> \n>> > Given this set of commands:\n>> >\n>> > $ echo \"newly added file\" >new\n>> > $ git add new\n>> > $ git rm new\n>> >\n>> > the file \"new\" was previously removed from the working\n>> > directory and the index. Because it was not in HEAD, it is\n>> > available only by searching for unreachable objects.\n>> \n>> I am not sure if this is a problem, as the user asked it to be\n>> removed. It somehow feels to me that the change (I am not\n>> convinced your patch should be called a \"fix\" rather than a\n>> \"behaviour change\") would cause more confusion.\n>\n> I agree it would add to confusion.\n>\n> It is well established that \"git rm\" also removes the file _in the working \n> directory_.\n>\n> If you don't want the file to be deleted, but only the corresponding \n> entry _in the index_, use \"git rm --cached\".\n\nActually, thinking about it a bit more, I think Jeff's patch is\nin line with the current behaviour.\n\nLooking at the cases where we prevent 'git rm' without '-f'\nsucceeding currently, the motivation is to save the user from\nmistakenly saying \"remove\" when the user earlier said \"this\nchange matters in the next commit\" to us.\n\nFor example, with \"edit existing-file; git add existing-file\",\nthe user said the new state of the file matters in the next\ncommit. And we refuse to remove, saying \"foo has changes\nstaged\".\n\nBy saying \"edit new-file; git add new-file\", the user expressed\nthe intent to add that content to the upcoming commit. Saying\n\"git rm\" later is reverting that intent. Jeff's patch does\nexactly the same thing for new files what we already do for\nexisting ones --- we ask for a confirmation when \"git rm\" is\ngiven for existing files that has staged changes, saying \"you\nearlier said you want changes to this file in the next commit.\nare you sure you have changed your mind?\"\n\nHaving said that, we do _not_ ask for confirmation when you do\n\"git add existing-file\" after doing \"edit ; git add\", which is\ntheoretically inconsistent, but rm is special so that is\nprobably Ok.\n"},{"id":"38072","messageId":"20070326222239.GA3498@coredump.intra.peff.net","threadId":"7413","inReplyTo":"7vslbrvgf1.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] git-rm: don't remove newly added file without -f","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-03-26T22:22:39Z","receivedAt":"2007-03-26T22:22:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 26, 2007 at 02:11:42PM -0700, Junio C Hamano wrote:\n\n> Unstaging request would have looked like \"git reset HEAD new\",\n> wouldn't it?\n\nYes, I think I am still in the habit of using \"git rm\" from its old\nusage. I unstage so rarely that I don't think my fingers have adjusted.\n\n> \"git rm\" later is reverting that intent. Jeff's patch does\n> exactly the same thing for new files what we already do for\n> existing ones --- we ask for a confirmation when \"git rm\" is\n\nYes, regardless of how git-rm is \"supposed\" to be used, I think this\npatch is worth it for two reasons:\n 1. consistency; we should treat newly added files just like existing\n files (after all, content is content, right?)\n 2. safety; I work under the assumption that I can do whatever I want\n with git-rm, and it will _never_ lose my data unless I use -f.\n Granted, without this patch my data would still be available\n somewhere in the object database, but it is hard to find and\n susceptible to pruning.\n\n> Having said that, we do _not_ ask for confirmation when you do\n> \"git add existing-file\" after doing \"edit ; git add\", which is\n> theoretically inconsistent, but rm is special so that is\n> probably Ok.\n\nHmm, yes, I agree that is inconsistent. However, it's such a common\nworkflow that forcing a '-f' would become meaningless. And you _can_\nrescue the blob from the object database in a pinch, though finding it\ncan be tedious.\n\n-Peff\n"},{"id":"38075","messageId":"7vk5x3vdhh.fsf@assigned-by-dhcp.cox.net","threadId":"7413","inReplyTo":"20070326222239.GA3498@coredump.intra.peff.net","subject":"Re: [PATCH] git-rm: don't remove newly added file without -f","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-03-26T22:36:42Z","receivedAt":"2007-03-26T22:36:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Hmm, yes, I agree that is inconsistent. However, it's such a common\n> workflow that forcing a '-f' would become meaningless. And you _can_\n> rescue the blob from the object database in a pinch, though finding it\n> can be tedious.\n\nThat's exactly the reason why I mentioned this inconsistency\nbetween add vs rm \"tongue-in-cheek\".\n\nWe really do not want to require -f for common situations so\nmuch that it ends up training people's fingers to type -f\nwithout thinking, which would render the safety option\nmeaningless.\n"}]}