Page MenuHomePhabricator
Create Task
Maniphest T9555

Exception with 'arc land' after manually rebasing a revision and resolving conflicts, leaves repository in unclean state
Closed, InvalidPublic
Actions

Description

It looks like arc land throws an exception when rebasing with conflicts or squashing commits that were rebased and contained conflicts (which continues to think there are conflicts... but I haven't discovered why). The resulting state of the repository is quite confusing and contains uncommitted changes.

Versions for arc/libphutil
cspeckrun@specktop ~/P/n/hgtest> arc version
arcanist 1773aad8559965f8896cd66087710ddbeee153ae (10 Oct 2015)
libphutil c5a7d67db29431dc67f930473f967c46477a7ad7 (10 Oct 2015)

To reproduce:

  1. With Mercurial repository create new branch off of non-head -- also ensure the repository has "history.immutable" : "false"
  2. Create several commits, with conflicting changes -- I believe this is key: there must be multiple commits not amended/squashed and there must be a conflict to disallow a clean rebase
State prior to any rebase or land
cspeckrun@specktop ~/P/n/hgtest> hg wip
@   41:2c012896d487 cspeck (3 seconds ago) tip test
|     addfile
o   40:1fd75fc107c0 cspeck (53 seconds ago)
|     more mods
o   39:82bbd834fa66 cspeck (4 minutes ago)
|     creating a branch
o   38:efb1653d421c cspeck (8 minutes ago)                <--- modifies 'readme' causing conflicts
|     modifying readme for COLLISIONS
o   37:c1a6ee8a65cf cspeck (9 minutes ago)
|     creating a branch
| o   36:d721d5b57fc9 cspeck (21 hours ago) master        <--- there are actually 15 commits between 'master' and revision 20 where the branch is rooted. within these commits have changes to the 'readme'
|/      more test data
o   20:136bb8213584 cspeck (8 days ago)
|     test hg.....
  1. Attempting to land fails due to conflict, manually rebase and resolve conflicts
  2. Run arc diff to update the revision after rebasing and resolving conflicts:
Update diff after manual rebase/conflict-resolve
cspeckrun@specktop ~/P/n/hgtest> arc diff
Linting...
No lint engine configured for this project.
Running unit tests...
No unit test engine is configured for this project.
 SKIP STAGING  Phabricator does not support staging areas for this repository.
Updated an existing Differential revision:
        Revision URI: http://192.168.0.133/D1

Included changes:
  A       .arcconfig
  M       README
  A       newfile
  A       supernewfile
cspeckrun@specktop ~/P/n/hgtest> hg wip
@   41:945ead78d871 cspeck (3 minutes ago) tip test
|     addfile
o   40:364c8f96f85c cspeck (4 minutes ago)
|     more mods
o   39:35f26c87dd0f cspeck (7 minutes ago)
|     creating a branch
o   38:2ae07ac41111 cspeck (11 minutes ago)
|     modifying readme for COLLISIONS
o   37:16a239549c36 cspeck (12 minutes ago)
|     creating a branch
o   36:d721d5b57fc9 cspeck (21 hours ago) master
|     more test data
  1. Run arc land then get placed in an unusual state
Running `land` throws exception
cspeckrun@specktop ~/P/n/hgtest> arc land test
Updating master...
The following commit(s) will be landed:

945ead78d871 addfile
364c8f96f85c more mods
35f26c87dd0f creating a branch
2ae07ac41111 modifying readme for COLLISIONS
16a239549c36 creating a branch

Switched to bookmark test. Identifying and merging...


    Revision 'D1: creating a branch' has not been accepted. Continue anyway?
    [y/N] y

Landing revision 'D1: creating a branch'...
 BUILDS PASSED  Harbormaster builds for the active diff completed successfully.
rebasing 37:16a239549c36 "creating a branch"
note: rebase of 37:16a239549c36 created no changes to commit
rebasing 38:2ae07ac41111 "modifying readme for COLLISIONS"
note: rebase of 38:2ae07ac41111 created no changes to commit
rebasing 39:35f26c87dd0f "creating a branch"
note: rebase of 39:35f26c87dd0f created no changes to commit
rebasing 40:364c8f96f85c "more mods"
unresolved conflicts (see hg resolve, then hg rebase --continue)
Exception
Command failed with error #255!
COMMAND
HGPLAIN=1 hg checkout 'test'

STDOUT
(empty)

STDERR
abort: outstanding uncommitted merge

(Run with `--trace` for a full exception trace.)
  1. At this point the repository ends up in an unclean state. Sometimes it appears as though my working set is at two different revisions. I think Mercurial is in the middle of a merge due to the squash because hg summary indicates:
In merge with uncommitted changes
cspeckrun@specktop ~/P/n/hgtest> hg summ
parent: 36:d721d5b57fc9 
 more test data
parent: 40:364c8f96f85c 
 more mods
branch: default
bookmarks: [test] master
commit: 1 modified, 2 added, 1 unresolved (merge)
update: 1 new changesets (update)
phases: 5 draft

Looking at ArcanistLandWorkflow and doing some investigation into the commands, I think this is what happens:

  1. Squash is performed by using hg rebase --collapse - [[ https://secure.phabricator.com/diffusion/ARC/browse/master/src/workflow/ArcanistLandWorkflow.php;172c930630a960ea79ada1dbeb50f2ad561d3870$840 | L840 ]]
  2. The squash fails so an attempt to abort is made by doing hg rebase --abort - [[ https://secure.phabricator.com/diffusion/ARC/browse/master/src/workflow/ArcanistLandWorkflow.php;172c930630a960ea79ada1dbeb50f2ad561d3870$840 | L847 ]]
  3. The abort does not clear out the working state, leaving uncommitted files
  4. Attempting to switch back to the bookmark using hg checkout results in failure/exception due to uncommitted changes [[ https://secure.phabricator.com/diffusion/ARC/browse/master/src/workflow/ArcanistLandWorkflow.php;172c930630a960ea79ada1dbeb50f2ad561d3870$1220 | L1220 ]]

Adding --clean to the checkout command fixes this:

-C --clean      discard uncommitted changes (no backup)

However it looks like there's some reliability on the checkout command being cross-VCS-compatible.

I have in my .hgrc my merge behavior set to merge=internal:fail which I believe is why I'm seeing the squash fail. When I remove this option the squash succeeds, although it does indicate that one of the rebased commits requires a merge. I'm guessing this is required as part of internal Mercurial for doing the squash, it essentially has to re-apply the change back on to master and resolve conflicts even though it was a descendant to begin with.

Related Objects

Event Timeline

cspeckmim created this task.Oct 13 2015, 4:06 AM
cspeckmim updated the task description. (Show Details)
cspeckmim added projects: Mercurial, Arcanist.
cspeckmim added a subscriber: cspeckmim.
cspeckmim updated the task description. (Show Details)Oct 13 2015, 4:11 AM
cspeckmim added a comment.Oct 13 2015, 4:23 AM

After fixing my .hgrc to allow for default merge behavior, and the landing continues past the squashing, I got this exception:

P1864 Remote Exception with `arc land`
1cspeckrun@specktop ~/P/n/hgtest> arc land test --trace
2libphutil loaded from '/Users/cspeckrun/Projects/phacility/libphutil/src'.
3arcanist loaded from '/Users/cspeckrun/Projects/phacility/arcanist/src'.
4Config: Reading user configuration file "/Users/cspeckrun/.arcrc"...
5Config: Did not find system configuration at "/etc/arcconfig".
6Working Copy: Reading .arcconfig from "/Users/cspeckrun/Projects/neandrake/hgtest/.arcconfig".
7Working Copy: Path "/Users/cspeckrun/Projects/neandrake/hgtest" is part of `hg` working copy "/Users/cspeckrun/Projects/neandrake/hgtest".
8Working Copy: Project root is at "/Users/cspeckrun/Projects/neandrake/hgtest".
9Config: Did not find local configuration at "/Users/cspeckrun/Projects/neandrake/hgtest/.hg/arc/config".
10>>> [0] <conduit> user.whoami() <bytes = 117>
11>>> [1] <http> http://192.168.0.133/api/user.whoami
12<<< [1] <http> 114,236 us
13<<< [0] <conduit> 114,780 us
14>>> [2] <exec> $ HGPLAIN=1 hg bookmarks
15<<< [2] <exec> 96,922 us
16>>> [3] <exec> $ HGPLAIN=1 hg bookmarks
17<<< [3] <exec> 106,534 us
18>>> [4] <exec> $ HGPLAIN=1 hg bookmarks
19<<< [4] <exec> 112,179 us
20>>> [5] <exec> $ HGPLAIN=1 hg help rebase
21<<< [5] <exec> 118,990 us
22>>> [6] <exec> $ HGPLAIN=1 hg status
23<<< [6] <exec> 134,823 us
24Updating master...
25>>> [7] <exec> $ HGPLAIN=1 hg pull
26<<< [7] <exec> 2,007,539 us
27>>> [8] <exec> $ HGPLAIN=1 hg branch
28<<< [8] <exec> 97,104 us
29>>> [9] <exec> $ HGPLAIN=1 hg help phase
30<<< [9] <exec> 104,958 us
31>>> [10] <exec> $ HGPLAIN=1 hg log -r 'master' --template '{phase}'
32<<< [10] <exec> 108,314 us
33>>> [11] <exec> $ HGPLAIN=1 hg log -l 1 --template '{node}' -r 'ancestor('\''master'\'','\''test'\'')' --
34<<< [11] <exec> 107,554 us
35>>> [12] <exec> $ HGPLAIN=1 hg log -r 'reverse(('\''d721d5b57fc9ef72e47ff9d4e0c583d74a46590c'\''::'\''test'\'') - '\''d721d5b57fc9ef72e47ff9d4e0c583d74a46590c'\'')' --template '{node|short} {desc|firstline}\n'
36<<< [12] <exec> 106,848 us
37The following commit(s) will be landed:
38
39945ead78d871 addfile
40364c8f96f85c more mods
4135f26c87dd0f creating a branch
422ae07ac41111 modifying readme for COLLISIONS
4316a239549c36 creating a branch
44
45>>> [13] <exec> $ HGPLAIN=1 hg bookmarks
46<<< [13] <exec> 103,020 us
47Switched to bookmark test. Identifying and merging...
48>>> [14] <exec> $ HGPLAIN=1 hg log -l 1 --template '{node}' -r 'ancestor('\''master'\'',.)' --
49<<< [14] <exec> 116,019 us
50>>> [15] <exec> $ HGPLAIN=1 hg log --template '{node}{desc}' --rev '('\''d721d5b57fc9ef72e47ff9d4e0c583d74a46590c'\''::. - '\''d721d5b57fc9ef72e47ff9d4e0c583d74a46590c'\'')' --branch 'default' --
51<<< [15] <exec> 109,840 us
52>>> [16] <conduit> differential.query() <bytes = 139>
53>>> [17] <http> http://192.168.0.133/api/differential.query
54<<< [17] <http> 135,782 us
55<<< [16] <conduit> 136,002 us
56
57
58 Revision 'D1: creating a branch' has not been accepted. Continue anyway?
59 [y/N] y
60
61>>> [18] <conduit> differential.getcommitmessage() <bytes = 147>
62>>> [19] <http> http://192.168.0.133/api/differential.getcommitmessage
63<<< [19] <http> 129,463 us
64<<< [18] <conduit> 129,780 us
65Landing revision 'D1: creating a branch'...
66>>> [20] <conduit> harbormaster.querybuildables() <bytes = 218>
67>>> [21] <http> http://192.168.0.133/api/harbormaster.querybuildables
68<<< [21] <http> 126,710 us
69<<< [20] <conduit> 126,976 us
70 BUILDS PASSED Harbormaster builds for the active diff completed successfully.
71>>> [22] <exec> $ HGPLAIN=1 hg log -l 1 --template '{node}' -r 'master' --
72<<< [22] <exec> 109,714 us
73>>> [23] <exec> $ HGPLAIN=1 hg log -l 1 --template '{node}' -r 'ancestor('\''master'\'', '\''test'\'')' --
74<<< [23] <exec> 117,455 us
75>>> [24] <exec> $ HGPLAIN=1 hg log -l 1 --template '{node}' -r 'test' --
76<<< [24] <exec> 119,290 us
77>>> [25] <exec> $ HGPLAIN=1 hg log -l 1 --template '{node}' -r 'first(('\''master'\''::'\''test'\'')-'\''master'\'')' --
78<<< [25] <exec> 117,949 us
79>>> [26] <exec> $ HGPLAIN=1 hg log --template '{node}\n' -r 'roots(descendants('\''16a239549c36d491544e6e8703fb72e0d0e8fdbc'\'')-descendants('\''test'\'')-('\''16a239549c36d491544e6e8703fb72e0d0e8fdbc'\''::'\''test'\''))'
80<<< [26] <exec> 117,646 us
81>>> [27] <exec> $ HGPLAIN=1 hg rebase --collapse --keep --logfile '/private/var/folders/tr/c72q6hn50_z_0js71p2gdy400000gq/T/evd1r79kjq8ks80o/16313-azEq0E' -r '('\''16a239549c36d491544e6e8703fb72e0d0e8fdbc'\''::'\''test'\'')' -d 'master'
82rebasing 37:16a239549c36 "creating a branch"
83note: rebase of 37:16a239549c36 created no changes to commit
84rebasing 38:2ae07ac41111 "modifying readme for COLLISIONS"
85note: rebase of 38:2ae07ac41111 created no changes to commit
86rebasing 39:35f26c87dd0f "creating a branch"
87note: rebase of 39:35f26c87dd0f created no changes to commit
88rebasing 40:364c8f96f85c "more mods"
89merging README
90note: rebase of 40:364c8f96f85c created no changes to commit
91rebasing 41:945ead78d871 "addfile" (tip test)
92note: rebase of 41:945ead78d871 created no changes to commit
93<<< [27] <exec> 195,705 us
94>>> [28] <exec> $ HGPLAIN=1 hg bookmarks
95<<< [28] <exec> 113,416 us
96>>> [29] <exec> $ HGPLAIN=1 hg bookmark -f 'master'
97<<< [29] <exec> 133,996 us
98>>> [30] <exec> $ HGPLAIN=1 hg bookmark -f 'test' -r '945ead78d87118052e3230c6dad7c51e9c08c4eb'
99<<< [30] <exec> 124,422 us
100>>> [31] <exec> $ HGPLAIN=1 hg log -r 'children('\''test'\'')' --template '{node}\n'
101<<< [31] <exec> 123,640 us
102>>> [32] <exec> $ HGPLAIN=1 hg checkout 'master'
103<<< [32] <exec> 114,024 us
104>>> [33] <event> land.willPushRevision <listeners = 0>
105<<< [33] <event> 112 us
106Pushing change...
107
108>>> [34] <exec> $ HGPLAIN=1 hg push -r 'master' ''
109pushing to ssh://phab-devel.code/diffusion/HGTEST/hgtest/
110searching for changes
111remote: adding changesets
112remote: adding manifests
113remote: adding file changes
114remote: added 1 changesets with 4 changes to 4 files
115remote: [2015-10-13 00:14:42] EXCEPTION: (CommandException) Command failed with error #255!
116remote: COMMAND
117remote: hg log --template '{node}' --rev 'ancestor('\''d721d5b57fc9ef72e47ff9d4e0c583d74a46590c'\'', '\''e309da2bfa84f01dc7a399b3dd5df0d7aaab2677'\'')'
118remote:
119remote: STDOUT
120remote: (empty)
121remote:
122remote: STDERR
123remote: abort: unknown revision 'e309da2bfa84f01dc7a399b3dd5df0d7aaab2677'!
124remote: at [<phutil>/src/future/exec/ExecFuture.php:416]
125remote: arcanist(head=master, ref.master=172c930630a9), phabricator(head=master, ref.master=3f3626c11a04), phutil(head=master, ref.master=dbf792a053fc)
126remote: #0 ExecFuture::resolvex() called at [<phabricator>/src/applications/repository/storage/PhabricatorRepository.php:346]
127remote: #1 PhabricatorRepository::execxLocalCommand(string, string, string) called at [<phabricator>/src/applications/diffusion/engine/DiffusionCommitHookEngine.php:968]
128remote: #2 DiffusionCommitHookEngine::findMercurialPushKeyRefUpdates() called at [<phabricator>/src/applications/diffusion/engine/DiffusionCommitHookEngine.php:693]
129remote: #3 DiffusionCommitHookEngine::findMercurialRefUpdates() called at [<phabricator>/src/applications/diffusion/engine/DiffusionCommitHookEngine.php:222]
130remote: #4 DiffusionCommitHookEngine::findRefUpdates() called at [<phabricator>/src/applications/diffusion/engine/DiffusionCommitHookEngine.php:124]
131remote: #5 DiffusionCommitHookEngine::execute() called at [<phabricator>/scripts/repository/commit_hook.php:133]
132remote: pushkey-abort: prepushkey.phabricator hook exited with status 255
133remote: transaction abort!
134remote: rollback completed
135abort: updating bookmark master failed!
136<<< [34] <exec> 3,116,171 us
137 PUSH FAILED!
138>>> [35] <exec> $ HGPLAIN=1 hg --config extensions.mq= strip 'master'
139<<< [35] <exec> 160,032 us
140>>> [36] <exec> $ HGPLAIN=1 hg checkout 'test'
141<<< [36] <exec> 135,399 us
142Switched back to bookmark test.
143Usage Exception: 'hg push' failed! Fix the error and push this change manually.
144
145[2015-10-13 04:14:43] EXCEPTION: (ArcanistUsageException) 'hg push' failed! Fix the error and push this change manually. at [<arcanist>/src/workflow/ArcanistLandWorkflow.php:1083]
146arcanist(head=stable, ref.master=9e65fc65164a, ref.stable=1773aad85599), phutil(head=stable, ref.master=55f554b618b9, ref.stable=c5a7d67db294)
147 #0 ArcanistLandWorkflow::push() called at [<arcanist>/src/workflow/ArcanistLandWorkflow.php:224]
148 #1 ArcanistLandWorkflow::run() called at [<arcanist>/scripts/arcanist.php:382]

With server on:

[cspeck@localhost phacility]$ arc version
arcanist 172c930630a960ea79ada1dbeb50f2ad561d3870 (4 Oct 2015)
libphutil dbf792a053fca78fb55b5ed193bb203c7f9049cb (8 Oct 2015)
[cspeck@localhost phabricator]$ git rev-parse HEAD
3f3626c11a04736181dac8e9b0962b34338cb2de
cspeckmim added a comment.Oct 13 2015, 4:31 AM

The other exception I noted in my previous comment might be unrelated - still investigating what's causing it.

For the main issue, as a workaround, I've found that stripping the branch and doing arc patch followed by arc land will usually succeed in place of trying to squash locally.

cspeckmim mentioned this in T12309: Differential comment that inlines a Paste and has code block after it renders in strange fashion.Feb 23 2017, 2:20 AM
epriestley moved this task from Backlog to arc land on the Mercurial board.Jan 5 2018, 6:55 PM
cspeckmim added a comment.Jan 8 2018, 5:58 PM

I tried a few scenarios for this and wasn't able to reproduce

  1. Single commit in diff that creates conflict
  2. Multiple commits in diff that all create conflicts
  3. Single commit in diff where first commit does not create conflict but second does

This can probably be closed until someone else stumbles across it

epriestley closed this task as Invalid.Jan 9 2018, 4:42 PM
epriestley added a subscriber: epriestley.

Ah, thanks! This is probably effectively covered by T9948 anyway -- one of the major changes for the Git flavor of that (T9657) was "put things back the way they were when anything goes wrong, even if we discard merge/rebase work", and that seems like a better behavior. I'll make a note there just in case.

Phabricator · Built by Phacility