{"thread":{"id":"8814","subject":"git-rm isn't the inverse action of git-add","startedAt":"2007-07-02T18:09:37Z","lastAt":"2007-07-14T10:14:04Z","messageCount":37,"participants":["Christian Jaeger","Yann Dirson","Matthieu Moy","Johannes Schindelin","Jeff King","Junio C Hamano","Jan Hudec","David Kastrup","Jakub Narebski"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"46263","messageId":"46893F61.5060401@jaeger.mine.nu","threadId":"8814","inReplyTo":null,"subject":"git-rm isn't the inverse action of git-add","fromName":"Christian Jaeger","fromEmail":"christian@jaeger.mine.nu","sentAt":"2007-07-02T18:09:37Z","receivedAt":"2007-07-02T18:09:37Z","isPatch":false,"sender":{"key":"christian@jaeger.mine.nu","avatar":null},"body":"Hello\n\nI'm coming from cogito. There you can run:\n\n cg-add $file ; cg-rm $file\n\nand everything is as before; it adds the file to the directory\nindex/cache, and just removes it again from the latter.\n\nWhereas with git,\n\n git-add $file; git-rm $file\n\nis giving the error\n\n error: '..file..' has changes staged in the index (hint: try -f)\n\nAnd sure enough, git rm -f $file will remove the file from the index,\nbut also unlink it from the directory. (Ok, I did remember that cogito's\n-f option is unlinking the file, so I was cautious and didn't try it on\nan important file, but still...)\n\nTurns out that\n\n git rm -f --cached $file\n\nwill do the same action as cg-rm $file.\n\nWhy so complicated? Why not just make git-rm without options behave like\ncg-rm? (Or at the very least, I'd change the hint to say \"try -f --cached\".)\n\nChristian.\n"},{"id":"46267","messageId":"20070702194237.GN7730@nan92-1-81-57-214-146.fbx.proxad.net","threadId":"8814","inReplyTo":"46893F61.5060401@jaeger.mine.nu","subject":"Re: git-rm isn't the inverse action of git-add","fromName":"Yann Dirson","fromEmail":"ydirson@altern.org","sentAt":"2007-07-02T19:42:37Z","receivedAt":"2007-07-02T19:42:37Z","isPatch":false,"sender":{"key":"ydirson@altern.org","avatar":"https://avatars.githubusercontent.com/u/1190950?v=4"},"body":"On Mon, Jul 02, 2007 at 08:09:37PM +0200, Christian Jaeger wrote:\n> Why so complicated? Why not just make git-rm without options behave like\n> cg-rm? (Or at the very least, I'd change the hint to say \"try -f --cached\".)\n\nIt is probably a matter of taste. Personally, I am really upset by\nthis behaviour that cvs, cogito, stgit and others share, which forces\nme to issue 2 commands to really delete a file from version control\nand from the filesystem.\n\nDo you really need to undo an add more often than you need to remove a\nfile from version-control ? It may be worth, however, to make things\neasier. Maybe \"git add --undo foo\" would be a solution ? Not sure\nwe'd want to add --undo to many git commands, though. Opinions ?\n\nBest regards,\n-- \nYann\n"},{"id":"46272","messageId":"46895EA4.5040803@jaeger.mine.nu","threadId":"8814","inReplyTo":"20070702194237.GN7730@nan92-1-81-57-214-146.fbx.proxad.net","subject":"Re: git-rm isn't the inverse action of git-add","fromName":"Christian Jaeger","fromEmail":"christian@jaeger.mine.nu","sentAt":"2007-07-02T20:23:00Z","receivedAt":"2007-07-02T20:23:00Z","isPatch":false,"sender":{"key":"christian@jaeger.mine.nu","avatar":null},"body":"Yann Dirson wrote:\n> On Mon, Jul 02, 2007 at 08:09:37PM +0200, Christian Jaeger wrote:\n> \n>> Why so complicated? Why not just make git-rm without options behave like\n>> cg-rm? (Or at the very least, I'd change the hint to say \"try -f --cached\".)\n>> \n>\n> It is probably a matter of taste. Personally, I am really upset by\n> this behaviour that cvs, cogito, stgit and others share, which forces\n> me to issue 2 commands to really delete a file from version control\n> and from the filesystem.\n> \n\nIt doesn't force you to issue 2 commands: the -f option to cg-rm unlinks\nthe file for you. So you only have to type an additional \"-f\".\n\nYes it's probably (partly) a matter of taste: in my bash startup files I\nhave mv aliased to mv -i and rm to rm -i, so that it asks me whether I'm\nsure to delete or overwrite a file. If I know in advance that I'm sure,\nI just type \"rm -f $file\", which then expands to \"rm -i -f $file\" where\nthe -f overrides the -i. cg-rm -f just fits very well into this scheme\n(the only difference being that \"cg-rm $file\" doesn't explicitely ask me\nwhether I also want the file to be unlinked). (BTW note that usually for\nremoving a file I use a \"trash\" (or shorter alias \"tra\") command, which\nmoves it to a trash can instead of deleting; so I use \"tra $file\" by\ndefault, and only for big files or when I'm sure I immediately want to\ndelete them, I use rm, and then if the paths are clear I add the -f\nflag, if not (like globbing involved), I don't add the -f and thus am\nasked for confirmation.)\n\nIf I could alias the git-rm command so that the default action is the\nreverse of git-add and adding an -f flag removes it from disk, that\nwould be fine for me.\n\n> Do you really need to undo an add more often than you need to remove a\n> file from version-control ? It may be worth, however, to make things\n> easier. Maybe \"git add --undo foo\" would be a solution ?\n\nThis doesn't sound very intuitive to me (and I couldn't fix it with an\nalias).\n\nI don't per se require undo actions. I just don't understand why git-rm\nrefuses to remove the file from the index, even if I didn't commit it.\nThe index is just an intermediate record of the changes in my\nunderstandings, and the rm action would also be intermediate until it's\nbeing committed. And a non-committed action being deleted shouldn't need\na special confirmation from me, especially not one which is consisting\nof a combination of two flags (of which one is a destructive one).\n\nChristian.\n"},{"id":"46274","messageId":"20070702204051.GP7730@nan92-1-81-57-214-146.fbx.proxad.net","threadId":"8814","inReplyTo":"46895EA4.5040803@jaeger.mine.nu","subject":"Re: git-rm isn't the inverse action of git-add","fromName":"Yann Dirson","fromEmail":"ydirson@altern.org","sentAt":"2007-07-02T20:40:51Z","receivedAt":"2007-07-02T20:40:51Z","isPatch":false,"sender":{"key":"ydirson@altern.org","avatar":"https://avatars.githubusercontent.com/u/1190950?v=4"},"body":"On Mon, Jul 02, 2007 at 10:23:00PM +0200, Christian Jaeger wrote:\n> I don't per se require undo actions. I just don't understand why git-rm\n> refuses to remove the file from the index, even if I didn't commit it.\n\nI'd say it does so, so you won't loose any uncommitted changes without\nknowing it - and \"git add -f\" is available when you have checked that\nyou indeed want to discard that data.\n\n> The index is just an intermediate record of the changes in my\n> understandings, and the rm action would also be intermediate until it's\n> being committed. And a non-committed action being deleted shouldn't need\n> a special confirmation from me, especially not one which is consisting\n> of a combination of two flags (of which one is a destructive one).\n\nIt already works as such: it will warn you if you have already staged\nthe file in the index, but it has not been committed, in which case\nthe data would be lost as well:\n\n$ echo foo > bar\n/tmp/test$ git rm bar\nfatal: pathspec 'bar' did not match any files\n/tmp/test$ git add bar\n/tmp/test$ git rm bar\nerror: 'bar' has changes staged in the index (hint: try -f)\n\nThat is, \"git rm\" will only ever remove the file without asking, when\nit is safe do so, in that you can retrieve your file from history. Or\ndo you think of another way, in which more safety would be needed ?\n\nBest regards,\n-- \nYann\n"},{"id":"46275","messageId":"vpq7ipittl2.fsf@bauges.imag.fr","threadId":"8814","inReplyTo":"20070702204051.GP7730@nan92-1-81-57-214-146.fbx.proxad.net","subject":"Re: git-rm isn't the inverse action of git-add","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2007-07-02T20:54:17Z","receivedAt":"2007-07-02T20:54:17Z","isPatch":false,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Yann Dirson writes:\n\n> That is, \"git rm\" will only ever remove the file without asking, when\n> it is safe do so, in that you can retrieve your file from history. Or\n> do you think of another way, in which more safety would be needed ?\n\nDefaulting to --cached would be an obvious way to avoid data-loss.\n_At least_, mentionning --cached in the error message in case of\nstaged changes would be a considerable step forward.\n\nAt the moment, the non-expert user will have difficulties to unversion\nthe file without deleting it. I just see it as\n\n$ git rm foo\nerror: 'foo' has changes staged in the index\n(hint: to hang yourself, try -f)\n$ _\n\n-- \nMatthieu\n"},{"id":"46277","messageId":"Pine.LNX.4.64.0707022205210.4071@racer.site","threadId":"8814","inReplyTo":"vpq7ipittl2.fsf@bauges.imag.fr","subject":"Re: git-rm isn't the inverse action of git-add","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-02T21:05:35Z","receivedAt":"2007-07-02T21:05:35Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Mon, 2 Jul 2007, Matthieu Moy wrote:\n\n> Defaulting to --cached would be an obvious way to avoid data-loss. _At \n> least_, mentionning --cached in the error message in case of staged \n> changes would be a considerable step forward.\n> \n> At the moment, the non-expert user will have difficulties to unversion \n> the file without deleting it. I just see it as\n> \n> $ git rm foo\n> error: 'foo' has changes staged in the index\n> (hint: to hang yourself, try -f)\n> $ _\n\nWhat's so wrong with our man pages? You know, there have been man hours \ninvested in them, and they are exclusively meant for consumption by people \nwho do not know about the usage of the commands...\n\nHth,\nDscho\n"},{"id":"46280","messageId":"46896C3B.1050406@jaeger.mine.nu","threadId":"8814","inReplyTo":"20070702204051.GP7730@nan92-1-81-57-214-146.fbx.proxad.net","subject":"Re: git-rm isn't the inverse action of git-add","fromName":"Christian Jaeger","fromEmail":"christian@jaeger.mine.nu","sentAt":"2007-07-02T21:20:59Z","receivedAt":"2007-07-02T21:20:59Z","isPatch":false,"sender":{"key":"christian@jaeger.mine.nu","avatar":null},"body":"Yann Dirson wrote:\n> On Mon, Jul 02, 2007 at 10:23:00PM +0200, Christian Jaeger wrote:\n> \n>> I don't per se require undo actions. I just don't understand why git-rm\n>> refuses to remove the file from the index, even if I didn't commit it.\n>> \n>\n> I'd say it does so, so you won't loose any uncommitted changes without\n> knowing it - and \"git add -f\" is available when you have checked that\n> you indeed want to discard that data.\n> \n\nI'm really realising that\n\ngit-rm $file # where $file *has* been committed previously\n\ndoes remove *and* unlink the file. (cg-rm does unlink only with the -f\nflag, as said.)\n\nSo there's no -f flag in normal git-rm usage. It thus has a different\nmeaning, namely \"force the operation pair of removing from index and\nunlinking\", not \"force this operation also onto the checked out files\"\nas is the case with cogito.\n\nSo I now understand better why they invented the -f flag to git-rm for\nthe case you're mentioning above and why the hint doesn't warn about\nit's danger, since git-rm is always dangerous. (Ok, as is \"rm\" without\nthe \"-i\"; I just found it normal that cogito behaved like my \"-i\" setup.)\n\nRegarding the issue of \"lost files\" because they have been created,\nadded, and removed again before committing: as far as I remember this\nhas never happened to me with cogito. I commit often, so if I add a file\nor a few, in most cases I commit just this fact (that they have been\nadded), before doing more fancy stuff. I'm maybe used to thinking in\ndatabase terms, work that isn't committed is lost. So if I create a file\nand add it, in my brain the \"attention, uncommitted work\" flag is on,\nand it usually doesn't happen that I later erroneously think the work\nhas been committed when in fact it isn't. (I can always check with a\nquick cg-status (which shows the files as \"A\", which makes them stand\nout better than in the git-status output)).\n\nJust before writing this mail I had a case where I wanted to remove a\nfile from versioning control, but keep it on disk (I used git-rm and\nthat's how I learned that it really also unlinks the local file without\nasking(*)). Note that this has not been an undo action; the file has\nbeen committed previously.\n\n(* thanks to git-reset I could get it back of course)\n\n>\n> That is, \"git rm\" will only ever remove the file without asking, when\n> it is safe do so, in that you can retrieve your file from history. \n\n(Well it's not safe if you want to remove the file *from the index* and\nnaively mis-use the -f flag as suggested by the hint.)\n\n> Or\n> do you think of another way, in which more safety would be needed ?\n> \n\nI think we have just two different points in our view where we think\nsafety matters.\n\nRegarding the man pages: yes the git-rm man page is fine, and it's nice\nto see the manuals are improving. As noted I came from cogito, and\ndidn't expect git to behave so different with the same named (but\ndifferent purpose) options, so I didn't read the man pages (I've been in\nirc and asked there, where someone suggested to bring this to the list;\nI'm too tired today to think further about it and will try to read more\ndocs and hope I'll get to understand the git philosophies more).\n\nChristian.\n"},{"id":"46321","messageId":"20070703041241.GA4007@coredump.intra.peff.net","threadId":"8814","inReplyTo":"46896C3B.1050406@jaeger.mine.nu","subject":"Re: git-rm isn't the inverse action of git-add","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-07-03T04:12:41Z","receivedAt":"2007-07-03T04:12:41Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jul 02, 2007 at 11:20:59PM +0200, Christian Jaeger wrote:\n\n> So there's no -f flag in normal git-rm usage. It thus has a different\n> meaning, namely \"force the operation pair of removing from index and\n> unlinking\", not \"force this operation also onto the checked out files\"\n> as is the case with cogito.\n\nYes, git-rm is used in several situations, and the idea is that it\nshould behave safely in all situations; that is, without -f you can't\ndelete any data that can't be recovered from your history (but maybe\nthat means we shouldn't suggest -f in so cavalier a fashion).\n\nEach file has three \"states\" that we care about: in the HEAD (H), in the\nindex (I), and in the working tree (W). Let's say you call 'git-rm'.\nHere's a table of possibilities (A is some content \"A\", B is some\ncontent not equal to \"A\", and N is \"non-existant\"):\n\nH I W | ok? | why?\n---------------------------------------------------\n* N * | no | file is not tracked\nN A N | ? | currently ok, but 'A' recoverable only through fsck\nN A A | no | 'A' recoverable only through fsck\nN A B | no | local modification 'B' would be lost\nA A N | yes | 'A' recoverable through history\nA A A | yes | 'A' recoverable through history\nA A B | no | local modification 'B' would be lost\nA B N | ? | currently ok, but 'B' recoverable only through fsck\nA B A | no | 'B' recoverable only through fsck\nA B B | no | 'B' recoverable only through fsck\nB * * | | equivalent to H=A\n\nWith --cached on, it is a little different:\n\nH I W | ok? | why?\n---------------------------------------------------\n* N * | no | file is not tracked\nN A N | ? | currently ok, but 'A' recoverable only through fsck\nN A A | ? | currently not ok, but 'A' still available in W\nN A B | no | 'A' recoverable only through fsck\nA A N | yes | 'A' recoverable through history\nA A A | yes | 'A' recoverable through history or working tree\nA A B | ? | currently not ok, but 'A' still available in H\nA B N | ? | currently ok, but 'B' recoverable only through fsck\nA B A | no | 'B' recoverable only through fsck\nA B B | ? | currently not ok, but 'B' still available in W\nB * * | | equivalent to H=A\n\nSo it looks like our safety valve is a bit overbearing in a few\nsituations, and still misses some situations where data has to be pulled\nout of the database with git-fsck.\n\nI think if we actually spell out these possible states in the code, we\ncan get more accurate behavior, but also more accurate error messages. I\nwill try to work up a patch.\n\n-Peff\n"},{"id":"46326","messageId":"7vhcomt7oa.fsf@assigned-by-dhcp.cox.net","threadId":"8814","inReplyTo":"20070703041241.GA4007@coredump.intra.peff.net","subject":"Re: git-rm isn't the inverse action of git-add","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-03T04:47:33Z","receivedAt":"2007-07-03T04:47:33Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> H I W | ok? | why?\n> ---------------------------------------------------\n> N A N | ? | currently ok, but 'A' recoverable only through fsck\n> A B N | ? | currently ok, but 'B' recoverable only through fsck\n\nThese were explicitly done per request from git-rm users (myself\nnot one of them) who wanted to:\n\n\trm the-file\n git rm the-file\n\nsequence not to barf. I suspect they were from CVS background\nwho are used to the SCM that complains if you still have the\nfile in the working tree when you say \"scm rm\".\n\nI would not mind requiring -f for these cases.\n\n> With --cached on, it is a little different:\n>\n> H I W | ok? | why?\n> ---------------------------------------------------\n> N A N | ? | currently ok, but 'A' recoverable only through fsck\n> N A A | ? | currently not ok, but 'A' still available in W\n> A A B | ? | currently not ok, but 'A' still available in H\n> A B N | ? | currently ok, but 'B' recoverable only through fsck\n> A B B | ? | currently not ok, but 'B' still available in W\n\nI personally do not think we would need any safety check for\n\"git rm --cached\", as it does not touch the working tree. If\none cares about the differences among three states, one would\nnot issue \"rm --cached\" anyway. The only reason \"rm --cached\"\nis used is because one _knows_ that any blob should not exist at\nthat path in the index.\n"},{"id":"46328","messageId":"20070703045948.GE4007@coredump.intra.peff.net","threadId":"8814","inReplyTo":"7vhcomt7oa.fsf@assigned-by-dhcp.cox.net","subject":"Re: git-rm isn't the inverse action of git-add","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-07-03T04:59:48Z","receivedAt":"2007-07-03T04:59:48Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jul 02, 2007 at 09:47:33PM -0700, Junio C Hamano wrote:\n\n> These were explicitly done per request from git-rm users (myself\n> not one of them) who wanted to:\n> \n> \trm the-file\n> git rm the-file\n\nAh, makes sense (if such a thing can be said about CVS behavior).\n\n> > H I W | ok? | why?\n> > ---------------------------------------------------\n> > N A N | ? | currently ok, but 'A' recoverable only through fsck\n> > N A A | ? | currently not ok, but 'A' still available in W\n> > A A B | ? | currently not ok, but 'A' still available in H\n> > A B N | ? | currently ok, but 'B' recoverable only through fsck\n> > A B B | ? | currently not ok, but 'B' still available in W\n> \n> I personally do not think we would need any safety check for\n> \"git rm --cached\", as it does not touch the working tree. If\n\nIt depends on how we want to define \"lost\" data. In many cases, we are\nprotecting against losing content that will still be available until the\nnext git-prune. Should our safety valve protect against that case, or\nshould it not? We are totally inconsistent.\n\nThe main one for --cached, of course, is when that content exists _only_\nin the index, but no longer in the working tree (!A A N or !A A B). You\nreally should be using regular git-rm (in the first case, since you are\nsaying \"I don't want this file anymore\") or git-add (throw out the old\ndata, use my new version).\n\nOTOH, clearly git-add can \"lose\" data in this way as well, since a\n\"modify, git-add, modify, git-add\" will \"lose\" any reference to the\nindex state after the first add. So maybe that is not worth worrying\nabout at all (in which case our safety valve is too strict in many\nplaces).\n\nWe could also issue a warning when \"losing\" reference to data that is in\nthe object db, which would include the sha1; in that case, an immediate\n\"oops\" could be rectified with git-show.\n\n> one cares about the differences among three states, one would\n> not issue \"rm --cached\" anyway. The only reason \"rm --cached\"\n> is used is because one _knows_ that any blob should not exist at\n> that path in the index.\n\nHow about:\n\n git-add foo\n echo changes >>foo\n # oops, I don't want to commit foo just yet\n git-rm --cached foo\n\nbut in that case, maybe the user doesn't actually _care_ about that\nintermediate state of 'foo'.\n\n-Peff\n"},{"id":"46329","messageId":"7v4pkmt6nk.fsf@assigned-by-dhcp.cox.net","threadId":"8814","inReplyTo":"20070703045948.GE4007@coredump.intra.peff.net","subject":"Re: git-rm isn't the inverse action of git-add","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-03T05:09:35Z","receivedAt":"2007-07-03T05:09:35Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> OTOH, clearly git-add can \"lose\" data in this way as well, since a\n> \"modify, git-add, modify, git-add\" will \"lose\" any reference to the\n> index state after the first add. So maybe that is not worth worrying\n> about at all (in which case our safety valve is too strict in many\n> places).\n\nExactly. And not considering that lossage helps us keep our\nsanity. I think \"git rm --cached\" falls into the same\ncategory. If the user wants to discard what is in the index\nwithout losing a copy in the working tree, I think we should let\nhim do without fuss.\n\n> git-add foo\n> echo changes >>foo\n> # oops, I don't want to commit foo just yet\n> git-rm --cached foo\n>\n> but in that case, maybe the user doesn't actually _care_ about that\n> intermediate state of 'foo'.\n\nYes, that is (at least, \"used to be\") exactly the use case \"rm\n--cached\" is supposed to help. Added something prematurely to\nthe index, not ready to commit that part of the changes yet.\nOf course you could do partial commits with \"add --interactive\"\nthese days, so there is not as much need for this as it used to\nbe anymore.\n"},{"id":"46330","messageId":"20070703051254.GA6477@coredump.intra.peff.net","threadId":"8814","inReplyTo":"7v4pkmt6nk.fsf@assigned-by-dhcp.cox.net","subject":"Re: git-rm isn't the inverse action of git-add","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-07-03T05:12:54Z","receivedAt":"2007-07-03T05:12:54Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jul 02, 2007 at 10:09:35PM -0700, Junio C Hamano wrote:\n\n> Exactly. And not considering that lossage helps us keep our\n> sanity. I think \"git rm --cached\" falls into the same\n> category. If the user wants to discard what is in the index\n> without losing a copy in the working tree, I think we should let\n> him do without fuss.\n\nOK. So should we _remove_ the safety valve in all cases where we're just\nlosing stuff that's in the index? It is, after all, recoverable. Should\nthere be a warning (I suspect it would get annoying very quickly)?\n\nI think this would help by making the use of '-f' more rare, which is\nthe thing that can _really_ screw you, since it turns off the safety\nvalve even for things that aren't recoverable.\n\n-Peff\n"},{"id":"46334","messageId":"7vsl86roin.fsf@assigned-by-dhcp.cox.net","threadId":"8814","inReplyTo":"20070703051254.GA6477@coredump.intra.peff.net","subject":"Re: git-rm isn't the inverse action of git-add","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-03T06:26:40Z","receivedAt":"2007-07-03T06:26:40Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Mon, Jul 02, 2007 at 10:09:35PM -0700, Junio C Hamano wrote:\n>\n>> Exactly. And not considering that lossage helps us keep our\n>> sanity. I think \"git rm --cached\" falls into the same\n>> category. If the user wants to discard what is in the index\n>> without losing a copy in the working tree, I think we should let\n>> him do without fuss.\n>\n> OK. So should we _remove_ the safety valve in all cases where we're just\n> losing stuff that's in the index? It is, after all, recoverable. Should\n> there be a warning (I suspect it would get annoying very quickly)?\n\nI personally do not think we would need any safety check for\n\"git rm --cached\", as it does not touch the working tree. For\nnon-cached case I think the current behaviour is fine.\n\nBut I should warn you that I rarely use \"git rm\" myself.\n"},{"id":"46344","messageId":"vpqoditkc23.fsf@bauges.imag.fr","threadId":"8814","inReplyTo":"Pine.LNX.4.64.0707022205210.4071@racer.site","subject":"Re: git-rm isn't the inverse action of git-add","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2007-07-03T10:37:40Z","receivedAt":"2007-07-03T10:37:40Z","isPatch":false,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Johannes Schindelin writes:\n\n> What's so wrong with our man pages? You know, there have been man hours \n> invested in them, and they are exclusively meant for consumption by people \n> who do not know about the usage of the commands...\n\nWhat's wrong is just that I shouldn't have to read a man page to avoid\ndata-loss. I should have to read them to do non-trivial things, for\nsure.\n\nUseability is not just about documenting surprising behaviors, it's\nreally about avoiding them.\n\n-- \nMatthieu\n"},{"id":"46353","messageId":"Pine.LNX.4.64.0707031308170.4071@racer.site","threadId":"8814","inReplyTo":"vpqoditkc23.fsf@bauges.imag.fr","subject":"Re: git-rm isn't the inverse action of git-add","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-03T12:09:59Z","receivedAt":"2007-07-03T12:09:59Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 3 Jul 2007, Matthieu Moy wrote:\n\n> Johannes Schindelin writes:\n> \n> > What's so wrong with our man pages? You know, there have been man \n> > hours invested in them, and they are exclusively meant for consumption \n> > by people who do not know about the usage of the commands...\n> \n> What's wrong is just that I shouldn't have to read a man page to avoid\n> data-loss.\n\nOkay, Mr Moy. How did you learn that \"rm\" leads to data-loss? Because it \ndoes. Hmm. How did you expect then, that git-rm does _not_ lead to data \nloss? If in doubt, you _have_ to read the manual. Especially if the tool \nis powerful.\n\nCiao,\nDscho\n"},{"id":"46364","messageId":"vpqir91hagz.fsf@bauges.imag.fr","threadId":"8814","inReplyTo":"Pine.LNX.4.64.0707031308170.4071@racer.site","subject":"Re: git-rm isn't the inverse action of git-add","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2007-07-03T13:40:12Z","receivedAt":"2007-07-03T13:40:12Z","isPatch":false,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Johannes Schindelin writes:\n\n> Hi,\n>\n> On Tue, 3 Jul 2007, Matthieu Moy wrote:\n>\n>> Johannes Schindelin writes:\n>> \n>> > What's so wrong with our man pages? You know, there have been man \n>> > hours invested in them, and they are exclusively meant for consumption \n>> > by people who do not know about the usage of the commands...\n>> \n>> What's wrong is just that I shouldn't have to read a man page to avoid\n>> data-loss.\n>\n> Okay, Mr Moy.\n\nGlad to be called by my name. Is it a tradition here, or a way to make\nfun of me?\n\n> How did you learn that \"rm\" leads to data-loss? Because it does.\n\nIt obviously does, and I can't imagine any other behavior than\ndeleting the file for a command like \"rm\".\n\n> Hmm. How did you expect then, that git-rm does _not_ lead to data\n> loss? \n\nBecause there are tons of possible behaviors for \"$VCS rm\", and I'd\nexpect it to be safe even if VCS=git, since it is with all the other\nVCS I know.\n\nWhat's wrong with the behavior of \"hg rm\"?\nWhat's wrong with the behavior of \"svn rm\"?\nWhat's wrong with the behavior of \"bzr rm\"?\n(no, I won't do it with CVS ;-) )\n\nNone of these commands have the problem that git-rm has.\n\n-- \nMatthieu\n"},{"id":"46370","messageId":"Pine.LNX.4.64.0707031518380.4071@racer.site","threadId":"8814","inReplyTo":"vpqir91hagz.fsf@bauges.imag.fr","subject":"Re: git-rm isn't the inverse action of git-add","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-03T14:21:15Z","receivedAt":"2007-07-03T14:21:15Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 3 Jul 2007, Matthieu Moy wrote:\n\n> Johannes Schindelin writes:\n> \n> > On Tue, 3 Jul 2007, Matthieu Moy wrote:\n> >\n> >> Johannes Schindelin writes:\n> >> \n> >> > What's so wrong with our man pages? You know, there have been man \n> >> > hours invested in them, and they are exclusively meant for \n> >> > consumption by people who do not know about the usage of the \n> >> > commands...\n> >> \n> >> What's wrong is just that I shouldn't have to read a man page to \n> >> avoid data-loss.\n> >\n> > Okay, Mr Moy.\n> \n> Glad to be called by my name. Is it a tradition here, or a way to make \n> fun of me?\n\nI tried to be funny, by introducing some diversity...\n\n> > How did you learn that \"rm\" leads to data-loss? Because it does.\n> \n> It obviously does, and I can't imagine any other behavior than deleting \n> the file for a command like \"rm\".\n> \n> > Hmm. How did you expect then, that git-rm does _not_ lead to data\n> > loss? \n> \n> Because there are tons of possible behaviors for \"$VCS rm\", and I'd \n> expect it to be safe even if VCS=git, since it is with all the other VCS \n> I know.\n\nWhich proves exactly my point. There are a ton of interpretations that \nmake sense. So I would always look into the man page.\n\n> What's wrong with the behavior of \"hg rm\"?\n> What's wrong with the behavior of \"svn rm\"?\n> What's wrong with the behavior of \"bzr rm\"?\n> (no, I won't do it with CVS ;-) )\n> \n> None of these commands have the problem that git-rm has.\n\nGuess what. I do not know how they operate! I have no idea what the \nbehaviour of the commands you mentioned is. So before I would answer (if \nthey were not rethoric questions), I would actually really read the man \npage to know what they are supposed to do.\n\nCiao,\nDscho\n"},{"id":"46508","messageId":"20070704200806.GA3991@efreet.light.src","threadId":"8814","inReplyTo":"vpqir91hagz.fsf@bauges.imag.fr","subject":"Re: git-rm isn't the inverse action of git-add","fromName":"Jan Hudec","fromEmail":"bulb@ucw.cz","sentAt":"2007-07-04T20:08:06Z","receivedAt":"2007-07-04T20:08:06Z","isPatch":false,"sender":{"key":"bulb@ucw.cz","avatar":null},"body":"On Tue, Jul 03, 2007 at 15:40:12 +0200, Matthieu Moy wrote:\n> Johannes Schindelin writes:\n> > Hmm. How did you expect then, that git-rm does _not_ lead to data\n> > loss? \n> \n> Because there are tons of possible behaviors for \"$VCS rm\", and I'd\n> expect it to be safe even if VCS=git, since it is with all the other\n> VCS I know.\n> \n> What's wrong with the behavior of \"hg rm\"?\n> What's wrong with the behavior of \"svn rm\"?\n> What's wrong with the behavior of \"bzr rm\"?\n> (no, I won't do it with CVS ;-) )\n> \n> None of these commands have the problem that git-rm has.\n\nHm. They all behave roughly the same: They unversion the file and unlink it,\nunless it is modified, in which case they unversion it and leave it alone.\n\nNow git has the extra complexity that index contains also content of the\nfile. But the behaviour can be easily adapted like this (HEAD = version in\nHEAD, index = version in index, tree = version in tree):\n - if (HEAD == index && index == version) unversion and unlink\n - else if (HEAD == index || index == version) unversion\n - else print message and do nothing\n\nWould you consider that a sane behaviour?\n\n-- \n\t\t\t\t\t\t Jan 'Bulb' Hudec \n"},{"id":"46547","messageId":"vpqd4z7q820.fsf@bauges.imag.fr","threadId":"8814","inReplyTo":"20070704200806.GA3991@efreet.light.src","subject":"Re: git-rm isn't the inverse action of git-add","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2007-07-05T13:44:23Z","receivedAt":"2007-07-05T13:44:23Z","isPatch":false,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Jan Hudec writes:\n\n>> What's wrong with the behavior of \"hg rm\"?\n>> What's wrong with the behavior of \"svn rm\"?\n>> What's wrong with the behavior of \"bzr rm\"?\n>> (no, I won't do it with CVS ;-) )\n>> \n>> None of these commands have the problem that git-rm has.\n>\n> Hm. They all behave roughly the same: They unversion the file and unlink it,\n> unless it is modified, in which case they unversion it and leave it\n> alone.\n\nYes. Roughly, they'll ask you a --force flag whenever you'd risk\ndata-loss. bzr gives you the choice between --force and --keep (that\nwould be --cached in git) if the file doesn't match HEAD.\n\n> Now git has the extra complexity that index contains also content of the\n> file. But the behaviour can be easily adapted like this (HEAD = version in\n> HEAD, index = version in index, tree = version in tree):\n ^^^^- I suppose you meant \"version\"\n here since you don't use\n \"tree\" after.\n\n> - if (HEAD == index && index == version) unversion and unlink\n\nJust to be more precise:\n\n - if (HEAD == index && index == version) unversion and\n * if (--cached is not given) unlink\n * else do nothing\n\n> - else if (HEAD == index || index == version) unversion\n> - else print message and do nothing\n>\n> Would you consider that a sane behaviour?\n\nTo me, that's a sane behavior.\n\nIt makes a few senarios easy and safe, like this:\n\n $ git add \n # Ooops, no, I didn't want to version this one :-(\n $ git rm some-file\n # Cool, I just cancelled my mistake without loosing anything ;-)\n \nOne benefit is: you don't have to use \"-f\" for a non-dangerous\nsenario. That seems stupid, but for the plain \"rm\" command, the \"-rf\"\nis hardcoded in the fingers of many unix users, and I know several\npeople having lost data by typing it a bit too mechanically (with a\ntypo behind, like forgetting the \"*\" in \"*~\" ;-).\n\nI'll try writting patch for that if people agree that this is saner\nthat the current behavior.\n\n-- \nMatthieu\n"},{"id":"46550","messageId":"86abubkl0v.fsf@lola.quinscape.zz","threadId":"8814","inReplyTo":"vpqd4z7q820.fsf@bauges.imag.fr","subject":"Re: git-rm isn't the inverse action of git-add","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2007-07-05T14:00:48Z","receivedAt":"2007-07-05T14:00:48Z","isPatch":false,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Matthieu Moy writes:\n\n> One benefit is: you don't have to use \"-f\" for a non-dangerous\n> senario. That seems stupid, but for the plain \"rm\" command, the\n> \"-rf\" is hardcoded in the fingers of many unix users, and I know\n> several people having lost data by typing it a bit too mechanically\n> (with a typo behind, like forgetting the \"*\" in \"*~\" ;-).\n\nJust a few days ago, I used rm -rf * in a temporary directory. I\nwould now advise people against doing that without an absolute path.\nThe problem was that at some later point of time, some history\nsearch/key fsckup popped that line back into the shell and executed\nit.\n\nAt that time, in my home directory. This was definitely annoying,\neven though the files and directories .* (and thus most configuration\ndata) were spared.\n\n> I'll try writting patch for that if people agree that this is saner\n> that the current behavior.\n\nSounds like it.\n\n-- \nDavid Kastrup\n"},{"id":"46769","messageId":"vpqfy3yajbj.fsf_-_@bauges.imag.fr","threadId":"8814","inReplyTo":"vpqd4z7q820.fsf@bauges.imag.fr","subject":"[RFC][PATCH] Re: git-rm isn't the inverse action of git-add","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2007-07-08T17:36:48Z","receivedAt":"2007-07-08T17:36:48Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Matthieu Moy writes:\n\n>> - if (HEAD == index && index == version) unversion and unlink\n>\n> Just to be more precise:\n>\n> - if (HEAD == index && index == version) unversion and\n> * if (--cached is not given) unlink\n> * else do nothing\n>\n>> - else if (HEAD == index || index == version) unversion\n>> - else print message and do nothing\n>>\n>> Would you consider that a sane behaviour?\n\n[...]\n\n> I'll try writting patch for that if people agree that this is saner\n> that the current behavior.\n\nHere's a first attempt (I'm still not familiar with the git codebase,\nso the patch is probably not so good).\n\nNote: currently, git-rm still shows those \"rm '...'\" messages on\nstdout. AAUI, they were actually useful at a time when git-rm didn't\nactually remove the files, and people actually ran the \"rm\" commands\nafter. They can probably be removed now, but that's another topic.\n\n\n>From f4f4aa047b2b9050d968704d1f2db07b2a1a79cc Mon Sep 17 00:00:00 2001\nFrom: Matthieu Moy \nDate: Sun, 8 Jul 2007 19:27:44 +0200\nSubject: [PATCH] Make git-rm obey in more circumstances.\n\nIn the previous behavior of git-rm, git refused to do anything in case of\na difference between the file on disk, the index, and the HEAD. As a\nresult, the -f flag is forced even for simple senarios like:\n\n$ git add foo\n# oops, I didn't want to version it\n$ git rm -f [--cached] foo\n# foo is deleted on disk if --cached isn't provided.\n\nThis patch proposes a saner behavior. When there are no difference at all\nbetween file, index and HEAD, the file is removed both from the index and\nthe tree, as before.\n\nOtherwise, if the index matches either the file on disk or the HEAD, the\nfile is removed from the index, but the file is kept on disk, it may\ncontain important data.\n\nOtherwise, that's an error, and git-rm aborts.\n\nThe above senario becomes\n\n$ git add foo\n$ git rm foo\n# back to the initial state.\n\nSigned-off-by: Matthieu Moy \n---\n Documentation/git-rm.txt | 9 ++++---\n builtin-rm.c | 55 ++++++++++++++++++++++++++++++++++++---------\n 2 files changed, 49 insertions(+), 15 deletions(-)\n\ndiff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt\nindex 78f45dc..180671c 100644\n--- a/Documentation/git-rm.txt\n+++ b/Documentation/git-rm.txt\n@@ -11,10 +11,11 @@ SYNOPSIS\n \n DESCRIPTION\n -----------\n-Remove files from the working tree and from the index. The\n-files have to be identical to the tip of the branch, and no\n-updates to its contents must have been placed in the staging\n-area (aka index).\n+Remove files from the working tree and from the index. The content\n+placed in the staging area (aka index) must match either the content\n+of the file on disk, or the tip of the branch. If it matches only one\n+of them, the file is kept on disk for safety, but is still removed\n+from the index.\n \n \n OPTIONS\ndiff --git a/builtin-rm.c b/builtin-rm.c\nindex 4a0bd93..d2a8998 100644\n--- a/builtin-rm.c\n+++ b/builtin-rm.c\n@@ -12,18 +12,27 @@\n static const char builtin_rm_usage[] =\n \"git-rm [-f] [-n] [-r] [--cached] [--ignore-unmatch] [--quiet] [--] ...\";\n \n+struct file_info {\n+\tconst char *name;\n+\tint local_changes;\n+\tint staged_changes;\n+};\n+\n static struct {\n \tint nr, alloc;\n-\tconst char **name;\n+\tstruct file_info * files;\n } list;\n \n static void add_list(const char *name)\n {\n \tif (list.nr >= list.alloc) {\n \t\tlist.alloc = alloc_nr(list.alloc);\n-\t\tlist.name = xrealloc(list.name, list.alloc * sizeof(const char *));\n+\t\tlist.files = xrealloc(list.files, list.alloc * sizeof(const char *));\n \t}\n-\tlist.name[list.nr++] = name;\n+\tlist.files[list.nr].name = name;\n+\tlist.files[list.nr].local_changes = 0;\n+\tlist.files[list.nr].staged_changes = 0;\n+\tlist.nr++;\n }\n \n static int remove_file(const char *name)\n@@ -46,6 +55,26 @@ static int remove_file(const char *name)\n \treturn ret;\n }\n \n+static int remove_file_maybe(const struct file_info fi, int quiet)\n+{\n+\tconst char *path = fi.name;\n+\tif (!fi.local_changes && !fi.staged_changes) {\n+\t\t/* The file matches either the index or the HEAD.\n+\t\t * It's content exists somewhere else, it's safe to\n+\t\t * delete it.\n+\t\t */\n+\t\treturn remove_file(path);\n+\t} else {\n+\t\tif (!quiet)\n+\t\t\tfprintf(stderr, \n+\t\t\t\t\"note: file '%s' not removed \"\n+\t\t\t\t\"(doesn't match %s).\\n\",\n+\t\t\t\tpath,\n+\t\t\t\tfi.local_changes?\"the index\":\"HEAD\");\n+\t\treturn 0;\n+\t}\n+}\n+\n static int check_local_mod(unsigned char *head)\n {\n \t/* items in list are already sorted in the cache order,\n@@ -62,7 +91,7 @@ static int check_local_mod(unsigned char *head)\n \t\tstruct stat st;\n \t\tint pos;\n \t\tstruct cache_entry *ce;\n-\t\tconst char *name = list.name[i];\n+\t\tconst char *name = list.files[i].name;\n \t\tunsigned char sha1[20];\n \t\tunsigned mode;\n \n@@ -87,13 +116,17 @@ static int check_local_mod(unsigned char *head)\n \t\t\tcontinue;\n \t\t}\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\t\tlist.files[i].local_changes = 1;\n+\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\tlist.files[i].staged_changes = 1;\n+\n+\t\tif (list.files[i].local_changes && \n+\t\t list.files[i].staged_changes)\n+\t\t\terrs = error(\"'%s' doesn't match neither HEAD nor the index \"\n \t\t\t\t \"(hint: try -f)\", name);\n \t}\n \treturn errs;\n@@ -201,7 +234,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)\n \t * the index unless all of them succeed.\n \t */\n \tfor (i = 0; i < list.nr; i++) {\n-\t\tconst char *path = list.name[i];\n+\t\tconst char *path = list.files[i].name;\n \t\tif (!quiet)\n \t\t\tprintf(\"rm '%s'\\n\", path);\n \n@@ -224,13 +257,13 @@ int cmd_rm(int argc, const char **argv, const char *prefix)\n \tif (!index_only) {\n \t\tint removed = 0;\n \t\tfor (i = 0; i < list.nr; i++) {\n-\t\t\tconst char *path = list.name[i];\n-\t\t\tif (!remove_file(path)) {\n+\t\t\tif (!remove_file_maybe(list.files[i], quiet)) {\n \t\t\t\tremoved = 1;\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!removed)\n-\t\t\t\tdie(\"git-rm: %s: %s\", path, strerror(errno));\n+\t\t\t\tdie(\"git-rm: %s: %s\", \n+\t\t\t\t list.files[i].name, strerror(errno));\n \t\t}\n \t}\n \n-- \n1.5.3.rc0.63.gc956-dirty\n\n\n\n-- \nMatthieu\n"},{"id":"46772","messageId":"Pine.LNX.4.64.0707081855300.4248@racer.site","threadId":"8814","inReplyTo":"vpqfy3yajbj.fsf_-_@bauges.imag.fr","subject":"Re: [RFC][PATCH] Re: git-rm isn't the inverse action of git-add","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-08T18:10:59Z","receivedAt":"2007-07-08T18:10:59Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 8 Jul 2007, Matthieu Moy wrote:\n\n> Subject: [PATCH] Make git-rm obey in more circumstances.\n\nThis is not really a good patch title. Since it only obeys your \nparticular understanding of what it should do. You are changing \nsemantics, and you should say so.\n\n> In the previous behavior of git-rm, git refused to do anything in case \n> of a difference between the file on disk, the index, and the HEAD. As a \n> result, the -f flag is forced even for simple senarios like:\n> \n> $ git add foo\n> # oops, I didn't want to version it\n> $ git rm -f [--cached] foo\n> # foo is deleted on disk if --cached isn't provided.\n> \n> This patch proposes a saner behavior. When there are no difference at \n> all between file, index and HEAD, the file is removed both from the \n> index and the tree, as before.\n> \n> Otherwise, if the index matches either the file on disk or the HEAD, the \n> file is removed from the index, but the file is kept on disk, it may \n> contain important data.\n\nHowever, if some of the files are of the first kind, and some are of the \nsecond kind, you happily apply with mixed strategies. IMO that is wrong.\n\n> static struct {\n> \tint nr, alloc;\n> -\tconst char **name;\n> +\tstruct file_info * files;\n> } list;\n> \n> static void add_list(const char *name)\n> {\n> \tif (list.nr >= list.alloc) {\n> \t\tlist.alloc = alloc_nr(list.alloc);\n> -\t\tlist.name = xrealloc(list.name, list.alloc * sizeof(const char *));\n> +\t\tlist.files = xrealloc(list.files, list.alloc * sizeof(const char *));\n\nThis is wrong, too. Yes, it works. But it really should be \n\"sizeof(struct file_info *)\". Remember, code is also documentation.\n\n> +static int remove_file_maybe(const struct file_info fi, int quiet)\n> +{\n> +\tconst char *path = fi.name;\n> +\tif (!fi.local_changes && !fi.staged_changes) {\n> +\t\t/* The file matches either the index or the HEAD.\n> +\t\t * It's content exists somewhere else, it's safe to\n> +\t\t * delete it.\n> +\t\t */\n> +\t\treturn remove_file(path);\n> +\t} else {\n\nSuperfluous \"{ .. }\".\n\n> +\t\tif (!quiet)\n> +\t\t\tfprintf(stderr, \n> +\t\t\t\t\"note: file '%s' not removed \"\n> +\t\t\t\t\"(doesn't match %s).\\n\",\n> +\t\t\t\tpath,\n> +\t\t\t\tfi.local_changes?\"the index\":\"HEAD\");\n> +\t\treturn 0;\n> +\t}\n> +}\n\nI suspect that this case does never fail. 0 means success for \nremove_file(). Not good. You should at least have a way to ensure that \nit removed the files from the working tree from a script. Otherwise there \nis not much point in returning a value to begin with.\n\n> @@ -224,13 +257,13 @@ int cmd_rm(int argc, const char **argv, const char *prefix)\n> \tif (!index_only) {\n> \t\tint removed = 0;\n> \t\tfor (i = 0; i < list.nr; i++) {\n> -\t\t\tconst char *path = list.name[i];\n> -\t\t\tif (!remove_file(path)) {\n> +\t\t\tif (!remove_file_maybe(list.files[i], quiet)) {\n> \t\t\t\tremoved = 1;\n> \t\t\t\tcontinue;\n> \t\t\t}\n> \t\t\tif (!removed)\n> -\t\t\t\tdie(\"git-rm: %s: %s\", path, strerror(errno));\n> +\t\t\t\tdie(\"git-rm: %s: %s\", \n> +\t\t\t\t list.files[i].name, strerror(errno));\n> \t\t}\n> \t}\n\nStyle: the old code set and used \"path\" for readability. You should do \nthe same (with \"file\", probably).\n\nAdditionally, since this changes semantics, you better provide test cases \nto show what is expected to work, and _ensure_ that it actually works.\n\nCiao,\nDscho\n"},{"id":"46787","messageId":"vpq1wfi8wjl.fsf@bauges.imag.fr","threadId":"8814","inReplyTo":"Pine.LNX.4.64.0707081855300.4248@racer.site","subject":"Re: [RFC][PATCH] Re: git-rm isn't the inverse action of git-add","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2007-07-08T20:34:06Z","receivedAt":"2007-07-08T20:34:06Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Johannes Schindelin writes:\n\n>> This patch proposes a saner behavior. When there are no difference at \n>> all between file, index and HEAD, the file is removed both from the \n>> index and the tree, as before.\n>> \n>> Otherwise, if the index matches either the file on disk or the HEAD, the \n>> file is removed from the index, but the file is kept on disk, it may \n>> contain important data.\n>\n> However, if some of the files are of the first kind, and some are of the \n> second kind, you happily apply with mixed strategies. IMO that is wrong.\n\nI'm not sure whether this is really wrong. The things git should\nreally care about are the index and the repository itself, and the\nproposed behavior is consistant regarding that (either remove all\nfiles from the index, or remove none).\n\nI'm not opposed to your proposal, but I'd like to have other\nopinion(s) on that before changing the code.\n\n>> static struct {\n>> \tint nr, alloc;\n>> -\tconst char **name;\n>> +\tstruct file_info * files;\n>> } list;\n>> \n>> static void add_list(const char *name)\n>> {\n>> \tif (list.nr >= list.alloc) {\n>> \t\tlist.alloc = alloc_nr(list.alloc);\n>> -\t\tlist.name = xrealloc(list.name, list.alloc * sizeof(const char *));\n>> +\t\tlist.files = xrealloc(list.files, list.alloc * sizeof(const char *));\n>\n> This is wrong, too. Yes, it works. But it really should be \n> \"sizeof(struct file_info *)\". Remember, code is also documentation.\n\nYou don't need to argue, that was a typo. My code is definitely wrong,\nbut you're wrong too ;-). That's actually sizeof(struct file_info).\n\n>> +\t\tif (!quiet)\n>> +\t\t\tfprintf(stderr, \n>> +\t\t\t\t\"note: file '%s' not removed \"\n>> +\t\t\t\t\"(doesn't match %s).\\n\",\n>> +\t\t\t\tpath,\n>> +\t\t\t\tfi.local_changes?\"the index\":\"HEAD\");\n>> +\t\treturn 0;\n>> +\t}\n>> +}\n>\n> I suspect that this case does never fail. 0 means success for \n> remove_file(). Not good. You should at least have a way to ensure that \n> it removed the files from the working tree from a script. Otherwise there \n> is not much point in returning a value to begin with.\n\nI've changed it to have exit_status = 1 if git-rm aborted before\nstarting, and 2 if git-rm skiped some file removals (and of course, 0\nif everything is done as expected).\n\n>> @@ -224,13 +257,13 @@ int cmd_rm(int argc, const char **argv, const char *prefix)\n>> \tif (!index_only) {\n>> \t\tint removed = 0;\n>> \t\tfor (i = 0; i < list.nr; i++) {\n>> -\t\t\tconst char *path = list.name[i];\n>> -\t\t\tif (!remove_file(path)) {\n>> +\t\t\tif (!remove_file_maybe(list.files[i], quiet)) {\n>> \t\t\t\tremoved = 1;\n>> \t\t\t\tcontinue;\n>> \t\t\t}\n>> \t\t\tif (!removed)\n>> -\t\t\t\tdie(\"git-rm: %s: %s\", path, strerror(errno));\n>> +\t\t\t\tdie(\"git-rm: %s: %s\", \n>> +\t\t\t\t list.files[i].name, strerror(errno));\n>> \t\t}\n>> \t}\n>\n> Style: the old code set and used \"path\" for readability. You should do \n> the same (with \"file\", probably).\n\nDone.\n\n> Additionally, since this changes semantics, you better provide test cases \n> to show what is expected to work, and _ensure_ that it actually works.\n\nSure. I forgot to mention it in my message, but I wanted to have\nfeedback before getting into the testsuite stuff.\n\nI'm posting the updated patch for info, but it should anyway not be\nmerged until\n\n* We agree on the behavior when different files have different kinds\n of changes\n\n* I add a testcase.\n\n\n\n>From f39ae646049b95b055e34da378ea470ef3f3caef Mon Sep 17 00:00:00 2001\nFrom: Matthieu Moy \nDate: Sun, 8 Jul 2007 19:27:44 +0200\nSubject: [PATCH] Change the behavior of git-rm to let it obey in more circumstances without -f.\n\nIn the previous behavior of git-rm, git refused to do anything in case of\na difference between the file on disk, the index, and the HEAD. As a\nresult, the -f flag is forced even for simple senarios like:\n\n$ git add foo\n$ git rm -f [--cached] foo\n\nThis patch proposes a saner behavior. When there are no difference at all\nbetween file, index and HEAD, the file is removed both from the index and\nthe tree, as before.\n\nOtherwise, if the index matches either the file on disk or the HEAD, the\nfile is removed from the index, but the file is kept on disk, it may\ncontain important data.\n\nOtherwise, that's an error, and git-rm aborts.\n\nThe above senario becomes\n\n$ git add foo\n$ git rm foo\n\nSigned-off-by: Matthieu Moy \n---\n Documentation/git-rm.txt | 9 +++--\n builtin-rm.c | 71 ++++++++++++++++++++++++++++++++++++++--------\n 2 files changed, 64 insertions(+), 16 deletions(-)\n\ndiff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt\nindex 78f45dc..180671c 100644\n--- a/Documentation/git-rm.txt\n+++ b/Documentation/git-rm.txt\n@@ -11,10 +11,11 @@ SYNOPSIS\n \n DESCRIPTION\n -----------\n-Remove files from the working tree and from the index. The\n-files have to be identical to the tip of the branch, and no\n-updates to its contents must have been placed in the staging\n-area (aka index).\n+Remove files from the working tree and from the index. The content\n+placed in the staging area (aka index) must match either the content\n+of the file on disk, or the tip of the branch. If it matches only one\n+of them, the file is kept on disk for safety, but is still removed\n+from the index.\n \n \n OPTIONS\ndiff --git a/builtin-rm.c b/builtin-rm.c\nindex 4a0bd93..08af5de 100644\n--- a/builtin-rm.c\n+++ b/builtin-rm.c\n@@ -12,20 +12,30 @@\n static const char builtin_rm_usage[] =\n \"git-rm [-f] [-n] [-r] [--cached] [--ignore-unmatch] [--quiet] [--] ...\";\n \n+struct file_info {\n+\tconst char *name;\n+\tint local_changes;\n+\tint staged_changes;\n+};\n+\n static struct {\n \tint nr, alloc;\n-\tconst char **name;\n+\tstruct file_info *files;\n } list;\n \n static void add_list(const char *name)\n {\n \tif (list.nr >= list.alloc) {\n \t\tlist.alloc = alloc_nr(list.alloc);\n-\t\tlist.name = xrealloc(list.name, list.alloc * sizeof(const char *));\n+\t\tlist.files = xrealloc(list.files, list.alloc * sizeof(struct file_info));\n \t}\n-\tlist.name[list.nr++] = name;\n+\tlist.files[list.nr].name = name;\n+\tlist.files[list.nr].local_changes = 0;\n+\tlist.files[list.nr].staged_changes = 0;\n+\tlist.nr++;\n }\n \n+/* Returns -1 on error, zero on success */\n static int remove_file(const char *name)\n {\n \tint ret;\n@@ -46,6 +56,30 @@ static int remove_file(const char *name)\n \treturn ret;\n }\n \n+/* Returns 0 if the file was actually deleted, -1 if the file removal\n+ was a failure, and 1 if remove_file wasn't actually called */\n+static int remove_file_maybe(const struct file_info fi, int quiet)\n+{\n+\tconst char *path = fi.name;\n+\tif (!fi.local_changes && !fi.staged_changes)\n+\t\t/* The file matches either the index or the HEAD.\n+\t\t * It's content exists somewhere else, it's safe to\n+\t\t * delete it.\n+\t\t */\n+\t\treturn remove_file(path);\n+\telse {\n+\t\tif (!quiet)\n+\t\t\tfprintf(stderr, \n+\t\t\t\t\"note: file '%s' not removed \"\n+\t\t\t\t\"(%s).\\n\",\n+\t\t\t\tpath,\n+\t\t\t\tfi.local_changes ? \n+\t\t\t\t\"the index doesn't match HEAD\" :\n+\t\t\t\t\"the file doesn't match the index\");\n+\t\treturn 1;\n+\t}\n+}\n+\n static int check_local_mod(unsigned char *head)\n {\n \t/* items in list are already sorted in the cache order,\n@@ -62,7 +96,7 @@ static int check_local_mod(unsigned char *head)\n \t\tstruct stat st;\n \t\tint pos;\n \t\tstruct cache_entry *ce;\n-\t\tconst char *name = list.name[i];\n+\t\tconst char *name = list.files[i].name;\n \t\tunsigned char sha1[20];\n \t\tunsigned mode;\n \n@@ -87,13 +121,17 @@ static int check_local_mod(unsigned char *head)\n \t\t\tcontinue;\n \t\t}\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\t\tlist.files[i].local_changes = 1;\n+\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\tlist.files[i].staged_changes = 1;\n+\n+\t\tif (list.files[i].local_changes && \n+\t\t list.files[i].staged_changes)\n+\t\t\terrs = error(\"'%s' doesn't match neither HEAD nor the index \"\n \t\t\t\t \"(hint: try -f)\", name);\n \t}\n \treturn errs;\n@@ -108,6 +146,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)\n \tint ignore_unmatch = 0;\n \tconst char **pathspec;\n \tchar *seen;\n+\tint exit_status = 0;\n \n \tgit_config(git_default_config);\n \n@@ -201,7 +240,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)\n \t * the index unless all of them succeed.\n \t */\n \tfor (i = 0; i < list.nr; i++) {\n-\t\tconst char *path = list.name[i];\n+\t\tconst char *path = list.files[i].name;\n \t\tif (!quiet)\n \t\t\tprintf(\"rm '%s'\\n\", path);\n \n@@ -224,13 +263,21 @@ int cmd_rm(int argc, const char **argv, const char *prefix)\n \tif (!index_only) {\n \t\tint removed = 0;\n \t\tfor (i = 0; i < list.nr; i++) {\n-\t\t\tconst char *path = list.name[i];\n-\t\t\tif (!remove_file(path)) {\n+\t\t\tstruct file_info file = list.files[i];\n+\t\t\tint status = remove_file_maybe(file, quiet);\n+\t\t\tif (status == 0) {\n \t\t\t\tremoved = 1;\n \t\t\t\tcontinue;\n+\t\t\t} else if (status == 1) {\n+\t\t\t\t/* Let the user know from a script\n+\t\t\t\t * that a file was not deleted on disk\n+\t\t\t\t */\n+\t\t\t\texit_status = 2;\n+\t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!removed)\n-\t\t\t\tdie(\"git-rm: %s: %s\", path, strerror(errno));\n+\t\t\t\tdie(\"git-rm: %s: %s\", \n+\t\t\t\t file.name, strerror(errno));\n \t\t}\n \t}\n \n@@ -240,5 +287,5 @@ int cmd_rm(int argc, const char **argv, const char *prefix)\n \t\t\tdie(\"Unable to write new index file\");\n \t}\n \n-\treturn 0;\n+\treturn exit_status;\n }\n-- \n1.5.3.rc0.64.gf4f4a-dirty\n\n\n\n-- \nMatthieu\n"},{"id":"46801","messageId":"Pine.LNX.4.64.0707082240510.4248@racer.site","threadId":"8814","inReplyTo":"vpq1wfi8wjl.fsf@bauges.imag.fr","subject":"Re: [RFC][PATCH] Re: git-rm isn't the inverse action of git-add","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-08T21:49:10Z","receivedAt":"2007-07-08T21:49:10Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 8 Jul 2007, Matthieu Moy wrote:\n\n> Johannes Schindelin writes:\n> \n> >> This patch proposes a saner behavior. When there are no difference at \n> >> all between file, index and HEAD, the file is removed both from the \n> >> index and the tree, as before.\n> >> \n> >> Otherwise, if the index matches either the file on disk or the HEAD, \n> >> the file is removed from the index, but the file is kept on disk, it \n> >> may contain important data.\n> >\n> > However, if some of the files are of the first kind, and some are of \n> > the second kind, you happily apply with mixed strategies. IMO that is \n> > wrong.\n> \n> I'm not sure whether this is really wrong. The things git should\n> really care about are the index and the repository itself, and the\n> proposed behavior is consistant regarding that (either remove all\n> files from the index, or remove none).\n\nWell, I think it is wrong for the same reason as it is wrong to apply the \nchanges to _any_ file when one would fail. And since \"git apply\" shares \nmy understanding, I think \"git rm\" should, too.\n\n> >> static struct {\n> >> \tint nr, alloc;\n> >> -\tconst char **name;\n> >> +\tstruct file_info * files;\n> >> } list;\n> >> \n> >> static void add_list(const char *name)\n> >> {\n> >> \tif (list.nr >= list.alloc) {\n> >> \t\tlist.alloc = alloc_nr(list.alloc);\n> >> -\t\tlist.name = xrealloc(list.name, list.alloc * sizeof(const char *));\n> >> +\t\tlist.files = xrealloc(list.files, list.alloc * sizeof(const char *));\n> >\n> > This is wrong, too. Yes, it works. But it really should be \n> > \"sizeof(struct file_info *)\". Remember, code is also documentation.\n> \n> You don't need to argue, that was a typo. My code is definitely wrong, \n> but you're wrong too ;-). That's actually sizeof(struct file_info).\n\nHeh, right.\n\n> >> +\t\tif (!quiet)\n> >> +\t\t\tfprintf(stderr, \n> >> +\t\t\t\t\"note: file '%s' not removed \"\n> >> +\t\t\t\t\"(doesn't match %s).\\n\",\n> >> +\t\t\t\tpath,\n> >> +\t\t\t\tfi.local_changes?\"the index\":\"HEAD\");\n> >> +\t\treturn 0;\n> >> +\t}\n> >> +}\n> >\n> > I suspect that this case does never fail. 0 means success for \n> > remove_file(). Not good. You should at least have a way to ensure that \n> > it removed the files from the working tree from a script. Otherwise there \n> > is not much point in returning a value to begin with.\n> \n> I've changed it to have exit_status = 1 if git-rm aborted before\n> starting, and 2 if git-rm skiped some file removals (and of course, 0\n> if everything is done as expected).\n\nOh, so you do not take the return value of this function to determine if \nit has or has not done something with the files? That's a bit confusing.\n\nBesides, it would be all the more a reason for a test case, so that I can \nsee that I am actually wrong.\n\n> > Additionally, since this changes semantics, you better provide test \n> > cases to show what is expected to work, and _ensure_ that it actually \n> > works.\n> \n> Sure. I forgot to mention it in my message, but I wanted to have \n> feedback before getting into the testsuite stuff.\n\nI think it should be the other way. If you change semantics with the \npatch, but another revision changes semantics _differently_, it is really \neasy to get lost. In order to demonstrate what should be true, you have \nto provide examples. And if you are already providing examples, just wrap \nthem into\n\n\ttest_description \n\t. ./test-lib.sh\n\n\t...\n\n\ttest_done\n\nand prefix each test with \"test_expect_success\", and you're done. It is \nreally not something requiring a wizard.\n\n> I'm posting the updated patch for info, but it should anyway not be\n> merged until\n> \n> * We agree on the behavior when different files have different kinds\n> of changes\n\nI'd understand better what you wish to accomplish with the...\n\n> * I add a testcase.\n\n... testcase. So those are not two distinct points.\n\n> >From f39ae646049b95b055e34da378ea470ef3f3caef Mon Sep 17 00:00:00 2001\n> From: Matthieu Moy \n> Date: Sun, 8 Jul 2007 19:27:44 +0200\n> Subject: [PATCH] Change the behavior of git-rm to let it obey in more circumstances without -f.\n\nPlease do not do this.\n\nI meant to complain about your OP, but this time it is even worse. The \nbest way to guarantee that a patch gets lost in a thread is to move it _at \nthe end_ of a reply.\n\nPlease follow the form that you change the subject, still reply, but but \nthe quoted mail with your answers to that text between the \"---\" and the \ndiffstat.\n\nIf that text is too long, you should use a separate email for the patch.\n\nCiao,\nDscho\n"},{"id":"46846","messageId":"vpqsl7xkj0j.fsf@bauges.imag.fr","threadId":"8814","inReplyTo":"Pine.LNX.4.64.0707082240510.4248@racer.site","subject":"Re: [RFC][PATCH] Re: git-rm isn't the inverse action of git-add","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2007-07-09T09:45:32Z","receivedAt":"2007-07-09T09:45:32Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Johannes Schindelin writes:\n\n> Hi,\n>\n> On Sun, 8 Jul 2007, Matthieu Moy wrote:\n>\n>> I'm not sure whether this is really wrong. The things git should\n>> really care about are the index and the repository itself, and the\n>> proposed behavior is consistant regarding that (either remove all\n>> files from the index, or remove none).\n>\n> Well, I think it is wrong for the same reason as it is wrong to apply the \n> changes to _any_ file when one would fail. And since \"git apply\" shares \n> my understanding, I think \"git rm\" should, too.\n\nOK, let's say I'm convinced ;-).\n\n>> > I suspect that this case does never fail. 0 means success for \n>> > remove_file(). Not good. You should at least have a way to ensure that \n>> > it removed the files from the working tree from a script. Otherwise there \n>> > is not much point in returning a value to begin with.\n>> \n>> I've changed it to have exit_status = 1 if git-rm aborted before\n>> starting, and 2 if git-rm skiped some file removals (and of course, 0\n>> if everything is done as expected).\n>\n> Oh, so you do not take the return value of this function to determine if \n> it has or has not done something with the files? That's a bit confusing.\n\nI did, but previously, I kept the code that \"die()\"s if the first call\nto remove_file() \"fails\". In remove_file_maybe(), not removing a file\nbecause it's not sure it's safe to delete it is not a failure, so I\nhad to put a \"return 0;\" here to avoid the fatal error. My first patch\nhad return status !=0 if we tried to remove the file, and it failed. I\nchanged that.\n\n> I meant to complain about your OP, but this time it is even worse. The \n> best way to guarantee that a patch gets lost in a thread is to move it _at \n> the end_ of a reply.\n\nI had posted the patch for info, but I did expect this one to get\nlost, since it's definitely not complete.\n\nI'll post an updated patch with a testcase and an appropriate subject\nline within a few days (I don't have time right now).\n\nThanks,\n\n-- \nMatthieu\n"},{"id":"47027","messageId":"f72hu8$65g$1@sea.gmane.org","threadId":"8814","inReplyTo":"46895EA4.5040803@jaeger.mine.nu","subject":"Re: git-rm isn't the inverse action of git-add","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-07-11T12:20:24Z","receivedAt":"2007-07-11T12:20:24Z","isPatch":false,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Christian Jaeger wrote:\n\n> I don't per se require undo actions. I just don't understand why git-rm\n> refuses to remove the file from the index, even if I didn't commit it.\n> The index is just an intermediate record of the changes in my\n> understandings, and the rm action would also be intermediate until it's\n> being committed. And a non-committed action being deleted shouldn't need\n> a special confirmation from me, especially not one which is consisting\n> of a combination of two flags (of which one is a destructive one).\n\nShould git-rm refuse to remove index entry if it is different from working\ndirectory version or not?\n\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"},{"id":"47061","messageId":"20070711185644.GA3069@efreet.light.src","threadId":"8814","inReplyTo":"f72hu8$65g$1@sea.gmane.org","subject":"Re: git-rm isn't the inverse action of git-add","fromName":"Jan Hudec","fromEmail":"bulb@ucw.cz","sentAt":"2007-07-11T18:56:44Z","receivedAt":"2007-07-11T18:56:44Z","isPatch":false,"sender":{"key":"bulb@ucw.cz","avatar":null},"body":"On Wed, Jul 11, 2007 at 14:20:24 +0200, Jakub Narebski wrote:\n> Christian Jaeger wrote:\n> > I don't per se require undo actions. I just don't understand why git-rm\n> > refuses to remove the file from the index, even if I didn't commit it.\n> > The index is just an intermediate record of the changes in my\n> > understandings, and the rm action would also be intermediate until it's\n> > being committed. And a non-committed action being deleted shouldn't need\n> > a special confirmation from me, especially not one which is consisting\n> > of a combination of two flags (of which one is a destructive one).\n> \n> Should git-rm refuse to remove index entry if it is different from working\n> directory version or not?\n\nIMHO it should refuse to remove index entry if it is different from both\nworking-tree version and versions in all parents.\n\nIf index matches any of that, but the working tree version does not match any\nparent, the index entry should be removed (which currently isn't -- that's\nthe proposed change), but the file left in wokring tree. That would make\ngit-add + git-rm get you right back where you started, with nothing in index\nand unversioned file in working tree.\n\n-- \n\t\t\t\t\t\t Jan 'Bulb' Hudec \n"},{"id":"47082","messageId":"7vps2y3a4n.fsf@assigned-by-dhcp.cox.net","threadId":"8814","inReplyTo":"20070711185644.GA3069@efreet.light.src","subject":"Re: git-rm isn't the inverse action of git-add","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-11T21:26:16Z","receivedAt":"2007-07-11T21:26:16Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jan Hudec writes:\n\n> If index matches any of that, but the working tree version does not match any\n> parent, the index entry should be removed (which currently isn't -- that's\n> the proposed change), but the file left in wokring tree. That would make\n> git-add + git-rm get you right back where you started, with nothing in index\n> and unversioned file in working tree.\n\nDon't think of 'rm' as inverse of 'add'. That would only\nconfuse you.\n\nA natural inverse of 'add' is 'un-add', and that operation is\ncalled 'rm --cached', because we use that to name the option to\ninvoke an \"index-only\" variant of a command when the command can\noperate on index and working tree file (e.g. \"diff --cached\",\n\"apply --cached\").\n\nA life of a file that does _not_ make into a commit goes like\nthis:\n\n\t[1]$ edit a-new-file\n\nThis is 'create', not 'add'. git is not involved in this step.\n\n\t[2]$ git add a-new-file\n\nThis is 'add'; place an existing file in the index. When you do\nnot want it in the index, you 'un-add' it.\n\n\t[3]$ git rm --cached a-new-file\n\nThis removes the entry from the index, without touching the\nworking tree file. If you do not want that file at all (as\nopposed to, \"I am making a series of partial commits, and the\naddition of this path does not belong to the first commit of the\nseries, so I am unstaging\"), this is followed by\n\n\t[4]$ rm -f a-new-file\n\nAgain, git is not involved in this step.\n\nThe thing is, people sometimes want to have steps 3 and 4\ncombined, and it meshes well with the users' expectation when\nthey see the word \"rm\". Think of \"git rm\" without \"--cached\" as\na shorthand to do 3 and 4 in one go to meet that expectation.\n\nObviously, we cannot usefully combine steps 1 and 2. We could\nhave \"git add --create a-new-file\" launch an editor to create a\nnew file, but that would not be very useful in practice.\n\nThe fact that steps 3 and 4 can be naturally combined, but steps\n1 and 2 cannot be, makes \"add\" and \"rm\" not inverse of each\nother.\n"},{"id":"47276","messageId":"vpq8x9k9peu.fsf@bauges.imag.fr","threadId":"8814","inReplyTo":"Pine.LNX.4.64.0707082240510.4248@racer.site","subject":"Re: [RFC][PATCH] Re: git-rm isn't the inverse action of git-add","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2007-07-13T17:36:25Z","receivedAt":"2007-07-13T17:36:25Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Johannes Schindelin writes:\n\n>> > However, if some of the files are of the first kind, and some are of \n>> > the second kind, you happily apply with mixed strategies. IMO that is \n>> > wrong.\n>> \n>> I'm not sure whether this is really wrong. The things git should\n>> really care about are the index and the repository itself, and the\n>> proposed behavior is consistant regarding that (either remove all\n>> files from the index, or remove none).\n>\n> Well, I think it is wrong for the same reason as it is wrong to apply the \n> changes to _any_ file when one would fail. And since \"git apply\" shares \n> my understanding, I think \"git rm\" should, too.\n\nOK, I've been thinking about it for some time (not having time to hack\ncan be good, it lets time for thinking instead ;-) ).\n\nI'm actually still not convinced that my proposal was wrong, but I\nthink we disagree because we disagree on what is a \"failure\". I\nconsider leaving the file in the working tree to be just a safety\nprecaution, not a failure, and to me, it's OK to do that only for the\nfiles that need it.\n\nFixing my patch by just \"applying the same strategy to all files\"\nwould be wrong: leaving _all_ the files on disk when just one has\nlocal modifications is very misleading, and if the user notices it\nafter running the command, he or she does not always have an easy way\nto get back to a clean situation (re-running the same command with -f\nwouldn't work for example).\n\nSo, I went a shorter way from the current semantics:\n\n* Allow --cached in more situations, so that -f is really needed in\n very particular situation (as I mentionned above, forcing -f too\n often means the -f gets hardcoded in the fingers, and makes it\n useless).\n\n* Better error message, which points to --cached in addition to -f.\n\nThat's very close to what bzr does, BTW.\n\nDrawback: it still doesn't solve the \"rm isn't the inverse of add\".\n\nThe patch is quite straightforward, and will be in a followup email.\n\n-- \nMatthieu\n"},{"id":"47277","messageId":"11843484982037-git-send-email-Matthieu.Moy@imag.fr","threadId":"8814","inReplyTo":"vpq8x9k9peu.fsf@bauges.imag.fr","subject":"[PATCH] More permissive \"git-rm --cached\" behavior without -f.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2007-07-13T17:41:38Z","receivedAt":"2007-07-13T17:41:38Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"In the previous behavior, \"git-rm --cached\" (without -f) had the same\nrestriction as \"git-rm\". This forced the user to use the -f flag in\nsituations which weren't actually dangerous, like:\n\n$ git add foo # oops, I didn't want this\n$ git rm --cached foo # back to initial situation\n\nPreviously, the index had to match the file *and* the HEAD. With\n--cached, the index must now match the file *or* the HEAD. The behavior\nwithout --cached is unchanged, but provides better error messages.\n\nSigned-off-by: Matthieu Moy \n---\n Documentation/git-rm.txt | 3 ++-\n builtin-rm.c | 32 ++++++++++++++++++++++++++------\n t/t3600-rm.sh | 34 ++++++++++++++++++++++++++++++++++\n 3 files changed, 62 insertions(+), 7 deletions(-)\n\ndiff --git a/Documentation/git-rm.txt b/Documentation/git-rm.txt\nindex 78f45dc..be61a82 100644\n--- a/Documentation/git-rm.txt\n+++ b/Documentation/git-rm.txt\n@@ -14,7 +14,8 @@ DESCRIPTION\n Remove files from the working tree and from the index. The\n files have to be identical to the tip of the branch, and no\n updates to its contents must have been placed in the staging\n-area (aka index).\n+area (aka index). When --cached is given, the staged content has to\n+match either the tip of the branch *or* the file on disk.\n \n \n OPTIONS\ndiff --git a/builtin-rm.c b/builtin-rm.c\nindex 4a0bd93..9a808c1 100644\n--- a/builtin-rm.c\n+++ b/builtin-rm.c\n@@ -46,7 +46,7 @@ static int remove_file(const char *name)\n \treturn ret;\n }\n \n-static int check_local_mod(unsigned char *head)\n+static int check_local_mod(unsigned char *head, int index_only)\n {\n \t/* items in list are already sorted in the cache order,\n \t * so we could do this a lot more efficiently by using\n@@ -65,6 +65,8 @@ static int check_local_mod(unsigned char *head)\n \t\tconst char *name = list.name[i];\n \t\tunsigned char sha1[20];\n \t\tunsigned mode;\n+\t\tint local_changes = 0;\n+\t\tint staged_changes = 0;\n \n \t\tpos = cache_name_pos(name, strlen(name));\n \t\tif (pos < 0)\n@@ -87,14 +89,32 @@ static int check_local_mod(unsigned char *head)\n \t\t\tcontinue;\n \t\t}\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\t\tlocal_changes = 1;\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\t\tstaged_changes = 1;\n+\n+\t\tif (local_changes && staged_changes)\n+\t\t\terrs = error(\"'%s' has staged content different \"\n+\t\t\t\t \"from both the file and the HEAD\\n\"\n+\t\t\t\t \"(use -f to force removal)\", name);\n+\t\telse if (!index_only) {\n+\t\t\t/* It's not dangerous to git-rm --cached a\n+\t\t\t * file if the index matches the file or the\n+\t\t\t * HEAD, since it means the deleted content is\n+\t\t\t * still available somewhere.\n+\t\t\t */\n+\t\t\tif (staged_changes)\n+\t\t\t\terrs = error(\"'%s' has changes staged in the index\\n\"\n+\t\t\t\t\t \"(use --cached to keep the file, \"\n+\t\t\t\t\t \"or -f to force removal)\", name);\n+\t\t\tif (local_changes)\n+\t\t\t\terrs = error(\"'%s' has local modifications\\n\"\n+\t\t\t\t\t \"(use --cached to keep the file, \"\n+\t\t\t\t\t \"or -f to force removal)\", name);\n+\t\t}\n \t}\n \treturn errs;\n }\n@@ -192,7 +212,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)\n \t\tunsigned char sha1[20];\n \t\tif (get_sha1(\"HEAD\", sha1))\n \t\t\thashclr(sha1);\n-\t\tif (check_local_mod(sha1))\n+\t\tif (check_local_mod(sha1, index_only))\n \t\t\texit(1);\n \t}\n \ndiff --git a/t/t3600-rm.sh b/t/t3600-rm.sh\nindex 13a461f..5c001aa 100755\n--- a/t/t3600-rm.sh\n+++ b/t/t3600-rm.sh\n@@ -46,6 +46,40 @@ test_expect_success \\\n 'git rm --cached foo'\n \n test_expect_success \\\n+ 'Test that git rm --cached foo succeeds if the index matches the file' \\\n+ 'echo content > foo\n+ git add foo\n+ git rm --cached foo'\n+\n+test_expect_success \\\n+ 'Test that git rm --cached foo succeeds if the index matches the file' \\\n+ 'echo content > foo\n+ git add foo\n+ git commit -m foo\n+ echo \"other content\" > foo\n+ git rm --cached foo'\n+\n+test_expect_failure \\\n+ 'Test that git rm --cached foo fails if the index matches neither the file nor HEAD' \\\n+ 'echo content > foo\n+ git add foo\n+ git commit -m foo\n+ echo \"other content\" > foo\n+ git add foo\n+ echo \"yet another content\" > foo\n+ git rm --cached foo'\n+\n+test_expect_success \\\n+ 'Test that git rm --cached -f foo works in case where --cached only did not' \\\n+ 'echo content > foo\n+ git add foo\n+ git commit -m foo\n+ echo \"other content\" > foo\n+ git add foo\n+ echo \"yet another content\" > foo\n+ git rm --cached -f foo'\n+\n+test_expect_success \\\n 'Post-check that foo exists but is not in index after git rm foo' \\\n '[ -f foo ] && ! git ls-files --error-unmatch foo'\n \n-- \n1.5.3.rc1.4.gaf83-dirty\n"},{"id":"47279","messageId":"20070713175737.GA20416@coredump.intra.peff.net","threadId":"8814","inReplyTo":"11843484982037-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH] More permissive \"git-rm --cached\" behavior without -f.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-07-13T17:57:37Z","receivedAt":"2007-07-13T17:57:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jul 13, 2007 at 07:41:38PM +0200, Matthieu Moy wrote:\n\n> Previously, the index had to match the file *and* the HEAD. With\n> --cached, the index must now match the file *or* the HEAD. The behavior\n> without --cached is unchanged, but provides better error messages.\n\nThis does make more sense, but there are still some inconsistencies. Is\nit OK to lose content that is only in the index, or not?\n\nIf it is OK, then --cached shouldn't need _any_ safety valve (and after\nall, anything you remove in that manner is recoverable with git-fsck\nuntil the next prune).\n\nIf it isn't OK, then you are not addressing the cases where git-rm\nwithout --cached loses index content (that is different than HEAD and\nthe working tree).\n\n-Peff\n"},{"id":"47283","messageId":"vpq8x9kp231.fsf@bauges.imag.fr","threadId":"8814","inReplyTo":"20070713175737.GA20416@coredump.intra.peff.net","subject":"Re: [PATCH] More permissive \"git-rm --cached\" behavior without -f.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2007-07-13T18:53:38Z","receivedAt":"2007-07-13T18:53:38Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Jeff King writes:\n\n> On Fri, Jul 13, 2007 at 07:41:38PM +0200, Matthieu Moy wrote:\n>\n>> Previously, the index had to match the file *and* the HEAD. With\n>> --cached, the index must now match the file *or* the HEAD. The behavior\n>> without --cached is unchanged, but provides better error messages.\n>\n> This does make more sense, but there are still some inconsistencies. Is\n> it OK to lose content that is only in the index, or not?\n\nI'd say it isn't OK. At least, that's what the previous git-rm\nconsidered.\n\n> If it is OK, then --cached shouldn't need _any_ safety valve (and after\n> all, anything you remove in that manner is recoverable with git-fsck\n> until the next prune).\n>\n> If it isn't OK, then you are not addressing the cases where git-rm\n> without --cached loses index content (that is different than HEAD and\n> the working tree).\n\nEither I didn't understand your question, or the answer is \"yes, I\ndo.\". The behavior without --cached is not modified, except for the\nerror message, and the previous was to require -f whenever the index\ndoesn't match the head, *or* doesn't match the file. So, without\n--cached, you need to have file=index=HEAD to be able to git-rm.\n\nIf I missunderstand you, please, provide a senario where my patch\ndoesn't do the expected.\n\n-- \nMatthieu\n"},{"id":"47300","messageId":"f7968s$j44$1@sea.gmane.org","threadId":"8814","inReplyTo":"11843484982037-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH] More permissive \"git-rm --cached\" behavior without -f.","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2007-07-14T00:44:14Z","receivedAt":"2007-07-14T00:44:14Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Matthieu Moy wrote:\n\n> In the previous behavior, \"git-rm --cached\" (without -f) had the same\n> restriction as \"git-rm\". This forced the user to use the -f flag in\n> situations which weren't actually dangerous, like:\n> \n> $ git add foo           # oops, I didn't want this\n> $ git rm --cached foo   # back to initial situation\n> \n> Previously, the index had to match the file *and* the HEAD. With\n> --cached, the index must now match the file *or* the HEAD. The behavior\n> without --cached is unchanged, but provides better error messages.\n\nSensible.\n\nThere might be some discussion if what git-rm without --cached does\nis right, but that is besides scope of this patch.\n-- \nJakub Narebski\nWarsaw, Poland\nShadeHawk on #git\n"},{"id":"47308","messageId":"20070714034221.GA23328@coredump.intra.peff.net","threadId":"8814","inReplyTo":"vpq8x9kp231.fsf@bauges.imag.fr","subject":"Re: [PATCH] More permissive \"git-rm --cached\" behavior without -f.","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-07-14T03:42:21Z","receivedAt":"2007-07-14T03:42:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jul 13, 2007 at 08:53:38PM +0200, Matthieu Moy wrote:\n\n> do.\". The behavior without --cached is not modified, except for the\n> error message, and the previous was to require -f whenever the index\n> doesn't match the head, *or* doesn't match the file. So, without\n> --cached, you need to have file=index=HEAD to be able to git-rm.\n> \n> If I missunderstand you, please, provide a senario where my patch\n> doesn't do the expected.\n\nRight, my point was that there is a case where running without --cached\ncould lose content: when there is no working tree file. However,\nthinking about it more, I recall that Junio made the point that allowing\nthat behavior means the CVS idiom of \"rm file; git-rm file\" will just\nwork.\n\nNot that that was a problem you introduced; I merely wanted to push for\ntotal consistency rather than just handling --cached. But I think the\nnon --cached behavior is actually right now, so let me retract my\ncomplaint.\n\nAnd assuming the \"git-rm when no working tree file\" current behavior is\nOK, then I think your patch removes the last consistency problem that I\nmentioned in my state table here:\n\n http://article.gmane.org/gmane.comp.version-control.git/51449\n\nSo in a round-about way, I totally approve of your patch. Sorry for the\nconfusion.\n\n-Peff\n"},{"id":"47320","messageId":"7vfy3rlbnp.fsf@assigned-by-dhcp.cox.net","threadId":"8814","inReplyTo":"11843484982037-git-send-email-Matthieu.Moy@imag.fr","subject":"Re: [PATCH] More permissive \"git-rm --cached\" behavior without -f.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-14T06:52:42Z","receivedAt":"2007-07-14T06:52:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Although I would not be using it often myself, I think this\nwould make \"git rm\" more pleasant to use.\n\nThanks for the patch, and my thanks also go to people who\ncommented on the patch.\n"},{"id":"47323","messageId":"7vy7hjjw01.fsf@assigned-by-dhcp.cox.net","threadId":"8814","inReplyTo":"7vfy3rlbnp.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] More permissive \"git-rm --cached\" behavior without -f.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-14T07:16:14Z","receivedAt":"2007-07-14T07:16:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Although I would not be using it often myself, I think this\n> would make \"git rm\" more pleasant to use.\n>\n> Thanks for the patch, and my thanks also go to people who\n> commented on the patch.\n\nHaving said that, I think this comment is not quite right.\n\n+\t\telse if (!index_only) {\n+\t\t\t/* It's not dangerous to git-rm --cached a\n+\t\t\t * file if the index matches the file or the\n+\t\t\t * HEAD, since it means the deleted content is\n+\t\t\t * still available somewhere.\n+\t\t\t */\n\nPersonally I do not think \"rm --cached\" needs any such \"safety\",\neven though I'll keep the check for now, primarily because\nloosening the restriction later is always easier than adding new\nrestriction. I really do not think this is about protecting the\nuser from \"deleted content is not available anywhere else\".\n\nIn this sequence:\n\n\tedit a-new-file\n\tgit add a-new-file\n edit a-new-file\n git add a-new-file\n\nwe do not complain, even though we are *losing* the contents we\nearlier staged. If you replace the second \"git add\" with\n\"git-rm --cached\", the sequence should work the same way. In\neither case, you are working towards your next commit, and most\nlikely are doing a partial commit (iow, your working tree does\nnot match any of the commit you create in the middle). Earlier\nyou thought you would want one state of the file in the next\ncommit, but now you decided against putting that new file in the\nfirst commit in the series. You may make further updates to the\nindex and would make a commit, but after making the commit, your\nworking tree still has \"a-new-file\" and you can add the contents\nfrom it for the later commit.\n"},{"id":"47332","messageId":"vpqy7hjl2c3.fsf@bauges.imag.fr","threadId":"8814","inReplyTo":"7vy7hjjw01.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] More permissive \"git-rm --cached\" behavior without -f.","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@imag.fr","sentAt":"2007-07-14T10:14:04Z","receivedAt":"2007-07-14T10:14:04Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Junio C Hamano writes:\n\n> Junio C Hamano writes:\n>\n>> Although I would not be using it often myself, I think this\n>> would make \"git rm\" more pleasant to use.\n>>\n>> Thanks for the patch, and my thanks also go to people who\n>> commented on the patch.\n>\n> Having said that, I think this comment is not quite right.\n>\n> +\t\telse if (!index_only) {\n> +\t\t\t/* It's not dangerous to git-rm --cached a\n> +\t\t\t * file if the index matches the file or the\n> +\t\t\t * HEAD, since it means the deleted content is\n> +\t\t\t * still available somewhere.\n> +\t\t\t */\n>\n> Personally I do not think \"rm --cached\" needs any such \"safety\",\n> even though I'll keep the check for now, primarily because\n> loosening the restriction later is always easier than adding new\n> restriction. I really do not think this is about protecting the\n> user from \"deleted content is not available anywhere else\".\n\nI agree that this is something you can argue about.\n\nBut in this case, the behavior without -f should be changed too. If\nthe file matches HEAD, then \"git-rm file\" should work, regardless of\nthe index then (but this situation is less frequent).\n\nIn any case, the situation where you might lose content in the index\nby doing git-rm are rare: it means you started working on a file, did\n\"git-add\" at least once, and edited the file again later, and then\ndecided you wanted to remove the file. So, requiring the -f flag in\nthose situation is not a real problem, even if the situation is\nslightly-dangerous-but-not-quite-so.\n\nI'm willing to work on another patch on top of this one if there's an\nagreement on a better semantics. This one was about fixing something\nwhich was IMHO wrong, but doesn't necessarily achieve perfection ;-).\n\n-- \nMatthieu\n"}]}