Re: Fatal bug on revert with --author

From: Jeff King <peff@peff.net>
Date: May 30, 2016, 18:55 UTC
Message-ID: <20160530185529.GA18074@sigill.intra.peff.net>
In-Reply-To: <CAKYJjwgkH5gtQHsV_=4O0SGqb6GEXWC=rdFWO9Jv36dL-NaPcw@mail.gmail.com>

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

Show 10 quoted lines

> 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.

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