This is an archive of the discontinued Mercurial Phabricator instance.

Differential D11788

dirstate: do no use `set_clean` in revert
ClosedPublic

Authored by marmoute on Nov 24 2021, 6:12 AM.

Download Raw Diff

Details

Reviewers

Alphare

Group Reviewers

hg-reviewers

Commits

rHG9f1b9e128788: dirstate: do no use `set_clean` in revert

Summary

The current set_clean usage is racy (the file might be modified between its
restoration and the set_clean call).

So we simply leave the file as ambiguous and the next status will fix that.

We still have to make sure the copy information is dropped, so we teach dirstate
how to do that.

The win32txt extension is confused after this because current logic is broken in
more location. However this series will ultimately fix that so we "ignore" it
for now. Fixing it now is complicated without some extra fix landing later.

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

marmoute created this revision.Nov 24 2021, 6:12 AM

Herald added a reviewer: hg-reviewers. · View Herald TranscriptNov 24 2021, 6:12 AM

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

marmoute added a child revision: D11789: dirstate: make it mandatory to provide parentfiledata in `set_clean`.Nov 24 2021, 6:12 AM

Alphare accepted this revision.Nov 24 2021, 8:34 AM

Alphare added a subscriber: Alphare.

Alphare added inline comments.

mercurial/cmdutil.py
3650	"merge" twice

This revision is now accepted and ready to land.Nov 24 2021, 8:34 AM

marmoute edited the summary of this revision. (Show Details)Nov 30 2021, 6:53 PM

marmoute updated this revision to Diff 31227.

You've edited the commit message and made it... less readable? This was probably a mistake

This revision now requires changes to proceed.Dec 1 2021, 5:15 AM

marmoute edited the summary of this revision. (Show Details)Dec 1 2021, 8:21 AM

Alphare accepted this revision.Dec 1 2021, 10:15 AM

This revision is now accepted and ready to land.Dec 1 2021, 10:15 AM

marmoute added a commit: rHG9f1b9e128788: dirstate: do no use `set_clean` in revert.Dec 1 2021, 10:21 AM

Closed by commit rHG9f1b9e128788: dirstate: do no use `set_clean` in revert (authored by marmoute). · Explain Why

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			mercurial/cmdutil.py (20 lines)
M			mercurial/dirstate.py (9 lines)
M			tests/test-revert.t (2 lines)
M			tests/test-win32text.t (1 line)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	31110		Nov 24 2021, 6:12 AM	★	★
Diff 2	31227		Nov 30 2021, 6:53 PM	★	★
Diff 3	31250	rHG9f1b9e128788776186f49fee0f74df6891639838	Nov 22 2021, 9:58 AM	★	★

Status	Author	Revision
Closed	marmoute	D11800 largefile: use the proper "mtime boundary" logic during fixup
Closed	marmoute	D11799 status: move the boundary comparison logic within the timestamp module
Closed	marmoute	D11798 tests: remove potential mtime ambiguity in a dirstate test
Closed	marmoute	D11797 dirstate: cleanup remaining of "now" during write
Closed	marmoute	D11796 dirstate: remove need_delay logic
Closed	marmoute	D11795 dirstate: remove `lastnormaltime` mechanism
Closed	marmoute	D11794 status: use filesystem time boundary to invalidate racy mtime
Closed	marmoute	D11793 test: use a different timestamp for the updated file
Closed	marmoute	D11792 win32text: drop associated dirstate cache information on revert
Closed	marmoute	D11791 dirstate: stop gathering parentfiledata in update_file
Closed	marmoute	D11790 dirstate: stop gathering parentfiledata in update_file_p1
Closed	marmoute	D11789 dirstate: make it mandatory to provide parentfiledata in `set_clean`
Closed	marmoute	D11788 dirstate: do no use `set_clean` in revert
Closed	marmoute	D11787 status: adapt the "keyword" extensions to gather stats at lookup time
Closed	marmoute	D11786 status: adapt largefile to gather stats at lookup time
Closed	marmoute	D11785 status: gather fixup info at comparison time
Closed	marmoute	D11784 update: filter the ambiguous mtime in update directly
Closed	marmoute	D11783 dirstate: move "get fs now" in the timestamp utility module
Closed	marmoute	D11782 dirstate-item: allow mtime to be None in "parentdata"
Closed	marmoute	D11781 dirstate: add a comment about a racy piece of code during updates
Closed	marmoute	D11780 tests: add missing `head` for when things go wrong
Closed	marmoute	D11779 tests: ensure a status will have non ambiguous mtime in some race test
Closed	marmoute	D11778 tests: make sure no ambiguities remains after the commit
Closed	marmoute	D11777 dirstate: clarify a `hg update` invocation in a test
Closed	marmoute	D11833 test: mark rhg output as flaky

Diff 31250

mercurial/cmdutil.py

	else:			else:
	prntstatusmsg(b'remove', f)			prntstatusmsg(b'remove', f)
	doremove(f)			doremove(f)
	for f in actions[b'drop'][0]:			for f in actions[b'drop'][0]:
	audit_path(f)			audit_path(f)
	prntstatusmsg(b'drop', f)			prntstatusmsg(b'drop', f)
	repo.dirstate.set_untracked(f)			repo.dirstate.set_untracked(f)

	normal = None			# We are reverting to our parent. If possible, we had like `hg status`
	if node == parent:			# to report the file as clean. We have to be less agressive for
				AlphareUnsubmitted Not Done "merge" twice Alphare: "merge" twice
	# We're reverting to our parent. If possible, we'd like status			# merges to avoid losing information about copy introduced by the merge.
	# to report the file as clean. We have to use normallookup for			# This might comes with bugs ?
	# merges to avoid losing information about merged/dirty files.			reset_copy = p2 == repo.nullid
	if p2 != repo.nullid:
	normal = repo.dirstate.set_tracked			def normal(filename):
	else:			return repo.dirstate.set_tracked(filename, reset_copy=reset_copy)
	normal = repo.dirstate.set_clean

	newlyaddedandmodifiedfiles = set()			newlyaddedandmodifiedfiles = set()
	if interactive:			if interactive:
	# Prompt the user for changes to revert			# Prompt the user for changes to revert
	torevert = [f for f in actions[b'revert'][0] if f not in excluded_files]			torevert = [f for f in actions[b'revert'][0] if f not in excluded_files]
	m = scmutil.matchfiles(repo, torevert)			m = scmutil.matchfiles(repo, torevert)
	diffopts = patch.difffeatureopts(			diffopts = patch.difffeatureopts(
	repo.ui,			repo.ui,

	for f in actions[b'add'][0]:			for f in actions[b'add'][0]:
	# Don't checkout modified files, they are already created by the diff			# Don't checkout modified files, they are already created by the diff
	if f not in newlyaddedandmodifiedfiles:			if f not in newlyaddedandmodifiedfiles:
	prntstatusmsg(b'add', f)			prntstatusmsg(b'add', f)
	checkout(f)			checkout(f)
	repo.dirstate.set_tracked(f)			repo.dirstate.set_tracked(f)

	normal = repo.dirstate.set_tracked
	if node == parent and p2 == repo.nullid:
	normal = repo.dirstate.set_clean
	for f in actions[b'undelete'][0]:			for f in actions[b'undelete'][0]:
	if interactive:			if interactive:
	choice = repo.ui.promptchoice(			choice = repo.ui.promptchoice(
	_(b"add back removed file %s (Yn)?$$ &Yes $$ &No") % f			_(b"add back removed file %s (Yn)?$$ &Yes $$ &No") % f
	)			)
	if choice == 0:			if choice == 0:
	prntstatusmsg(b'undelete', f)			prntstatusmsg(b'undelete', f)
	checkout(f)			checkout(f)

mercurial/dirstate.py


	def copied(self, file):			def copied(self, file):
	return self._map.copymap.get(file, None)			return self._map.copymap.get(file, None)

	def copies(self):			def copies(self):
	return self._map.copymap			return self._map.copymap

	@requires_no_parents_change			@requires_no_parents_change
	def set_tracked(self, filename):			def set_tracked(self, filename, reset_copy=False):
	"""a "public" method for generic code to mark a file as tracked			"""a "public" method for generic code to mark a file as tracked

	This function is to be called outside of "update/merge" case. For			This function is to be called outside of "update/merge" case. For
	example by a command like `hg add X`.			example by a command like `hg add X`.

				if reset_copy is set, any existing copy information will be dropped.

	return True the file was previously untracked, False otherwise.			return True the file was previously untracked, False otherwise.
	"""			"""
	self._dirty = True			self._dirty = True
	entry = self._map.get(filename)			entry = self._map.get(filename)
	if entry is None or not entry.tracked:			if entry is None or not entry.tracked:
	self._check_new_tracked_filename(filename)			self._check_new_tracked_filename(filename)
	return self._map.set_tracked(filename)			pre_tracked = self._map.set_tracked(filename)
				if reset_copy:
				self._map.copymap.pop(filename, None)
				return pre_tracked

	@requires_no_parents_change			@requires_no_parents_change
	def set_untracked(self, filename):			def set_untracked(self, filename):
	"""a "public" method for generic code to mark a file as untracked			"""a "public" method for generic code to mark a file as untracked

	This function is to be called outside of "update/merge" case. For			This function is to be called outside of "update/merge" case. For
	example by a command like `hg remove X`.			example by a command like `hg remove X`.

tests/test-revert.t

	$ hg revert newa			$ hg revert newa
	$ hg st a newa			$ hg st a newa
	? newa			? newa

	Also true for move overwriting an existing file			Also true for move overwriting an existing file

	$ hg mv --force a b/b			$ hg mv --force a b/b
	$ hg revert b/b			$ hg revert b/b
	$ hg status a b/b			$ hg status a b/b --copies

	$ cd ..			$ cd ..

	$ hg init ignored			$ hg init ignored
	$ cd ignored			$ cd ignored
	$ echo '^ignored$' > .hgignore			$ echo '^ignored$' > .hgignore
	$ echo '^ignoreddir$' >> .hgignore			$ echo '^ignoreddir$' >> .hgignore
	$ echo '^removed$' >> .hgignore			$ echo '^removed$' >> .hgignore

tests/test-win32text.t

	% just linefeed			% just linefeed
	$ hg st -q			$ hg st -q
	$ echo modified >> linefeed			$ echo modified >> linefeed
	$ hg st -q			$ hg st -q
	M linefeed			M linefeed
	$ hg revert -a			$ hg revert -a
	reverting linefeed			reverting linefeed
	$ hg st -q			$ hg st -q
				M linefeed (known-bad-output !)
	$ cat linefeed			$ cat linefeed
	% just linefeed\r (esc)			% just linefeed\r (esc)

	$ cd ..			$ cd ..