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 3/9] odb: return newly created in-memory sources
Date: Thu, 7 Aug 2025 18:16:32 -0400 [thread overview]
Message-ID: <aJUlwLJyxSmD4EQF@nand.local> (raw)
In-Reply-To: <20250807-b4-pks-midx-deduplicate-source-info-v2-3-bcffb8fc119c@pks.im>
On Thu, Aug 07, 2025 at 10:09:53AM +0200, Patrick Steinhardt wrote:
> Callers have no trivial way to obtain the newly created object database
> source when adding it to the in-memory list of alternates. While not yet
> needed anywhere, a subsequent commit will want to obtain that pointer.
>
> Refactor the function to return the source to make it easily accessible.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> odb.c | 37 +++++++++++++++++++++----------------
> odb.h | 4 ++--
> 2 files changed, 23 insertions(+), 18 deletions(-)
>
> diff --git a/odb.c b/odb.c
> index 61104b7cb8..7793816f81 100644
> --- a/odb.c
> +++ b/odb.c
> @@ -139,23 +139,22 @@ static void read_info_alternates(struct object_database *odb,
> const char *relative_base,
> int depth);
>
> -static int link_alt_odb_entry(struct object_database *odb,
> - const struct strbuf *entry,
> - const char *relative_base,
> - int depth,
> - const char *normalized_objdir)
> +static struct odb_source *link_alt_odb_entry(struct object_database *odb,
> + const char *entry,
> + const char *relative_base,
> + int depth,
> + const char *normalized_objdir)
Beyond returning an odb_source pointer from this function, it looks like
"entry" was changed form a strbuf to a bare const char *. Is that
intentional? If so, it might be worth mentioning it in the commit
message, or optionally doing that as a preparatory step.
> @@ -316,17 +315,23 @@ void odb_add_to_alternates_file(struct object_database *odb,
> free(alts);
> }
>
> -void odb_add_to_alternates_memory(struct object_database *odb,
> - const char *reference)
> +struct odb_source *odb_add_to_alternates_memory(struct object_database *odb,
> + const char *reference)
Not the fault of your patch, but while we're refactoring, do you think
it makes sense to rename the second parameter here to "dir", or changing
"dir" to "reference" in the declaration of this function over in odb.h?
Thanks,
Taylor
next prev parent reply other threads:[~2025-08-07 22:16 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
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 [this message]
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=aJUlwLJyxSmD4EQF@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).