On Mon, Mar 19, 2018 at 8:39 PM, Jonathan Tan [off-list ref] wrote:

On Sat,  3 Mar 2018 18:36:03 +0700
Nguyễn Thái Ngọc Duy  [off-list ref] wrote:

quoted

From: Stefan Beller <redacted>

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.

quoted

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]

quoted

@@ -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