D4158: fix: determine fixer tool failure by exit code instead of stderr

[hooper (Danny Hooper)]

This revision was automatically updated to reflect the committed changes.
Closed by commit rHGa009589cd32a: fix: determine fixer tool failure by exit code instead of stderr (authored by hooper, committed by ).

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D4158?vs=10078&id=10130

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

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
@@ -502,30 +502,51 @@
 
   $ cd ..
 
-When a fixer prints to stderr, we assume that it has failed. We should show the
-error messages to the user, and we should not let the failing fixer affect the
-file it was fixing (many code formatters might emit error messages on stderr
-and nothing on stdout, which would cause us the clear the file). We show the
-user which fixer failed and which revision, but we assume that the fixer will
-print the filename if it is relevant.
+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
+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,
+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).
 
   $ hg init showstderr
   $ cd showstderr
 
   $ printf "hello\n" > hello.txt
   $ hg add
   adding hello.txt
-  $ cat >> $TESTTMP/cmd.sh <<'EOF'
+  $ cat > $TESTTMP/fail.sh <<'EOF'
   > printf 'HELLO\n'
   > printf "$@: some\nerror" >&2
+  > exit 0 # success despite the stderr output
   > EOF
-  $ hg --config "fix.fail:command=sh $TESTTMP/cmd.sh {rootpath}" \
+  $ 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
+  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
+  $ cat hello.txt
+  goodbye
+
+  $ hg --config "fix.fail:command=exit 42" \
+  >    --config "fix.fail:fileset=hello.txt" \
+  >    fix --working-dir
+  [wdir] fail: exited with status 42
 
   $ cd ..
 
diff --git a/hgext/fix.py b/hgext/fix.py
--- a/hgext/fix.py
+++ b/hgext/fix.py
@@ -435,6 +435,9 @@
     starting with the file's content in the fixctx. Fixers that support line
     ranges will affect lines that have changed relative to any of the basectxs
     (i.e. they will only avoid lines that are common to all basectxs).
+
+    A fixer tool's stdout will become the file's new content if and only if it
+    exits with code zero.
     """
     newdata = fixctx[path].data()
     for fixername, fixer in fixers.iteritems():
@@ -454,8 +457,11 @@
             newerdata, stderr = proc.communicate(newdata)
             if stderr:
                 showstderr(ui, fixctx.rev(), fixername, stderr)
-            else:
+            if proc.returncode == 0:
                 newdata = newerdata
+            elif not stderr:
+                showstderr(ui, fixctx.rev(), fixername,
+                           _('exited with status %d\n') % (proc.returncode,))
     return newdata
 
 def showstderr(ui, rev, fixername, stderr):



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