[MERGE] reconcile patch to correct the ancestry graph.

[Martin Pool]

On  3 May 2006, Robert Collins <robertc at robertcollins.net> wrote:
> > > @@ -188,7 +200,9 @@
> > >              else:
> > >                  mutter('found ghost %s', parent)
> > >          self._rev_graph[rev_id] = parents   
> > > -        if set(self.inventory.get_parents(rev_id)) != set(parents):
> > > +        if (set(self.inventory.get_parents(rev_id)) != set(parents) or
> > > +            (len(self.inventory.get_parents(rev_id)) and len(parents) and
> > > +             parents[0] != self.inventory.get_parents(rev_id)[0])):
> > >              self.inconsistent_parents += 1
> > >              mutter('Inconsistent inventory parents: id {%s} '
> > >                     'inventory claims %r, '
> > 
> > Arguably it would be simpler to say 
> > 
> >         if (set(self.inventory.get_parents(rev_id)) != set(parents) or
> >             (self.inventory.get_parents(rev_id)[:1] != parents[:1]):
> > 
> > rather than testing the length.  
> 
> That would test a different case :).

Why?  (Aside from my missing one parenthesis.)

> > There's a new assumption here which is probably reasonable but might not
> > be expected: after reconciliation, the primary parent is guaranteed to
> > be correct but the others may be out of order.  (Or is there a stronger
> > guarantee?)  I think that should be written down somewhere - a docstring
> > on the reconcile function or class would do.
> 
> Its currently written down in the tests for this patch. I think the
> right place to write this down is Repository.get_graph and
> get_graph_with_ghosts.

Yes, or in the documentation for Knit.

-- 
Martin