GNOME Bugzilla – Bug 653302
bzr check is completely inappropriate for testing the presence of a bzr-managed tree
Last modified: 2011-07-18 22:47:46 UTC
Created attachment 190554 [details] [review] patch that makes meld start up in less than an hour in a large bzr branch I was unpleasantly surprised when running meld in a large bzr branch (launchpad) to find that the reason the UI had gone responsive was that it was running "bzr check". "bzr check" is an extremely CPU intensive command that recalculates and validates all the checksums and other data in the branch -- it will probably take several hours for launchpad. I see that trunk now runs "bzr check --tree --branch" which is better, but still inappropriate. I'm not entirely sure what the check is asking, but I think running "bzr root" is appropriate -- this returns the root of the working tree, if any, and errors if not. I'll attach a patch changing the call to this (it works for me, in limited testing).
Having read the log message that introduced the 'check' call, perhaps 'bzr info' will be closer in intent. But anything but check.
You say 'completely inappropriate', but what we want to know with this call is "Is this directory a valid bzr repo?" which seems to be *exactly* what bzr check is supposed to do. Having said that, even the restricted bzr check we currently do is about four times slower than bzr status or something, so yes, we should probably change. bzr root is the fastest of the obvious choices (root/status/info). I have no idea which is the most appropriate or most reliable; if you'd like to make a case for using something specific, please do.
I think 'bzr status' is probably a good check to use here. It will use a reasonable portion of the data structures on disk to do with the working tree and is still reasonably quick (0.2s for 'bzr st' vs 8s for 'bzr check' for me on the Launchpad tree -- that's a bit more than 4 times :p). Of course there can be corruption that escapes this check, but I don't mind waiting 0.2s longer for meld to pop up...
I've pushed a patch to HEAD that just replaces 'check' with 'status'. Hopefully this should make performance acceptable. Thanks for the bug report.
Looks fine, thanks for fixing :)