1232738 - Autoland should use "internal:merge"

Status: RESOLVED FIXED

(MozReview Graveyard :: General, defect)

Description

[Dave Townsend [:mossop]]

I attempted an autoland with this review: https://reviewboard.mozilla.org/r/27677/

It failed to rebase so I rebased myself, pushed to review and then tried again. It failed again with this error::

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. hg error in cmd: hg rebase -s 84603e4d1109 -d ec86fd78c958: rebasing 296730:84603e4d1109 "Bug 1231828: Fix JS parsing errors in toolkit and browser XBL files. r=Felipe" (tip) unresolved conflicts (see hg resolve, then hg rebase --continue) 

I have no idea what source changeset it is using (84603e4d1109). That isn't the revision I pushed and doesn't appear in the review repository.

Rebasing my local changesets on tip of ec86fd78c958 worked with no conflicts so I don't know what code it was trying to rebase.

Comment 1

[Dan Minor [:dminor]]

I believe what has happened is that you have r=felipe in your commit string, but the user name in MozReview is Felipe, so Autoland amended your commit message to be r=Felipe, which is where commit 84603e4d1109 came from.

I'll investigate why the rebase failed.

Comment 2

[Dan Minor [:dminor]]

We set "merge = internal:fail" on autoland to prevent manual merges when autolanding.

If I run with this setting locally, the rebase fails as on autoland. If I run without this setting, I see "merging toolkit/content/jar.mn" in the output, but no manual merge is required, so it seems like this setting is too aggressive in preventing manual merges.

I recently overrode the default hglib behaviour to run a non-interactive session in autoland, so we might be able to remove "merge = internal:fail" now.

Comment 3

[Gregory Szorc [:gps]]

So, a file merge occurs when *any* part of the file changes, not just the part inside the patch/diff. I'm pretty sure internal:fail will cause all file merges to fast fail.

We want to use internal:merge. If there are conflicts, it will leave markers and abort and `hg resolve` will be required to mark files as resolved before continuing. This error should cause autoland to fail, which is the appropriate thing to happen in case of merge conflict (we want humans to resolve the problem).

We'll definitely want test coverage for file merge handling.

Comment 4

[Dan Minor [:dminor]]

Attachment: MozReview Request: autoland: use internal:merge (bug 1232738) r?gps

Review commit: https://reviewboard.mozilla.org/r/28045/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/28045/

Comment 5

[Gregory Szorc [:gps]]

Comment on attachment 8698610 [details]
MozReview Request: autoland: use internal:merge (bug 1232738) r?gps

https://reviewboard.mozilla.org/r/28045/#review25119

There is plenty of room for follow-up work to improve the messaging on failure. It would be really nice if we could tell people exactly what command to run to perform the rebase, for example. But that can be deferred to another bug.