{"thread":{"id":"37486","subject":"[BUG (maybe)] git rev-parse --verify --quiet isn't quiet","startedAt":"2014-09-04T11:20:45Z","lastAt":"2014-09-05T07:15:32Z","messageCount":4,"participants":["Øystein Walle","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"248830","messageId":"loom.20140904T131954-274@post.gmane.org","threadId":"37486","inReplyTo":null,"subject":"[BUG (maybe)] git rev-parse --verify --quiet isn't quiet","fromName":"Øystein Walle","fromEmail":"oystwa@gmail.com","sentAt":"2014-09-04T11:20:45Z","receivedAt":"2014-09-04T11:20:45Z","isPatch":false,"sender":{"key":"oystwa@gmail.com","avatar":"https://avatars.githubusercontent.com/u/794585?v=4"},"body":"Hi,\n\nI noticed this while writing a small script for myself:\n\n    $ git rev-parse --verify --quiet '@{u}'\n    fatal: No upstream configured for branch 'local'\n\nThe functions that get run when rev-parse does its thing all return a\npointer to a strbuf, return the length of the modified buf or something\nto that effect. However get_upstream_branch() calls die() which writes\nto stderr unconditionally.\n\nCall hierarchy in the above case: \n\ndie()\n  get_upstream_branch()\n    interpret_upstream_mark()\n      interpret_branch_name()\n        substitute_branch_name()\n          dwim_ref()\n            get_sha1_basic()\n              get_sha1_1()\n                get_sha1_with_context_1()\n                  get_sha1_with_context()\n                    get_sha1()\n                      cmd_rev_parse()\n\nPassing quiet = 1 all the way down I assume isn't feasible. Changing\nget_upstream_branch() to not die but rather signify an error could work.\nBut interpret_branch_name() has four different callers belonging to\ndifferent parts of Git.\n\nI marked this as \"maybe\" because I guess it's fair to say that rev-parse\n'@{u}' is not the correct way to check whether a branch as a configured\nupstream. Furthermore even if it writes to stderr the exit status is\nstill non-zero so the only way to get bitten by this is if you doing\nsomething like:\n\n    foo=$(git rev-parse --verify --quiet $bar 2>&1) # pointless redirection\n\n*and* you check whether the exit code is equal to 1, not unequal to zero.\n\nOn the other hand, it does break the expectations that the documentation\nand the other uses of git rev-parse one encounters give. At least for\nme. It should be able to verify that $foo is valid, and it should do so\nquietly.\n\nOn a related note:\n\n    $ git branch origin/master\n    $ git rev-parse --verify --quiet origin/master\n    warning: refname 'origin/master' is ambiguous.\n    7c07808349fd0fc2c61a169833eeb55163cf3df4   \n\n\nBest regards,\nØsse\n"},{"id":"248846","messageId":"xmqq1trruv3b.fsf@gitster.dls.corp.google.com","threadId":"37486","inReplyTo":"loom.20140904T131954-274@post.gmane.org","subject":"Re: [BUG (maybe)] git rev-parse --verify --quiet isn't quiet","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-09-04T17:57:44Z","receivedAt":"2014-09-04T17:57:44Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Øystein Walle <oystwa@gmail.com> writes:\n\n> I noticed this while writing a small script for myself:\n>\n>     $ git rev-parse --verify --quiet '@{u}'\n>     fatal: No upstream configured for branch 'local'\n>\n> The functions that get run when rev-parse does its thing all return a\n> pointer to a strbuf, return the length of the modified buf or something\n> to that effect. However get_upstream_branch() calls die() which writes\n> to stderr unconditionally.\n\nI would suspect that this may be fine.\n\n\"rev-parse --verify\" makes sure the named object exists, but in this\ncase @{u} does not even name any object, does it?\n\n> On a related note:\n>\n>     $ git branch origin/master\n>     $ git rev-parse --verify --quiet origin/master\n>     warning: refname 'origin/master' is ambiguous.\n>     7c07808349fd0fc2c61a169833eeb55163cf3df4   \n\nAnd this warning is in the same vein, I would suspect.\n"},{"id":"248848","messageId":"xmqqppfbtfu0.fsf@gitster.dls.corp.google.com","threadId":"37486","inReplyTo":"xmqq1trruv3b.fsf@gitster.dls.corp.google.com","subject":"Re: [BUG (maybe)] git rev-parse --verify --quiet isn't quiet","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-09-04T18:12:39Z","receivedAt":"2014-09-04T18:12:39Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> I would suspect that this may be fine.\n>\n> \"rev-parse --verify\" makes sure the named object exists, but in this\n> case @{u} does not even name any object, does it?\n\nHmph, but \"rev-parse --verify no-such-branch\" does *not* name any\nobject, we would want to see it barf, and we probably would want to\nbe able to squelch the message.  So it is unclear if @{u} barfing is\na good idea.\n\nWhat is the reason why it is inpractical to pass 'quiet' down the\ncallchain?\n"},{"id":"248891","messageId":"loom.20140905T085616-576@post.gmane.org","threadId":"37486","inReplyTo":"xmqqppfbtfu0.fsf@gitster.dls.corp.google.com","subject":"Re: [BUG (maybe)] git rev-parse --verify --quiet isn't quiet","fromName":"Øystein Walle","fromEmail":"oystwa@gmail.com","sentAt":"2014-09-05T07:15:32Z","receivedAt":"2014-09-05T07:15:32Z","isPatch":false,"sender":{"key":"oystwa@gmail.com","avatar":"https://avatars.githubusercontent.com/u/794585?v=4"},"body":"Junio C Hamano <gitster <at> pobox.com> writes:\n\n> \n> Junio C Hamano <gitster <at> pobox.com> writes:\n> \n> > I would suspect that this may be fine.\n> >\n> > \"rev-parse --verify\" makes sure the named object exists, but in this\n> > case  <at> {u} does not even name any object, does it?\n> \n> Hmph, but \"rev-parse --verify no-such-branch\" does *not* name any\n> object, we would want to see it barf, and we probably would want to\n> be able to squelch the message.  So it is unclear if  <at> {u} barfing is\n> a good idea.\n> \n\nThat was my counter-argument :) The \"vibe\" I get from rev-parse --verify\n--quiet is that it should handle anything.\n\n> \n> What is the reason why it is inpractical to pass 'quiet' down the\n> callchain?\n> \n\nMaybe I'm missing something obvious, but wouldn't that mean changing the\nsignature of 9 different functions, and consequently all of their calls\nthroughout Git? \n\nThat's perhaps not a good argument. Who cares whether a diff is small or\nlarge if it fixes a bug properly?  But most (or all) of those functions\ndo not concern themselves with printing stuff so maybe an additional\n\"quiet?\" argument would look foreign in most places and make the code\nharder to read.\n\nØsse\n"}]}