bookmarks: check HG_PENDING strictly
authorFUJIWARA Katsunori <foozy@lares.dti.ne.jp>
Tue, 21 Feb 2017 01:21:00 +0900
changeset 31052 0332b8fafd05
parent 31051 96eaefd350ae
child 31053 6afd8a87a657
bookmarks: check HG_PENDING strictly Before this patch, checking HG_PENDING in bookmarks.py might cause unintentional reading unrelated '.hg/bookmarks.pending' in, because it just examines existence of HG_PENDING environment variable. This patch uses txnutil.trypending() to check HG_PENDING strictly. This patch also changes share extension. Enabling share extension (+ bookmark sharing) makes bookmarks._getbkfile() receive repo to be shared (= "srcrepo"). On the other hand, HG_PENDING always refers current working repo (= "currepo"), and bookmarks.pending is written only into currepo. Therefore, we should try to read .hg/bookmarks.pending of currepo in at first. If it doesn't exist, we try to read .hg/bookmarks of srcrepo in. Even after this patch, an external hook spawned in currepo can't see pending changes in currepo via srcrepo, even though such changes become visible after closing transaction, because there is no easy and cheap way to know existence of pending changes in currepo via srcrepo. Please see https://www.mercurial-scm.org/wiki/SharedRepository, too. BTW, this patch may cause failure of bisect in the repository of Mercurial itself, if examination at bisecting assumes that an external hook can see all pending changes while nested transactions across repositories. This invisibility issue will be fixed by subsequent patch, which allows HG_PENDING to refer multiple repositories.
hgext/share.py
mercurial/bookmarks.py
tests/test-bookmarks.t
tests/test-share.t
--- a/hgext/share.py	Tue Feb 21 01:20:59 2017 +0900
+++ b/hgext/share.py	Tue Feb 21 01:21:00 2017 +0900
@@ -48,6 +48,7 @@
     error,
     extensions,
     hg,
+    txnutil,
     util,
 )
 
@@ -171,7 +172,28 @@
     if _hassharedbookmarks(repo):
         srcrepo = _getsrcrepo(repo)
         if srcrepo is not None:
+            # just orig(srcrepo) doesn't work as expected, because
+            # HG_PENDING refers repo.root.
+            try:
+                fp, pending = txnutil.trypending(repo.root, repo.vfs,
+                                                 'bookmarks')
+                if pending:
+                    # only in this case, bookmark information in repo
+                    # is up-to-date.
+                    return fp
+                fp.close()
+            except IOError as inst:
+                if inst.errno != errno.ENOENT:
+                    raise
+
+            # otherwise, we should read bookmarks from srcrepo,
+            # because .hg/bookmarks in srcrepo might be already
+            # changed via another sharing repo
             repo = srcrepo
+
+            # TODO: Pending changes in repo are still invisible in
+            # srcrepo, because bookmarks.pending is written only into repo.
+            # See also https://www.mercurial-scm.org/wiki/SharedRepository
     return orig(repo)
 
 def recordchange(orig, self, tr):
--- a/mercurial/bookmarks.py	Tue Feb 21 01:20:59 2017 +0900
+++ b/mercurial/bookmarks.py	Tue Feb 21 01:21:00 2017 +0900
@@ -19,6 +19,7 @@
     error,
     lock as lockmod,
     obsolete,
+    txnutil,
     util,
 )
 
@@ -29,17 +30,8 @@
     bookmarks or the committed ones. Other extensions (like share)
     may need to tweak this behavior further.
     """
-    bkfile = None
-    if 'HG_PENDING' in encoding.environ:
-        try:
-            bkfile = repo.vfs('bookmarks.pending')
-        except IOError as inst:
-            if inst.errno != errno.ENOENT:
-                raise
-    if bkfile is None:
-        bkfile = repo.vfs('bookmarks')
-    return bkfile
-
+    fp, pending = txnutil.trypending(repo.root, repo.vfs, 'bookmarks')
+    return fp
 
 class bmstore(dict):
     """Storage for bookmarks.
--- a/tests/test-bookmarks.t	Tue Feb 21 01:20:59 2017 +0900
+++ b/tests/test-bookmarks.t	Tue Feb 21 01:21:00 2017 +0900
@@ -896,3 +896,58 @@
   $ touch $TESTTMP/unpause
 
   $ cd ..
+
+check whether HG_PENDING makes pending changes only in related
+repositories visible to an external hook.
+
+(emulate a transaction running concurrently by copied
+.hg/bookmarks.pending in subsequent test)
+
+  $ cat > $TESTTMP/savepending.sh <<EOF
+  > cp .hg/bookmarks.pending .hg/bookmarks.pending.saved
+  > exit 1 # to avoid adding new bookmark for subsequent tests
+  > EOF
+
+  $ hg init unrelated
+  $ cd unrelated
+  $ echo a > a
+  $ hg add a
+  $ hg commit -m '#0'
+  $ hg --config hooks.pretxnclose="sh $TESTTMP/savepending.sh" bookmarks INVISIBLE
+  transaction abort!
+  rollback completed
+  abort: pretxnclose hook exited with status 1
+  [255]
+  $ cp .hg/bookmarks.pending.saved .hg/bookmarks.pending
+
+(check visible bookmarks while transaction running in repo)
+
+  $ cat > $TESTTMP/checkpending.sh <<EOF
+  > echo "@repo"
+  > hg -R $TESTTMP/repo bookmarks
+  > echo "@unrelated"
+  > hg -R $TESTTMP/unrelated bookmarks
+  > exit 1 # to avoid adding new bookmark for subsequent tests
+  > EOF
+
+  $ cd ../repo
+  $ hg --config hooks.pretxnclose="sh $TESTTMP/checkpending.sh" bookmarks NEW
+  @repo
+   * NEW                       6:81dcce76aa0b
+     X2                        1:925d80f479bb
+     Y                         4:125c9a1d6df6
+     Z                         5:5fb12f0f2d51
+     Z@1                       1:925d80f479bb
+     Z@2                       4:125c9a1d6df6
+     foo                       3:9ba5f110a0b3
+     foo@1                     0:f7b1eb17ad24
+     foo@2                     2:db815d6d32e6
+     four                      3:9ba5f110a0b3
+     should-end-on-two         2:db815d6d32e6
+     x  y                      2:db815d6d32e6
+  @unrelated
+  no bookmarks set
+  transaction abort!
+  rollback completed
+  abort: pretxnclose hook exited with status 1
+  [255]
--- a/tests/test-share.t	Tue Feb 21 01:20:59 2017 +0900
+++ b/tests/test-share.t	Tue Feb 21 01:21:00 2017 +0900
@@ -154,6 +154,67 @@
    * bm1                       2:c2e0ac586386
      bm3                       2:c2e0ac586386
 
+check whether HG_PENDING makes pending changes only in relatd
+repositories visible to an external hook.
+
+In "hg share" case, another transaction can't run in other
+repositories sharing same source repository, because starting
+transaction requires locking store of source repository.
+
+Therefore, this test scenario ignores checking visibility of
+.hg/bookmakrs.pending in repo2, which shares repo1 without bookmarks.
+
+  $ cat > $TESTTMP/checkbookmarks.sh <<EOF
+  > echo "@repo1"
+  > hg -R $TESTTMP/repo1 bookmarks
+  > echo "@repo2"
+  > hg -R $TESTTMP/repo2 bookmarks
+  > echo "@repo3"
+  > hg -R $TESTTMP/repo3 bookmarks
+  > exit 1 # to avoid adding new bookmark for subsequent tests
+  > EOF
+
+  $ cd ../repo1
+  $ hg --config hooks.pretxnclose="sh $TESTTMP/checkbookmarks.sh" -q book bmX
+  @repo1
+     bm1                       2:c2e0ac586386
+     bm3                       2:c2e0ac586386
+   * bmX                       2:c2e0ac586386
+  @repo2
+   * bm2                       3:0e6e70d1d5f1
+  @repo3
+     bm1                       2:c2e0ac586386
+   * bm3                       2:c2e0ac586386
+     bmX                       2:c2e0ac586386
+  transaction abort!
+  rollback completed
+  abort: pretxnclose hook exited with status 1
+  [255]
+  $ hg book bm1
+
+FYI, in contrast to above test, bmX is invisible in repo1 (= shared
+src), because (1) HG_PENDING refers only repo3 and (2)
+"bookmarks.pending" is written only into repo3.
+
+  $ cd ../repo3
+  $ hg --config hooks.pretxnclose="sh $TESTTMP/checkbookmarks.sh" -q book bmX
+  @repo1
+   * bm1                       2:c2e0ac586386
+     bm3                       2:c2e0ac586386
+  @repo2
+   * bm2                       3:0e6e70d1d5f1
+  @repo3
+     bm1                       2:c2e0ac586386
+     bm3                       2:c2e0ac586386
+   * bmX                       2:c2e0ac586386
+  transaction abort!
+  rollback completed
+  abort: pretxnclose hook exited with status 1
+  [255]
+  $ hg book bm3
+
+  $ cd ../repo1
+
 test that commits work
 
   $ echo 'shared bookmarks' > a