On Thu, May 23, 2019 at 08:59:59AM +0000, Eric Wong wrote:
> > We never delete entries from the in-memory packed_git list; a reprepare
> > only adds to the list. You'd need to teach update_server_info() to
> > ignore packs which are no longer present (or switch to exec-ing a
> > separate update-server-info binary).
>
> Ah, checking files_exists() and setting a bit seems sufficient.
Yes, though we do we even need to store the bit?
I.e.,
> @@ -199,12 +200,16 @@ static void init_pack_info(const char *infofile, int
> force)
> */
> if (!p->pack_local)
> continue;
> + if (!file_exists(p->pack_name)) {
> + p->pack_unlinked = 1;
> + continue;
> + }
> i++;
> }
> num_pack = i;
> info = xcalloc(num_pack, sizeof(struct pack_info *));
> for (i = 0, p = get_all_packs(the_repository); p; p = p->next) {
> - if (!p->pack_local)
> + if (!p->pack_local || p->pack_unlinked)
> continue;
> assert(i < num_pack);
> info[i] = xcalloc(1, sizeof(struct pack_info));
If we just check file_exists() in the second loop, then this is entirely
local to update_server_info(). And other users of packed_git do not have
to wonder who is responsible for setting that flag in the global list.
It does mean you'd over-allocate the array (and num_pack would have to
be adjusted down to "i" after the second loop), but that's not a big
deal. I do think the whole two-loop thing would be more readable if we
simply grew it on the fly with ALLOC_GROW().
-Peff