[merge][#288751] avoid creating text deltas spanning repository stacks
John Arbash Meinel
john at arbash-meinel.com
Mon Nov 17 23:07:55 GMT 2008
-----BEGIN PGP SIGNED MESSAGE-----
Martin Pool wrote:
> This patch fixes a bug which causes some trouble with
> stacked repositories. The design was that text deltas would
> not span repository stacks, for the sake of robustness, and
> so that we can upgrade the inventory representation in
> underlying repositories without causing trouble to the
> stacked repository. However, this was not checked properly.
> The KnitPackRepository was previously trying to enforce this
> constraint, but it didn't cover all paths, and it only
> checked on file texts not on inventories.
> This adds has_key(key) to some index(like) objects. I
> realize we don't generally want people checking for one key
> at a time, but in some cases that's all the caller
> reasonably has, and it's better than making them repeat
> boilerplate to use get_parent_map.
> There are some effort tests for stacking that I updated.
> I'm glad they're there. I do a bit more or less work in
> some cases; I think it's still reasonable.
> If this is merged well before 1.10 I think it would be worth putting
> into a 1.9.1 release.
> This won't automatically repack existing repositories (unless I'm
> really lucky :-)
> Martin <http://launchpad.net/~mbp/>
This patch conflicts slightly with bzr.dev, because of a small change in
the _abort_write_group logic.
I'm not particularly fond of 'has_key()' though I can agree that having
all callers have to repeat the "key in get_parent_map([key])" is not
particularly pleasant either.
However, is there a reason you have to be checking the keys one-by-one
rather than all in one go?
Specifically, when the fetch is almost done, why not check external
references then, and then you can do a get_parent_map() across all of
them at once, rather than one at a time? It also handles when you get
entries in "reverse" order. As Aaron points out, I think if you had the
But you got the inventories in the order, C, B, A, this would cause C to
be inserted as a full text, then B, then A, and none of them have a
delta to build against.
Anyway, I can understand getting a fix in, as we need to close the
currently open bug now.
Also, to reply to Aaron "bzr pack" as it stands does *not* fill in the
missing holes. It would certainly be reasonable, even though the fix
would really prefer to be in "bzr reconcile", even though that is a bit
too heavyweight right now.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
-----END PGP SIGNATURE-----
More information about the bazaar