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

From: Taylor Blau <[email-masked]>
Date: Thu, 7 Aug 2025 18:12:59 -0400

On Thu, Aug 07, 2025 at 10:09:52AM +0200, Patrick Steinhardt wrote:
> 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 <[email-masked]>
> ---
>  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