1482222 - moz-phab does not restore repository after successful completion and messes up histedit

Status: RESOLVED WORKSFORME

(Conduit :: moz-phab, defect)

Description

[Tom Ritter [:tjr]]

I had commits like [-central] -> A -> B -> C -> D

I ran the command 'moz-phab submit B C', but after it completed, it did not restore D.

Here is a try push showing my tree before running moz-phab:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=8209629c03c647c33ab1d86afb4d8bb3831847b8

Then I run moz-phab as shown here:

moz-phab submit 480792 480793 -r Alex_Gaynor
Submitting 2 commits:
480793:472c5aaa6002 Bug 1478061 Use a separate toolchain for CFI vcall r=Alex_Gaynor
480792:b1b40def0337 Bug 1478061 Add CFI vcall r=Alex_Gaynor
b1b40def0337 is based off non-public commit d8306504c69c
Submit to Phabricator (YES/No/Always)? YES

Creating new revision:
480792:b1b40def0337 Bug 1478061 Add CFI vcall r=Alex_Gaynor
 SKIP STAGING  Phabricator does not support staging areas for this repository.
Linting...
No lint engine configured for this project.
Running unit tests...
No unit test engine is configured for this project.
Created a new Differential revision:
        Revision URI: https://phabricator.services.mozilla.com/D3050

Included changes:
  M       build/moz.configure/toolchain.configure
  A       build/sanitizers/cfi_suppressions.txt
created new head
rebasing 480792:b1b40def0337 "Bug 1478061 Add CFI vcall"
rebasing 480797:dff985e0254b "Bug 1478061 Add CFI vcall r=Alex_Gaynor" (tip)
rebasing 480793:472c5aaa6002 "Bug 1478061 Use a separate toolchain for CFI vcall"
rebasing 480794:3d69696be880 "Disable elfhack"

Creating new revision:
480799:e99ffff38862 Bug 1478061 Use a separate toolchain for CFI vcall r=Alex_Gaynor
 SKIP STAGING  Phabricator does not support staging areas for this repository.
Linting...
No lint engine configured for this project.
Running unit tests...
No unit test engine is configured for this project.
Created a new Differential revision:
        Revision URI: https://phabricator.services.mozilla.com/D3051

Included changes:
  A       build/build-clang/clang-cfi-linux64.json
  M       taskcluster/ci/build/linux.yml
  M       taskcluster/ci/toolchain/linux.yml
  A       taskcluster/scripts/misc/build-clang-cfi-linux.sh
created new head
rebasing 480799:e99ffff38862 "Bug 1478061 Use a separate toolchain for CFI vcall"
rebasing 480803:7b366443cb7b "Bug 1478061 Use a separate toolchain for CFI vcall r=Alex_Gaynor" (tip)
rebasing 480800:a938d1e54b3e "Disable elfhack"
saved backup bundle to /home/tom/Documents/moz/mozilla-unified-cfi-vcall/.hg/strip-backup/e99ffff38862-97de45e8-backup.hg

Completed
480793:50a534d0cbda Bug 1478061 Use a separate toolchain for CFI vcall r=Alex_Gaynor
-> https://phabricator.services.mozilla.com/D3051
480792:2804790a9996 Bug 1478061 Add CFI vcall r=Alex_Gaynor
-> https://phabricator.services.mozilla.com/D3050




Then I run hg log:

changeset:   480793:50a534d0cbda
user:        Tom Ritter <[e-mail]>
date:        Thu Aug 09 11:03:50 2018 -0500
summary:     Bug 1478061 Use a separate toolchain for CFI vcall r=Alex_Gaynor

(It's missing the 'Disable elfhack commit')



I import that manually from my try push and run hg histedit.  I expect to see all my local changes; however I only see the single commit I just imported.

Comment 1

[:glob ✱]

can you provide the output of `hg wip`.

if you have the opportunity to perform the same actions again, can you set the DEBUG env var and provide the generated output:
`DEBUG=1 moz-phab submit ...`


thanks.

Comment 2

[Tom Ritter [:tjr]]

Attachment: log.txt

It looks like it might just be missing a final `hg up` to the original revision.

Comment 3

[:glob ✱]

ah, good.  it looks like your tree is correct; it's just, as you say, the current revision is left on the last submitted commit instead of the one that was active prior to submission.

this fix for this probably involves:
- refactoring the calls to hg that perform successor mapping out of Mercurial.refresh_commit_stack() to a separate helper method
- storing the current revision's sha1 before submission starts
- after submission use the successor mapping method to get the updated sha1 from the starting point
- `hg update {sha}`

Comment 4

[Chris Peterson [:cpeterson]]

I hit this situation when I tried to push the following changeset to Phabricator using moz-phab:

  "Bug 1336712 - Remove performance.now() test checks for Windows XP. r?bz"
  https://hg.mozilla.org/try/rev/05bb8b177a52290a175f9ec1835a5fe4a30eb46d

and then moz-phab aborted the submission on an error:

Creating new revision:
493060:6ecff2d9e8f3 Bug 1336712 - dom: Remove performance.now() test checks for Windows XP. r?bz
 Exception 
Error parsing field "Reviewers": The objects you have listed include objects which do not exist (bz).
(Run with `--trace` for a full exception trace.)
Exception: command 'arc' failed to complete successfully

Seeing that the subsequent changesets appeared to be lost after moz-phab aborted with an error was a scary few moments. :) Fortunately `hg wip` showed that I just needed to checkout my bookmark again.

Comment 5

[Piotr Zalewa [:zalun]]

I think we're OK to close this bug.
moz-phab should leave the tree untouched if submission was successful.
Comment #4 is about a different issue.

Comment 6

[Chris Peterson [:cpeterson]]

(In reply to Piotr Zalewa [:zalun] from comment #5)
> I think we're OK to close this bug.
> moz-phab should leave the tree untouched if submission was successful.
> Comment #4 is about a different issue.

Thanks. SGTM!