1253393 - Autoland failed while rewriting or rebasing commits

Status: RESOLVED FIXED

(Conduit Graveyard :: Transplant, defect)

Description

[:Felipe Gomes (needinfo for replies!)]

Got the message telling me to file a bug:
"We're sorry - something has gone wrong while rewriting or rebasing your commits. The commits being pushed no longer match what was requested. Please file a bug."

From https://reviewboard.mozilla.org/r/36655/

One thing I noticed (not sure if it's related or not): in the Autoland dialog, the first patch has a msg saying "Warning: commit has changed since review was granted", even though that patch didn't change.

Comment 1

[Dan Minor [:dminor]]

Thank you for filing!

Relevant portion of the autoland log:

02:48:56,41 autoland INFO initiating transplant from tree: gecko rev: 7bb0cd4180684b52edcdc428ae69ea66184a31b3 to destination: ssh://hg.mozilla.org/integration/mozilla-inbound
02:49:18,323 autoland INFO base revision: 09ab134f2c0f
02:52:28,153 autoland INFO rebased (tip) revision: 86e363e39707
02:52:29,280 autoland ERROR unexpected outgoing commits:
02:52:29,280 autoland ERROR outgoing: b5aab6d60a1fcdf13afdd881d9ff9d7916a2cc56: Bug 1249845 - Set up structure for e10srollout system add-on. r=glandium,standard8

MozReview-Commit-ID: LUbiKq9rimo
02:52:29,280 autoland ERROR outgoing: 86e363e39707ad6e9627a3187d17fffbb69ca5c1: Bug 1249845 - Store the e10s rollout cohort in the telemetry environment. r=gfritzsche

MozReview-Commit-ID: BUPA7bEv8RC
02:52:29,340 autoland INFO transplant failed: tree: gecko rev: 7bb0cd4180684b52edcdc428ae69ea66184a31b3 destination: ssh://hg.mozilla.org/integration/mozilla-inbound error: We're sorry - something has gone wrong while rewriting or rebasing your commits. The commits being pushed no longer match what was requested. Please file a bug.
02:52:29,340 autoland INFO elapsed transplant time: 0:03:33.299074

Comment 2

[Dan Minor [:dminor]]

Based on the output, it looks like the last commit (7bb0cd418068 on MozReview) was going to be dropped from the push and so Autoland aborted instead. Not sure about the cause right now.

Comment 3

[Gregory Szorc [:gps]]

If Mercurial rebases a changeset that produces no diff (the changeset has already landed for example), it will drop the changeset automatically. That may have been what happened here. Since autoland verifies # input changesets == # output changesets, this behavior would cause a mismatch and cause autoland to abort.

There's no easy way to detect this scenario. Although now that we have MozReview-Commit-ID, we could potentially look at that and trust Mercurial. But we may want to report discards. Not sure if you can detect that easily from the public Mercurial APIs.

Comment 4

[Mark Côté [:mcote]]

I just got this same error when trying to autoland https://reviewboard.mozilla.org/r/60132/.  I too did a rebase without changes that led to the warning mentioned in comment 0.  glob says that there's a record of the second commit trying to land without the first.

Comment 5

[:glob ✱]

this happened to me on bug 1284678, so i dug deeper.
the bug has three commits: a67a8901d325, c1f3035316c6, and cf645bfe6b69.

the request from reviewboard:

{
  "push_bookmark": "@",
  "tree": "version-control-tools",
  "destination": "ssh://hg.mozilla.org/hgcustom/version-control-tools",
  "rev": "cf645bfe6b69e4d4e21dc92b42908a48f8765ef4",
  "ldap_username": "[e-mail]",
  "pingback_url": "https://reviewboard.mozilla.org/api/extensions/mozreview.extension.MozReviewExtension/autoland-request-updates/",
  "commit_descriptions": {
    "a67a8901d325": "mozreview: refactor bugzilla flag code (bug 1284678) r=smacleod..",
    "c1f3035316c6": "mozreview: Don't clear feedback flags unless a new patch is pushed (bug 1284678) r=gps,smacleod..",
    "cf645bfe6b69": "testing: add tests for feedback flag management (bug 1284678) r=gps,smacleod.."
  }
}

this created a single row in the database:

id           5567
destination  ssh://hg.mozilla.org/hgcustom/version-control-tools
request      {..json..}
landed       
result       
last_updated
created      2016-07-13 06:04:24.074729

here's the log entries from the landing attempt:

06:04:24,230 autoland INFO initiating transplant from tree: version-control-tools rev: cf645bfe6b69e4d4e21dc92b42908a48f8765ef4 to destination: ssh://hg.mozilla.org/hgcustom/version-control-tools
06:04:25,179 autoland INFO base revision: 329503b5e09c
06:04:25,297 autoland INFO transplant failed - manual rebase required: tree: version-control-tools rev: cf645bfe6b69e4d4e21dc92b42908a48f8765ef4 destination: ssh://hg.mozilla.org/hgcustom/version-control-tools error: hg error in cmd: hg rewritecommitdescriptions --descriptions=/tmp/tmpRS4LD3 cf645bfe6b69e4d4e21dc92b42908a48f8765ef4: rebasing 4491:329503b5e09c "mozreview: Don't clear feedback flags unless a new patch is pushed (bug 1284678) r=gps,smacleod"
merging pylib/mozreview/mozreview/bugzilla/attachments.py
warning: conflicts while merging pylib/mozreview/mozreview/bugzilla/attachments.py! (edit, then use 'hg resolve --mark')
unresolved conflicts (see hg resolve, then hg rebase --continue)
06:04:25,298 autoland INFO elapsed transplant time: 0:00:01.067161
06:04:25,301 autoland INFO trying to post mozreview update to: https://reviewboard.mozilla.org/api/extensions/mozreview.extension.MozReviewExtension/autoland-request-updates/ for request: 5567
06:04:25,302 requests.packages.urllib3.connectionpool INFO Starting new HTTPS connection (1): reviewboard.mozilla.org
06:04:25,653 requests.packages.urllib3.connectionpool DEBUG "POST /api/extensions/mozreview.extension.MozReviewExtension/autoland-request-updates/ HTTP/1.1" 200 14

i'm not sure, but looks like it picked the wrong base revision to rebase.

Comment 6

[Gregory Szorc [:gps]]

I'd have to look at the code, but it is worrying that the request to autoland only contains the tip changeset (cf645bfe6b69e4d4e21dc92b42908a48f8765ef4). If we don't specify a base changeset to begin the rebase from, bad things can happen. We should specify the base and tip revisions to rebase, otherwise behavior is undefined.

Comment 7

[:glob ✱]

i think i know what's going on..

the base revision is detected by the rewritecommitdescriptions extension:
https://github.com/mozilla/version-control-tools/blob/master/autoland/autoland/transplant.py#L104-L111
and used to rebase
https://github.com/mozilla/version-control-tools/blob/master/autoland/autoland/transplant.py#L125

rewritecommitdescriptions returns the "oldest modified commit"
https://github.com/mozilla/version-control-tools/blob/master/autoland/hgext/rewritecommitdescriptions.py#L93-L95

however that's the oldest commit that required a commit description rewrite:
https://github.com/mozilla/version-control-tools/blob/master/autoland/hgext/rewritecommitdescriptions.py#L57-L64


when i work i'll update the commit message in my local repo to r= once that revision has received an r+.  this helps me keep track of the work i need to do.

so the descriptions of the revisions on reviewboard-hg are:

a67a8901d325 mozreview: refactor bugzilla flag code (bug 1284678) r=smacleod 
c1f3035316c6 mozreview: Don't clear feedback flags unless a new patch is pushed (bug 1284678) r=smacleod
cf645bfe6b69 testing: add tests for feedback flag management (bug 1284678) r=smacleod 

i received an r+ from gps on c1f3035316c6 and cf645bfe6b69, so those revisions needed to be rewritten by autoland to add gps to the r= identifier.  however a67a8901d325's description didn't need modification, so it was skipped by prune_unchanged(), resulting in it incorrectly trying to rebase off c1f3035316c6/329503b5e09c.


rewritecommitdescriptions does output a message when this happens:
https://github.com/mozilla/version-control-tools/blob/master/autoland/hgext/rewritecommitdescriptions.py#L62
this is consumed by run_hg and removed to extract the base revision:
https://github.com/mozilla/version-control-tools/blob/master/autoland/autoland/transplant.py#L111-L116

Comment 8

[:glob ✱]

to test my theory i updated the commit messages to:

557b07a77023 mozreview: refactor bugzilla flag code (bug 1284678) r?smacleod
26035ccd4e70 mozreview: Don't clear feedback flags unless a new patch is pushed (bug 1284678) r?smacleod
13725902c4de testing: add tests for feedback flag management (bug 1284678) r?smacleod

autoland was able to land these changes.

Comment 9

[:glob ✱]

the easiest fix for this is to add a "base_rev" property to the autoland request.

the warning/error logging situation from the rewritecommitdescriptions extension needs addressing too; probably by ensuring they are written to stderr instead of stdout, and updating transplat.py to log all stderr output to the autoland log file.

Comment 10

[:glob ✱]

Attachment: autoland: fix PEP8 issues (bug 1253393)

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

Comment 11

[:glob ✱]

Attachment: autoland: Update rewritecommitdescriptions to output all revisions (bug 1253393)

Under certain conditions rewritecommitdescriptions is selecting the wrong
revision as to base the rebase off.

Rework the extension to never skip revisions listed in the
commit-descriptions.json, and output the old and new sha1's for every commit to
simplify debugging.

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

Comment 12

[:glob ✱]

Attachment: autoland: add test for rewritecommitdescriptions with partial rewrites (bug 1253393)

Adds a test where we update the description of the second revision, but not the
first.

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

Comment 13

[Gregory Szorc [:gps]]

Comment on attachment 8772716 [details]
autoland: fix PEP8 issues (bug 1253393)

https://reviewboard.mozilla.org/r/65486/#review62642

Comment 14

[Steven MacLeod [:smacleod]]

Comment on attachment 8772716 [details]
autoland: fix PEP8 issues (bug 1253393)

https://reviewboard.mozilla.org/r/65486/#review62682

Comment 15

[Gregory Szorc [:gps]]

Comment on attachment 8772717 [details]
autoland: Update rewritecommitdescriptions to output all revisions (bug 1253393)

https://reviewboard.mozilla.org/r/65488/#review63208

Comment 16

[Gregory Szorc [:gps]]

Comment on attachment 8772718 [details]
autoland: add test for rewritecommitdescriptions with partial rewrites (bug 1253393)

https://reviewboard.mozilla.org/r/65490/#review63210

Comment 17

[Steven MacLeod [:smacleod]]

Comment on attachment 8772717 [details]
autoland: Update rewritecommitdescriptions to output all revisions (bug 1253393)

https://reviewboard.mozilla.org/r/65488/#review62684

I had a few questions open when I was reviewing this earlier, but I was able to answer them myself :D. I didn't do a fully thorough review since gps has already reviewed this, but it looks good to me

Comment 18

[Steven MacLeod [:smacleod]]

Comment on attachment 8772718 [details]
autoland: add test for rewritecommitdescriptions with partial rewrites (bug 1253393)

https://reviewboard.mozilla.org/r/65490/#review64128

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/hgcustom/version-control-tools/rev/b908c0161912
autoland: fix PEP8 issues r=gps,smacleod
https://hg.mozilla.org/hgcustom/version-control-tools/rev/cfc505c2a26a
autoland: Update rewritecommitdescriptions to output all revisions r=gps,smacleod
https://hg.mozilla.org/hgcustom/version-control-tools/rev/a6cbb6b02a74
autoland: add test for rewritecommitdescriptions with partial rewrites r=gps,smacleod

Comment 20

[:glob ✱]

deployed.