updated: Re: [MERGE] trees - add get_parent_ids

[John Arbash Meinel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Robert Collins wrote:
> On Fri, 2006-06-16 at 23:51 +1000, Robert Collins wrote:
>> This patch adds a get_parent_ids() call to all our tree objects (at
>> least, I think to all of them ;)).
>>
>> This call returns:
>> [] for a tree with unknown parents
>> ['null:'] for a tree descending from null:
>> [revid, ...] in general.
>>
>> EmptyTree descends from ['null:'].
>> RevisionTree mirrors the revision graph.
>> WorkingTree provides the union of last_revision and pending merges.
> 
> This uses the new get_parent_ids code to tweak commit a little. This
> will also make commit nicer for use with non-working trees.
> 
> -Rob
> 
> 

I assume this is the only real change:


>      def _cleanup(self):
>          """Cleanup any open locks, progress bars etc."""
> @@ -452,13 +444,11 @@
>          """Record the parents of a merge for merge detection."""
>          # TODO: Make sure that this list doesn't contain duplicate 
>          # entries and the order is preserved when doing this.
> -        pending_merges = self.work_tree.pending_merges()
> -        self.parents = []
> +        self.parents = self.work_tree.parent_ids()
> +        # remove NULL_REVISION which is not to be recorded in the commit.
> +        while self.parents.index(bzrlib.revision.NULL_REVISION):
> +            del self.parents[self.parents.index(bzrlib.revision.NULL_REVISION)]
>          self.parent_invs = []
> -        precursor_id = self.branch.last_revision()
> -        if precursor_id:
> -            self.parents.append(precursor_id)
> -        self.parents += pending_merges
>          for revision in self.parents:
>              if self.branch.repository.has_revision(revision):
>                  inventory = self.branch.repository.get_inventory(revision)


I really don't like the look of the 'while self.parents.index()' call.
Index raises an exception (ValueError) if it isn't found. Which means
that if there are no NULL_REVISION entries in the parents index, you'll
get an exception and not be able to continue. And since it is in a while
loop, it will always fail.

Did you actually test this?

I think you may have wanted to do:

while self.parents.count(NULL_REVISION):
  del self.parents[self.parents.index(NULL_REVISION)]

Though could also 'pop' self.parents.pop(self.parents.index(...

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.2 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFEksfZJdeBCYSNAAMRAvE1AKDIZaXUG5aAv3NAoAcPMoL84aL/6QCcDtUw
KGTHGVOqtENtrjozFOH0gig=
=KgWV
-----END PGP SIGNATURE-----