[MERGE][0.92] Fix bug 155730: force file versions with unreferenced parents to be stored as fulltexts.

Ian Clatworthy ian.clatworthy at internode.on.net
Fri Oct 26 06:22:08 BST 2007


Andrew Bennetts wrote:
> This bundle fixes bug 155730, which is marked critical because it makes it hard
> for affected repositories (like bzr.dev!) to be converted to the just-merged
> pack format.

bb: tweak

I'm 95% confident about this and poolie has indicated to spiv it's ok so
we ought to land it. I'm pretty confident about the code - the tests
less so. That's no reflection on the tests - just my depth of
understanding of them.

My comments over and above the earlier ones:

>              vf_checker = self.repo.get_versioned_file_checker(
> -                versions, revision_versions)
> -            versions_with_bad_parents = vf_checker.check_file_version_parents(
> -                vf, file_id)
> -            if len(versions_with_bad_parents) == 0:
> +                vf.versions(), revision_versions)
> +            versions_with_bad_parents, dangling_file_versions = \
> +                vf_checker.check_file_version_parents(vf, file_id)

Buried in that change, get_versioned_file_checker now takes
vf.versions() as it 1st arg rather than versions (which was obtained
from the revision_store). Is this deliberate or doesn't it matter? If
there's a subtle difference, then it might be worth a comment mentioning
why the vf.versions() must be used now.

> +            full_text_versions = set()
> +            unused_versions = set()
> +            if (len(versions_with_bad_parents) == 0 and
> +                len(dangling_file_versions) == 0):
>                  continue

The if stmt should go before the 2 initialisation statements here.

> +        :returns: A tuple of (wrong_parents, dangling_file_version).
> +            wrong_parents is a dict mapping {revision_id: (stored_parents,
> +            correct_parents)} for each revision_id where the stored parents
> +            are not correct.  dangling_file_version is a set of revision_ids

Actually, it a set of (file-id,revision-id) tuples.

>      def all_versions(self):
> -        return ['rev1a', 'rev2', 'rev4', 'rev2b', 'rev4', 'rev2c', 'rev5']
> +        return ['rev1a', 'rev2c', 'rev4', 'rev5']

I originally thought this was a bad merge because 'rev2' etc clearly are
in the list of versions in the populated_parents and corrected_parents
given immediately below. Renaming this from all_versions to something
more explicit as discussed on IRC would be good.

Ian C.



More information about the bazaar mailing list