vcssync: wrong revision selected as destination after merge

RESOLVED FIXED

Status

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: glob, Assigned: glob)

Tracking

Details

Attachments

(1 attachment)

(Assignee)

Description

2 years ago
the code walks the dest revision's ancestors and stops when it finds one that was overlaid.  however this code chooses the incorrect revision following the merge of faba0400cbe2.

this is because mercurial's ancestors() method doesn't traverse in purely reverse storage order when merges are present: it only guarantees that a parent is emitted after all its children.

we'd likely want to do something like
> sorted(changelog.ancestors(), reverse=True)
Comment hidden (mozreview-request)
(Assignee)

Comment 2

2 years ago
Comment on attachment 8837587 [details]
overlay: fix iteration of destination revision's ancestors (bug 1339689)

found some issues with this.
Attachment #8837587 - Attachment is obsolete: true
Attachment #8837587 - Flags: review?(smacleod)
Comment hidden (mozreview-request)

Comment 4

2 years ago
mozreview-review
Comment on attachment 8837587 [details]
overlay: fix iteration of destination revision's ancestors (bug 1339689)

https://reviewboard.mozilla.org/r/112712/#review114178

::: hgext/overlay/__init__.py:183
(Diff revision 2)
>                           sourcecl.ancestors([sourcerevs[-1]], inclusive=True))
>  
>      # Attempt to find an incoming changeset in dest and prune already processed
>      # source revisions.
>      lastsourcectx = None
> -    for rev in destrepo.changelog.ancestors([destctx.rev()], inclusive=True):
> +    for rev in sorted(destrepo.changelog.ancestors([destctx.rev()],

I'm wondering, how fast is this operation on a repository of mozilla-central's size? the ancestors of our changeset is pretty much the entire repository history :/. Does this lazily sort or something?
(Assignee)

Comment 5

2 years ago
mozreview-review-reply
Comment on attachment 8837587 [details]
overlay: fix iteration of destination revision's ancestors (bug 1339689)

https://reviewboard.mozilla.org/r/112712/#review114178

> I'm wondering, how fast is this operation on a repository of mozilla-central's size? the ancestors of our changeset is pretty much the entire repository history :/. Does this lazily sort or something?

i didn't notice any performance difference between that and the original line on a slow ec2 instance.

Comment 6

2 years ago
mozreview-review-reply
Comment on attachment 8837587 [details]
overlay: fix iteration of destination revision's ancestors (bug 1339689)

https://reviewboard.mozilla.org/r/112712/#review114178

> i didn't notice any performance difference between that and the original line on a slow ec2 instance.

Okay, that's good to know, dropping.

Comment 7

2 years ago
mozreview-review
Comment on attachment 8837587 [details]
overlay: fix iteration of destination revision's ancestors (bug 1339689)

https://reviewboard.mozilla.org/r/112712/#review114186

Tentative r+, please also provide a test - or if it's important to get things running, you can land but followup with a test.
Attachment #8837587 - Flags: review?(smacleod) → review+

Comment 8

2 years ago
Pushed by bjones@mozilla.com:
https://hg.mozilla.org/hgcustom/version-control-tools/rev/e01531f0d4e1
overlay: fix iteration of destination revision's ancestors r=smacleod
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
(Assignee)

Updated

2 years ago
Blocks: 1340032
(Assignee)

Comment 9

2 years ago
landed and pushed to prod.
filed bug 1340032 for the tests, which i'll work on once some other prod issues are resolved.
(Assignee)

Updated

2 years ago
Component: General → Servo VCS Sync
You need to log in before you can comment on or make changes to this bug.