[MERGE] Remove most calls to safe_file_id and safe_revision_id.

Martin Pool mbp at sourcefrog.net
Wed Sep 26 07:03:29 BST 2007


On Wed, 2007-09-26 at 15:49 +1000, Ian Clatworthy wrote:

> > === modified file 'bzrlib/workingtree.py'
> > --- bzrlib/workingtree.py	2007-09-13 22:41:38 +0000
> > +++ bzrlib/workingtree.py	2007-09-25 08:14:12 +0000
> 
> > -        # TODO: jam 20070209 This should be redundant, as the revision_id
> > -        #       as all callers should have already converted the revision_id to
> > -        #       utf8
> > -        inventory.revision_id = osutils.safe_revision_id(revision_id)
> >          return xml7.serializer_v7.write_inventory_to_string(inventory)
> 
> This is a semantic change in that inventory.revision_id is no longer
> being set here. Was that deliberate? (It stands out as being more than a
> mechanical change as 99% of the other changes in this patch are.) I
> haven't analysed the various data flows but it doesn't look a safe
> change.

You're right, that is an incorrect mechanical change.  I thought I had
run the test suite on this so it suggests it may not be totally covered,
or maybe I just made a mistake.

> > @@ -1975,15 +1951,6 @@
> >      @needs_tree_write_lock
> >      def set_root_id(self, file_id):
> >          """Set the root id for this tree."""
> > -        # for compatability 
> > -        if file_id is None:
> > -            symbol_versioning.warn(symbol_versioning.zero_twelve
> > -                % 'WorkingTree.set_root_id with fileid=None',
> > -                DeprecationWarning,
> > -                stacklevel=3)
> > -            file_id = ROOT_ID
> > -        else:
> > -            file_id = osutils.safe_file_id(file_id)
> >          self._set_root_id(file_id)
> 
> This change breaks the test suite:
> 
>   File
> "/home/ian/bzr/bzr.tmp/bzrlib/tests/workingtree_implementations/test_set_root_id.py",
> line 33, in test_set_None
>     tree.set_root_id, None)
> AssertionError: not equal:
> a = ['WorkingTree.set_root_id with fileid=None was deprecated in version
> 0.12.']
> b = []
> 
> It wouldn't hurt to leave the if clause there. Otherwise, the test suite
> will need a tweak if you'd prefer (0.12 was a while ago now).

I'll do that.

-- 
Martin



More information about the bazaar mailing list