Patchwork D6588: abort: added support for merge

login
register
mail settings
Submitter phabricator
Date June 29, 2019, 7:39 p.m.
Message ID <differential-rev-PHID-DREV-col4klp5jfmjoi7qcgmy-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/40724/
State Superseded
Headers show

Comments

phabricator - June 29, 2019, 7:39 p.m.
taapas1128 created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  This adds support of `hg merge --abort` to `hg abort`.
  Results are shown as tests.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D6588

AFFECTED FILES
  mercurial/commands.py
  mercurial/hg.py
  tests/test-abort.t

CHANGE DETAILS




To: taapas1128, #hg-reviewers
Cc: mercurial-devel
phabricator - June 30, 2019, 7:23 p.m.
pulkit added inline comments.

INLINE COMMENTS

> commands.py:151
>          raise error.Abort(_('no operation in progress'))
> +    if abortstate._opname == 'merge':
> +        return hg.merge(repo, abort=True)

let's refactor some code and create a function for merge abort instead of special casing it.

> hg.py:963
>      else:
>          ms = mergemod.mergestate.read(repo)
>          if ms.active():

We can compute node required here again, and hence take this abort related code into a separate function.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6588/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D6588

To: taapas1128, #hg-reviewers
Cc: pulkit, mercurial-devel
phabricator - July 4, 2019, 3 p.m.
pulkit added inline comments.

INLINE COMMENTS

> hg.py:964
> +        ui = repo.ui
> +        stats = abortmerge(ui, repo, labels=labels, asflag=True)
>  

we can directly pass repo.ui here. No need to have a ui variable

> hg.py:964
> +        ui = repo.ui
> +        stats = abortmerge(ui, repo, labels=labels, asflag=True)
>  

we actually don't need stats back here is we call `_showstats()` in abort merge. The stats related lines below can be moved to if condition above. Hence removing the asflag argument.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6588/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D6588

To: taapas1128, #hg-reviewers
Cc: pulkit, mercurial-devel
phabricator - July 4, 2019, 5:39 p.m.
pulkit added inline comments.

INLINE COMMENTS

> test-commit-unresolved.t:45
>    [255]
> +#endif
> +

since there are only two cases, you can use `#else` here.

> test-commit-unresolved.t:169
>  
> +#if abortflag
>    $ hg merge --abort

We can put only the command in `if` and `else` part. The output is same on the both sides.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6588/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D6588

To: taapas1128, #hg-reviewers
Cc: pulkit, mercurial-devel
phabricator - July 4, 2019, 5:40 p.m.
pulkit added inline comments.

INLINE COMMENTS

> test-commit-unresolved.t:1
> +#testcases hgabort abortflag
> +

`abortcommand` will be better name instead of `hgabort` as that will match the way we named `abortflag` and will be more easier to realise the difference between them.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6588/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D6588

To: taapas1128, #hg-reviewers
Cc: pulkit, mercurial-devel
phabricator - July 4, 2019, 5:49 p.m.
pulkit added inline comments.

INLINE COMMENTS

> test-commit-unresolved.t:48
> +#if abortflag
>    $ hg merge --abort
>    abort: no merge in progress

`hg abort` can be used. we can case the output here.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6588/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D6588

To: taapas1128, #hg-reviewers
Cc: pulkit, mercurial-devel
phabricator - July 10, 2019, 5:12 p.m.
pulkit added inline comments.

INLINE COMMENTS

> hg.py:985
> +                            labels=labels)
> +    _showstats(repo, stats)
> +

We lost `return stats.unresolvedcount > 0` in this code movement.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6588/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D6588

To: taapas1128, #hg-reviewers, pulkit
Cc: pulkit, mercurial-devel
phabricator - July 10, 2019, 8:25 p.m.
This revision now requires changes to proceed.
pulkit added inline comments.
pulkit requested changes to this revision.

INLINE COMMENTS

> pulkit wrote in hg.py:985
> We lost `return stats.unresolvedcount > 0` in this code movement.

This is not yet addressed.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6588/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D6588

To: taapas1128, #hg-reviewers, pulkit
Cc: pulkit, mercurial-devel
phabricator - July 10, 2019, 8:46 p.m.
pulkit added inline comments.

INLINE COMMENTS

> hg.py:987
> +    if stats.unresolvedcount:
> +        repo.ui.status(_("use 'hg resolve' to retry unresolved file merges "
> +                         "or 'hg merge --abort' to abandon\n"))

Here the user is already running `hg merge --abort`, so we should not show this message.

Ideally, this `if` condition should not never when aborting a merge.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6588/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D6588

To: taapas1128, #hg-reviewers, pulkit
Cc: pulkit, mercurial-devel
phabricator - July 10, 2019, 8:50 p.m.
This revision is now accepted and ready to land.
pulkit added inline comments.
pulkit accepted this revision.

INLINE COMMENTS

> pulkit wrote in hg.py:987
> Here the user is already running `hg merge --abort`, so we should not show this message.
> 
> Ideally, this `if` condition should not never when aborting a merge.

Turns out I am blind and the previous version already had everything correct. I will either apply previous version of fix things in flight.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D6588/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D6588

To: taapas1128, #hg-reviewers, pulkit
Cc: pulkit, mercurial-devel

Patch

diff --git a/tests/test-abort.t b/tests/test-abort.t
--- a/tests/test-abort.t
+++ b/tests/test-abort.t
@@ -628,9 +628,8 @@ 
 Unshelve abort fails with appropriate message if there's no unshelve in
 progress
   $ hg abort
-  abort: merge does not support 'hg abort'
-  (use 'hg commit' or 'hg merge --abort')
-  [255]
+  aborting the merge, updating back to 9451eaa6eee3
+  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
   $ cd ..
 
 Abort due to pending changes
@@ -902,3 +901,59 @@ 
   $ hg abort
   abort: no operation in progress
   [255]
+  $ cd ..
+
+####TEST `hg abort` operation merge
+
+  $ addcommit () {
+  >     echo $1 > $1
+  >     hg add $1
+  >     hg commit -d "${2} 0" -m $1
+  > }
+
+  $ commit () {
+  >     hg commit -d "${2} 0" -m $1
+  > }
+
+  $ hg init a
+  $ cd a
+  $ addcommit "A" 0
+  $ addcommit "B" 1
+  $ echo "C" >> A
+  $ commit "C" 2
+
+  $ hg update -C 0
+  1 files updated, 0 files merged, 1 files removed, 0 files unresolved
+  $ echo "D" >> A
+  $ commit "D" 3
+  created new head
+
+State before the merge
+
+  $ hg status
+  $ hg id
+  e45016d2b3d3 tip
+  $ hg summary
+  parent: 3:e45016d2b3d3 tip
+   D
+  branch: default
+  commit: (clean)
+  update: 2 new changesets, 2 branch heads (merge)
+  phases: 4 draft
+
+Testing the abort functionality first in case of conflicts
+
+  $ hg abort
+  abort: no operation in progress
+  [255]
+  $ hg merge
+  merging A
+  warning: conflicts while merging A! (edit, then use 'hg resolve --mark')
+  1 files updated, 0 files merged, 0 files removed, 1 files unresolved
+  use 'hg resolve' to retry unresolved file merges or 'hg merge --abort' to abandon
+  [1]
+
+  $ hg abort
+  aborting the merge, updating back to e45016d2b3d3
+  1 files updated, 0 files merged, 1 files removed, 0 files unresolved
+
diff --git a/mercurial/hg.py b/mercurial/hg.py
--- a/mercurial/hg.py
+++ b/mercurial/hg.py
@@ -952,8 +952,8 @@ 
 
     return ret
 
-def merge(repo, node, force=None, remind=True, mergeforce=False, labels=None,
-          abort=False):
+def merge(repo, node=None, force=None, remind=True, mergeforce=False,
+          labels=None, abort=False):
     """Branch merge with node, resolving changes. Return true if any
     unresolved conflicts."""
     if not abort:
diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -148,6 +148,8 @@ 
             break
     if not abortstate:
         raise error.Abort(_('no operation in progress'))
+    if abortstate._opname == 'merge':
+        return hg.merge(repo, abort=True)
     if abortstate._abortfunc:
         return abortstate._abortfunc(ui, repo)
     raise error.Abort((_("%s does not support 'hg abort'") %