This is an archive of the discontinued Mercurial Phabricator instance.

Differential D9416

copies: avoid unwanted side effect from one branch to another
ClosedPublic

Authored by Alphare on Nov 27 2020, 11:11 AM.

Download Raw Diff

Details

Reviewers

pulkit

Group Reviewers

hg-reviewers

Commits

rHGb6b7626d3e06: copies: avoid unwanted side effect from one branch to another
rHGf1cb44527819: copies: avoid unwanted side effect from one branch to another

Summary

Without this copy, change in a one descendant branch (With "remove" change
only) could affect computation on another descendant branches.

This was not caugh by the test because the test graph are "too simple". I
started writing more test in that regards, but I a submitting this changes
earlier because I want to get more code landed to allow other optimisation work
to happens.

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

Alphare created this revision.Nov 27 2020, 11:11 AM

Herald added a reviewer: hg-reviewers. · View Herald TranscriptNov 27 2020, 11:11 AM

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

Alphare added a child revision: D9417: copies: simplify the call to _merge_copies_dict.Nov 27 2020, 11:11 AM

marmoute updated this revision to Diff 23814.Nov 29 2020, 6:13 AM

I started writing more test in that regards, but I a submitting this changes earlier because I want to get more code landed to allow other optimisation work to happens.

Shall we expect tests for this future in series?

marmoute edited parent revisions, added: D9307: copies-rust: parse the changed-file sidedata directly in rust; removed: D9415: copies: fast path no-op merge.Nov 29 2020, 3:29 PM

marmoute added a child revision: D9418: copies: clarify the return of _merge_copies_dict.

marmoute updated this revision to Diff 23866.

In D9416#142205, @pulkit wrote:

I started writing more test in that regards, but I a submitting this changes earlier because I want to get more code landed to allow other optimisation work to happens.

Shall we expect tests for this future in series?

Yep, I have a list of new cases to get tested that I'll implement and submit later. I just wanted the large code movement to be out of the way first to help other to start working on performance. I also plan to get some rust unnittest for some of the lower lever bits, but mu Rust foo was a bit too low at the time of writing this. Now that Raphaël Gomès is no longer in vacation and Simon Sapin is around, I'll write follow up about that.

pulkit accepted this revision.Nov 30 2020, 11:39 AM

This revision is now accepted and ready to land.Nov 30 2020, 11:39 AM

marmoute added a commit: rHGf1cb44527819: copies: avoid unwanted side effect from one branch to another.Nov 30 2020, 12:24 PM

Closed by commit rHGf1cb44527819: copies: avoid unwanted side effect from one branch to another (authored by marmoute). · Explain Why

This revision was automatically updated to reflect the committed changes.

marmoute added a commit: rHGb6b7626d3e06: copies: avoid unwanted side effect from one branch to another.Nov 30 2020, 4:14 PM

Revision Contents
Changeset List

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

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	23732		Nov 27 2020, 11:11 AM	★	★
Diff 2	23814		Nov 29 2020, 6:13 AM	★	★
Diff 3	23866		Nov 29 2020, 3:28 PM	★	★
Diff 4	23889	rHGf1cb44527819ab3b0efaa009dc6e4bb705c2801a	Nov 20 2020, 4:38 AM	★	★

Status	Author	Revision
Closed	Alphare	D9415 copies: fast path no-op merge
Closed	Alphare	D9417 copies: simplify the call to _merge_copies_dict
Closed	Alphare	D9418 copies: clarify the return of _merge_copies_dict
Closed	Alphare	D9416 copies: avoid unwanted side effect from one branch to another
Closed	marmoute	D9307 copies-rust: parse the changed-file sidedata directly in rust
Closed	marmoute	D9306 copies-rust: encapsulate internal sets on `changes`
Closed	marmoute	D9305 copies-rust: pre-indent some code to clarify the next patch
Closed	marmoute	D9304 copies-rust: combine the iteration over remove and copies into one
Closed	marmoute	D9303 copies-rust: move is_ancestor caching within the rust code
Closed	marmoute	D9302 copies-rust: leverage the immutability for efficient update
Closed	marmoute	D9301 copies-rust: pre-indent some code to clarify the next changeset
Closed	marmoute	D9300 copies-rust: use immutable "OrdMap" to store copies information
Closed	marmoute	D9299 copies: use the rust code for `combine_changeset_copies`
Closed	marmoute	D9298 copies: introduce the hg-cpython wrapper for `combine_changeset_copies`
Closed	marmoute	D9297 copies: introduce a basic Rust function for `combine_changeset_copies`
Closed	marmoute	D9296 copies: cache the ancestor checking call when tracing copy

Diff 23889

mercurial/copies.py

	if childcopies:			if childcopies:
	newcopies = copies.copy()			newcopies = copies.copy()
	for dest, source in pycompat.iteritems(childcopies):			for dest, source in pycompat.iteritems(childcopies):
	prev = copies.get(source)			prev = copies.get(source)
	if prev is not None and prev[1] is not None:			if prev is not None and prev[1] is not None:
	source = prev[1]			source = prev[1]
	newcopies[dest] = (c, source)			newcopies[dest] = (c, source)
	assert newcopies is not copies			assert newcopies is not copies
	if changes is not None:			if changes is not None and changes.removed:
				if newcopies is copies:
				newcopies = copies.copy()
	for f in changes.removed:			for f in changes.removed:
	if f in newcopies:			if f in newcopies:
	if newcopies is copies:			if newcopies is copies:
	# copy on write to avoid affecting potential other			# copy on write to avoid affecting potential other
	# branches. when there are no other branches, this			# branches. when there are no other branches, this
	# could be avoided.			# could be avoided.
	newcopies = copies.copy()			newcopies = copies.copy()
	newcopies[f] = (c, None)			newcopies[f] = (c, None)