[PATCH] Fix for bug #314251 [mv via rm+add breaking dirstate]

Ian Clatworthy ian.clatworthy at internode.on.net
Thu Jan 8 05:25:34 GMT 2009


Charles Duffy wrote:
> Oops. Here's another try.
> 

bb:tweak

Please add a NEWS item into the BUGS section.

> +            if file_id_entry[1][0][0] == 'a':
> +                if file_id_entry[0] != (dirname, basename, file_id):
> +                    # set the old name's current operation to rename
> +                    self.update_minimal(file_id_entry[0],
> +                        'r',
> +                        path_utf8='',
> +                        packed_stat='',
> +                        fingerprint=utf8path
> +                    )
> +                    rename_from = file_id_entry[0][0:2]
> +            else:
> +                path = osutils.pathjoin(file_id_entry[0][0], file_id_entry[0][1])
> +                kind = DirState._minikind_to_kind[file_id_entry[1][0][0]]
> +                info = '%s:%s' % (kind, path)
> +                raise errors.DuplicateFileId(file_id, info)

I'm not sure if the inner if test needs an else clause or not?
If they do match, is that OK or a logic/data error we ought to
report? Please check this with a dirstate guru, e.g. lifeless
or jam.

> +        if rename_from:
> +            if rename_from[0]:
> +                old_path_utf8 = '%s/%s' % rename_from
> +            else:
> +                old_path_utf8 = rename_from[1]
> +            parent_info[0] = ('r', old_path_utf8, 0, False, '')

The top "if" will be slightly more efficient if written as

  if rename_from is not None:

> -    def _get_entry(self, tree_index, fileid_utf8=None, path_utf8=None):
> +    def _get_entry(self, tree_index, fileid_utf8=None, path_utf8=None, include_deleted=False):
>          """Get the dirstate entry for path in tree tree_index.

Please add a docstring entry for the new param, making it clear
that it only applies when path_utf8 is None.

Ian C.



More information about the bazaar mailing list