Fatal bug on revert with --author

So I learned today that --author is not a supported argument to git revert. It took me a long time to realize this, though, because the error I got was very cryptic:

  fatal: BUG: expected exactly one commit from walk

Here's a very simple reproducible case: https://gist.github.com/anlutro/67e0cec1a6a419e6d44131b0bc1deff6

I was recommended to send this report here by #git on irc.freenode.net.

Thanks, Andreas

On Mon, May 30, 2016 at 04:16:50PM +0200, Andreas Lutro wrote:

Hmm. It _is_ a supported command-line argument, as you can pass arbitrary revision options to revert. So:

  git revert --author peff HEAD

should revert all of my commits (whether that is _useful_, I am not sure, but it is a consequence of the fact that revert simply passes its arguments to the regular revision machinery).

But in your example, you pass the author "test", which matches nothing. And so we end up with nothing to revert. But rather than saying so, we fall into a backwards-compatibility code path:

        /*
         * If we were called as "git cherry-pick <commit>", just
         * cherry-pick/revert it, set CHERRY_PICK_HEAD /
         * REVERT_HEAD, and don't touch the sequencer state.
         * This means it is possible to cherry-pick in the middle
         * of a cherry-pick sequence.
         */
        if (opts->revs->cmdline.nr == 1 &&
            opts->revs->cmdline.rev->whence == REV_CMD_REV &&
            opts->revs->no_walk &&
            !opts->revs->cmdline.rev->flags) {
                struct commit *cmit;
                if (prepare_revision_walk(opts->revs))
                        die(_("revision walk setup failed"));
                cmit = get_revision(opts->revs);
                if (!cmit || get_revision(opts->revs))
                        die("BUG: expected exactly one commit from walk");
                return single_pick(cmit, opts);
        }

I think this conditional should not be triggering, because even though we did receive a single argument, we _also_ got options which imply that the user expected us to walk and find other commits.

-Peff