This is an archive of the discontinued Mercurial Phabricator instance.

Differential D8616

merge: chain copies with existing copies in working copy
Changes PlannedPublic

Authored by martinvonz on Jun 5 2020, 9:39 PM.

Download Raw Diff

Details

Reviewers

None

Group Reviewers

hg-reviewers

Summary

This makes the merge code chain and filter copies when grafting with
copies already in the working copy. For example, if the working copy
has renamed file A to B and you somehow graft in a change that renames
B to C, then that will now become a rename from A to C. That will
soon be necessary for hg rebase --collapse. It seems that we don't
have any existing cases where chaining makes a difference.

Diff Detail

Repository

rHG Mercurial

Branch

default

Lint

No Linters Available

Unit

No Unit Test Coverage

Event Timeline

martinvonz created this revision.Jun 5 2020, 9:39 PM

Herald added a reviewer: hg-reviewers. · View Herald TranscriptJun 5 2020, 9:39 PM

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

martinvonz added a parent revision: D8595: copies: make _chain() and _filter() public.Jun 5 2020, 9:39 PM

martinvonz added a child revision: D8597: rebase: use merge.graft() instead of merge.update().

This patch seems related to D8596 which @martinvonz pointed as "not meant to be merged". What should we do with this diff ? is it intended for landing ? If not could get we it out of need-review and marke it RFC ?

In D8616#128712, @marmoute wrote:

This patch seems related to D8596 which @martinvonz pointed as "not meant to be merged". What should we do with this diff ? is it intended for landing ? If not could get we it out of need-review and marke it RFC ?

That was on an earlier version of the series; this series is now ready.

martinvonz updated this revision to Diff 21605.Jun 10 2020, 1:54 PM

marmoute added inline comments.Jun 11 2020, 9:45 AM

mercurial/merge.py
1916–1921	I am not sure why we gain all these new cases? where do they comes from?

martinvonz added inline comments.Jun 11 2020, 9:51 AM

mercurial/merge.py
1916–1921	Did you miss the comment I added on line 1914 or do you think it's not enough? Each case is documented there.

martinvonz updated this revision to Diff 21608.Jun 11 2020, 10:34 AM

(I have an unsubmitted question laying around for over a month…)

mercurial/merge.py
1916–1921	I don't understand how it answser my question of "why does these new cases pops up?"

martinvonz added inline comments.Jul 15 2020, 12:22 PM

mercurial/merge.py
1916–1921	I guess I don't understand what your question was in that case. Can you rephrase it?

marmoute added inline comments.Jul 16 2020, 10:33 AM

mercurial/merge.py
1916–1921	Previously, theses extra `ifs` clause were not necessary, Now they appears. Why do they become necessary now ?

Changes planned to earlier patch in the series

martinvonz added inline comments.Jul 16 2020, 11:28 AM

mercurial/merge.py
1916–1921	Ah, I see. See `copies.filter()` (called `copies._filter()` at head) for details.

Revision Contents
Changeset List

			Path	Packages
M			mercurial/merge.py (16 lines)

Commit	Parents	Author	Summary	Date
1b6b55fbacab	b65946a65603	Martin von Zweigbergk		Jun 5 2020, 3:04 PM

Status	Author	Revision
Changes Planned	martinvonz	D8599 rebase: remove now-unused arguments from rebasenode()
Changes Planned	martinvonz	D8598 rebase: drop duplicate call to copies.graftcopies()
Changes Planned	martinvonz	D8597 rebase: use merge.graft() instead of merge.update()
Changes Planned	martinvonz	D8616 merge: chain copies with existing copies in working copy
Changes Planned	martinvonz	D8595 copies: make _chain() and _filter() public
Changes Planned	martinvonz	D8596 merge: mark copies in in-memory context when merging
Closed	martinvonz	D8615 merge: move an inspection of the dirstate from record to calculate phase

Diff 21564

mercurial/merge.py

	_(			_(
	b'(warning: large working directory being used without '			b'(warning: large working directory being used without '
	b'fsmonitor enabled; enable fsmonitor to improve performance; '			b'fsmonitor enabled; enable fsmonitor to improve performance; '
	b'see "hg help -e fsmonitor")\n'			b'see "hg help -e fsmonitor")\n'
	)			)
	)			)

	wantfiledata = updatedirstate and not branchmerge			wantfiledata = updatedirstate and not branchmerge
	copies = {}			resulting_copies = {}
	if not overwrite:			if not overwrite:
	copies = _extract_copies(actions, branchmerge)			existing_copies = wc.p2copies()
				existing_copies.update(wc.p1copies())
				new_copies = _extract_copies(actions, branchmerge)
				resulting_copies = copies.chain(existing_copies, new_copies)

	stats, getfiledata = applyupdates(			stats, getfiledata = applyupdates(
	repo, actions, wc, p2, overwrite, wantfiledata, labels=labels			repo, actions, wc, p2, overwrite, wantfiledata, labels=labels
	)			)

	if updatedirstate:			if updatedirstate:
	with repo.dirstate.parentchange():			with repo.dirstate.parentchange():
	repo.setparents(fp1, fp2)			repo.setparents(fp1, fp2)
	wc = repo[None]			wc = repo[None]
	mergestatemod.recordupdates(			mergestatemod.recordupdates(
	repo, actions, branchmerge, getfiledata			repo, actions, branchmerge, getfiledata
	)			)
	# update completed, clear state			# update completed, clear state
	util.unlink(repo.vfs.join(b'updatestate'))			util.unlink(repo.vfs.join(b'updatestate'))

	if not branchmerge:			if not branchmerge:
	repo.dirstate.setbranch(p2.branch())			repo.dirstate.setbranch(p2.branch())

	for dst, src in copies.items():			# This replicates copies.filter() but is modified to work with merges
				for dst, src in resulting_copies.items():
				if dst == src:
				continue
				if dst not in wc:
				continue
				if not (src in wc.p1() or src in wc.p2()):
				continue
				marmouteUnsubmitted Not Done I am not sure why we gain all these new cases? where do they comes from? marmoute: I am not sure why we gain all these new cases? where do they comes from?
				martinvonzAuthorUnsubmitted Done Did you miss the comment I added on line 1914 or do you think it's not enough? Each case is documented there. martinvonz: Did you miss the comment I added on line 1914 or do you think it's not enough? Each case is…
				marmouteUnsubmitted Not Done I don't understand how it answser my question of "why does these new cases pops up?" marmoute: I don't understand how it answser my question of "why does these new cases pops up?"
				martinvonzAuthorUnsubmitted Done I guess I don't understand what your question was in that case. Can you rephrase it? martinvonz: I guess I don't understand what your question was in that case. Can you rephrase it?
				marmouteUnsubmitted Not Done Previously, theses extra `ifs` clause were not necessary, Now they appears. Why do they become necessary now ? marmoute: Previously, theses extra `ifs` clause were not necessary, Now they appears. Why do they become…
				martinvonzAuthorUnsubmitted Done Ah, I see. See `copies.filter()` (called `copies._filter()` at head) for details. martinvonz: Ah, I see. See `copies.filter()` (called `copies._filter()` at head) for details.
	wc[dst].markcopied(src)			wc[dst].markcopied(src)

	# If we're updating to a location, clean up any stale temporary includes			# If we're updating to a location, clean up any stale temporary includes
	# (ex: this happens during hg rebase --abort).			# (ex: this happens during hg rebase --abort).
	if not branchmerge:			if not branchmerge:
	sparse.prunetemporaryincludes(repo)			sparse.prunetemporaryincludes(repo)

	if updatedirstate:			if updatedirstate:

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	21564		Jun 5 2020, 9:39 PM	★	★
Diff 2	21605		Jun 10 2020, 1:54 PM	★	★
Diff 3	21608		Jun 11 2020, 10:34 AM	★	★