[patch][0.15] better DirState._validate

Martin Pool mbp at sourcefrog.net
Thu Mar 29 03:39:56 BST 2007


On 3/27/07, John Arbash Meinel <john at arbash-meinel.com> wrote:
> Martin Pool wrote:
> > Following on from John's move_children_correctly here is a patch that:
>
> Thanks. I'm sorry that I didn't get a chance to catch all the other edge
> cases before I submitted it. (I can only blame it being 7pm on a Friday)

That's quite ok, you did the hard work by catching the original bug.
> v- I think having a check that all entries are the right width (number
> of columns) is reasonable. In case anyone starts poking at _dirblocks
> directly. Since they are lists, there is no strong constraint, but they
> do have a very specific form.

Yes, done.

I think you may have been looking at a slightly old version.  (I may
not have updated it.)  Here is the current version, which is in
0.15rc3

> v- You say 'accumulate' but you don't seem to be doing it. Is it a
> change you later removed?

Yes, thanks

    def _validate(self):
        """Check that invariants on the dirblock are correct.

        This can be useful in debugging; it shouldn't be necessary in
        normal code.

        This must be called with a lock held.
        """
        # NOTE: This must always raise AssertionError not just assert,
        # otherwise it may not behave properly under python -O
        #
        # TODO: All entries must have some content that's not 'a' or 'r',
        # otherwise it could just be removed.
        #
        # TODO: All relocations must point directly to a real entry.
        #
        # TODO: No repeated keys.
        #
        # -- mbp 20070325
        from pprint import pformat
        self._read_dirblocks_if_needed()
        if len(self._dirblocks) > 0:
            if not self._dirblocks[0][0] == '':
                raise AssertionError(
                    "dirblocks don't start with root block:\n" + \
                    pformat(dirblocks))
        if len(self._dirblocks) > 1:
            if not self._dirblocks[1][0] == '':
                raise AssertionError(
                    "dirblocks missing root directory:\n" + \
                    pformat(dirblocks))
        # the dirblocks are sorted by their path components, name, and dir id
        dir_names = [d[0].split('/')
                for d in self._dirblocks[1:]]
        if dir_names != sorted(dir_names):
            raise AssertionError(
                "dir names are not in sorted order:\n" + \
                pformat(self._dirblocks) + \
                "\nkeys:\n" +
                pformat(dir_names))
        for dirblock in self._dirblocks:
            # within each dirblock, the entries are sorted by filename and
            # then by id.
            for entry in dirblock[1]:
                if dirblock[0] != entry[0][0]:
                    raise AssertionError(
                        "entry key for %r"
                        "doesn't match directory name in\n%r" %
                        (entry, pformat(dirblock)))
            if dirblock[1] != sorted(dirblock[1]):
                raise AssertionError(
                    "dirblock for %r is not sorted:\n%s" % \
                    (dirblock[0], pformat(dirblock)))

        # For each file id, for each tree: either
        # the file id is not present at all; all rows with that id in the
        # key have it marked as 'absent'
        # OR the file id is present under exactly one name; any other entries
        # that mention that id point to the correct name.
        #
        # We check this with a dict per tree pointing either to the present
        # name, or None if absent.
        tree_count = self._num_present_parents() + 1
        id_path_maps = [dict() for i in range(tree_count)]
        # Make sure that all renamed entries point to the correct location.
        for entry in self._iter_entries():
            file_id = entry[0][2]
            this_path = osutils.pathjoin(entry[0][0], entry[0][1])
            if len(entry[1]) != tree_count:
                raise AssertionError(
                "wrong number of entry details for row\n%s" \
                ",\nexpected %d" % \
                (pformat(entry), tree_count))
            for tree_index, tree_state in enumerate(entry[1]):
                this_tree_map = id_path_maps[tree_index]
                minikind = tree_state[0]
                # have we seen this id before in this column?
                if file_id in this_tree_map:
                    previous_path = this_tree_map[file_id]
                    # any later mention of this file must be consistent with
                    # what was said before
                    if minikind == 'a':
                        if previous_path is not None:
                            raise AssertionError(
                            "file %s is absent in row %r but also present " \
                            "at %r"% \
                            (file_id, entry, previous_path))
                    elif minikind == 'r':
                        target_location = tree_state[1]
                        if previous_path != target_location:
                            raise AssertionError(
                            "file %s relocation in row %r but also at %r" \
                            % (file_id, entry, previous_path))
                    else:
                        # a file, directory, etc - may have been previously
                        # pointed to by a relocation, which must point here
                        if previous_path != this_path:
                            raise AssertionError(
                            "entry %r inconsistent with previous path %r" % \
                            (entry, previous_path))
                else:
                    if minikind == 'a':
                        # absent; should not occur anywhere else
                        this_tree_map[file_id] = None
                    elif minikind == 'r':
                        # relocation, must occur at expected location
                        this_tree_map[file_id] = tree_state[1]
                    else:
                        this_tree_map[file_id] = this_path

    def _wipe_state(self):


-- 
Martin



More information about the bazaar mailing list