Patchwork D7996: merge: don't auto-pick destination with `hg merge 'wdir()'`

login
register
mail settings
Submitter phabricator
Date Jan. 25, 2020, 12:17 a.m.
Message ID <differential-rev-PHID-DREV-tknfn3hfqev4ikooy6h3-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/44644/
State Superseded
Headers show

Comments

phabricator - Jan. 25, 2020, 12:17 a.m.
martinvonz created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  If the user doesn't specify a commit to merge with, we'll have
  `node==None` in `commands.merge()`. We'll then try to find a good
  commit to merge with. However, if the user, for some strange reason,
  runs `hg merge 'wdir()'`, we'll also have `node==None` and we'll do
  that same. That's clearly not the intent, so let's not do that.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  mercurial/commands.py

CHANGE DETAILS




To: martinvonz, #hg-reviewers
Cc: mercurial-devel
phabricator - Jan. 25, 2020, 1:27 a.m.
pulkit added a comment.


  Looks like we don't have tests for `hg merge 'wdir()'`. Also I am not sure what should be the correct behavior in that case.

REPOSITORY
  rHG Mercurial

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

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

To: martinvonz, #hg-reviewers
Cc: pulkit, mercurial-devel
phabricator - Jan. 25, 2020, 1:42 a.m.
martinvonz added a comment.


  In D7996#117931 <https://phab.mercurial-scm.org/D7996#117931>, @pulkit wrote:
  
  > Looks like we don't have tests for `hg merge 'wdir()'`. Also I am not sure what should be the correct behavior in that case.
  
  Yes, I just didn't care enough to add tests. I don't care much because it's such a weird thing for the user to do. I changed this only in order to clarify the control flow and how `node` gets assigned. I can add a test if you want.

REPOSITORY
  rHG Mercurial

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

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

To: martinvonz, #hg-reviewers
Cc: pulkit, mercurial-devel
phabricator - Jan. 25, 2020, 1:49 a.m.
martinvonz added a comment.


  In D7996#117951 <https://phab.mercurial-scm.org/D7996#117951>, @martinvonz wrote:
  
  > In D7996#117931 <https://phab.mercurial-scm.org/D7996#117931>, @pulkit wrote:
  >
  >> Looks like we don't have tests for `hg merge 'wdir()'`. Also I am not sure what should be the correct behavior in that case.
  >
  > Yes, I just didn't care enough to add tests. I don't care much because it's such a weird thing for the user to do. I changed this only in order to clarify the control flow and how `node` gets assigned. I can add a test if you want.
  
  Turns out we actually crash :P Let me add a test.

REPOSITORY
  rHG Mercurial

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

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

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

Patch

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -4866,8 +4866,7 @@ 
 
     if node:
         node = scmutil.revsingle(repo, node).node()
-
-    if not node:
+    else:
         if ui.configbool(b'commands', b'merge.require-rev'):
             raise error.Abort(
                 _(