This is an archive of the discontinued Mercurial Phabricator instance.

Differential D3896

copies: handle case when both merge cset are not descendant of merge base
Needs RevisionPublic

Authored by pulkit on Jul 8 2018, 5:11 AM.

Download Raw Diff

Details

Reviewers

baymax

Group Reviewers

hg-reviewers

Summary

There can be cases when both the changesets which we are merging are not
descendants of the merge base. In those cases dirtyc1 and dirtyc2 both will be
true. The existing code assumes that either of them will be true always but that
is not a right assumption.

I found this while working with content-divergence resolution. In
content-divergence resolution, we use the common predecessor as the merge base
for resolving content divergence and there it can be possible that the merge
base is not the descendant of both the content-divergence changsets.

The code in content-divergence which does this is at:
https://www.mercurial-scm.org/repo/evolve/file/b81fd1487e04/hgext3rd/evolve/evolvecmd.py#l507

Diff Detail

Repository

rHG Mercurial

Lint

Lint Skipped

Unit

Unit Tests Skipped

Event Timeline

pulkit created this revision.Jul 8 2018, 5:11 AM

Herald added a reviewer: hg-reviewers. · View Herald TranscriptJul 8 2018, 5:11 AM

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

pulkit added inline comments.Jul 8 2018, 5:14 AM

mercurial/copies.py
496	Since we can have both dirtyc1, and dirtyc2 true, I am not sure whether this else should be turned into it's own if statement?

Can you add some tests that make both dirtyc1 and dirtyc2 set, and
trigger copy tracing?

Since we can have both dirtyc1, and dirtyc2 true, I am not sure whether
this else should be turned into it's own if statement?

No idea. I doubt if the current copy tracing can handle such cases. From
my vague memory, the current algorithm relies on the fact that the pseudo
base revision exists in between the c1 and the c2.

Any updates on this? I'm poking at updating this function in core, and not too excited to have to also modify the copy in evolve that seems to have spawned :/

In D3896#65986, @spectral wrote:

Any updates on this? I'm poking at updating this function in core, and not too excited to have to also modify the copy in evolve that seems to have spawned :/

I am yet to write tests. Feel free to update the one in core, I will take care of the evolve copy.

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:

This revision now requires changes to proceed.Jan 24 2020, 12:32 PM

Pretty sure this was made obsolete by D6255

Revision Contents
Changeset List

			Path	Packages
M			mercurial/copies.py (5 lines)

Commit	Parents	Author	Summary	Date
		Pulkit Goyal		Jul 6 2018, 7:36 AM

Diff 9471

mercurial/copies.py

	copy = dict(data1['copy'])			copy = dict(data1['copy'])
	copy.update(data2['copy'])			copy.update(data2['copy'])
	fullcopy = dict(data1['fullcopy'])			fullcopy = dict(data1['fullcopy'])
	fullcopy.update(data2['fullcopy'])			fullcopy.update(data2['fullcopy'])

	if dirtyc1:			if dirtyc1:
	_combinecopies(data2['incomplete'], data1['incomplete'], copy, diverge,			_combinecopies(data2['incomplete'], data1['incomplete'], copy, diverge,
	incompletediverge)			incompletediverge)
	else:			else:
				pulkitAuthorUnsubmitted Not Done Since we can have both dirtyc1, and dirtyc2 true, I am not sure whether this else should be turned into it's own if statement? pulkit: Since we can have both dirtyc1, and dirtyc2 true, I am not sure whether this else should be…
	_combinecopies(data1['incomplete'], data2['incomplete'], copy, diverge,			_combinecopies(data1['incomplete'], data2['incomplete'], copy, diverge,
	incompletediverge)			incompletediverge)

	renamedelete = {}			renamedelete = {}
	renamedeleteset = set()			renamedeleteset = set()
	divergeset = set()			divergeset = set()
	for of, fl in list(diverge.items()):			for of, fl in list(diverge.items()):
	if len(fl) == 1 or of in c1 or of in c2:			if len(fl) == 1 or of in c1 or of in c2:
	'fullcopy': {},			'fullcopy': {},
	'incomplete': {},			'incomplete': {},
	'diverge': bothdiverge,			'diverge': bothdiverge,
	'incompletediverge': bothincompletediverge			'incompletediverge': bothincompletediverge
	}			}
	for f in bothnew:			for f in bothnew:
	_checkcopies(c1, c2, f, base, tca, dirtyc1, limit, both1)			_checkcopies(c1, c2, f, base, tca, dirtyc1, limit, both1)
	_checkcopies(c2, c1, f, base, tca, dirtyc2, limit, both2)			_checkcopies(c2, c1, f, base, tca, dirtyc2, limit, both2)
	if dirtyc1:
				if dirtyc1 and dirtyc2:
				pass
				elif dirtyc1:
	# incomplete copies may only be found on the "dirty" side for bothnew			# incomplete copies may only be found on the "dirty" side for bothnew
	assert not both2['incomplete']			assert not both2['incomplete']
	remainder = _combinecopies({}, both1['incomplete'], copy, bothdiverge,			remainder = _combinecopies({}, both1['incomplete'], copy, bothdiverge,
	bothincompletediverge)			bothincompletediverge)
	elif dirtyc2:			elif dirtyc2:
	assert not both1['incomplete']			assert not both1['incomplete']
	remainder = _combinecopies({}, both2['incomplete'], copy, bothdiverge,			remainder = _combinecopies({}, both2['incomplete'], copy, bothdiverge,
	bothincompletediverge)			bothincompletediverge)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	9471			Jul 8 2018, 5:11 AM	★	★