Patchwork [resend] dispatch: add fail-* family of hooks

login
register
mail settings
Submitter Jordi Gutiérrez Hermoso
Date May 8, 2016, 9:39 p.m.
Message ID <1462743594.3630.1.camel@octave.org>
Download mbox | patch
Permalink /patch/14976/
State Accepted
Headers show

Comments

Jordi Gutiérrez Hermoso - May 8, 2016, 9:39 p.m.
# HG changeset patch
# User Jordi Gutiérrez Hermoso <jordigh@octave.org>
# Date 1461854267 14400
#      Thu Apr 28 10:37:47 2016 -0400
# Node ID fe6711339ce840a3c08a8c129fdc614ac8f9a8a1
# Parent  c5565fc8848dd084d104ca40c33d1acdfcff8bc6
dispatch: add fail-* family of hooks

The post-* family of hooks will not run in case a command fails (i.e.
raises an exception). This makes it inconvenient to hook into events
such as doing something in case of a failed push.

We catch all exceptions to run the failure hook. I am not sure if this
is too aggressive, but tests apparently pass.
Pierre-Yves David - May 9, 2016, 3 p.m.
On 05/08/2016 11:39 PM, Jordi Gutiérrez Hermoso wrote:
> # HG changeset patch
> # User Jordi Gutiérrez Hermoso <jordigh@octave.org>
> # Date 1461854267 14400
> #      Thu Apr 28 10:37:47 2016 -0400
> # Node ID fe6711339ce840a3c08a8c129fdc614ac8f9a8a1
> # Parent  c5565fc8848dd084d104ca40c33d1acdfcff8bc6
> dispatch: add fail-* family of hooks
>
> The post-* family of hooks will not run in case a command fails (i.e.
> raises an exception). This makes it inconvenient to hook into events
> such as doing something in case of a failed push.

excellent idea.

> We catch all exceptions to run the failure hook. I am not sure if this
> is too aggressive, but tests apparently pass.

If find it a bit scary too, but I'm short on better idea, I took the 
patch as is.

Thanks.
Yuya Nishihara - May 11, 2016, 11:25 a.m.
On Sun, 08 May 2016 17:39:54 -0400, Jordi Gutiérrez Hermoso wrote:
> # HG changeset patch
> # User Jordi Gutiérrez Hermoso <jordigh@octave.org>
> # Date 1461854267 14400
> #      Thu Apr 28 10:37:47 2016 -0400
> # Node ID fe6711339ce840a3c08a8c129fdc614ac8f9a8a1
> # Parent  c5565fc8848dd084d104ca40c33d1acdfcff8bc6
> dispatch: add fail-* family of hooks
> 
> The post-* family of hooks will not run in case a command fails (i.e.
> raises an exception). This makes it inconvenient to hook into events
> such as doing something in case of a failed push.
> 
> We catch all exceptions to run the failure hook. I am not sure if this
> is too aggressive, but tests apparently pass.
> 
> diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
> --- a/mercurial/dispatch.py
> +++ b/mercurial/dispatch.py
> @@ -627,10 +627,16 @@ def runcommand(lui, repo, cmd, fullargs,
>      # run pre-hook, and abort if it fails
>      hook.hook(lui, repo, "pre-%s" % cmd, True, args=" ".join(fullargs),
>                pats=cmdpats, opts=cmdoptions)
> -    ret = _runcommand(ui, options, cmd, d)
> -    # run post-hook, passing command result
> -    hook.hook(lui, repo, "post-%s" % cmd, False, args=" ".join(fullargs),
> -              result=ret, pats=cmdpats, opts=cmdoptions)
> +    try:
> +        ret = _runcommand(ui, options, cmd, d)
> +        # run post-hook, passing command result
> +        hook.hook(lui, repo, "post-%s" % cmd, False, args=" ".join(fullargs),
> +                  result=ret, pats=cmdpats, opts=cmdoptions)
> +    except Exception:
> +        # run failure hook and re-raise
> +        hook.hook(lui, repo, "fail-%s" % cmd, False, args=" ".join(fullargs),
> +                  pats=cmdpats, opts=cmdoptions)
> +        raise

Somewhat related, it's unclear to me what is "failure" (and what is "success".)

If a command returns 1, post-* hook is called and the process exits with 1.
On the other hand, if a command raises InterventionRequired, fail-* hook is
called and the process exits with 1.

Patch

diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
--- a/mercurial/dispatch.py
+++ b/mercurial/dispatch.py
@@ -627,10 +627,16 @@  def runcommand(lui, repo, cmd, fullargs,
     # run pre-hook, and abort if it fails
     hook.hook(lui, repo, "pre-%s" % cmd, True, args=" ".join(fullargs),
               pats=cmdpats, opts=cmdoptions)
-    ret = _runcommand(ui, options, cmd, d)
-    # run post-hook, passing command result
-    hook.hook(lui, repo, "post-%s" % cmd, False, args=" ".join(fullargs),
-              result=ret, pats=cmdpats, opts=cmdoptions)
+    try:
+        ret = _runcommand(ui, options, cmd, d)
+        # run post-hook, passing command result
+        hook.hook(lui, repo, "post-%s" % cmd, False, args=" ".join(fullargs),
+                  result=ret, pats=cmdpats, opts=cmdoptions)
+    except Exception:
+        # run failure hook and re-raise
+        hook.hook(lui, repo, "fail-%s" % cmd, False, args=" ".join(fullargs),
+                  pats=cmdpats, opts=cmdoptions)
+        raise
     return ret
 
 def _getlocal(ui, rpath, wd=None):
diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
--- a/mercurial/help/config.txt
+++ b/mercurial/help/config.txt
@@ -811,6 +811,15 @@  variables it is passed are listed with n
   dictionary of options (with unspecified options set to their defaults).
   ``$HG_PATS`` is a list of arguments. Hook failure is ignored.
 
+``fail-<command>``
+  Run after a failed invocation of an associated command. The contents
+  of the command line are passed as ``$HG_ARGS``. Parsed command line
+  arguments are passed as ``$HG_PATS`` and ``$HG_OPTS``. These contain
+  string representations of the python data internally passed to
+  <command>. ``$HG_OPTS`` is a dictionary of options (with unspecified
+  options set to their defaults). ``$HG_PATS`` is a list of arguments.
+  Hook failure is ignored.
+
 ``pre-<command>``
   Run before executing the associated command. The contents of the
   command line are passed as ``$HG_ARGS``. Parsed command line arguments
diff --git a/tests/test-push-warn.t b/tests/test-push-warn.t
--- a/tests/test-push-warn.t
+++ b/tests/test-push-warn.t
@@ -785,4 +785,14 @@  outgoing:
   no changes found
   [1]
 
+Test fail hook
+
+  $ hg push inner --config hooks.fail-push="echo running fail-push hook"
+  pushing to inner
+  searching for changes
+  running fail-push hook
+  abort: push creates new remote head 7d0f4fb6cf04 on branch 'A'!
+  (merge or see "hg help push" for details about pushing new heads)
+  [255]
+
   $ cd ..