commit 04a0e98515a7b6e2e43eddea239aacd2a5a71632
author Jeff King <peff@peff.net> Wed Aug 26 16:13:05 2020 -0400
committer Junio C Hamano <gitster@pobox.com> Wed Aug 26 13:30:08 2020 -0700
tree 9b198b1e7d51de576dafc99a2aa432cf37b74f79
parent 47ae905ffb98cc4d4fd90083da6bc8dab55d9ecc

revision: set rev_input_given in handle_revision_arg()

Commit 7ba826290a (revision: add rev_input_given flag, 2017-08-02) added
a flag to rev_info to tell whether we got any revision arguments. As
explained there, this is necessary because some revision arguments may
not produce any pending traversal objects, but should still inhibit
default behaviors (e.g., a glob that matches nothing).

However, it only set the flag in the globbing code, but not for
revisions we get on the command-line or via stdin. This leads to two
problems:

  - the command-line code keeps its own separate got_rev_arg flag; this
    isn't wrong, but it's confusing and an extra maintenance burden

  - even specifically-named rev arguments might end up not adding any
    pending objects: if --ignore-missing is set, then specifying a
    missing object is a noop rather than an error.

And that leads to some user-visible bugs:

  - when deciding whether a default rev like "HEAD" should kick in, we
    check both got_rev_arg and rev_input_given. That means that
    "--ignore-missing $ZERO_OID" works on the command-line (where we set
    got_rev_arg) but not on --stdin (where we don't)

  - when rev-list decides whether it should complain that it wasn't
    given a starting point, it relies on rev_input_given. So it can't
    even get the command-line "--ignore-missing $ZERO_OID" right

Let's consistently set the flag if we got any revision argument. That
lets us clean up the redundant got_rev_arg, and fixes both of those bugs
(but note there are three new tests: we'll confirm the already working
git-log command-line case).

A few implementation notes:

  - conceptually we want to set the flag whenever handle_revision_arg()
    finds an actual revision arg ("handles" it, you might say). But it
    covers a ton of cases with early returns. Rather than annotating
    each one, we just wrap it and use its success exit-code to set the
    flag in one spot.

  - the new rev-list test is in t6018, which is titled to cover globs.
    This isn't exactly a glob, but it made sense to stick it with the
    other tests that handle the "even though we got a rev, we have no
    pending objects" case, which are globs.

  - the tests check for the oid of a missing object, which it's pretty
    clear --ignore-missing should ignore. You can see the same behavior
    with "--ignore-missing a-ref-that-does-not-exist", because
    --ignore-missing treats them both the same. That's perhaps less
    clearly correct, and we may want to change that in the future. But
    the way the code and tests here are written, we'd continue to do the
    right thing even if it does.

Reported-by: Bryan Turner <bturner@atlassian.com>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>