Incorrect results from annotate on packs

Lukáš Lalinský lalinsky at gmail.com
Sat Dec 29 08:53:39 GMT 2007


On Dec 29, 2007 9:14 AM, Aaron Bentley <aaron at aaronbentley.com> wrote:
> Lukáš Lalinský wrote:
> > On Dec 29, 2007 5:52 AM, Aaron Bentley <aaron at aaronbentley.com> wrote:
> >> As for why it's attributed to revno 3052, that version is a merge of
> >> 'lalinsky at gmail.com-20071128172710-eaf9o9y8vw2z5joa' and
> >> 'abentley at panoramicfeedback.com-20071127194032-f51sqjvldlwz7nc1'.
> >>
> >> These versions disagree about the origin of the line, so our rule is to
> >> assign it to the current revision, i.e. revno 3052.
> >
> > I'm afraid I don't see this. According to bzr log,
> > 'pqm at pqm.ubuntu.com-20071129184101-u9506rihe4zbzyyz' is a merge of
> > 'pqm at pqm.ubuntu.com-20071129180655-yv661adx0qb6a50z' and
> > 'abentley at panoramicfeedback.com-20071129173223-vxdci7trlqn5h083'.
> > 'lalinsky at gmail.com-20071128172710-eaf9o9y8vw2z5joa' has only one
> > child revision, and it's a single-parent revision, not a merge. Same
> > for 'abentley at panoramicfeedback.com-20071127194032-f51sqjvldlwz7nc1',
> > except that in this case the revision itself is a merge. But as far as
> > I can see, neither of these revision touches the copyright line
> > against any of their parents.
>
> A version need not touch a line in order to be annotated as the
> originator.  If a version merges two versions that each claim to have
> originated the line, that version is marked as the originator of the line.
>
> >>> Any ideas why it gives wrong results on packs?
> >> I'd say it's too early to say that the packs result is wrong.
> >
> > I still think it's wrong.
>
> Well, I don't.  My research shows that the ancestry of this line is
> quite complicated.  I've attached a DAG in dot and SVG format, as well
> as the patch I used to generate it.
>
> But to me it looks like the line was originated in four different
> revisions, and until revno 3052, we never reached synchronization.
>
> It does seem like it ought to allow only head revisions to be candidate
> origins.

I think we have different definitions of "annotate", see below.

> > I can understand why results on knits might
> > be different or possibly wrong, but on packs it reannotates everything
> > with the same sequence matcher, without any historical bugs.
>
> You are talking of your code or the pack implementation of annotate?

I was comparing knits with cached annotation and possible bugs in them
against anything else. Sure, you still use matching blocks from knit
deltas, which are not produced by patiencediff, but that doesn't
affect the case of merges we have here.

> > And I
> > really don't see how PQM could be author of any line if it refuses a
> > clean merge
>
> I'm not sure what you mean.  PQM could not work if it refused clean merges.

Sorry, I meant non-clean merge.

> > (and the merge algorithm uses the same sequence matcher as
> > annotate)
>
> Basically everything in bzrlib uses patience diff.

But this wasn't always the case.

> > in which case the lines should be attributed to their real
> > authors, not PQM.
>
> Doesn't follow.  In this case, it looks like the revision for
> attribution would be
> "mbp at sourcefrog.net-20070221053456-vyr6o0ehqnbetrvb", because this is
> the first revision I see that is a descendant of all four originating
> revisions.  But it could just as easily have been
> pqm at pqm.ubuntu.com-20070213114814-9606106906ac312f, if only
> mbp at sourcefrog.net-20070213052239-09atqsahwth6zdm1 and
> abentley at panoramicfeedback.com-20070209182321-b4tw6uhxfx1o4xqi has
> originated the line.

As I said, we seem to have different definitions of what annotate
should do. With knits it shows the first revision, according to the
topological order from bzrlib.topo_sort, in which the line appeared.
That is, if you have a revision C, which merges A and B, both of which
independantly change the same line the same way, the one is that is
lower in the topo order wins (or differently, you never overwrite
origins of identical lines with newer revisions). But
annotate.reannotate uses revision C, because revisions A and B don't
agree on the origin. I personally don't find this information useful,
but at least now I know it's correct according to the current
definition in bzrlib. I expected annotate to behave the same way on
knits and packs, so I thought it was incorrect.

Thanks,

Lukas



More information about the bazaar mailing list