fix: add a config to abort when a fixer tool fails
authorDanny Hooper <hooper@google.com>
Wed, 31 Oct 2018 13:11:51 -0700
changeset 40532 93bab80993f4
parent 40531 e6c8a0fd3db4
child 40533 2ecf5c24d0cd
fix: add a config to abort when a fixer tool fails This allows users to stop and address tool failures before proceeding, instead of the default behavior of continuing to apply any tools that didn't fail. For example, a code formatting tool could fail if you have syntax errors, and you might want your repo to stay in its current state while you fix the syntax error before re-running 'hg fix'. It's conceivable that this would even be necessary for the correctness of some fixer tools across a chain of revisions. Differential Revision: https://phab.mercurial-scm.org/D5200
hgext/fix.py
tests/test-fix.t
--- a/hgext/fix.py	Tue Nov 06 11:05:13 2018 +0100
+++ b/hgext/fix.py	Wed Oct 31 13:11:51 2018 -0700
@@ -19,9 +19,11 @@
 
 The :command suboption forms the first part of the shell command that will be
 used to fix a file. The content of the file is passed on standard input, and the
-fixed file content is expected on standard output. If there is any output on
-standard error, the file will not be affected. Some values may be substituted
-into the command::
+fixed file content is expected on standard output. Any output on standard error
+will be displayed as a warning. If the exit status is not zero, the file will
+not be affected. A placeholder warning is displayed if there is a non-zero exit
+status but no standard error output. Some values may be substituted into the
+command::
 
   {rootpath}  The path of the file being fixed, relative to the repo root
   {basename}  The name of the file being fixed, without the directory path
@@ -42,7 +44,15 @@
 processed by :hg:`fix`::
 
   [fix]
-  maxfilesize=2MB
+  maxfilesize = 2MB
+
+Normally, execution of configured tools will continue after a failure (indicated
+by a non-zero exit status). It can also be configured to abort after the first
+such failure, so that no files will be affected if any tool fails. This abort
+will also cause :hg:`fix` to exit with a non-zero status::
+
+  [fix]
+  failure = abort
 
 """
 
@@ -100,6 +110,20 @@
 # user.
 configitem('fix', 'maxfilesize', default='2MB')
 
+# Allow fix commands to exit non-zero if an executed fixer tool exits non-zero.
+# This helps users do shell scripts that stop when a fixer tool signals a
+# problem.
+configitem('fix', 'failure', default='continue')
+
+def checktoolfailureaction(ui, message, hint=None):
+    """Abort with 'message' if fix.failure=abort"""
+    action = ui.config('fix', 'failure')
+    if action not in ('continue', 'abort'):
+        raise error.Abort(_('unknown fix.failure action: %s') % (action,),
+                          hint=_('use "continue" or "abort"'))
+    if action == 'abort':
+        raise error.Abort(message, hint=hint)
+
 allopt = ('', 'all', False, _('fix all non-public non-obsolete revisions'))
 baseopt = ('', 'base', [], _('revisions to diff against (overrides automatic '
                              'selection, and applies to every revision being '
@@ -465,9 +489,14 @@
                 showstderr(ui, fixctx.rev(), fixername, stderr)
             if proc.returncode == 0:
                 newdata = newerdata
-            elif not stderr:
-                showstderr(ui, fixctx.rev(), fixername,
-                           _('exited with status %d\n') % (proc.returncode,))
+            else:
+                if not stderr:
+                    message = _('exited with status %d\n') % (proc.returncode,)
+                    showstderr(ui, fixctx.rev(), fixername, message)
+                checktoolfailureaction(
+                    ui, _('no fixes will be applied'),
+                    hint=_('use --config fix.failure=continue to apply any '
+                           'successful fixes anyway'))
     return newdata
 
 def showstderr(ui, rev, fixername, stderr):
--- a/tests/test-fix.t	Tue Nov 06 11:05:13 2018 +0100
+++ b/tests/test-fix.t	Wed Oct 31 13:11:51 2018 -0700
@@ -130,9 +130,11 @@
   
   The :command suboption forms the first part of the shell command that will be
   used to fix a file. The content of the file is passed on standard input, and
-  the fixed file content is expected on standard output. If there is any output
-  on standard error, the file will not be affected. Some values may be
-  substituted into the command:
+  the fixed file content is expected on standard output. Any output on standard
+  error will be displayed as a warning. If the exit status is not zero, the file
+  will not be affected. A placeholder warning is displayed if there is a non-
+  zero exit status but no standard error output. Some values may be substituted
+  into the command:
   
     {rootpath}  The path of the file being fixed, relative to the repo root
     {basename}  The name of the file being fixed, without the directory path
@@ -153,7 +155,15 @@
   processed by 'hg fix':
   
     [fix]
-    maxfilesize=2MB
+    maxfilesize = 2MB
+  
+  Normally, execution of configured tools will continue after a failure
+  (indicated by a non-zero exit status). It can also be configured to abort
+  after the first such failure, so that no files will be affected if any tool
+  fails. This abort will also cause 'hg fix' to exit with a non-zero status:
+  
+    [fix]
+    failure = abort
   
   list of commands:
   
@@ -508,7 +518,9 @@
 on stderr and nothing on stdout, which would cause us the clear the file,
 except that they also exit with a non-zero code. We show the user which fixer
 emitted the stderr, and which revision, but we assume that the fixer will print
-the filename if it is relevant (since the issue may be non-specific).
+the filename if it is relevant (since the issue may be non-specific). There is
+also a config to abort (without affecting any files whatsoever) if we see any
+tool with a non-zero exit status.
 
   $ hg init showstderr
   $ cd showstderr
@@ -516,32 +528,51 @@
   $ printf "hello\n" > hello.txt
   $ hg add
   adding hello.txt
-  $ cat > $TESTTMP/fail.sh <<'EOF'
+  $ cat > $TESTTMP/work.sh <<'EOF'
   > printf 'HELLO\n'
-  > printf "$@: some\nerror" >&2
+  > printf "$@: some\nerror that didn't stop the tool" >&2
   > exit 0 # success despite the stderr output
   > EOF
+  $ hg --config "fix.work:command=sh $TESTTMP/work.sh {rootpath}" \
+  >    --config "fix.work:fileset=hello.txt" \
+  >    fix --working-dir
+  [wdir] work: hello.txt: some
+  [wdir] work: error that didn't stop the tool
+  $ cat hello.txt
+  HELLO
+
+  $ printf "goodbye\n" > hello.txt
+  $ printf "foo\n" > foo.whole
+  $ hg add
+  adding foo.whole
+  $ cat > $TESTTMP/fail.sh <<'EOF'
+  > printf 'GOODBYE\n'
+  > printf "$@: some\nerror that did stop the tool\n" >&2
+  > exit 42 # success despite the stdout output
+  > EOF
+  $ hg --config "fix.fail:command=sh $TESTTMP/fail.sh {rootpath}" \
+  >    --config "fix.fail:fileset=hello.txt" \
+  >    --config "fix.failure=abort" \
+  >    fix --working-dir
+  [wdir] fail: hello.txt: some
+  [wdir] fail: error that did stop the tool
+  abort: no fixes will be applied
+  (use --config fix.failure=continue to apply any successful fixes anyway)
+  [255]
+  $ cat hello.txt
+  goodbye
+  $ cat foo.whole
+  foo
+
   $ hg --config "fix.fail:command=sh $TESTTMP/fail.sh {rootpath}" \
   >    --config "fix.fail:fileset=hello.txt" \
   >    fix --working-dir
   [wdir] fail: hello.txt: some
-  [wdir] fail: error
-  $ cat hello.txt
-  HELLO
-
-  $ printf "goodbye\n" > hello.txt
-  $ cat > $TESTTMP/work.sh <<'EOF'
-  > printf 'GOODBYE\n'
-  > printf "$@: some\nerror\n" >&2
-  > exit 42 # success despite the stdout output
-  > EOF
-  $ hg --config "fix.fail:command=sh $TESTTMP/work.sh {rootpath}" \
-  >    --config "fix.fail:fileset=hello.txt" \
-  >    fix --working-dir
-  [wdir] fail: hello.txt: some
-  [wdir] fail: error
+  [wdir] fail: error that did stop the tool
   $ cat hello.txt
   goodbye
+  $ cat foo.whole
+  FOO
 
   $ hg --config "fix.fail:command=exit 42" \
   >    --config "fix.fail:fileset=hello.txt" \