Comment 2 for bug 406687

The linked branch does an ok job at noticing missing CHK root entries for added inventories, which would have caught the problem that inspired this bug report.

After chatting with Robert today it would be good to be more paranoid, though: essentially do a reasonably complete "bzr check" on every added revision. The end goal is that if commit_write_group accepts it, we have a very high degree of confidence that the data is sound, so we can be confident that we'll be intercepting corruptions before they damage repositories. This gives better user experience and better bug reports.

Specifically for each added revision verify that an inventory is present (not necessarily a fulltext), and that the set of chks and texts referenced from that inventory (vs. the present parents) are also present. This is essentially what Repository.check already does (although it does even more besides, but perhaps we want that too). Unfortunately, bug 406597 means that simply asking Repository.check to verify new revisions won't work, because it is overly strict with stacked branches. Currently the linked branch is using Repository.check in _commit_write_group and so demonstrates this problem.

So, my short term plan is to propose a merge for a patch doesn't reuse check, but will at least catch missing chk entries. This will insulate us from gross bugs to do with using the wrong generic fetch code (like bug 406597), and is a good incremental change in the desired direction. (Perhaps this partial fix will be considered adequate for the 2.0 release, but I think Robert does not consider that enough by itself.)

After that, I'll try to reuse check, which means fixing bug 406597 and possibly other undiscovered bugs (I've already found that check sometimes explodes rather than reports cleanly in the presence of some problems like missing chk entries). Hopefully that goes well; if not I may just reimplement the logic for detecting missing chk entries and referenced texts and hope we can unify with check later.