[MERGE][Bug #64783] Fixing check to understand the split-up .bzr format

[Ian Clatworthy]

Daniel Mark Watkins wrote:
> One month later...

This is really close. My main gripe is that workingtree.check() is now a
public method and therefore must have a test (so that accidentally
removing it or changing its signature gets detected) and a better
docstring. Having said that, I'd be ok with making that method private
given I think the high level tests arguably cover it well enough.

I'd also like some other minor tweaks (in addition to those raised by
Aaron) ...

> -    """Validate consistency of branch history.
> -
> -    This command checks various invariants about the branch storage to
> -    detect data corruption or bzr bugs.
> -
> -    Output fields:
> +    """Validates working tree structure, branch consistency and repository
> +    history.
> +

For consistency with the other command summaries given when running

  bzr help commands

I want the first word in the docstring to be Validate, not Validates.

> +    @classmethod
> +    def open_containing_tree_branch_or_repository(klass, location):
> +        """Return the branch, working tree and repo contained by a location.
> +
> +        Returns (tree, branch, repository).

Given the order of the result tuple, I'd like the docstring to mention
working tree before branch in its 1st line summary here. (Sorry if that
seems picky but it's the sort of thing that leads to accidental errors.)

> +from bzrlib.symbol_versioning import deprecated_function, one_three

> + at deprecated_function(one_three)

As Aaron indicated, this needs to be 1.5 now.

> +    @needs_read_lock
> +    def check(self):
> +        # bit hacky, check the tree parent is accurate
> +        tree_basis = self.basis_tree()

The "bit hacky" comment doesn't add much value IMO. Perhaps you need to
either explain why your approach is hacky or be more explicit (TODO:
...) about how it might be improved in the future. Removing it
altogether is another option. :-)

Finally, thanks for your work on this. 'check' is the sort of command
that isn't often run but really needs to DTRT when it is. I think these
changes, and those you have planned, will get it much closer to that state.

Ian C.