[PATCH 2 of 2] commit: fix the dirstate race from issue2264 for all scripts/extensions

Greg Ward greg-hg at gerg.ca
Sat Jan 29 15:11:47 CST 2011


# HG changeset patch
# User Greg Ward <greg-hg at gerg.ca>
# Date 1296335278 18000
# Branch stable
# Node ID 4a74ec2ae4f1359c27bf82fbf00646ad1e32ea04
# Parent  10d88a8557f9eaa7e681bba661d49cf6081b5e5b
commit: fix the dirstate race from issue2264 for all scripts/extensions.

Instead of marking all files in a transplanted changeset with
dirstate.normallookup() (need to check file contents) in transplant,
do it in localrepository.commit().  Doing it in transplant obviously
helps transplant, but doesn't help other scripts/extensions that do
multiple commits in the same process.  In particular, any
script/extension that meets the following criteria:
  - multiple commits in the same process
  - two commits in the same second (or whatever filesystem resolution is)
  - while holding the same repo lock
  - where the second commit modifies a file also modified in the first
    commit without changing its size
is affected by this dirstate race: the file in question will not be
committed in the second commit, and will be left 'M'odfied in the
working dir.

diff --git a/hgext/transplant.py b/hgext/transplant.py
--- a/hgext/transplant.py
+++ b/hgext/transplant.py
@@ -263,11 +263,6 @@
 
         n = repo.commit(message, user, date, extra=extra, match=m)
 
-        # force repo.status() to look harder at each file in this patch
-        # when committing the next patch (avoids a dirstate race)
-        for fn in files:
-            repo.dirstate.normallookup(fn)
-
         if not n:
             # Crash here to prevent an unclear crash later, in
             # transplants.write().  This can happen if patch.patch()
diff --git a/mercurial/localrepo.py b/mercurial/localrepo.py
--- a/mercurial/localrepo.py
+++ b/mercurial/localrepo.py
@@ -968,9 +968,13 @@
                         _('note: commit message saved in %s\n') % msgfn)
                 raise
 
-            # update dirstate and mergestate
+            # update dirstate and mergestate: call normallookup(),
+            # rather than normal(), in case we do a second commit in the
+            # same process in the same second while holding the same
+            # repo lock, and the second commit modifies a file without
+            # changing its size (issue2264)
             for f in changes[0] + changes[1]:
-                self.dirstate.normal(f)
+                self.dirstate.normallookup(f)
             for f in changes[2]:
                 self.dirstate.forget(f)
             self.dirstate.setparents(ret)
diff --git a/tests/test-transplant-multiple.t b/tests/test-transplant-multiple.t
--- a/tests/test-transplant-multiple.t
+++ b/tests/test-transplant-multiple.t
@@ -78,3 +78,39 @@
   $ hg status --rev 4:5
   M bugfix
   M file1
+
+now test that we fixed the bug for all scripts/extensions
+  $ cat > $TESTTMP/committwice.py <<__EOF__
+  > from mercurial import ui, hg, match, node
+  > 
+  > def replacebyte(fn, b):
+  >     f = open("file1", "rb+")
+  >     f.seek(0, 0)
+  >     f.write(b)
+  >     f.close()
+  > 
+  > repo = hg.repository(ui.ui(), '.')
+  > assert len(repo) == 6, \
+  >        "initial: len(repo) == %d, expected 6" % len(repo)
+  > try:
+  >     wlock = repo.wlock()
+  >     lock = repo.lock()
+  >     m = match.exact(repo.root, '', ['file1'])
+  >     replacebyte("file1", "x")
+  >     n = repo.commit(text="x", user="test", date=(0, 0), match=m)
+  >     print "commit 1: len(repo) == %d" % len(repo)
+  >     replacebyte("file1", "y")
+  >     n = repo.commit(text="y", user="test", date=(0, 0), match=m)
+  >     print "commit 2: len(repo) == %d" % len(repo)
+  > finally:
+  >     lock.release()
+  >     wlock.release()
+  > __EOF__
+  $ $PYTHON $TESTTMP/committwice.py
+  commit 1: len(repo) == 7
+  commit 2: len(repo) == 8
+  $ hg status
+  $ hg log --template "{rev}  {desc}  {files}\n" -r5:
+  5  fix 2  bugfix file1
+  6  x  file1
+  7  y  file1


More information about the Mercurial-devel mailing list