git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: Justin Tobler <jltobler@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH 01/10] packfile: create store via its owning source
Date: Tue, 16 Dec 2025 09:36:05 +0100	[thread overview]
Message-ID: <aUEZ9RvivPRCjIQ_@pks.im> (raw)
In-Reply-To: <7rbnw67kn3xe3mpkpssiy22ewvjihzteole3sjhosocqo4sr7a@cig7o2dauljd>

On Mon, Dec 15, 2025 at 03:30:44PM -0600, Justin Tobler wrote:
> On 25/12/15 08:36AM, Patrick Steinhardt wrote:
> > diff --git a/packfile.c b/packfile.c
> > index c88bd92619..0a05a10daa 100644
> > --- a/packfile.c
> > +++ b/packfile.c
> > @@ -876,7 +876,7 @@ struct packed_git *packfile_store_load_pack(struct packfile_store *store,
> >  
> >  	p = strmap_get(&store->packs_by_path, key.buf);
> >  	if (!p) {
> > -		p = add_packed_git(store->odb->repo, idx_path,
> > +		p = add_packed_git(store->source->odb->repo, idx_path,
> >  				   strlen(idx_path), local);
> >  		if (p)
> >  			packfile_store_add_pack(store, p);
> > @@ -1068,8 +1068,8 @@ void packfile_store_prepare(struct packfile_store *store)
> >  	if (store->initialized)
> >  		return;
> >  
> > -	odb_prepare_alternates(store->odb);
> > -	for (source = store->odb->sources; source; source = source->next) {
> > +	odb_prepare_alternates(store->source->odb);
> > +	for (source = store->source->odb->sources; source; source = source->next) {
> 
> huh so IIUC, even though there is a packfile store per ODB source, we
> will add the alternate sources to the same packfile store? 

Not quite -- what we're doing here is that whenever we prepare a
packfile source, we'll implicitly also prepare all the other packfile
sources part of the same ODB. They will still be separate packfile
sources, but regardless of that this logic is of course still quite
flawed.

> This is feels very awkward, but is maybe part of the "weird in-between
> state" you mentioned in the commit message.

It very much is, yes. It gets cleaned up at a later point in this patch
series, but I'm not a huge fan of this intermediate step where we are in
this in-between state. I have been pondering over it for quite a while
though and wasn't able to figure out a better way to structure the
series to avoid this :/

Patrick

  reply	other threads:[~2025-12-16  8:36 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-15  7:36 [PATCH 00/10] Start tracking packfiles per object database source Patrick Steinhardt
2025-12-15  7:36 ` [PATCH 01/10] packfile: create store via its owning source Patrick Steinhardt
2025-12-15 21:30   ` Justin Tobler
2025-12-16  8:36     ` Patrick Steinhardt [this message]
2025-12-15  7:36 ` [PATCH 02/10] packfile: pass source to `prepare_pack()` Patrick Steinhardt
2025-12-15 21:38   ` Justin Tobler
2025-12-15  7:36 ` [PATCH 03/10] packfile: refactor kept-pack cache to work with packfile stores Patrick Steinhardt
2025-12-15 21:56   ` Justin Tobler
2025-12-16  9:09     ` Patrick Steinhardt
2025-12-15  7:36 ` [PATCH 04/10] packfile: refactor misleading code when unusing pack windows Patrick Steinhardt
2025-12-15  7:36 ` [PATCH 05/10] packfile: move packfile store into object source Patrick Steinhardt
2025-12-18  0:52   ` Justin Tobler
2025-12-18  6:50     ` Patrick Steinhardt
2025-12-15  7:36 ` [PATCH 06/10] packfile: only prepare owning store in `packfile_store_get_packs()` Patrick Steinhardt
2025-12-18  0:58   ` Justin Tobler
2025-12-15  7:36 ` [PATCH 07/10] packfile: only prepare owning store in `packfile_store_prepare()` Patrick Steinhardt
2025-12-15  7:36 ` [PATCH 08/10] packfile: inline `find_kept_pack_entry()` Patrick Steinhardt
2025-12-18  1:06   ` Justin Tobler
2025-12-18  6:48     ` Patrick Steinhardt
2025-12-15  7:36 ` [PATCH 09/10] packfile: refactor `find_pack_entry()` to work on the packfile store Patrick Steinhardt
2025-12-15  7:36 ` [PATCH 10/10] packfile: move MIDX into " Patrick Steinhardt
2025-12-18  6:55 ` [PATCH v2 00/10] Start tracking packfiles per object database source Patrick Steinhardt
2025-12-18  6:55   ` [PATCH v2 01/10] packfile: create store via its owning source Patrick Steinhardt
2025-12-18  6:55   ` [PATCH v2 02/10] packfile: pass source to `prepare_pack()` Patrick Steinhardt
2025-12-18  6:55   ` [PATCH v2 03/10] packfile: refactor kept-pack cache to work with packfile stores Patrick Steinhardt
2025-12-18  6:55   ` [PATCH v2 04/10] packfile: refactor misleading code when unusing pack windows Patrick Steinhardt
2025-12-18  6:55   ` [PATCH v2 05/10] packfile: move packfile store into object source Patrick Steinhardt
2025-12-18  6:55   ` [PATCH v2 06/10] packfile: only prepare owning store in `packfile_store_get_packs()` Patrick Steinhardt
2025-12-18  6:55   ` [PATCH v2 07/10] packfile: only prepare owning store in `packfile_store_prepare()` Patrick Steinhardt
2025-12-18  6:55   ` [PATCH v2 08/10] packfile: inline `find_kept_pack_entry()` Patrick Steinhardt
2025-12-18  6:55   ` [PATCH v2 09/10] packfile: refactor `find_pack_entry()` to work on the packfile store Patrick Steinhardt
2025-12-18  6:55   ` [PATCH v2 10/10] packfile: move MIDX into " 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=aUEZ9RvivPRCjIQ_@pks.im \
    --to=ps@pks.im \
    --cc=git@vger.kernel.org \
    --cc=jltobler@gmail.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).