changeset 34107:4f60720cf0df

blackbox: fix rotation with chg

The added test will show:

  $ $PYTHON showsize.py .hg/blackbox*
  .hg/blackbox.log: < 500
  .hg/blackbox.log.1: < 500
  .hg/blackbox.log.2: < 500
  .hg/blackbox.log.3: < 500
  .hg/blackbox.log.4: < 500
  .hg/blackbox.log.5: >= 500

with previous code.

The issue is caused by blackbox caching file objects *by path*, and the
rotation size check could run on a wrong file object (i.e. it should check
"blackbox.log", but `filehandles["blackbox.log"]` contains a file object
that has been renamed to "blackbox.log.5").

This patch removes the "filehandlers" global cache added by 45313f5a3a8c to
solve the issue.

I think the original patch was trying to make different ui objects use a same
file object if their blackbox.log path is the same. In theory it could also
be problematic in the rotation case. Anyway, that should become unnecessary
after D650.

Differential Revision: https://phab.mercurial-scm.org/D648

--- a/hgext/blackbox.py	Wed Sep 06 18:31:25 2017 -0700
+++ b/hgext/blackbox.py	Wed Sep 06 19:27:30 2017 -0700
@@ -73,21 +73,6 @@
 
 lastui = None
 
-filehandles = {}
-
-def _openlog(vfs):
-    path = vfs.join('blackbox.log')
-    if path in filehandles:
-        return filehandles[path]
-    filehandles[path] = fp = vfs('blackbox.log', 'a')
-    return fp
-
-def _closelog(vfs):
-    path = vfs.join('blackbox.log')
-    fp = filehandles[path]
-    del filehandles[path]
-    fp.close()
-
 def wrapui(ui):
     class blackboxui(ui.__class__):
         def __init__(self, src=None):
@@ -132,21 +117,23 @@
                         self.debug("warning: cannot rename '%s' to '%s': %s\n" %
                                    (newpath, oldpath, err.strerror))
 
-            fp = _openlog(self._bbvfs)
             maxsize = self.configbytes('blackbox', 'maxsize')
+            name = 'blackbox.log'
             if maxsize > 0:
-                st = self._bbvfs.fstat(fp)
-                if st.st_size >= maxsize:
-                    path = fp.name
-                    _closelog(self._bbvfs)
-                    maxfiles = self.configint('blackbox', 'maxfiles', 7)
-                    for i in xrange(maxfiles - 1, 1, -1):
-                        rotate(oldpath='%s.%d' % (path, i - 1),
-                               newpath='%s.%d' % (path, i))
-                    rotate(oldpath=path,
-                           newpath=maxfiles > 0 and path + '.1')
-                    fp = _openlog(self._bbvfs)
-            return fp
+                try:
+                    st = self._bbvfs.stat(name)
+                except OSError:
+                    pass
+                else:
+                    if st.st_size >= maxsize:
+                        path = self._bbvfs.join(name)
+                        maxfiles = self.configint('blackbox', 'maxfiles', 7)
+                        for i in xrange(maxfiles - 1, 1, -1):
+                            rotate(oldpath='%s.%d' % (path, i - 1),
+                                   newpath='%s.%d' % (path, i))
+                        rotate(oldpath=path,
+                               newpath=maxfiles > 0 and path + '.1')
+            return self._bbvfs(name, 'a')
 
         def _bbwrite(self, fmt, *args):
             self._bbfp.write(fmt % args)

--- a/tests/test-blackbox.t	Wed Sep 06 18:31:25 2017 -0700
+++ b/tests/test-blackbox.t	Wed Sep 06 19:27:30 2017 -0700
@@ -230,3 +230,57 @@
 
 cleanup
   $ cd ..
+
+#if chg
+
+when using chg, blackbox.log should get rotated correctly
+
+  $ cat > $TESTTMP/noop.py << EOF
+  > from __future__ import absolute_import
+  > import time
+  > from mercurial import registrar, scmutil
+  > cmdtable = {}
+  > command = registrar.command(cmdtable)
+  > @command('noop')
+  > def noop(ui, repo):
+  >     pass
+  > EOF
+
+  $ hg init blackbox-chg
+  $ cd blackbox-chg
+
+  $ cat > .hg/hgrc << EOF
+  > [blackbox]
+  > maxsize = 500B
+  > [extensions]
+  > # extension change forces chg to restart
+  > noop=$TESTTMP/noop.py
+  > EOF
+
+  $ $PYTHON -c 'print("a" * 400)' > .hg/blackbox.log
+  $ chg noop
+  $ chg noop
+  $ chg noop
+  $ chg noop
+  $ chg noop
+
+  $ cat > showsize.py << 'EOF'
+  > import os, sys
+  > limit = 500
+  > for p in sys.argv[1:]:
+  >     size = os.stat(p).st_size
+  >     if size >= limit:
+  >         desc = '>='
+  >     else:
+  >         desc = '<'
+  >     print('%s: %s %d' % (p, desc, limit))
+  > EOF
+
+  $ $PYTHON showsize.py .hg/blackbox*
+  .hg/blackbox.log: < 500
+  .hg/blackbox.log.1: >= 500
+  .hg/blackbox.log.2: >= 500
+
+  $ cd ..
+
+#endif