mercurial-scm/hg: mercurial/merge.py comparison

comparison mercurial/merge.py @ 42456:87a34c767384

merge: fix race that could cause wrong size in dirstate The problem is that hg merge/update/etc work the following way: 1. figure out what files to update 2. apply the update to disk 3. apply the update to in-memory dirstate 4. write dirstate where step3 looks at the filesystem and assumes it sees the result of step2. If a file is changed between step2 and step3, step3 will record incorrect information in the dirstate. I avoid this by passing the size step3 needs directly from step2, for the common path (not implemented for change/delete conflicts for instance). I didn't fix the same race for the exec bit for now, because it's less likely to be problematic and I had trouble due to the fact that the dirstate stores the permissions differently from the manifest (st_mode vs '' 'l' 'x'), in combination with tests that pretend that symlinks are not supported. However, I moved the lstat from step3 to step2, which should tighten the race window markedly, both for the exec bit and for the mtime. Differential Revision: https://phab.mercurial-scm.org/D6475

author	Valentin Gatien-Baron <valentin.gatienbaron@gmail.com>
date	Mon, 27 May 2019 16:55:46 -0400
parents	127937874395
children	d29db0a0c4eb

comparison

equal deleted inserted replaced

-:5ca136bbd3f6
+:87a34c767384
 from __future__ import absolute_import
 import errno
 import hashlib
 import shutil
+import stat
 import struct
 from .i18n import _
 from .node import (
 addednodeid,
 return actions
 def recordactions(self):
 """record remove/add/get actions in the dirstate"""
 branchmerge = self._repo.dirstate.p2() != nullid
-recordupdates(self._repo, self.actions(), branchmerge)
+recordupdates(self._repo, self.actions(), branchmerge, None)
 def queueremove(self, f):
 """queues a file to be removed from the dirstate
 Meant for use by custom merge drivers."""
 # cwd was removed in the course of removing files; print a helpful
 # warning.
 repo.ui.warn(_("current directory was removed\n"
 "(consider changing to repo root: %s)\n") % repo.root)
-def batchget(repo, mctx, wctx, actions):
+def batchget(repo, mctx, wctx, wantfiledata, actions):
 """apply gets to the working directory
 mctx is the context to get from
-yields tuples for progress updates
+Yields arbitrarily many (False, tuple) for progress updates, followed by
+exactly one (True, filedata). When wantfiledata is false, filedata is an
+empty list. When wantfiledata is true, filedata[i] is a triple (mode, size,
+mtime) of the file written for action[i].
 """
+filedata = []
 verbose = repo.ui.verbose
 fctx = mctx.filectx
 ui = repo.ui
 i = 0
 with repo.wvfs.backgroundclosing(ui, expectedcount=len(actions)):
 conflicting = p
 break
 if repo.wvfs.lexists(conflicting):
 orig = scmutil.backuppath(ui, repo, conflicting)
 util.rename(repo.wjoin(conflicting), orig)
-wctx[f].clearunknown()
+wfctx = wctx[f]
+wfctx.clearunknown()
 atomictemp = ui.configbool("experimental", "update.atomic-file")
-wctx[f].write(fctx(f).data(), flags, backgroundclose=True,
+size = wfctx.write(fctx(f).data(), flags,
-atomictemp=atomictemp)
+backgroundclose=True,
+atomictemp=atomictemp)
+if wantfiledata:
+s = wfctx.lstat()
+mode = s.st_mode
+mtime = s[stat.ST_MTIME]
+filedata.append((mode, size, mtime)) # for dirstate.normal
 if i == 100:
-yield i, f
+yield False, (i, f)
 i = 0
 i += 1
 if i > 0:
-yield i, f
+yield False, (i, f)
+yield True, filedata
 def _prefetchfiles(repo, ctx, actions):
 """Invoke ``scmutil.prefetchfiles()`` for the files relevant to the dict
 of merge actions.  ``ctx`` is the context being merged in."""
 ACTION_EXEC,
 ACTION_KEEP,
 ACTION_PATH_CONFLICT,
 ACTION_PATH_CONFLICT_RESOLVE))
-def applyupdates(repo, actions, wctx, mctx, overwrite, labels=None):
+def applyupdates(repo, actions, wctx, mctx, overwrite, wantfiledata,
+labels=None):
 """apply the merge action list to the working directory
 wctx is the working copy context
 mctx is the context to be merged into the working copy
-Return a tuple of counts (updated, merged, removed, unresolved) that
+Return a tuple of (counts, filedata), where counts is a tuple
-describes how many files were affected by the update.
+(updated, merged, removed, unresolved) that describes how many
+files were affected by the update, and filedata is as described in
+batchget.
 """
 _prefetchfiles(repo, mctx, actions)
 updated, merged, removed = 0, 0, 0
 progress.increment(item=f)
 # get in parallel.
 threadsafe = repo.ui.configbool('experimental',
 'worker.wdir-get-thread-safe')
-prog = worker.worker(repo.ui, cost, batchget, (repo, mctx, wctx),
+prog = worker.worker(repo.ui, cost, batchget,
+(repo, mctx, wctx, wantfiledata),
 actions[ACTION_GET],
-threadsafe=threadsafe)
+threadsafe=threadsafe,
-for i, item in prog:
+hasretval=True)
-progress.increment(step=i, item=item)
+getfiledata = []
+for final, res in prog:
+if final:
+getfiledata = res
+else:
+i, item = res
+progress.increment(step=i, item=item)
 updated = len(actions[ACTION_GET])
 if [a for a in actions[ACTION_GET] if a[0] == '.hgsubstate']:
 subrepoutil.submerge(repo, wctx, mctx, wctx, overwrite, labels)
 extraactions = ms.actions()
 if extraactions:
 mfiles = set(a[0] for a in actions[ACTION_MERGE])
 for k, acts in extraactions.iteritems():
 actions[k].extend(acts)
+if k == ACTION_GET and wantfiledata:
+# no filedata until mergestate is updated to provide it
+getfiledata.extend([None] * len(acts))
 # Remove these files from actions[ACTION_MERGE] as well. This is
 # important because in recordupdates, files in actions[ACTION_MERGE]
 # are processed after files in other actions, and the merge driver
 # might add files to those actions via extraactions above. This can
 # lead to a file being recorded twice, with poor results. This is
 actions[ACTION_MERGE] = [a for a in actions[ACTION_MERGE]
 if a[0] in mfiles]
 progress.complete()
-return updateresult(updated, merged, removed, unresolved)
+assert len(getfiledata) == (len(actions[ACTION_GET]) if wantfiledata else 0)
+return updateresult(updated, merged, removed, unresolved), getfiledata
-def recordupdates(repo, actions, branchmerge):
+def recordupdates(repo, actions, branchmerge, getfiledata):
 "record merge actions to the dirstate"
 # remove (must come first)
 for f, args, msg in actions.get(ACTION_REMOVE, []):
 if branchmerge:
 repo.dirstate.remove(f)
 # keep
 for f, args, msg in actions.get(ACTION_KEEP, []):
 pass
 # get
-for f, args, msg in actions.get(ACTION_GET, []):
+for i, (f, args, msg) in enumerate(actions.get(ACTION_GET, [])):
 if branchmerge:
 repo.dirstate.otherparent(f)
 else:
-repo.dirstate.normal(f)
+parentfiledata = getfiledata[i] if getfiledata else None
+repo.dirstate.normal(f, parentfiledata=parentfiledata)
 # merge
 for f, args, msg in actions.get(ACTION_MERGE, []):
 f1, f2, fa, move, anc = args
 if branchmerge:
 repo.ui.warn(
 _('(warning: large working directory being used without '
 'fsmonitor enabled; enable fsmonitor to improve performance; '
 'see "hg help -e fsmonitor")\n'))
-stats = applyupdates(repo, actions, wc, p2, overwrite, labels=labels)
+updatedirstate = not partial and not wc.isinmemory()
+wantfiledata = updatedirstate and not branchmerge
-if not partial and not wc.isinmemory():
+stats, getfiledata = applyupdates(repo, actions, wc, p2, overwrite,
+wantfiledata, labels=labels)
+if updatedirstate:
 with repo.dirstate.parentchange():
 repo.setparents(fp1, fp2)
-recordupdates(repo, actions, branchmerge)
+recordupdates(repo, actions, branchmerge, getfiledata)
 # update completed, clear state
 util.unlink(repo.vfs.join('updatestate'))
 if not branchmerge:
 repo.dirstate.setbranch(p2.branch())

Mercurial > public > mercurial-scm > hg

comparison mercurial/merge.py @ 42456:87a34c767384