{"thread":{"id":"37922","subject":"2.2.0-rc behavior changes (1/2)","startedAt":"2014-11-10T08:47:32Z","lastAt":"2014-11-10T10:43:37Z","messageCount":3,"participants":["Bryan Turner","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"251661","messageId":"CAGyf7-GxJ6XHjKqwktKqpo_mFuC_D3gzeOGNTdt4sweAnFqNRg@mail.gmail.com","threadId":"37922","inReplyTo":null,"subject":"2.2.0-rc behavior changes (1/2)","fromName":"Bryan Turner","fromEmail":"bturner@atlassian.com","sentAt":"2014-11-10T08:47:32Z","receivedAt":"2014-11-10T08:47:32Z","isPatch":false,"sender":{"key":"bturner@atlassian.com","avatar":"https://gravatar.com/avatar/16bcf3167981c1ef7c804e502642366d888a35b0d0b0a4ca01fdc442aa1acb1e?d=mp&s=160"},"body":"I've been running a test suite we use to verify Git behaviors across\nversions, and the 2.2.0 RCs (0 and 1 both) have a couple of small\nbehavioral differences. I'm sending them in separate e-mails just to\nmake the contents easier to grok.\n\nImportant: It's entirely possible neither of these is a _bug_; they\nmay both be intentional changes in behavior.\n\nFirst change: git update-ref -d /refs/heads/nonexistent\n<some-valid-sha1> now produces an error about ref locking that it\ndidn't produce before\n\nGit 2.1.x and prior produced this output:\nerror: unable to resolve reference refs/heads/nonexistent: No such\nfile or directory\n\nNow, in the 2.2.0 RCs, it says:\nerror: unable to resolve reference refs/heads/nonexistent: No such\nfile or directory\nerror: Cannot lock the ref 'refs/heads/nonexistent'.\n\nThis one feels more like a bug, but again may not be. I say it feels\nlike a bug because of the order of the messages: If git has decided\nthe ref doesn't exist, why is it still trying to lock it?\n\nThis change bisects to:\n\nbturner@felurian:~/Development/git/git$ git bisect bad\n7521cc4611a783f4a8174bd0fcec5f4a47357ac1 is the first bad commit\ncommit 7521cc4611a783f4a8174bd0fcec5f4a47357ac1\nAuthor: Ronnie Sahlberg <sahlberg@google.com>\nDate:   Wed Apr 30 09:22:45 2014 -0700\n\n    refs.c: make delete_ref use a transaction\n\nBest regards,\nBryan Turner\n"},{"id":"251663","messageId":"20141110092219.GA11387@peff.net","threadId":"37922","inReplyTo":"CAGyf7-GxJ6XHjKqwktKqpo_mFuC_D3gzeOGNTdt4sweAnFqNRg@mail.gmail.com","subject":"Re: 2.2.0-rc behavior changes (1/2)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-11-10T09:22:19Z","receivedAt":"2014-11-10T09:22:19Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 10, 2014 at 07:47:32PM +1100, Bryan Turner wrote:\n\n> First change: git update-ref -d /refs/heads/nonexistent\n> <some-valid-sha1> now produces an error about ref locking that it\n> didn't produce before\n> \n> Git 2.1.x and prior produced this output:\n> error: unable to resolve reference refs/heads/nonexistent: No such\n> file or directory\n> \n> Now, in the 2.2.0 RCs, it says:\n> error: unable to resolve reference refs/heads/nonexistent: No such\n> file or directory\n> error: Cannot lock the ref 'refs/heads/nonexistent'.\n> \n> This one feels more like a bug, but again may not be. I say it feels\n> like a bug because of the order of the messages: If git has decided\n> the ref doesn't exist, why is it still trying to lock it?\n\nI don't think this is a bug. The order you see is because the code goes\nsomething like this:\n\n  1. the parent function calls a sub-function to lock\n\n  2. the sub-function generates the error \"no such file or directory\"\n     and returns failure to the caller\n\n  3. the caller reports that acquiring the lock failed\n\nThe only thing that has changed between the two is step (3), but it is\nnot an extra lock action after the error. It is just a more verbose\nreport of the same error.\n\nThat being said, the sub-function (lock_ref_sha1_basic) gives a much\nmore useful message. So it would be a nice enhancement to make sure that\nit prints something useful in every return case, and then drop the\nmessage from the caller.\n\nAs an aside, I'm also slightly confused by your output. Are you feeding\n\"/refs/heads/nonexistent\" (with a leading slash), or\n\"refs/heads/nonexistent\" (no leading slash)? If the latter, then that\nshould silently succeed (and seems to in my tests). If the former, then\nthe real problem is not ENOENT, but rather EINVAL; that name is not a\nvalid refname.\n\nOlder versions of git would produce:\n\n  error: unable to resolve reference /refs/heads/nonexistent: No such file or directory\n\nwhich is like the error you showed, but note that the refname is\nreported with the leading slash. In v2.2.0-rc1, this is:\n\n  error: unable to resolve reference /refs/heads/nonexistent: Invalid argument\n  error: Cannot lock the ref '/refs/heads/nonexistent'.\n\nwhich is more accurate. I could explain the differences in our output\nfrom some simple transcription errors when writing your email, but I\nwanted to make sure I am not missing something.\n\n-Peff\n"},{"id":"251667","messageId":"CAGyf7-HSJfQtceWTQferNvxEMDLU1DEz6xf4kemCXyo72WuNRQ@mail.gmail.com","threadId":"37922","inReplyTo":"20141110092219.GA11387@peff.net","subject":"Re: 2.2.0-rc behavior changes (1/2)","fromName":"Bryan Turner","fromEmail":"bturner@atlassian.com","sentAt":"2014-11-10T10:43:37Z","receivedAt":"2014-11-10T10:43:37Z","isPatch":false,"sender":{"key":"bturner@atlassian.com","avatar":"https://gravatar.com/avatar/16bcf3167981c1ef7c804e502642366d888a35b0d0b0a4ca01fdc442aa1acb1e?d=mp&s=160"},"body":"On Mon, Nov 10, 2014 at 8:22 PM, Jeff King <peff@peff.net> wrote:\n> On Mon, Nov 10, 2014 at 07:47:32PM +1100, Bryan Turner wrote:\n>\n>> First change: git update-ref -d /refs/heads/nonexistent\n>> <some-valid-sha1> now produces an error about ref locking that it\n>> didn't produce before\n>>\n>> Git 2.1.x and prior produced this output:\n>> error: unable to resolve reference refs/heads/nonexistent: No such\n>> file or directory\n>>\n>> Now, in the 2.2.0 RCs, it says:\n>> error: unable to resolve reference refs/heads/nonexistent: No such\n>> file or directory\n>> error: Cannot lock the ref 'refs/heads/nonexistent'.\n>>\n>> This one feels more like a bug, but again may not be. I say it feels\n>> like a bug because of the order of the messages: If git has decided\n>> the ref doesn't exist, why is it still trying to lock it?\n>\n> I don't think this is a bug. The order you see is because the code goes\n> something like this:\n>\n>   1. the parent function calls a sub-function to lock\n>\n>   2. the sub-function generates the error \"no such file or directory\"\n>      and returns failure to the caller\n>\n>   3. the caller reports that acquiring the lock failed\n>\n> The only thing that has changed between the two is step (3), but it is\n> not an extra lock action after the error. It is just a more verbose\n> report of the same error.\n>\n> That being said, the sub-function (lock_ref_sha1_basic) gives a much\n> more useful message. So it would be a nice enhancement to make sure that\n> it prints something useful in every return case, and then drop the\n> message from the caller.\n>\n> As an aside, I'm also slightly confused by your output. Are you feeding\n> \"/refs/heads/nonexistent\" (with a leading slash), or\n> \"refs/heads/nonexistent\" (no leading slash)? If the latter, then that\n> should silently succeed (and seems to in my tests). If the former, then\n> the real problem is not ENOENT, but rather EINVAL; that name is not a\n> valid refname.\n>\n> Older versions of git would produce:\n>\n>   error: unable to resolve reference /refs/heads/nonexistent: No such file or directory\n>\n> which is like the error you showed, but note that the refname is\n> reported with the leading slash. In v2.2.0-rc1, this is:\n>\n>   error: unable to resolve reference /refs/heads/nonexistent: Invalid argument\n>   error: Cannot lock the ref '/refs/heads/nonexistent'.\n>\n> which is more accurate. I could explain the differences in our output\n> from some simple transcription errors when writing your email, but I\n> wanted to make sure I am not missing something.\n\nSorry, no, you're not missing anything. That is indeed a transcription\nerror from my e-mail. The test in question is using\n\"refs/heads/nonexistent\".\n\nThanks for the quick response, Jeff. With the sub-function the\nordering of the messages makes perfect sense.\n\n>\n> -Peff\n"}]}