[BUG (maybe)] git rev-parse --verify --quiet isn't quiet

Hi,

I noticed this while writing a small script for myself:

    $ git rev-parse --verify --quiet '@{u}'
    fatal: No upstream configured for branch 'local'

The functions that get run when rev-parse does its thing all return a pointer to a strbuf, return the length of the modified buf or something to that effect. However get_upstream_branch() calls die() which writes to stderr unconditionally.

Call hierarchy in the above case: 

die()
  get_upstream_branch()
    interpret_upstream_mark()
      interpret_branch_name()
        substitute_branch_name()
          dwim_ref()
            get_sha1_basic()
              get_sha1_1()
                get_sha1_with_context_1()
                  get_sha1_with_context()
                    get_sha1()
                      cmd_rev_parse()

Passing quiet = 1 all the way down I assume isn't feasible. Changing get_upstream_branch() to not die but rather signify an error could work. But interpret_branch_name() has four different callers belonging to different parts of Git.

I marked this as "maybe" because I guess it's fair to say that rev-parse '@{u}' is not the correct way to check whether a branch as a configured upstream. Furthermore even if it writes to stderr the exit status is still non-zero so the only way to get bitten by this is if you doing something like:

    foo=$(git rev-parse --verify --quiet $bar 2>&1) # pointless redirection

*and* you check whether the exit code is equal to 1, not unequal to zero.

On the other hand, it does break the expectations that the documentation and the other uses of git rev-parse one encounters give. At least for me. It should be able to verify that $foo is valid, and it should do so quietly.

On a related note:

    $ git branch origin/master
    $ git rev-parse --verify --quiet origin/master
    warning: refname 'origin/master' is ambiguous.
    7c07808349fd0fc2c61a169833eeb55163cf3df4   

Best regards, Øsse

Øystein Walle <oystwa@gmail.com> writes:

I would suspect that this may be fine.

"rev-parse --verify" makes sure the named object exists, but in this case @{u} does not even name any object, does it?

And this warning is in the same vein, I would suspect.

Junio C Hamano <gitster@pobox.com> writes:

Hmph, but "rev-parse --verify no-such-branch" does *not* name any object, we would want to see it barf, and we probably would want to be able to squelch the message. So it is unclear if @{u} barfing is a good idea.

What is the reason why it is inpractical to pass 'quiet' down the callchain?

Junio C Hamano <gitster <at> pobox.com> writes:

That was my counter-argument :) The "vibe" I get from rev-parse --verify --quiet is that it should handle anything.

Maybe I'm missing something obvious, but wouldn't that mean changing the signature of 9 different functions, and consequently all of their calls throughout Git?

That's perhaps not a good argument. Who cares whether a diff is small or large if it fixes a bug properly? But most (or all) of those functions do not concern themselves with printing stuff so maybe an additional "quiet?" argument would look foreign in most places and make the code harder to read.

Øsse