All of lore.kernel.org
 help / color / mirror / Atom feed
From: Taylor Blau <me@ttaylorr.com>
To: Patrick Steinhardt <ps@pks.im>
Cc: git@vger.kernel.org, Toon Claes <toon@iotcl.com>
Subject: Re: [PATCH v2 2/9] odb: allow `odb_find_source()` to fail
Date: Thu, 7 Aug 2025 18:12:59 -0400	[thread overview]
Message-ID: <aJUk68fgQttR2gMe@nand.local> (raw)
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:
> 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

  reply	other threads:[~2025-08-07 22:13 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-29 14:12 [PATCH 0/8] midx: stop deduplicating info redundant with their sources Patrick Steinhardt
2025-07-29 14:12 ` [PATCH 1/8] odb: store locality in object database sources Patrick Steinhardt
2025-08-06 16:39   ` Toon Claes
2025-08-07  6:15     ` Patrick Steinhardt
2025-08-07  8:12   ` Karthik Nayak
2025-07-29 14:12 ` [PATCH 2/8] odb: allow `odb_find_source()` to fail Patrick Steinhardt
2025-07-29 14:12 ` [PATCH 3/8] odb: return newly created in-memory sources Patrick Steinhardt
2025-08-06 16:40   ` Toon Claes
2025-08-07  6:16     ` Patrick Steinhardt
2025-08-07  8:21   ` Karthik Nayak
2025-07-29 14:12 ` [PATCH 4/8] midx: drop redundant `struct repository` parameter Patrick Steinhardt
2025-07-29 14:12 ` [PATCH 5/8] midx: load multi-pack indices via their source Patrick Steinhardt
2025-08-07  8:49   ` Karthik Nayak
2025-08-07  8:51     ` Karthik Nayak
2025-07-29 14:12 ` [PATCH 6/8] midx: write " Patrick Steinhardt
2025-08-07  8:55   ` Karthik Nayak
2025-07-29 14:12 ` [PATCH 7/8] midx: stop duplicating info redundant with its owning source Patrick Steinhardt
2025-07-29 14:12 ` [PATCH 8/8] midx: compute paths via their source Patrick Steinhardt
2025-07-29 18:33 ` [PATCH 0/8] midx: stop deduplicating info redundant with their sources Junio C Hamano
2025-07-30  5:21   ` Patrick Steinhardt
2025-08-07  8:09 ` [PATCH v2 0/9] midx: stop duplicating " Patrick Steinhardt
2025-08-07  8:09   ` [PATCH v2 1/9] odb: store locality in object database sources Patrick Steinhardt
2025-08-07 22:10     ` Taylor Blau
2025-08-07  8:09   ` [PATCH v2 2/9] odb: allow `odb_find_source()` to fail Patrick Steinhardt
2025-08-07 22:12     ` Taylor Blau [this message]
2025-08-11 11:56       ` Patrick Steinhardt
2025-08-07  8:09   ` [PATCH v2 3/9] odb: return newly created in-memory sources Patrick Steinhardt
2025-08-07 22:16     ` Taylor Blau
2025-08-11 11:56       ` Patrick Steinhardt
2025-08-07  8:09   ` [PATCH v2 4/9] odb: simplify calling `link_alt_odb_entry()` Patrick Steinhardt
2025-08-07 22:21     ` Taylor Blau
2025-08-07  8:09   ` [PATCH v2 5/9] midx: drop redundant `struct repository` parameter Patrick Steinhardt
2025-08-07  8:09   ` [PATCH v2 6/9] midx: load multi-pack indices via their source Patrick Steinhardt
2025-08-07 22:25     ` Taylor Blau
2025-08-07  8:09   ` [PATCH v2 7/9] midx: write " Patrick Steinhardt
2025-08-07 22:25     ` Taylor Blau
2025-08-07  8:09   ` [PATCH v2 8/9] midx: stop duplicating info redundant with its owning source Patrick Steinhardt
2025-08-07  8:09   ` [PATCH v2 9/9] midx: compute paths via their source Patrick Steinhardt
2025-08-07 22:27   ` [PATCH v2 0/9] midx: stop duplicating info redundant with their sources Taylor Blau
2025-08-11 11:56     ` Patrick Steinhardt
2025-08-07  8:58 ` [PATCH 0/8] midx: stop deduplicating " Karthik Nayak
2025-08-11 13:46 ` [PATCH v3 00/10] midx: stop duplicating " Patrick Steinhardt
2025-08-11 13:46   ` [PATCH v3 01/10] odb: store locality in object database sources Patrick Steinhardt
2025-08-11 13:46   ` [PATCH v3 02/10] odb: allow `odb_find_source()` to fail Patrick Steinhardt
2025-08-11 13:46   ` [PATCH v3 03/10] odb: consistently use "dir" to refer to alternate's directory Patrick Steinhardt
2025-08-11 13:46   ` [PATCH v3 04/10] odb: return newly created in-memory sources Patrick Steinhardt
2025-08-11 13:46   ` [PATCH v3 05/10] odb: simplify calling `link_alt_odb_entry()` Patrick Steinhardt
2025-08-11 13:46   ` [PATCH v3 06/10] midx: drop redundant `struct repository` parameter Patrick Steinhardt
2025-08-11 13:46   ` [PATCH v3 07/10] midx: load multi-pack indices via their source Patrick Steinhardt
2025-08-11 13:46   ` [PATCH v3 08/10] midx: write " Patrick Steinhardt
2025-08-11 13:46   ` [PATCH v3 09/10] midx: stop duplicating info redundant with its owning source Patrick Steinhardt
2025-08-11 13:46   ` [PATCH v3 10/10] midx: compute paths via their source Patrick Steinhardt
2025-08-28 22:46   ` [PATCH v3 00/10] midx: stop duplicating info redundant with their sources Junio C Hamano
2025-08-29  0:34     ` Taylor Blau
2025-08-30 13:39       ` Derrick Stolee
2025-09-02  6:36         ` Patrick Steinhardt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aJUk68fgQttR2gMe@nand.local \
    --to=me@ttaylorr.com \
    --cc=git@vger.kernel.org \
    --cc=ps@pks.im \
    --cc=toon@iotcl.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.