Patchwork D5785: blackbox: take regex patterns for blackbox.track

login
register
mail settings
Submitter phabricator
Date Jan. 31, 2019, 10:07 p.m.
Message ID <differential-rev-PHID-DREV-kj4xqpdfyxse546dh35i-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/38270/
State New
Headers show

Comments

phabricator - Jan. 31, 2019, 10:07 p.m.
spectral created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D5785

AFFECTED FILES
  hgext/blackbox.py
  tests/test-blackbox.t

CHANGE DETAILS




To: spectral, #hg-reviewers
Cc: mercurial-devel
Yuya Nishihara - Feb. 1, 2019, 1:06 p.m.
> +        track = ui.configlist('blackbox', 'track')
> +        if not track:
> +            self._active = False
> +        elif b'*' in track:
> +            self._trackedevents = re.compile(b'.*')
> +        else:
> +            try:
> +                self._trackedevents = re.compile(b'|'.join(track))
> +            except re.error as e:
> +                ui.warn('invalid blackbox.track setting: %s\n' % e)
> +                self._active = False

Can't we process the pattern as a glob? It's compatible with `*` and much
easier to write. Or do you need the power of the regexp?
phabricator - Feb. 1, 2019, 1:19 p.m.
yuja added a comment.


  > +        track = ui.configlist('blackbox', 'track')
  >  +        if not track:
  >  +            self._active = False
  >  +        elif b'*' in track:
  >  +            self._trackedevents = re.compile(b'.*')
  >  +        else:
  >  +            try:
  >  +                self._trackedevents = re.compile(b'|'.join(track))
  >  +            except re.error as e:
  >  +                ui.warn('invalid blackbox.track setting: %s\n' % e)
  >  +                self._active = False
  
  Can't we process the pattern as a glob? It's compatible with `*` and much
  easier to write. Or do you need the power of the regexp?

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D5785

To: spectral, #hg-reviewers
Cc: yuja, mercurial-devel
phabricator - Feb. 4, 2019, 8:42 p.m.
spectral planned changes to this revision.
spectral added a comment.


  In https://phab.mercurial-scm.org/D5785#84935, @yuja wrote:
  
  > Can't we process the pattern as a glob? It's compatible with `*` and much
  >  easier to write. Or do you need the power of the regexp?
  
  
  I generally prefer regex, but I might be an outlier. We do not need the power of regexp, especially if we get a negative filter (next commit), so I'll do that (eventually).  Marking as "plan changes" for now...

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D5785

To: spectral, #hg-reviewers
Cc: yuja, mercurial-devel

Patch

diff --git a/tests/test-blackbox.t b/tests/test-blackbox.t
--- a/tests/test-blackbox.t
+++ b/tests/test-blackbox.t
@@ -23,8 +23,8 @@ 
   > confuse = log --limit 3
   > so-confusing = confuse --style compact
   > [blackbox]
-  > track = backupbundle, branchcache, command, commandalias, commandexception,
-  >         commandfinish, debug, exthook, incoming, pythonhook, tagscache
+  > track = backupbundle, branchcache, command.*,
+  >         debug, exthook, incoming, pythonhook, tagscache
   > EOF
 
   $ hg init blackboxtest
@@ -383,6 +383,19 @@ 
   1970/01/01 00:00:00 bob @0000000000000000000000000000000000000000 (5000) [command]> blackbox
   $ cd $TESTTMP
 
+invalid regex entries in blackbox.track are interpreted as log nothing
+
+  $ hg --config blackbox.track='**' init track_invalid_regex
+  invalid blackbox.track setting: nothing to repeat
+  $ cd track_invalid_regex
+  $ cat >> .hg/hgrc << EOF
+  > [blackbox]
+  > track = **
+  > EOF
+  $ hg blackbox
+  invalid blackbox.track setting: nothing to repeat
+  $ cd $TESTTMP
+
 #if chg
 
 when using chg, blackbox.log should get rotated correctly
diff --git a/hgext/blackbox.py b/hgext/blackbox.py
--- a/hgext/blackbox.py
+++ b/hgext/blackbox.py
@@ -9,20 +9,26 @@ 
 """log repository events to a blackbox for debugging
 
 Logs event information to .hg/blackbox.log to help debug and diagnose problems.
-The events that get logged can be configured via the blackbox.track config key.
+
+The events that get logged can be configured via the blackbox.track config key;
+this takes a list of regular expressions. As a special rule, if any item in the
+list is the string '*', everything is logged (default: '*').
 
 Examples::
 
   [blackbox]
+  # log everything
   track = *
   # dirty is *EXPENSIVE* (slow);
   # each log entry indicates `+` if the repository is dirty, like :hg:`id`.
   dirty = True
   # record the source of log messages
   logsource = True
 
   [blackbox]
-  track = command, commandfinish, commandexception, exthook, pythonhook
+  # log every event starting with 'command' (including 'command' itself), and
+  # everything ending with 'hook' (including 'hook' itself).
+  track = command.*, .*hook
 
   [blackbox]
   track = incoming
@@ -93,13 +99,24 @@ 
 class blackboxlogger(object):
     def __init__(self, ui, repo):
         self._repo = repo
-        self._trackedevents = set(ui.configlist('blackbox', 'track'))
+        self._active = True
+        track = ui.configlist('blackbox', 'track')
+        if not track:
+            self._active = False
+        elif b'*' in track:
+            self._trackedevents = re.compile(b'.*')
+        else:
+            try:
+                self._trackedevents = re.compile(b'|'.join(track))
+            except re.error as e:
+                ui.warn('invalid blackbox.track setting: %s\n' % e)
+                self._active = False
         self._maxfiles = ui.configint('blackbox', 'maxfiles')
         self._maxsize = ui.configbytes('blackbox', 'maxsize')
         self._inlog = False
 
     def tracked(self, event):
-        return b'*' in self._trackedevents or event in self._trackedevents
+        return self._active and self._trackedevents.match(event)
 
     def log(self, ui, event, msg, opts):
         # self._log() -> ctx.dirty() may create new subrepo instance, which
@@ -138,7 +155,7 @@ 
                 fp.write(fmt % args)
         except (IOError, OSError) as err:
             # deactivate this to avoid failed logging again
-            self._trackedevents.clear()
+            self._active = False
             ui.debug('warning: cannot write to blackbox.log: %s\n' %
                      encoding.strtolocal(err.strerror))
             return