Re: [PATCH 1/2] replay: die descriptively when invalid commit-ish

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 23, 2025, 13:41 UTC
Message-ID: <xmqqpl85pb7k.fsf@gitster.g>
In-Reply-To: <a5822145-5a5c-4a2b-84b9-7c061b3bab97@gmail.com>

Phillip Wood <phillip.wood123@gmail.com> writes:

Show 6 quoted lines

> There are only two callers so I think that is a good idea. If you give 
> an invalid commit name to "--advance" then it dies with
>
>      fatal: argument to --advance must be a reference
>
> so arguably we only need to check the return value when parsing "--onto"

So, in determine_replay_mode(), ...

	if (onto_name) {
		*onto = peel_committish(repo, onto_name);

... here is where we must see *onto is NULL and barf and then ...

		if (rinfo.positive_refexprs <
		    strset_get_size(&rinfo.positive_refs))
			die(_("all positive revisions given must be references"));
	} else if (*advance_name) {
		struct object_id oid;
		char *fullname = NULL;

		*onto = peel_committish(repo, *advance_name);

... for symmetry, we would want to do the same.

In addition, we probably would want to let the following code that uses the same *advance_name (and requires that it just not names a commit-ish object, but is actually a ref, which is a different requirement that is probably a bit tighter) ...

		if (repo_dwim_ref(repo, *advance_name, strlen(*advance_name),
			     &oid, &fullname, 0) == 1) {
			free(*advance_name);
			*advance_name = fullname;
		} else {
			die(_("argument to --advance must be a reference"));
		}

... first, and then compute *onto after that by moving code a bit, perhaps?

Thanks.