Code review comment for lp:~spiv/bzr/ids-only-necessary-texts

Revision history for this message
Andrew Bennetts (spiv) wrote :

This fixes bug 399140. As explained in the bug discussion, the issue is that InterDifferingSerializer doesn't behave correctly with stacked sources. It should take care to copy any parent inventories that are present for the revisions it is copying, and also take care to only try to copy texts necessary for the revisions it is copying. Due to a combination of these factors it was generating an inventory delta from null for the first revision in a stacked repository during upgrade, and then expected to be able to find every text named in that delta.

The test is a somewhat copied-and-pasted variant of other stacking tests in per_interrepository/test_fetch.py. So it's both long and somewhat repetitive compared to the existing tests. We should refactor those tests at some point... Anyway, the test does fail with bzr.dev's copy of repository.py, and passes with the fix, of course :)

I expect this will have a minor performance impact on InterDifferingSerializer, but I don't expect it to be very large. Correctness beats performance, anyway.

« Back to merge proposal