On Wed, Apr 27, 2016 at 03:56:46PM -0700, Junio C Hamano wrote:
> Eric Wong <normalperson@xxxxxxxx> writes:
>
> > It can be tempting for a server admin to want a stable set of
> > long-lived packs for dumb clients; but also want to enable
> > bitmaps to serve smart clients more quickly.
> >
> > Unfortunately, such a configuration is impossible;
> > so at least warn users of this incompatibility since
> > commit 21134714787a02a37da15424d72c0119b2b8ed71
> > ("pack-objects: turn off bitmaps when we split packs").
> >
> > Tested the warning by inspecting the output of:
> >
> > make -C t t5310-pack-bitmaps.sh GIT_TEST_OPTS=-v
>
> While I do not think the update in this patch is wrong, this makes
> me wonder what happens to a server admin who wants a stable set of
> long-lived objects in a pack, and other objects in another pack that
> is frequently recreated, by first packing objects needed for the
> latest released version into a single pack and marking it with .keep
> and then running "git repack" to create the second pack for the
> remainder.
>
> There is no automatic split involved, so we would get a bitmap file
> for each of these two packs. Would that pose a problem? The pack
> with the newer part of the history would not satisfy "all of the
> reachable objects are in the same pack" condition.
You will not get two bitmaps in such a case. In add_object_entry(), if
we exclude an object via want_object_in_pack(), we will issue a warning
and turn off bitmaps. That includes finding that one of the reachable
objects we would need is in a .keep pack.
You could in theory construct a broken non-closed bitmap by feeding an
arbitrary set of objects to pack-objects, but we turn off bitmap writing
entirely unless --all is used. So the test in add_object_entry() should
be sufficient for all cases there (it's actually too conservative; it's
possible that the object is not reachable from the other objects that
are going into the pack, but this is so impractical that it's not even
worth trying to catch).
The split-pack check from 211347147 had to come separately, because we
only find out about the split much later during the write phase (and
again, it is too conservative; it's _possible_ that the objects being
split aren't reachable from anything in the previous pack, but it's
exceedingly unlikely and not worth caring about).
Adding a warning as Eric's patch does is a definite improvement, and
something I should have done in the original (we _could_ just use the
same no_closure_warning, as that is technically what is going on, but I
think it is nice to mention pack-splitting, which is more likely to lead
the user in the right direction to fixing it).
> [discussion of doc fixes]
I do not mind overly if pack-splitting mentions disabling bitmaps. But I
think it may be simpler to keep the documentation in the bitmap section,
rather than trying to cross-reference in both directions. It is the
bitmap code which is not prepared to work with pack-splits (and other
things, like .keep), not the other way around.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html