зеркало из https://github.com/microsoft/git.git
midx.c: lookup MIDX by object directory during expire
Before a new MIDX can be written, expire_midx_packs() first loads the
existing MIDX, figures out which packs can be expired, and then writes a
new MIDX based on that information.
In order to load the existing MIDX, it uses load_multi_pack_index(),
which mmaps the multi-pack-index file, but does not store the resulting
`struct multi_pack_index *` in the object store.
write_midx_internal() also needs to open the existing MIDX, and it does
so by iterating the results of get_multi_pack_index(), so that it reuses
the same pointer held by the object store. But before it can move the
new MIDX into place, it close_object_store() to munmap() the
multi-pack-index file to accommodate platforms like Windows which don't
allow overwriting files which are memory mapped.
That's where things get weird. Since expire_midx_packs has its own
*separate* memory mapped copy of the MIDX, the MIDX file is still memory
mapped! Interestingly, this doesn't seem to cause a problem in our
tests. (I believe that this has much more to do with my own lack of
familiarity with Windows than it does a lack of coverage in our tests).
In any case, we can side-step the whole issue by teaching
expire_midx_packs() to use the `struct multi_pack_index` pointer it
found via the object store instead of maintain its own copy. That way,
when write_midx_internal() calls `close_object_store()`, we know that
there are no memory mapped copies of the MIDX laying around.
A couple of other small notes about this patch:
- As far as I can tell, passing `local == 1` to the call to
load_multi_pack_index() was an error, since object_dir could be an
alternate. But it doesn't matter, since even though we write
`m->local = 1`, we never read that field back later on.
- Setting `m = NULL` after write_midx_internal() was likely to prevent
a double-free back from when that function took a `struct
multi_pack_index *` that it called close_midx() on itself. We can
rely on write_midx_internal() to call that for us now.
Finally, this enforces the same "the value of --object-dir must be the
local object store, or an alternate" rule from f57a739691
(midx: avoid
opening multiple MIDXs when writing, 2021-09-01) to the `expire`
sub-command, too.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
Родитель
504131ac26
Коммит
98926e0d01
7
midx.c
7
midx.c
|
@ -1696,7 +1696,7 @@ int expire_midx_packs(struct repository *r, const char *object_dir, unsigned fla
|
|||
{
|
||||
uint32_t i, *count, result = 0;
|
||||
struct string_list packs_to_drop = STRING_LIST_INIT_DUP;
|
||||
struct multi_pack_index *m = load_multi_pack_index(object_dir, 1);
|
||||
struct multi_pack_index *m = lookup_multi_pack_index(r, object_dir);
|
||||
struct progress *progress = NULL;
|
||||
|
||||
if (!m)
|
||||
|
@ -1741,12 +1741,11 @@ int expire_midx_packs(struct repository *r, const char *object_dir, unsigned fla
|
|||
|
||||
free(count);
|
||||
|
||||
if (packs_to_drop.nr) {
|
||||
if (packs_to_drop.nr)
|
||||
result = write_midx_internal(object_dir, NULL, &packs_to_drop, NULL, NULL, flags);
|
||||
m = NULL;
|
||||
}
|
||||
|
||||
string_list_clear(&packs_to_drop, 0);
|
||||
|
||||
return result;
|
||||
}
|
||||
|
||||
|
|
Загрузка…
Ссылка в новой задаче