D6957: rebase: unconditionally clear merge state when `--stop`ing a rebase

[baymax (Baymax, Your Personal Patch-care Companion)]

This revision now requires changes to proceed.
baymax added a comment.
baymax requested changes to this revision.


  There seems to have been no activities on this Diff for the past 3 Months.
  
  By policy, we are automatically moving it out of the `need-review` state.
  
  Please, move it back to `need-review` without hesitation if this diff should 
still be discussed.
  
  :baymax:need-review-idle:

REPOSITORY
  rHG Mercurial

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

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

To: durin42, martinvonz, #hg-reviewers, baymax
Cc: marmoute, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D6957: rebase: unconditionally clear merge state when `--stop`ing a rebase

[durin42 (Augie Fackler)]

durin42 added a comment.


  In D6957#101985 , @martinvonz 
wrote:
  
  > In D6957#101984 , @marmoute 
wrote:
  >
  >> In D6957#101977 , @martinvonz 
wrote:
  >>
  >>> How was it failing for you?
  >>> I don't think I've used `hg rebase --stop`, but I had been wondering what 
it would do with the working directory. It's unclear to me what it should do, 
and now after looking at the code, it's also unclear what the author meant it 
to do. Maybe it would be best to just leave the working directory untouched? 
Oh, we should also compare to what `hg evolve --stop` does (I haven't checked).
  >>
  >> The evolve semantic, is to:
  >>
  >> - get out of the current conflict (so, reverting the working copy)
  >> - preserve all successful evolution so far.
  >>
  >> Since you cannot `continue` after the --stop, we would not be able to do 
anything good with the conflict if the user resolved it anyway.
  >> I think rebase should have the same logic and revert the current conflict 
(preserving all successfully rebased revision)
  >
  > True, you'd get to resolve the conflicts on the next `hg rebase/evolve` 
anyway, so I agree there's no need to preserve the working copy.
  > @durin42, any idea how you ended up with just one working copy parent?
  
  I don't have just one parent. If you want to inspect the state, look at 
`augie/a 59473`;)

REPOSITORY
  rHG Mercurial

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

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

To: durin42, martinvonz, #hg-reviewers
Cc: marmoute, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D6957: rebase: unconditionally clear merge state when `--stop`ing a rebase

[martinvonz (Martin von Zweigbergk)]

martinvonz added a comment.


  In D6957#101984 , @marmoute 
wrote:
  
  > In D6957#101977 , @martinvonz 
wrote:
  >
  >> How was it failing for you?
  >> I don't think I've used `hg rebase --stop`, but I had been wondering what 
it would do with the working directory. It's unclear to me what it should do, 
and now after looking at the code, it's also unclear what the author meant it 
to do. Maybe it would be best to just leave the working directory untouched? 
Oh, we should also compare to what `hg evolve --stop` does (I haven't checked).
  >
  > The evolve semantic, is to:
  >
  > - get out of the current conflict (so, reverting the working copy)
  > - preserve all successful evolution so far.
  >
  > Since you cannot `continue` after the --stop, we would not be able to do 
anything good with the conflict if the user resolved it anyway.
  > I think rebase should have the same logic and revert the current conflict 
(preserving all successfully rebased revision)
  
  True, you'd get to resolve the conflicts on the next `hg rebase/evolve` 
anyway, so I agree there's no need to preserve the working copy.
  
  @durin42, any idea how you ended up with just one working copy parent?

REPOSITORY
  rHG Mercurial

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

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

To: durin42, martinvonz, #hg-reviewers
Cc: marmoute, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D6957: rebase: unconditionally clear merge state when `--stop`ing a rebase

[marmoute (Pierre-Yves David)]

marmoute added a comment.


  In D6957#101977 , @martinvonz 
wrote:
  
  > How was it failing for you?
  > I don't think I've used `hg rebase --stop`, but I had been wondering what 
it would do with the working directory. It's unclear to me what it should do, 
and now after looking at the code, it's also unclear what the author meant it 
to do. Maybe it would be best to just leave the working directory untouched? 
Oh, we should also compare to what `hg evolve --stop` does (I haven't checked).
  
  The evolve semantic, is to:
  
  - get out of the current conflict (so, reverting the working copy)
  - preserve all successful evolution so far.
  
  Since you cannot `continue` after the --stop, we would not be able to do 
anything good with the conflict if the user resolved it anyway.
  
  I think rebase should have the same logic and revert the current conflict 
(preserving all successfully rebased revision)

REPOSITORY
  rHG Mercurial

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

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

To: durin42, martinvonz, #hg-reviewers
Cc: marmoute, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D6957: rebase: unconditionally clear merge state when `--stop`ing a rebase

[martinvonz (Martin von Zweigbergk)]

martinvonz added a comment.


  How was it failing for you?
  
  I don't think I've used `hg rebase --stop`, but I had been wondering what it 
would do with the working directory. It's unclear to me what it should do, and 
now after looking at the code, it's also unclear what the author meant it to 
do. Maybe it would be best to just leave the working directory untouched? Oh, 
we should also compare to what `hg evolve --stop` does (I haven't checked).

REPOSITORY
  rHG Mercurial

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

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

To: durin42, martinvonz, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

D6957: rebase: unconditionally clear merge state when `--stop`ing a rebase

[durin42 (Augie Fackler)]

durin42 created this revision.
Herald added a reviewer: martinvonz.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  I am NOT sure of the correctness of this change, but I've hit a point
  on a work repository where the needupdate function returns false and
  so the merge state never gets correctly cleared. As far as I can tell
  when we're *always* in a merge state at this point, so we should
  *always* clear the merge state.
  
  I'm not sure how to structure the test at the moment, but if others
  agree that this looks plausible I'll try and figure out what state my
  work repository is in to craft a test case that can be used to prevent
  any backsliding on this fix.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  hgext/rebase.py

CHANGE DETAILS

diff --git a/hgext/rebase.py b/hgext/rebase.py
--- a/hgext/rebase.py
+++ b/hgext/rebase.py
@@ -909,10 +909,9 @@
 hint=_('either enable obsmarkers to allow unstable '
'revisions or use --keep to keep original '
'changesets'))
-if needupdate(repo, rbsrt.state):
-# update to the current working revision
-# to clear interrupted merge
-hg.updaterepo(repo, rbsrt.originalwd, overwrite=True)
+# update to the current working revision
+# to clear interrupted merge
+hg.updaterepo(repo, rbsrt.originalwd, overwrite=True)
 rbsrt._finishrebase()
 return 0
 elif inmemory:



To: durin42, martinvonz, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel