( )⚙ D11784 update: filter the ambiguous mtime in update directly

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D11784

update: filter the ambiguous mtime in update directly
ClosedPublic

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

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Commits
rHG6dc3dc5e9636: update: filter the ambiguous mtime in update directly
Summary

Right now, this filtering is done by dirstate.write using the time of
dirstate.write method call. However that filtering is done "too late"
It works "fine" as most command are "fast enough", and race rare enough.
We are about to change the mtime filtering logic in the dirstate to be more
accurate and reliable.

However hg update will still need such filtering (mostly because it is
actually quite racy, even with the existing filtering). So we explicitly
implement a similar logic here. Before removing the older one later in the
series.

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:11 AM
Alphare requested changes to this revision.Nov 24 2021, 8:18 AM
Alphare added a subscriber: Alphare.
Alphare added inline comments.
mercurial/merge.py
2187

Not sure what this sentence was supposed to be

This revision now requires changes to proceed.Nov 24 2021, 8:18 AM
marmoute edited the summary of this revision. (Show Details)Nov 30 2021, 6:53 PM
marmoute updated this revision to Diff 31225.
Alphare accepted this revision.Dec 1 2021, 5:09 AM
This revision is now accepted and ready to land.Dec 1 2021, 5:09 AM
marmoute added a commit: rHG6dc3dc5e9636: update: filter the ambiguous mtime in update directly.Dec 1 2021, 10:21 AM
Closed by commit rHG6dc3dc5e9636: update: filter the ambiguous mtime in update directly (authored by marmoute). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/merge.py (58 lines)

Diff 31106

mercurial/merge.py

# no filedata until mergestate is updated to provide it # no filedata until mergestate is updated to provide it
for a in acts: for a in acts:
getfiledata[a[0]] = None getfiledata[a[0]] = None
assert len(getfiledata) == ( assert len(getfiledata) == (
mresult.len((mergestatemod.ACTION_GET,)) if wantfiledata else 0 mresult.len((mergestatemod.ACTION_GET,)) if wantfiledata else 0
) )
with repo.dirstate.parentchange(): with repo.dirstate.parentchange():
### Filter Filedata
#
# We gathered "cache" information for the clean file while
# updating them: mtime, size and mode.
#
# At the time this comment is written, they are various issues
# with how we gather this information (see the comment in
# `batchget`).
#
# We are going to smooth one of this issue here : mtime ambiguity.
#
# i.e. even if the mtime gathered during `batchget` was
# correct[1] a change happening right after it could change the
# content while keeping the same mtime[2].
#
# So, not we the update operation is finished, we will only
AlphareUnsubmitted
Not Done

Not sure what this sentence was supposed to be

Alphare: Not sure what this sentence was supposed to be
# keep the file data for files with mtime that are stricly in
# the past, i.e. whose mtime is strictly lower than the current
# time. This protect us from race condition from operation that
# could run right after this one, especially other Mercurial
# operation that could be waiting for the wlock to touch file
# content and the dirstate.
#
# In an ideal world, we could only get reliable information in
# `getfiledata` (from `getbatch`), however the current approach
# have been a successful compromise since many years.
#
# At the time this comment is written, not using any "cache"
# file data at all here would not be viable. As it would result is
# a very large amount of work (equivalement to the previous `hg
# update` during the next status after an update).
#
# [1] the current code cannot garantee that the mtime is
# correct, but the result is "okay in practice", so lets ignore
# that sub issue.
#
# [2] using nano-second precision can greatly help here because
# it makes the "different write with same mtime" issue
# virtually vanish. However, dirstate v1 cannot store such
# precision and a bunch of python runtime, operating system and
# filesystem does not provide use with such precision, so we
# have to operate as if it wasn't available.
if getfiledata:
ambiguous_mtime = {}
now = timestamp.get_fs_now(repo.vfs)
if now is None:
# we can't write to the FS, so we won't actually update
# the dirstate content anyway, no need to put cache
# information.
getfiledata = None
else:
now_sec = now[0]
for f, m in pycompat.iteritems(getfiledata):
if m is not None and m[2][0] >= now_sec:
ambiguous_mtime[f] = (m[0], m[1], None)
for f, m in pycompat.iteritems(ambiguous_mtime):
getfiledata[f] = m
repo.setparents(fp1, fp2) repo.setparents(fp1, fp2)
mergestatemod.recordupdates( mergestatemod.recordupdates(
repo, mresult.actionsdict, branchmerge, getfiledata repo, mresult.actionsdict, 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: