D6693: fix: ignore fixer tool configurations that are missing patterns

hooper (Danny Hooper) phabricator at mercurial-scm.org
Wed Jul 24 19:39:20 EDT 2019


hooper created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  This is to prevent a crash under the same circumstances.
  
  This is also to avoid data loss due to accidental application of a fixer tool
  to all files, if the matching logic somehow changed to that effect. Affecting
  all files until otherwise configured would be dangerous, and not very useful.
  
  We shouldn't abort because there may be other fixers, and it may still be
  useful to run them without having to adjust configuration. A user might not
  feel confident in changing configs, for example.

REPOSITORY
  rHG Mercurial

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

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

CHANGE DETAILS

diff --git a/tests/test-fix.t b/tests/test-fix.t
--- a/tests/test-fix.t
+++ b/tests/test-fix.t
@@ -1264,3 +1264,23 @@
 
   $ cd ..
 
+Tools configured without a pattern are ignored. It would be too dangerous to
+run them on all files, because this might happen while testing a configuration
+that also deletes all of the file content. There is no reasonable subset of the
+files to use as a default. Users should be explicit about what files are
+affected by a tool. This test also confirms that we don't crash when the
+pattern config is missing, and that we only warn about it once.
+
+  $ hg init nopatternconfigured
+  $ cd nopatternconfigured
+
+  $ printf "foo" > foo
+  $ printf "bar" > bar
+  $ hg add -q
+  $ hg fix --debug --working-dir --config "fix.nopattern:command=echo fixed"
+  fixer tool has no pattern configuration: nopattern
+  $ cat foo bar
+  foobar (no-eol)
+
+  $ cd ..
+
diff --git a/hgext/fix.py b/hgext/fix.py
--- a/hgext/fix.py
+++ b/hgext/fix.py
@@ -710,6 +710,14 @@
             setattr(fixers[name], pycompat.sysstr('_' + key),
                     attrs.get(key, default))
         fixers[name]._priority = int(fixers[name]._priority)
+        # Don't use a fixer if it has no pattern configured. It would be
+        # dangerous to let it affect all files. It would be pointless to let it
+        # affect no files. There is no reasonable subset of files to use as the
+        # default.
+        if fixers[name]._pattern is None:
+            ui.warn(
+                _('fixer tool has no pattern configuration: %s\n') % (name,))
+            del fixers[name]
     return collections.OrderedDict(
         sorted(fixers.items(), key=lambda item: item[1]._priority,
                reverse=True))
@@ -727,7 +735,8 @@
 
     def affects(self, opts, fixctx, path):
         """Should this fixer run on the file at the given path and context?"""
-        return scmutil.match(fixctx, [self._pattern], opts)(path)
+        return (self._pattern is not None and
+                scmutil.match(fixctx, [self._pattern], opts)(path))
 
     def shouldoutputmetadata(self):
         """Should the stdout of this fixer start with JSON and a null byte?"""



To: hooper, #hg-reviewers
Cc: mercurial-devel


More information about the Mercurial-devel mailing list