[PATCH 2 of 2 SPRINT] changelog: avoid redundant truncation of 00changelog.i at failure

[FUJIWARA Katsunori]

# HG changeset patch
# User FUJIWARA Katsunori <foozy at lares.dti.ne.jp>
# Date 1446574170 -32400
#      Wed Nov 04 03:09:30 2015 +0900
# Node ID 79fae5df4947f0665eb0adf762a4c95d823901c1
# Parent  9716bacc8e464e9ffaab609a74b9d31c9f93aa24
changelog: avoid redundant truncation of 00changelog.i at failure

Before this patch, '00changelog.i' is always truncated at transaction
failure, even though actual changes are written not into it but into
'00changelog.i.a' (aka "pending file").

This truncation changes behavior depending on timestamp of the file
(e.g. filecache). and may cause issues, which are difficult to
be certainly reproduced, (e.g. issue4876).

On the other hand, if once changes are written into '00changelog.i' by
'_finalize()', it should be truncated at failure.

To avoid such redundant truncation of '00changelog.i' at failure
correctly, this patch schedules invoking own '_avoidrollback()' at
failure by 'transaction.addabort()' in 'delayupdate()'.

It implies 'transaction._avoidrollback()' to avoid redundant
truncation of '00changelog.i' at failure

This patch introduces new status field '_finalized' to detect whether
truncation is needed or not, because combination of current inetrnal
status fields '_divert' and '_delayed' can't distinguish cases below:

  - transaction is aborted before any change (truncation not needed)
  - changelog is finalized successfully (truncation needed)

If changelog object in 'repo._filecache' isn't discarded as expected
at failure, this patch can reproduce problems like issue4876 more
frequently.

diff --git a/mercurial/changelog.py b/mercurial/changelog.py
--- a/mercurial/changelog.py
+++ b/mercurial/changelog.py
@@ -147,6 +147,7 @@ class changelog(revlog.revlog):
         self._delayed = False
         self._delaybuf = None
         self._divert = False
+        self._finalized = False
         self.filteredrevs = frozenset()
 
     def tip(self):
@@ -252,6 +253,7 @@ class changelog(revlog.revlog):
         self._delayed = True
         tr.addpending('cl-%i' % id(self), self._writepending)
         tr.addfinalize('cl-%i' % id(self), self._finalize)
+        tr.addabort('cl-%i' % id(self), self._avoidrollback)
 
     def _finalize(self, tr):
         "finalize index updates"
@@ -270,6 +272,7 @@ class changelog(revlog.revlog):
             fp.close()
             self._delaybuf = None
         self._divert = False
+        self._finalized = True
         # split when we're done
         self.checkinlinesize(tr)
 
@@ -318,6 +321,27 @@ class changelog(revlog.revlog):
 
         return False
 
+    def _avoidrollback(self, tr):
+        """Avoid redundant truncation of '00changelog.i' at failure
+
+        Without invocation of this function, '00changelog.i' is always
+        truncated at transaction failure, even though actual changes
+        are written not into it but into '00changelog.i.a'.
+
+        This truncation changes behavior depending on timestamp of
+        the file (e.g. filecache), and may cause issues, which are
+        difficult to be certainly reproduced.
+
+        On the other hand, if once changes are written into
+        '00changelog.i' (= finalized), it should be truncated at
+        failure.
+
+        This function is invoked as an abort hook of transaction and
+        avoids such truncation according to finalization status.
+        """
+        if not self._finalized:
+            tr._avoidrollback(self.indexfile)
+
     def checkinlinesize(self, tr, fp=None):
         if not self._delayed:
             revlog.revlog.checkinlinesize(self, tr, fp)
diff --git a/tests/test-commandserver.t b/tests/test-commandserver.t
--- a/tests/test-commandserver.t
+++ b/tests/test-commandserver.t
@@ -725,3 +725,146 @@ unix domain socket:
   [255]
 
 #endif
+
+  $ cd ..
+
+Test avoiding redundant trunaction of '00changelog.i' at transaction
+failure
+
+  $ hg init repo3
+  $ cd repo3
+
+- test for "failure before finalization"
+
+If repo object isn't discarded from filecache as expected at failure
+before finalization of changelog, subsequent command is aborted for
+accessing '00changelog.i.a' already removed.
+
+-- "failure before finalization" and "empty changelog"
+
+  $ echo foo > foo
+  $ hg add foo
+  >>> from hgclient import readchannel, runcommand, check
+  >>> @check
+  ... def abort(server):
+  ...     readchannel(server)
+  ...     runcommand(server, ['commit', '--config', 'hooks.pretxncommit=false',
+  ...                         '-mfoo'])
+  ...     runcommand(server, ['verify', '-q'])
+  *** runcommand commit --config hooks.pretxncommit=false -mfoo
+  transaction abort!
+  rollback completed
+  abort: pretxncommit hook exited with status 1
+   [255]
+  *** runcommand verify -q
+
+-- "failure before finalization" and "not-empty changelog"
+
+  $ echo bar > bar
+  $ hg add bar
+  >>> from hgclient import readchannel, runcommand, check
+  >>> @check
+  ... def abort(server):
+  ...     readchannel(server)
+  ...     runcommand(server, ['commit', '-mbar', 'bar'])
+  ...     runcommand(server, ['commit', '--config', 'hooks.pretxncommit=false',
+  ...                         '-mfoo', 'foo'])
+  ...     runcommand(server, ['verify', '-q'])
+  *** runcommand commit -mbar bar
+  *** runcommand commit --config hooks.pretxncommit=false -mfoo foo
+  transaction abort!
+  rollback completed
+  abort: pretxncommit hook exited with status 1
+   [255]
+  *** runcommand verify -q
+
+- test for "failure after finalization"
+
+If repo object isn't discarded from filecache as expected at failure
+after finalization of changelog, subsequent command shows already
+rollbacked revisions.
+
+  $ cd ..
+  $ rm -rf repo3
+  $ hg init repo3
+  $ cd repo3
+
+  $ cat > $TESTTMP/failafterfinalize.py <<EOF
+  > from mercurial import commands, error, extensions, lock as lockmod
+  > def fail(tr):
+  >     raise error.Abort('fail after finalization')
+  > def commitwrap(orig, ui, repo, *pats, **opts):
+  >     wlock = lock = tr = None
+  >     try:
+  >         wlock = repo.wlock()
+  >         lock = repo.lock()
+  >         tr = repo.transaction('failafterfinalize')
+  >         if ui.configbool('failafterfinalize', 'fail'):
+  >             # 'sorted()' by ASCII code on category names causes
+  >             # invoking 'fail' after finalization of changelog
+  >             # using "'cl-%i' % id(self)" as category name
+  >             tr.addfinalize('zzzzzzzz', fail)
+  >         result = orig(ui, repo, *pats, **opts)
+  >         tr.close()
+  >         return result
+  >     finally:
+  >         lockmod.release(tr, lock, wlock)
+  > def extsetup(ui):
+  >     extensions.wrapcommand(commands.table, 'commit', commitwrap)
+  > EOF
+  $ cat >> .hg/hgrc <<EOF
+  > [extensions]
+  > failafterfinalize = $TESTTMP/failafterfinalize.py
+  > EOF
+
+-- "failure after finalization" and "empty changelog"
+
+If transaction starts with empty changelog, finalization always
+renames from '00changelog.i.a' to '00changelog.i', and it causes
+reloading changelog from the file at next command execution, because
+removing should change i-node No of '00changelog.i'.
+
+Therefore, reloading from the file should always occurs at subsequent
+command execution, and test below pass safely, in fact. But this case
+is tested here, to detect regression like issue4876 quickly.
+
+  $ echo foo > foo
+  $ hg add foo
+  >>> from hgclient import readchannel, runcommand, check
+  >>> @check
+  ... def abort(server):
+  ...     readchannel(server)
+  ...     runcommand(server, ['commit', '--config', 'failafterfinalize.fail=true',
+  ...                         '-mfoo'])
+  ...     runcommand(server, ['log', '-T', '{rev} {desc|firstline} ({files})\\\\n'])
+  ...     runcommand(server, ['verify', '-q'])
+  *** runcommand commit --config failafterfinalize.fail=true -mfoo
+  transaction abort!
+  rollback completed
+  abort: fail after finalization
+   [255]
+  *** runcommand log -T {rev} {desc|firstline} ({files})\n
+  *** runcommand verify -q
+
+-- "failure after finalization" and "not-empty changelog"
+
+  $ echo bar > bar
+  $ hg add bar
+  >>> from hgclient import readchannel, runcommand, check
+  >>> @check
+  ... def abort(server):
+  ...     readchannel(server)
+  ...     runcommand(server, ['commit', '-mbar', 'bar'])
+  ...     runcommand(server, ['commit', '--config', 'failafterfinalize.fail=true',
+  ...                         '-mfoo', 'foo'])
+  ...     runcommand(server, ['log', '-T', '{rev} {desc|firstline} ({files})\\\\n'])
+  ...     runcommand(server, ['verify', '-q'])
+  *** runcommand commit -mbar bar
+  *** runcommand commit --config failafterfinalize.fail=true -mfoo foo
+  transaction abort!
+  rollback completed
+  abort: fail after finalization
+   [255]
+  *** runcommand log -T {rev} {desc|firstline} ({files})\n
+  0 bar (bar)
+  *** runcommand verify -q