On Mon, Mar 19, 2018 at 8:39 PM, Jonathan Tan <jonathantanmy@xxxxxxxxxx> wrote:
> On Sat, 3 Mar 2018 18:36:03 +0700
> Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> wrote:
>
>> From: Stefan Beller <sbeller@xxxxxxxxxx>
>>
>> In a process with multiple repositories open, packfile accessors
>> should be associated to a single repository and not shared globally.
>> Move packed_git and packed_git_mru into the_repository and adjust
>> callers to reflect this.
>>
>> [nd: while at there, wrap access to these two fields in get_packed_git()
>> and get_packed_git_mru(). This allows us to lazily initialize these
>> fields without caller doing that explicitly]
>
> The patches up to this one look good. (I didn't reply for each
> individual one to avoid unnecessarily sending messages to the list.)
>
> About this patch: will lazily initializing these fields be done in a
> later patch?
Yes.
>> Patch generated by
>>
>> 1. Moving the struct packed_git declaration to object-store.h
>> and packed_git, packed_git_mru globals to struct object_store.
>>
>> 2. Apply the following semantic patch to adjust callers:
>> @@ @@
>> - packed_git
>> + the_repository->objects.packed_git
>>
>> @@ @@
>> - packed_git_mru
>> + the_repository->objects.packed_git_mru
>
> This doesn't seem up-to-date - they are being replaced with a function
> call, not a field access. I would be OK with just removing this "Patch
> generated by" section.
I'll just drop this part. I'm manually updating the series anyway.
> [snip remainder of "Patch generated by" section]
>
>> @@ -246,7 +244,7 @@ static int unuse_one_window(struct packed_git *current)
>>
>> if (current)
>> scan_windows(current, &lru_p, &lru_w, &lru_l);
>> - for (p = packed_git; p; p = p->next)
>> + for (p = the_repository->objects.packed_git; p; p = p->next)
>> scan_windows(p, &lru_p, &lru_w, &lru_l);
>> if (lru_p) {
>> munmap(lru_w->base, lru_w->len);
>
> Here (and elsewhere), "the_repository->objects.packed_git" instead of
> "get_packed_git" is still used.
Yeah. I saw your other comment about the not converting in packfile.c
too. My view is, packfile has intimate knowledge about pack
manipulation and we don't need to try to "protect" from misuse of
packed_git pointer without prepare_packed_git. People how modify this
code probably know that by heart at this point. And we can't avoid it
completely anyway because it will have access to the static function
prepare_packed_git.
--
Duy
