Re: [PATCH v2 2/9] odb: allow `odb_find_source()` to fail

From: Taylor Blau <me@ttaylorr.com>
Date: Aug 7, 2025, 22:12 UTC
Message-ID: <aJUk68fgQttR2gMe@nand.local>
In-Reply-To: <20250807-b4-pks-midx-deduplicate-source-info-v2-2-bcffb8fc119c@pks.im>

On Thu, Aug 07, 2025 at 10:09:52AM +0200, Patrick Steinhardt wrote:

Show 25 quoted lines

> When trying to locate a source for an unknown object directory we will
> die right away. In subsequent patches we will add new callsites though
> that want to handle this situation gracefully instead.
>
> Refactor the function to return a `NULL` pointer if the source could not
> be found and adapt the callsites to die instead.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  builtin/commit-graph.c | 4 ++++
>  midx-write.c           | 2 ++
>  odb.c                  | 2 --
>  odb.h                  | 4 ++--
>  4 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
> index 25018a0b9d..dc2c1a5432 100644
> --- a/builtin/commit-graph.c
> +++ b/builtin/commit-graph.c
> @@ -102,6 +102,8 @@ static int graph_verify(int argc, const char **argv, const char *prefix,
>  		flags |= COMMIT_GRAPH_WRITE_PROGRESS;
>
>  	source = odb_find_source(the_repository->objects, opts.obj_dir);
> +	if (!source)
> +		die(_("could not find object directory matching %s"), opts.obj_dir);

Makes sense, and I am glad that we are trending towards having fewer internal functions that assume the caller wants to die() on failure. I wonder if it might be worth having a odb_find_source_or_die() counterpart here such that callers don't have to repeat the "could not find object directory ..." message.

Perhaps something like:

    struct odb_source *odb_find_source_or_die(struct object_database *odb,
                                              const char *obj_dir)
    {
        struct odb_source *source = odb_find_source(odb, obj_dir);
        if (!source)
            die(_("could not find object directory matching %s"),
                obj_dir);
        return source;
    }

Of course, callers that want to use a different message or otherwise handle a missing source differently would still be able to do so by calling odb_find_source() directly.

Thanks, Taylor