[Outreachy] fast-import: Use repo_for_each_pack() for duplicate checks

[Outreachy] fast-import: Use repo_for_each_pack() for duplicate checks

[Joie Joie]

Hi Patrick,

I was looking through `fast-import.c` and noticed that both
`store_object()` and `stream_blob()` still loop through `files->dirs`
to check whether an object is already in a pack.

I saw that Git already has `repo_for_each_pack()`, which is used in
places like `count-objects.c` and `pack-objects.c`.

Would it be worth using that iterator in these two places instead? The
idea would be to avoid accessing the files backend directly while
keeping the current packed-object lookup and duplicate-handling
behavior unchanged.

I haven't made any changes yet. I wanted to get your thoughts first
and check whether this would be a useful follow-up in the current
refactoring work.

Thanks!
Joy

Re: [Outreachy] fast-import: Use repo_for_each_pack() for duplicate checks

[Patrick Steinhardt]

Hi Joie,

On Fri, Nov 06, 2026 at 06:44:49PM +0100, Joie Joie wrote:
> Hi Patrick,
> 
> I was looking through `fast-import.c` and noticed that both
> `store_object()` and `stream_blob()` still loop through `files->dirs`
> to check whether an object is already in a pack.
> 
> I saw that Git already has `repo_for_each_pack()`, which is used in
> places like `count-objects.c` and `pack-objects.c`.
> 
> Would it be worth using that iterator in these two places instead? The
> idea would be to avoid accessing the files backend directly while
> keeping the current packed-object lookup and duplicate-handling
> behavior unchanged.

No. `repo_for_each_pack()` was introduced as a helper function to make
the migration towards pluggable object databases a bit easier. But it is
ultimately still flawed, as it assumes that the backend uses packfiles
in the first place. So the goal is rather to eventually get rid of this
macro altogether in favor of higher-level interfaces that can be
implemented by arbitrary backends.

For git-fast-import(1) this will be done by making use of transactions,
and Karthik has already started taking a look at it, so I've Cc'd him.
For other subsystems we'll need to investigate how to do it properly.

Thanks!

Patrick

Re: [Outreachy] fast-import: Use repo_for_each_pack() for duplicate checks

[Karthik Nayak]

[-- Attachment #1: Type: text/plain, Size: 1516 bytes --]

>Patrick Steinhardt <ps@pks.im> writes:

> Hi Joie,
>
> On Fri, Nov 06, 2026 at 06:44:49PM +0100, Joie Joie wrote:
>> Hi Patrick,
>>
>> I was looking through `fast-import.c` and noticed that both
>> `store_object()` and `stream_blob()` still loop through `files->dirs`
>> to check whether an object is already in a pack.
>>
>> I saw that Git already has `repo_for_each_pack()`, which is used in
>> places like `count-objects.c` and `pack-objects.c`.
>>
>> Would it be worth using that iterator in these two places instead? The
>> idea would be to avoid accessing the files backend directly while
>> keeping the current packed-object lookup and duplicate-handling
>> behavior unchanged.
>
> No. `repo_for_each_pack()` was introduced as a helper function to make
> the migration towards pluggable object databases a bit easier. But it is
> ultimately still flawed, as it assumes that the backend uses packfiles
> in the first place. So the goal is rather to eventually get rid of this
> macro altogether in favor of higher-level interfaces that can be
> implemented by arbitrary backends.
>
> For git-fast-import(1) this will be done by making use of transactions,
> and Karthik has already started taking a look at it, so I've Cc'd him.
> For other subsystems we'll need to investigate how to do it properly.
>

Yeah, that's the idea. I have started poking around a bit and don't have
anything concrete yet. I was hoping to send in a series cleaning up the
tests before getting into the weeds.

> Thanks!
>
> Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]