( )⚙ D11788 dirstate: do no use `set_clean` in revert

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.

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
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

marmoute created this revision.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
3632

"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.
Alphare requested changes to this revision.Dec 1 2021, 5:15 AM

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

PathPackages
Mmercurial/cmdutil.py (20 lines)
Mmercurial/dirstate.py (9 lines)
Mtests/test-revert.t (2 lines)
Mtests/test-win32text.t (1 line)

Diff 31110

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
if node == parent:
# We're reverting to our parent. If possible, we'd like status # We're reverting to our parent. If possible, we'd like status
# to report the file as clean. We have to use normallookup for # to report the file as clean. We have to be less agressive for merge
AlphareUnsubmitted
Not Done

"merge" twice

Alphare: "merge" twice
# merges to avoid losing information about merged/dirty files. # merges to avoid losing information about copy introduced by the merge.
if p2 != repo.nullid: # This might comes with bugs ?
normal = repo.dirstate.set_tracked reset_copy = p2 == repo.nullid
else:
normal = repo.dirstate.set_clean def normal(filename):
return repo.dirstate.set_tracked(filename, reset_copy=reset_copy)
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 ..