Re: git replace --graft does error checking too late

From: Christian Couder <christian.couder@gmail.com>
Date: Mar 27, 2019, 13:11 UTC
Message-ID: <CAP8UFD1qU_kJ97MdLFwzx+g3F6Q+fQ9LWOBxd=1m4vSi-fxF=Q@mail.gmail.com>
In-Reply-To: <mvmd0mcsjkf.fsf@suse.de>

On Wed, Mar 27, 2019 at 11:24 AM Andreas Schwab <schwab@suse.de> wrote:

>
> When running `git replace --graft A B' where B is a non-commit (eg. a
> tag) it displays an error,

Yeah, it seems that when A is a commit and B a tag I get:

"error: object A is a tag, not a commit"

which is wrong as A is a commit.

> but creates the replace ref anyway.  I think
> it should verify that B names a commit object before creating the ref.

Accepting a tag and using the commit the tag points to could be useful.

For example someone could look at the commit graph, then decide to tag the commit(s) that should be used when replacing, and then use `git replace --graft A <tag>...` using the created tag(s).

(I checked the code in builtin/replace.c and it seems that we use lookup_commit_reference() on each of the new parents, so we should be safe in case one of the given new parents cannot be peeled into a commit.)

So it seems to me that the issue is that it shows a wrong error when it shouldn't show anything, or perhaps only a warning.