This is an archive of the discontinued Mercurial Phabricator instance.

Differential D7996

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

Authored by martinvonz on Jan 24 2020, 7:16 PM.

Details

Reviewers
pulkit
Group Reviewers
hg-reviewers
Commits
rHG1850066f9e36: merge: don't auto-pick destination with `hg merge 'wdir()'`
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.

It turns out we'd instead crash on that command after this patch, so I
added special handling of it too.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

martinvonz created this revision.Jan 24 2020, 7:16 PM
pulkit added a subscriber: pulkit.Jan 24 2020, 8:27 PM

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.

martinvonz added a comment.Jan 24 2020, 8:42 PM
In 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.

martinvonz added a comment.Jan 24 2020, 8:49 PM
In D7996#117951, @martinvonz wrote:
In 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.

martinvonz edited the summary of this revision. (Show Details)Jan 25 2020, 1:05 AM
martinvonz updated this revision to Diff 19602.
pulkit accepted this revision.Jan 25 2020, 6:12 PM
This revision is now accepted and ready to land.Jan 25 2020, 6:12 PM

Revision Contents
Changeset List

PathPackages
Mmercurial/commands.py (3 lines)

Diff 19587

mercurial/commands.py

if opts.get(b'rev') and node: if opts.get(b'rev') and node:
raise error.Abort(_(b"please specify just one revision")) raise error.Abort(_(b"please specify just one revision"))
if not node: if not node:
node = opts.get(b'rev') node = opts.get(b'rev')
if node: if node:
node = scmutil.revsingle(repo, node).node() node = scmutil.revsingle(repo, node).node()
else:
if not node:
if ui.configbool(b'commands', b'merge.require-rev'): if ui.configbool(b'commands', b'merge.require-rev'):
raise error.Abort( raise error.Abort(
_( _(
b'configuration requires specifying revision to merge ' b'configuration requires specifying revision to merge '
b'with' b'with'
) )
) )
node = repo[destutil.destmerge(repo)].node() node = repo[destutil.destmerge(repo)].node()