{"thread":{"id":"26966","subject":"git commit fails under some circumstances","startedAt":"2011-04-02T15:18:54Z","lastAt":"2011-04-06T17:57:01Z","messageCount":8,"participants":["Laszlo Papp","Junio C Hamano","Will Palmer","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"164952","messageId":"AANLkTikHRM10p9e8XCzuiih7cYtQRG7Az2Qn5qtPzhZJ@mail.gmail.com","threadId":"26966","inReplyTo":null,"subject":"git commit fails under some circumstances","fromName":"Laszlo Papp","fromEmail":"djszapi@archlinux.us","sentAt":"2011-04-02T15:18:54Z","receivedAt":"2011-04-02T15:18:54Z","isPatch":false,"sender":{"key":"djszapi@archlinux.us","avatar":null},"body":"Hi,\n\nThere are 2 bugs and 1 wish in this case:\n1) It says \"Changes to be committed:\", but they are actually not\ncommitted, there cannot even be committed since they were not added\n\"properly\". This output is bogus\n2) error: Error building trees\n---\n3) It would be nice to have one command (with no (git) alias for sure)\nto see the difference like \"git diff\" but also the new files.\n\nBest Regards,\nLaszlo Papp\n\n========\n\nroot /home/djszapi/Projects/kde/attica/build # git add -N\n../lib/achievement.cpp ../lib/achievement.h\n../lib/achievementparser.cpp ../lib/achievementparser.h\nroot /home/djszapi/Projects/kde/attica/build # git status\n# On branch master\n# Changes to be committed:\n# (use \"git reset HEAD ...\" to unstage)\n#\n# new file: ../lib/achievement.cpp\n# new file: ../lib/achievement.h\n# new file: ../lib/achievementparser.cpp\n# new file: ../lib/achievementparser.h\n#\n# Changes not staged for commit:\n# (use \"git add ...\" to update what will be committed)\n# (use \"git checkout -- ...\" to discard changes in working directory)\n#\n# modified: ../lib/CMakeLists.txt\n# modified: ../lib/achievement.cpp\n# modified: ../lib/achievement.h\n# modified: ../lib/achievementparser.cpp\n# modified: ../lib/achievementparser.h\n# modified: ../lib/listjob_inst.cpp\n# modified: ../lib/provider.cpp\n# modified: ../lib/provider.h\n# modified: ../lib/providermanager.cpp\n#\n# Untracked files:\n# (use \"git add ...\" to include in what will be committed)\n#\n# ./\nroot /home/djszapi/Projects/kde/attica/build # gc\nlib/achievement.cpp: not added yet\nlib/achievement.h: not added yet\nlib/achievementparser.cpp: not added yet\nlib/achievementparser.h: not added yet\nerror: Error building trees\nroot /home/djszapi/Projects/kde/attica/build # alias gc\nalias gc='git commit --author=\"Laszlo Papp \"'\nroot /home/djszapi/Projects/kde/attica/build\n"},{"id":"164959","messageId":"7vhbagh3aw.fsf@alter.siamese.dyndns.org","threadId":"26966","inReplyTo":"AANLkTikHRM10p9e8XCzuiih7cYtQRG7Az2Qn5qtPzhZJ@mail.gmail.com","subject":"Re* git commit fails under some circumstances","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-04-02T19:16:23Z","receivedAt":"2011-04-02T19:16:23Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Laszlo Papp writes:\n\n> There are 2 bugs and 1 wish in this case:\n> 1) It says \"Changes to be committed:\", but they are actually not\n> committed, there cannot even be committed since they were not added\n> \"properly\". This output is bogus\n> 2) error: Error building trees\n\nThanks.\n\nYou gave us a concise reproduction recipe and the output you get from it,\nwhich is far better than what an average new user would do. But that is\nstill two out of three of what makes a good bug report.\n\nWhen you utter the word \"bug\" (or the word \"bogus\") to an existing\nproject, please make it a habit to mention what you think should happen as\nwell. This is not limited to git but in any open source project you are\nnot familiar with in general.\n\nIt helps tremendously to have a reproduction recipe and its output to\ndiagnose certain kinds of issues. If somebody else tries to run it, and\nif it gave results different from the output you are getting, you may have\nuncovered some unintended environment dependency bug.\n\nBut what if your output matches what the original developers expected to\nsee? Possibly after spending many hours to design how the thing ought to\nbehave, it is possible that they still might not have thought about a\nbetter behaviour in some cases, and you may have an idea to present the\nresult in a more useful way---and your complaint might be that the current\noutput does not match that idea of yours. In such a case, a bug report\nthat does not state what should be shown stops short of being useful.\n\nWhen running \"commit\" and \"status\" with files marked with \"intent to add\",\nI think there are three possible interpretations of what the user wants to\ndo.\n\n (1) I earlier said \"I'll decide the exact contents to be committed for\n these paths and tell you by running 'git add' later.\" when I said\n 'git add -N'. But I forgot to do so before running \"git commit\".\n Thanks for catching this mistake and erroring out.\n\n (2) I said \"I'll decide the exact content to be committed ... later.\"\n when I said 'git add -N'. I am running \"git commit\" now, but I still\n don't know what the final contents for this path should be. I\n changed my mind, and I do not want to include the addition of these\n paths in the commit I am making. Please do not error out, but just\n ignore the earlier 'add -N' for now.\n\n (3) I said \"I'll decide the exact content to be committed ... later.\"\n when I said 'git add -N'. I am running \"git commit\" now, without\n explicitly telling you with 'git add' about the final contents for\n these paths. Please take it as an implicit hint that I am happy with\n the contents in the working tree and commit them as if I said 'git\n add' on these paths, but do leave modifications to already tracked\n paths that I haven't added with 'git add'.\n\nThe current behaviour of \"git commit\" that refuse to commit without\ntelling git what the final contents for these pathse is a very deliberate\ndesign and implementation of (1).\n\nThe second one is a possible interpretation, but it is a very error prone\none. People who want to commit without these new files wouldn't have\nmarked them with \"add -N\" in the first place, and (2) feels more like\ndisabling a useful error checking than making the command useful. Also it\nis a silent bug to behave like (2) to people who expect (3), as they will\nonly later find out that some of the additions were not committed against\ntheir will.\n\nThe third one also is a possible interpretation, but it is a very\ninconsistent one. Why should new paths marked with 'git add -N' be\ntreated differently from paths that were already tracked and you made\nchanges to the working tree? In addition, it is a silent bug to behave\nlike (3) to people who expect (2), as they will only later find out that\nsome of the additions were committed against their will.\n\nNote that the user who may want to see variants of (2) and (3) can still\nname which paths to include (and to exclude by not naming them):\n\n $ edit foo bar baz\n $ git add baz\n $ git add -N foo bar\n $ git commit baz foo\n\nor include everything\n\n $ git commit -a\n\n\nI thought the above issues through when I wrote \"git add -N\" and\ndetermined that (1) is the only sane and useful behaviour, and I still\nthink it is.\n\nHaving said all that, we might want to differentiate these paths in the\noutput from 'status'. The 'status' command in its current form and\nsemantics came much later (v1.7.0) than \"add -N\" (introduced in v1.6.1),\nand because \"add -N\" is such a low-key corner case (adding new paths is\nfar less frequently done than modifying an existing paths, and showing\nonly intent to add new paths is even less frequently done), we probably\ndidn't think about special casing them.\n\nCurrently we show these paths as both added for commit (i.e. appears in\nthe top portion of the verbose output, and in the first column of the -s\noutput) and having local modification (i.e. appears in the second part or\nin the second column, respectively). For example, here is what I get:\n\n $ cp COPYING RENAMING\n $ git add -N RENAMING\n $ git status -s\n AM RENAMING\n $ git status\n # ... to be committed ...\n # new file: RENAMING\n # ... have local changes ...\n # modified: RENAMING\n\nWe should be able to say something like:\n\n $ git status -s\n IM RENAMING\n $ git status\n # ... to be committed ...\n # new file: RENAMING (needs 'git add')\n # ... have local changes ...\n # modified: RENAMING\n\nif we don't care about the backward compatibility.\n\n> 3) It would be nice to have one command (with no (git) alias for sure)\n> to see the difference like \"git diff\" but also the new files.\n\nDoesn't \"git diff\" show the difference for files you told git about via\nthe \"add -N\" interface? After the above \"addition\" of RENAMING, if I ask\n\"git diff\" (or \"git diff HEAD\"), I get what I expect to see (addition of\nthe contents taken from the whole file in the working tree).\n\nAgain, please describe what you think should be the right output if it\ndiffers from your expectation.\n\nAlso having said that, I notice that \"git diff --cached\" behaves as if an\nempty blob is added in such a case. I am not sure if we want to special\ncase this. After all paths marked with \"git add -N\" does _not_ have a\nconcrete contents in the index by definition (as the user told \"I'll tell\nyou later\" but hasn't done so), and may want to behave more like unmerged\nentries for certain operations (i.e. the path does exist, but comparing\nsomething with it does not have a usual meaning, etc.)\n"},{"id":"165172","messageId":"1301991295.2320.11.camel@wpalmer.simply-domain","threadId":"26966","inReplyTo":"7vhbagh3aw.fsf@alter.siamese.dyndns.org","subject":"Re: Re* git commit fails under some circumstances","fromName":"Will Palmer","fromEmail":"wmpalmer@gmail.com","sentAt":"2011-04-05T08:14:55Z","receivedAt":"2011-04-05T08:14:55Z","isPatch":false,"sender":{"key":"wmpalmer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/357044?v=4"},"body":"On Sat, 2011-04-02 at 12:16 -0700, Junio C Hamano wrote:\n> Laszlo Papp writes:\n> > 3) It would be nice to have one command (with no (git) alias for sure)\n> > to see the difference like \"git diff\" but also the new files.\n> \n> Doesn't \"git diff\" show the difference for files you told git about via\n> the \"add -N\" interface? After the above \"addition\" of RENAMING, if I ask\n> \"git diff\" (or \"git diff HEAD\"), I get what I expect to see (addition of\n> the contents taken from the whole file in the working tree).\n> \n> Again, please describe what you think should be the right output if it\n> differs from your expectation.\n> \n\nTo give some context: the problem here is that \"git add -N\" was being\nused as a glorified way of saying \"show the content of a file I have\njust added in \"git diff\" output along with unstaged content\", as is\ndescribed in the manual for \"git add\" as one of the purposes of -N. All\nthe other problems are somewhat invented issues stemming from this one.\nThat is, a result of blindly following instructions to \"try git add -N\nif you want to see the output in diff\", without the implications having\nbeen explained first.\nThe alternative, \"git add everything else, then use git diff --cached\" I\nbelieve is unsuitable because the goal is to have \"git diff\" 'just work'\nin future runs, without the need for additional commands being run.\nHonestly I do not quite understand the exact use-case.\n\nAn option such as --treat-new-as-unstaged might solve this better, but\nof course suffers from the problem of having a terrible name. I greatly\nsuspect that the name being terrible is a sign of the idea not being\nfleshed-out enough yet. There are also various cases involving renames\nwhere it would not be clear at all how to handle things.\n\n-- Will\n\n> Also having said that, I notice that \"git diff --cached\" behaves as if an\n> empty blob is added in such a case. I am not sure if we want to special\n> case this. After all paths marked with \"git add -N\" does _not_ have a\n> concrete contents in the index by definition (as the user told \"I'll tell\n> you later\" but hasn't done so), and may want to behave more like unmerged\n> entries for certain operations (i.e. the path does exist, but comparing\n> something with it does not have a usual meaning, etc.)\n> \n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n"},{"id":"165174","messageId":"7v8vvp5cxn.fsf@alter.siamese.dyndns.org","threadId":"26966","inReplyTo":"1301991295.2320.11.camel@wpalmer.simply-domain","subject":"Re: Re* git commit fails under some circumstances","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-04-05T08:18:12Z","receivedAt":"2011-04-05T08:18:12Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Will Palmer writes:\n\n> The alternative, \"git add everything else, then use git diff --cached\" I\n> believe is unsuitable\n\nIt sounds as if you are looking for \"git diff HEAD\" to me...\n"},{"id":"165175","messageId":"1301991804.2320.13.camel@wpalmer.simply-domain","threadId":"26966","inReplyTo":"7v8vvp5cxn.fsf@alter.siamese.dyndns.org","subject":"Re: Re* git commit fails under some circumstances","fromName":"Will Palmer","fromEmail":"wmpalmer@gmail.com","sentAt":"2011-04-05T08:23:24Z","receivedAt":"2011-04-05T08:23:24Z","isPatch":false,"sender":{"key":"wmpalmer@gmail.com","avatar":"https://avatars.githubusercontent.com/u/357044?v=4"},"body":"On Tue, 2011-04-05 at 01:18 -0700, Junio C Hamano wrote:\n> Will Palmer writes:\n> \n> > The alternative, \"git add everything else, then use git diff --cached\" I\n> > believe is unsuitable\n> \n> It sounds as if you are looking for \"git diff HEAD\" to me...\n> \n\nQuite possible another case of \"the mindset of someone used to using the\nindex, and so always wanting to reference it, clashing with the mindset\nof someone who never uses the index, and so doesn't even consider it\".\nQuite possible this is indeed the solution that was needed.\n"},{"id":"165190","messageId":"20110405173603.GD9965@sigill.intra.peff.net","threadId":"26966","inReplyTo":"7vhbagh3aw.fsf@alter.siamese.dyndns.org","subject":"Re: Re* git commit fails under some circumstances","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-04-05T17:36:04Z","receivedAt":"2011-04-05T17:36:04Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Apr 02, 2011 at 12:16:23PM -0700, Junio C Hamano wrote:\n\n> When running \"commit\" and \"status\" with files marked with \"intent to add\",\n> I think there are three possible interpretations of what the user wants to\n> do.\n> \n> (1) I earlier said \"I'll decide the exact contents to be committed for\n> these paths and tell you by running 'git add' later.\" when I said\n> 'git add -N'. But I forgot to do so before running \"git commit\".\n> Thanks for catching this mistake and erroring out.\n> \n> (2) I said \"I'll decide the exact content to be committed ... later.\"\n> when I said 'git add -N'. I am running \"git commit\" now, but I still\n> don't know what the final contents for this path should be. I\n> changed my mind, and I do not want to include the addition of these\n> paths in the commit I am making. Please do not error out, but just\n> ignore the earlier 'add -N' for now.\n> \n> (3) I said \"I'll decide the exact content to be committed ... later.\"\n> when I said 'git add -N'. I am running \"git commit\" now, without\n> explicitly telling you with 'git add' about the final contents for\n> these paths. Please take it as an implicit hint that I am happy with\n> the contents in the working tree and commit them as if I said 'git\n> add' on these paths, but do leave modifications to already tracked\n> paths that I haven't added with 'git add'.\n> \n> The current behaviour of \"git commit\" that refuse to commit without\n> telling git what the final contents for these pathse is a very deliberate\n> design and implementation of (1).\n\nI think that all three of those are reasonable things to want to do, and\nthat the current behavior of (1) is a nice, conservative default. But\nwhere we could do better is in helping the user understand what happened\nand what their options are.\n\nIn Laszlo's example, he saw:\n\n $ git commit\n lib/achievement.cpp: not added yet\n lib/achievement.h: not added yet\n lib/achievementparser.cpp: not added yet\n lib/achievementparser.h: not added yet\n error: Error building trees\n\nI see a couple of places to improve:\n\n 1. We say \"not added yet\", which is a good start. We at least point\n out the problematic paths. But it's perhaps a little confusing.\n Those paths _have_ been added. It's just that no content was added\n at them yet.\n\n 2. The \"error: Error building trees\" is unnecessarily scary, as it\n looks like git ran into some error while trying to do what you\n wanted (and indeed, that is the same message you get for something\n like \"oops, we couldn't write to the object db\"). It's not even\n clear from that output that the \"not added yet\" lines are the cause\n of the error, unless you understand how \"git commit\" is\n implemented.\n\n 3. There is no advice given on how to proceed.\n\nSo I think a much nicer output would be something like:\n\n $ git commit\n error: some paths could not be committed\n The following paths were marked with intent-to-add, but have\n not had any content added:\n\n lib/achievement.cpp\n lib/achievement.h\n lib/achievementparser.cpp\n lib/achievementparser.h\n\n If you want to commit them with their current content, use \"git add\".\n If you want git to forget about them, use \"git rm --cached\".\n\nWe could also provide some options to git commit to make those\noperations easier. Especially \"ignore these for this commit but leave\nthem in the index\" is a little awkward to do.\n\n> Having said all that, we might want to differentiate these paths in the\n> output from 'status'.\n\nYes, that was my first thought on reading Laszlo's message. We are\nsomewhat lying to say \"changes to be committed\". Your suggestion:\n\n> # ... to be committed ...\n> # new file: RENAMING (needs 'git add')\n> # ... have local changes ...\n> # modified: RENAMING\n\nYou could also give them a separate stanza, like:\n\n # Changes to be committed:\n # (use \"git reset HEAD ...\" to unsage)\n #\n # modified: some-other-file\n #\n # Paths marked for addition, but needing content:\n # (use \"git add ...\" to update what will be committed)\n #\n # new file: RENAMING\n\n> if we don't care about the backward compatibility.\n\nI don't know how much we need to care about \"git status\" output of this\nform. It is parsed mainly be editors' syntax highlighters. Adding this\nnew information might not get highlighted as nicely, but it's probably\nnot a big deal.\n\nI am much more concerned with whether and how this information would be\nrepresented in the \"git status --porcelain\" format.\n\n-Peff\n"},{"id":"165291","messageId":"7v1v1fjnsu.fsf@alter.siamese.dyndns.org","threadId":"26966","inReplyTo":"20110405173603.GD9965@sigill.intra.peff.net","subject":"Re: Re* git commit fails under some circumstances","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-04-06T17:24:17Z","receivedAt":"2011-04-06T17:24:17Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I am much more concerned with whether and how this information would be\n> represented in the \"git status --porcelain\" format.\n\nI earlier suggested using 'I'; a safer alternative would be to change\nnothing and let the callers figure it out. I slightly favor the former;\nwhile there is a definite risk of breaking scripts' expectations, they can\nbe tentatively marked \"this script does not work until you 'git add' paths\nyou used 'add -N'\" and I don't think it would be such a big deal.\n"},{"id":"165293","messageId":"20110406175700.GB8205@sigill.intra.peff.net","threadId":"26966","inReplyTo":"7v1v1fjnsu.fsf@alter.siamese.dyndns.org","subject":"Re: Re* git commit fails under some circumstances","fromName":"Jeff King","fromEmail":"peff@github.com","sentAt":"2011-04-06T17:57:01Z","receivedAt":"2011-04-06T17:57:01Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Apr 06, 2011 at 10:24:17AM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > I am much more concerned with whether and how this information would be\n> > represented in the \"git status --porcelain\" format.\n> \n> I earlier suggested using 'I'; a safer alternative would be to change\n> nothing and let the callers figure it out.\n\nAh, sorry, I missed that.\n\n> I slightly favor the former; while there is a definite risk of\n> breaking scripts' expectations, they can be tentatively marked \"this\n> script does not work until you 'git add' paths you used 'add -N'\" and\n> I don't think it would be such a big deal.\n\nI think that is reasonable. Maintaining backwards compatibility doesn't\nmean we can _never_ expand to cover new situations or give more\ninformation. In this case we would be maintaining syntactic\ncompatibility, and have identical output when the \"add -N\" feature is\nnot used.\n\n-Peff\n"}]}