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.

Download Raw Diff

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

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

martinvonz created this revision.Jan 24 2020, 7:16 PM

Herald added a reviewer: hg-reviewers. · View Herald TranscriptJan 24 2020, 7:16 PM

Herald added a subscriber: mercurial-devel. · View Herald Transcript

martinvonz added a child revision: D7997: merge: drop now-unused "abort" argument from hg.merge().Jan 24 2020, 7:16 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.

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.

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

martinvonz added a commit: rHG1850066f9e36: merge: don't auto-pick destination with `hg merge 'wdir()'`.Jan 25 2020, 7:25 PM

Closed by commit rHG1850066f9e36: merge: don't auto-pick destination with `hg merge 'wdir()'` (authored by martinvonz). · Explain Why

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			mercurial/commands.py (6 lines)
M			tests/test-merge2.t (4 lines)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	19587		Jan 24 2020, 7:16 PM	★	★
Diff 2	19602		Jan 25 2020, 1:05 AM	★	★
Diff 3	19615	rHG1850066f9e362529d197d569d89c4c2c7cb56069	Jan 24 2020, 8:49 PM	★	★

Status	Author	Revision
Closed	martinvonz	D8000 mq: don't tell user to commit merge that we already committed
Closed	martinvonz	D7999 merge: make hg.merge() take a context instead of a node
Closed	martinvonz	D7998 merge: replace a repo.lookup('.') by more typical repo['.'].node()
Closed	martinvonz	D7997 merge: drop now-unused "abort" argument from hg.merge()
Closed	martinvonz	D7996 merge: don't auto-pick destination with `hg merge 'wdir()'`
Closed	martinvonz	D7995 merge: call hg.abortmerge() directly and return early
Closed	martinvonz	D7994 merge: check that there are no conflicts after --abort
Closed	martinvonz	D7993 merge: use check_incompatible_arguments() for --abort

Diff 19615

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()

				if node is None:
				raise error.Abort(_(b'merging with the working copy has no effect'))

	if opts.get(b'preview'):			if opts.get(b'preview'):
	# find nodes that are ancestors of p2 but not of p1			# find nodes that are ancestors of p2 but not of p1
	p1 = repo.lookup(b'.')			p1 = repo.lookup(b'.')
	p2 = node			p2 = node
	nodes = repo.changelog.findmissing(common=[p1], heads=[p2])			nodes = repo.changelog.findmissing(common=[p1], heads=[p2])

	displayer = logcmdutil.changesetdisplayer(ui, repo, opts)			displayer = logcmdutil.changesetdisplayer(ui, repo, opts)
	for node in nodes:			for node in nodes:

tests/test-merge2.t

	$ hg remove b			$ hg remove b
	$ hg update 0			$ hg update 0
	0 files updated, 0 files merged, 0 files removed, 0 files unresolved			0 files updated, 0 files merged, 0 files removed, 0 files unresolved
	$ echo This is file b2 > b			$ echo This is file b2 > b
	$ hg commit -A -m "commit #2"			$ hg commit -A -m "commit #2"
	adding b			adding b
	created new head			created new head

				$ hg merge 'wdir()'
				abort: merging with the working copy has no effect
				[255]

	$ cd ..			$ cd ..