Re: [PATCH v3 8/8] commit-graph: drop COMMIT_GRAPH_WRITE_CHECK_OIDS flag

From: Jeff King <peff@peff.net>
Date: May 14, 2020, 18:09 UTC
Message-ID: <20200514180947.GC2430834@coredump.intra.peff.net>
In-Reply-To: <3cb0bd306cbb971715f84db07a8d1acac658f2d0.1589407015.git.me@ttaylorr.com>

On Wed, May 13, 2020 at 03:59:55PM -0600, Taylor Blau wrote:

Show 9 quoted lines

> -	result = lookup_commit_reference_gently(the_repository, &oid, 1);
> -	if (result)
> -		oidset_insert(commits, &result->object.oid);
> -	else
> -		return error(_("invalid commit object id: %s"), hash);
> +	result = deref_tag(the_repository, parse_object(the_repository, &oid),
> +			   NULL, 0);
> +	if (!result)
> +		return error(_("invalid object: %s"), hash);

OK. As you noted earlier, this is really "we don't have that object" _or_ "we don't have an object it points to". But since either is a corruption, and I'd expect parse_object() to produce a more detailed message anyway, I don't think it's worth trying to get more specific here.

> +	else if (object_as_type(the_repository, result, OBJ_COMMIT, 1))
> +		oidset_insert(commits, &result->oid);

I suspect this could just be "if (result->type == OBJ_COMMIT)", as we'd never see OBJ_NONE from a tag deref (which would have required the casting behavior), but I don't think it hurts to use this function to be on the safe side.

-Peff