[BUG] bogus name_version in bzr.newformat

[John A Meinel]

Martin Pool wrote:
> On 05/10/05, John A Meinel <john at arbash-meinel.com> wrote:
>  The one thing I thought about is the statement:
>
>>    On regular commits without merges, files that are unchanged retain
>>    the same text_version they had in the basis version, and files whose
>>    text is changed get a text_version equal to the new revision id. For
>>    merges, files that had different versions in any of the parent trees
>>    have their text version updated, even if the text is the same in all
>>    of them.
>>
>>So some of this might be because of a merge. But why is the revision
>>number for the "bzrlib" directory changing? As far as I know, it was
>>only changed one time. Does that mean from then on, because somewhere in
>>the ancestry (not the diverged ancestry, the common ancestry) it
>>changed, now it always gets a new revision id.
>>
>>That is my guess. That the code which says "if this has changed in the
>>ancestry, give it a new revision id", doesn't track the fact that the
>>change occurred in the common portion of the ancestry.
>>
>>Which make me think we need to update it. It should be more thorough
>>saying that "if the revision id for an entry is the same in both
>>branches, then it should not be updated". I think that gives us the
>>behaviour that we want.
>
>
> That is what it's meant to mean; that seems to imply there should be
> very few revisions of the bzrlib directory.

The current code says this:

    def _set_revision(self, rev, ie, parent_invs):
        """Set name version for a file.

        Done in a slightly lazy way: if the file is renamed or in a
merge revision
        it gets a new version, otherwise the same as before.
        """
        file_id = ie.file_id
        if ie.kind == 'root_directory':
            return
        if len(parent_invs) != 1:
            ie.revision = rev.revision_id
        else:
            old_inv = parent_invs[0]
            if not old_inv.has_id(file_id):
                ie.revision = rev.revision_id
            else:
                old_ie = old_inv[file_id]
                if (old_ie.parent_id != ie.parent_id
                    or old_ie.name != ie.name):
                    ie.revision = rev.revision_id
                else:
                    ie.revision = old_ie.revision

I think what you want is:

if len(parent_invs) == 0:
    ie.revision = rev.revision_id
elif len(parent_invs) >= 1:
    matched_id = None
    for p_inv in parent_invs:
      if not p_inv.has_id(file_id):
          ie.revision = rev.revision_id
          break
      old_ie = p_inv[file_id]
      if (old_ie.parent_id != ie.parent_id
          or old_ie.name != ie.name
          or old_ie.text_sha1 != ie.text_sha1
          or old_ie.text_size != ie.text_size):
          ie.revision = rev.revision_id
          break
      if matched_id is None:
          matched_id = old_ie.revision
      elif matched_id != old_ie.revision:
          ie.revision = rev.revision_id
          break
      matched_id = ie.revision
    else:
      assert matched_id is not None
      ie.revision = matched_id

I believe that means that it will check, and if all parent inventories
say exactly the same thing about the file/directory, then they will
continue to say the exact same thing about it.

John
=:->

>
> To answer a question from earlier in the thread, it is intended that
> renaming of a parent should not cause a new revision, only changing
> the parent (e.g. moving between directories) or changing the name.
>
> --
> Martin
>
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 253 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20051004/c9ebe915/attachment.pgp