This is an archive of the discontinued Mercurial Phabricator instance.

Differential D5200

fix: add a config to abort when a fixer tool fails
ClosedPublic

Authored by hooper on Oct 31 2018, 4:16 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG93bab80993f4: fix: add a config to abort when a fixer tool fails
Summary

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.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

hooper created this revision.Oct 31 2018, 4:16 PM
yuja added a subscriber: yuja.Nov 2 2018, 9:32 PM

Looks good, but test-check-config.t failed probably because the doc couldn't
be found by the checker.

@@ -464,9 +488,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'))

This runs after fork(), but it should work as long as no user interaction is
required.

hooper updated this revision to Diff 12435.Nov 5 2018, 6:49 PM
hooper added a comment.Nov 5 2018, 6:50 PM
In D5200#77865, @yuja wrote:

Looks good, but test-check-config.t failed probably because the doc couldn't
be found by the checker.

Looks like it's picky about whitespace.

Closed by commit rHG93bab80993f4: fix: add a config to abort when a fixer tool fails (authored by hooper). · Explain WhyNov 6 2018, 6:52 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mhgext/fix.py (43 lines)
Mtests/test-fix.t (75 lines)

Diff 12445

hgext/fix.py

[fix] [fix]
clang-format:command=clang-format --assume-filename={rootpath} clang-format:command=clang-format --assume-filename={rootpath}
clang-format:linerange=--lines={first}:{last} clang-format:linerange=--lines={first}:{last}
clang-format:fileset=set:**.cpp or **.hpp clang-format:fileset=set:**.cpp or **.hpp
The :command suboption forms the first part of the shell command that will be 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 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 fixed file content is expected on standard output. Any output on standard error
standard error, the file will not be affected. Some values may be substituted will be displayed as a warning. If the exit status is not zero, the file will
into the command:: 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 {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 {basename} The name of the file being fixed, without the directory path
If the :linerange suboption is set, the tool will only be run if there are If the :linerange suboption is set, the tool will only be run if there are
changed lines in a file. The value of this suboption is appended to the shell changed lines in a file. The value of this suboption is appended to the shell
command once for every range of changed lines in the file. Some values may be command once for every range of changed lines in the file. Some values may be
substituted into the command:: substituted into the command::
{first} The 1-based line number of the first line in the modified range {first} The 1-based line number of the first line in the modified range
{last} The 1-based line number of the last line in the modified range {last} The 1-based line number of the last line in the modified range
The :fileset suboption determines which files will be passed through each The :fileset suboption determines which files will be passed through each
configured tool. See :hg:`help fileset` for possible values. If there are file configured tool. See :hg:`help fileset` for possible values. If there are file
arguments to :hg:`fix`, the intersection of these filesets is used. arguments to :hg:`fix`, the intersection of these filesets is used.
There is also a configurable limit for the maximum size of file that will be There is also a configurable limit for the maximum size of file that will be
processed by :hg:`fix`:: processed by :hg:`fix`::
[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
""" """
from __future__ import absolute_import from __future__ import absolute_import
import collections import collections
import itertools import itertools
import os import os
import re import re
for key in FIXER_ATTRS: for key in FIXER_ATTRS:
configitem('fix', '.*(:%s)?' % key, default=None, generic=True) configitem('fix', '.*(:%s)?' % key, default=None, generic=True)
# A good default size allows most source code files to be fixed, but avoids # A good default size allows most source code files to be fixed, but avoids
# letting fixer tools choke on huge inputs, which could be surprising to the # letting fixer tools choke on huge inputs, which could be surprising to the
# user. # user.
configitem('fix', 'maxfilesize', default='2MB') 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')) allopt = ('', 'all', False, _('fix all non-public non-obsolete revisions'))
baseopt = ('', 'base', [], _('revisions to diff against (overrides automatic ' baseopt = ('', 'base', [], _('revisions to diff against (overrides automatic '
'selection, and applies to every revision being ' 'selection, and applies to every revision being '
'fixed)'), _('REV')) 'fixed)'), _('REV'))
revopt = ('r', 'rev', [], _('revisions to fix'), _('REV')) revopt = ('r', 'rev', [], _('revisions to fix'), _('REV'))
wdiropt = ('w', 'working-dir', False, _('fix the working directory')) wdiropt = ('w', 'working-dir', False, _('fix the working directory'))
wholeopt = ('', 'whole', False, _('always fix every line of a file')) wholeopt = ('', 'whole', False, _('always fix every line of a file'))
usage = _('[OPTION]... [FILE]...') usage = _('[OPTION]... [FILE]...')
stdin=subprocess.PIPE, stdin=subprocess.PIPE,
stdout=subprocess.PIPE, stdout=subprocess.PIPE,
stderr=subprocess.PIPE) stderr=subprocess.PIPE)
newerdata, stderr = proc.communicate(newdata) newerdata, stderr = proc.communicate(newdata)
if stderr: if stderr:
showstderr(ui, fixctx.rev(), fixername, stderr) showstderr(ui, fixctx.rev(), fixername, stderr)
if proc.returncode == 0: if proc.returncode == 0:
newdata = newerdata newdata = newerdata
elif not stderr: else:
showstderr(ui, fixctx.rev(), fixername, if not stderr:
_('exited with status %d\n') % (proc.returncode,)) 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 return newdata
def showstderr(ui, rev, fixername, stderr): def showstderr(ui, rev, fixername, stderr):
"""Writes the lines of the stderr string as warnings on the ui """Writes the lines of the stderr string as warnings on the ui
Uses the revision number and fixername to give more context to each line of Uses the revision number and fixername to give more context to each line of
the error message. Doesn't include file names, since those take up a lot of the error message. Doesn't include file names, since those take up a lot of
space and would tend to be included in the error message if they were space and would tend to be included in the error message if they were

tests/test-fix.t

[fix] [fix]
clang-format:command=clang-format --assume-filename={rootpath} clang-format:command=clang-format --assume-filename={rootpath}
clang-format:linerange=--lines={first}:{last} clang-format:linerange=--lines={first}:{last}
clang-format:fileset=set:**.cpp or **.hpp clang-format:fileset=set:**.cpp or **.hpp
The :command suboption forms the first part of the shell command that will be 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 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 the fixed file content is expected on standard output. Any output on standard
on standard error, the file will not be affected. Some values may be error will be displayed as a warning. If the exit status is not zero, the file
substituted into the command: 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 {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 {basename} The name of the file being fixed, without the directory path
If the :linerange suboption is set, the tool will only be run if there are If the :linerange suboption is set, the tool will only be run if there are
changed lines in a file. The value of this suboption is appended to the shell changed lines in a file. The value of this suboption is appended to the shell
command once for every range of changed lines in the file. Some values may be command once for every range of changed lines in the file. Some values may be
substituted into the command: substituted into the command:
{first} The 1-based line number of the first line in the modified range {first} The 1-based line number of the first line in the modified range
{last} The 1-based line number of the last line in the modified range {last} The 1-based line number of the last line in the modified range
The :fileset suboption determines which files will be passed through each The :fileset suboption determines which files will be passed through each
configured tool. See 'hg help fileset' for possible values. If there are file configured tool. See 'hg help fileset' for possible values. If there are file
arguments to 'hg fix', the intersection of these filesets is used. arguments to 'hg fix', the intersection of these filesets is used.
There is also a configurable limit for the maximum size of file that will be There is also a configurable limit for the maximum size of file that will be
processed by 'hg fix': processed by 'hg fix':
[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: list of commands:
fix rewrite file content in changesets or working directory fix rewrite file content in changesets or working directory
(use 'hg help -v -e fix' to show built-in aliases and global options) (use 'hg help -v -e fix' to show built-in aliases and global options)
There is no default behavior in the absence of --rev and --working-dir. There is no default behavior in the absence of --rev and --working-dir.
$ cd .. $ cd ..
When a fixer prints to stderr, we don't assume that it has failed. We show the When a fixer prints to stderr, we don't assume that it has failed. We show the
error messages to the user, and we still let the fixer affect the file it was error messages to the user, and we still let the fixer affect the file it was
fixing if its exit code is zero. Some code formatters might emit error messages fixing if its exit code is zero. Some code formatters might emit error messages
on stderr and nothing on stdout, which would cause us the clear the file, 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 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 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 $ hg init showstderr
$ cd showstderr $ cd showstderr
$ printf "hello\n" > hello.txt $ printf "hello\n" > hello.txt
$ hg add $ hg add
adding hello.txt adding hello.txt
$ cat > $TESTTMP/fail.sh <<'EOF' $ cat > $TESTTMP/work.sh <<'EOF'
> printf 'HELLO\n' > printf 'HELLO\n'
> printf "$@: some\nerror" >&2 > printf "$@: some\nerror that didn't stop the tool" >&2
> exit 0 # success despite the stderr output > exit 0 # success despite the stderr output
> EOF > EOF
$ hg --config "fix.fail:command=sh $TESTTMP/fail.sh {rootpath}" \ $ hg --config "fix.work:command=sh $TESTTMP/work.sh {rootpath}" \
> --config "fix.fail:fileset=hello.txt" \ > --config "fix.work:fileset=hello.txt" \
> fix --working-dir > fix --working-dir
[wdir] fail: hello.txt: some [wdir] work: hello.txt: some
[wdir] fail: error [wdir] work: error that didn't stop the tool
$ cat hello.txt $ cat hello.txt
HELLO HELLO
$ printf "goodbye\n" > hello.txt $ printf "goodbye\n" > hello.txt
$ cat > $TESTTMP/work.sh <<'EOF' $ printf "foo\n" > foo.whole
$ hg add
adding foo.whole
$ cat > $TESTTMP/fail.sh <<'EOF'
> printf 'GOODBYE\n' > printf 'GOODBYE\n'
> printf "$@: some\nerror\n" >&2 > printf "$@: some\nerror that did stop the tool\n" >&2
> exit 42 # success despite the stdout output > exit 42 # success despite the stdout output
> EOF > EOF
$ hg --config "fix.fail:command=sh $TESTTMP/work.sh {rootpath}" \ $ 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" \ > --config "fix.fail:fileset=hello.txt" \
> fix --working-dir > fix --working-dir
[wdir] fail: hello.txt: some [wdir] fail: hello.txt: some
[wdir] fail: error [wdir] fail: error that did stop the tool
$ cat hello.txt $ cat hello.txt
goodbye goodbye
$ cat foo.whole
FOO
$ hg --config "fix.fail:command=exit 42" \ $ hg --config "fix.fail:command=exit 42" \
> --config "fix.fail:fileset=hello.txt" \ > --config "fix.fail:fileset=hello.txt" \
> fix --working-dir > fix --working-dir
[wdir] fail: exited with status 42 [wdir] fail: exited with status 42
$ cd .. $ cd ..