[PATCH 1/2] cli: add mechanism for running user configurable hooks

[PATCH 1/2] cli: add mechanism for running user configurable hooks

[Jani Nikula]

Add support functions for running hooks configurable in the notmuch config
file. The hooks will be run using system(1).

TODO:

* Move notmuch_run_hook() out of notmuch-new.c. It's there and static only
  because the first user will be there.

* Consider merging this with the following patch, as this is slightly
  artificial as it is.

Signed-off-by: Jani Nikula <jani@nikula.org>
---
 notmuch-client.h |    7 +++++++
 notmuch-config.c |   21 +++++++++++++++++++++
 notmuch-new.c    |   25 +++++++++++++++++++++++++
 3 files changed, 53 insertions(+), 0 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index b50cb38..5e2fed2 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -87,6 +87,10 @@ typedef struct notmuch_show_params {
     int decrypt;
 } notmuch_show_params_t;
 
+typedef enum {
+    NOTMUCH_HOOK_PLACEHOLDER,
+} notmuch_hook_t;
+
 /* There's no point in continuing when we've detected that we've done
  * something wrong internally (as opposed to the user passing in a
  * bogus value).
@@ -235,6 +239,9 @@ void
 notmuch_config_set_maildir_synchronize_flags (notmuch_config_t *config,
 					      notmuch_bool_t synchronize_flags);
 
+const char *
+notmuch_config_get_hook (notmuch_config_t *config, notmuch_hook_t hook);
+
 notmuch_bool_t
 debugger_is_active (void);
 
diff --git a/notmuch-config.c b/notmuch-config.c
index 1a7ed58..8f1a038 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -608,6 +608,27 @@ notmuch_config_set_new_tags (notmuch_config_t *config,
     config->new_tags = NULL;
 }
 
+const char *
+notmuch_config_get_hook (notmuch_config_t *config, notmuch_hook_t hook)
+{
+    char *command;
+    const char *group, *key;
+
+    switch (hook) {
+    default:
+	INTERNAL_ERROR ("Unknown hook %d\n.", hook);
+    }
+
+    command = g_key_file_get_string (config->key_file, group, key, NULL);
+    if (command) {
+	char *p = command;
+	command = talloc_strdup (config, command);
+	free (p);
+    }
+
+    return command;
+}
+
 /* Given a configuration item of the form <group>.<key> return the
  * component group and key. If any error occurs, print a message on
  * stderr and return 1. Otherwise, return 0.
diff --git a/notmuch-new.c b/notmuch-new.c
index 81a9350..0c70e64 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -795,6 +795,31 @@ _remove_directory (void *ctx,
     notmuch_directory_destroy (directory);
 }
 
+static int
+notmuch_run_hook (notmuch_config_t *config, notmuch_hook_t hook)
+{
+    const char *command;
+    int r;
+
+    command = notmuch_config_get_hook (config, hook);
+    if (!command)
+	return NOTMUCH_STATUS_SUCCESS; /* It's okay not to have a hook */
+
+    r = system(command);
+    if (r) {
+	if (WIFSIGNALED(r))
+	    fprintf(stderr, "hook '%s' terminated with signal %d\n",
+		    command, WTERMSIG(r));
+	else
+	    fprintf(stderr, "hook '%s' failed with status %d\n",
+		    command, WEXITSTATUS(r));
+
+	r = NOTMUCH_STATUS_FILE_ERROR; /* FIXME */
+    }
+
+    return r;
+}
+
 int
 notmuch_new_command (void *ctx, int argc, char *argv[])
 {
-- 
1.7.5.4

[PATCH 2/2] cli: add support for running notmuch new pre and post hooks

[Jani Nikula]

Run notmuch new pre and post hooks if specified in the notmuch config
file. The hooks will be run before and after incorporating new messages to
the database. Also add command line option --no-hooks to notmuch new to
bypass the hooks.

With this patch, you can add hooks in your config, for example:

[new]
prehook=offlineimap
posthook=my-tagging-script

As the value is passed to system(1), you can actually have multiple
commands, redirections, pipes, etc. in there. If the tagging is simple
enough, you can just add the tagging in-line without a script.

TODO:

* Tests.

* Documentation (manpage and help).

Signed-off-by: Jani Nikula <jani@nikula.org>
---
 notmuch-client.h |    3 ++-
 notmuch-config.c |   16 +++++++++++++++-
 notmuch-new.c    |    9 +++++++++
 3 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index 5e2fed2..d2ebc73 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -88,7 +88,8 @@ typedef struct notmuch_show_params {
 } notmuch_show_params_t;
 
 typedef enum {
-    NOTMUCH_HOOK_PLACEHOLDER,
+    NOTMUCH_HOOK_PRE_NEW,
+    NOTMUCH_HOOK_POST_NEW,
 } notmuch_hook_t;
 
 /* There's no point in continuing when we've detected that we've done
diff --git a/notmuch-config.c b/notmuch-config.c
index 8f1a038..277f197 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -43,7 +43,13 @@ static const char new_config_comment[] =
     " The following options are supported here:\n"
     "\n"
     "\ttags	A list (separated by ';') of the tags that will be\n"
-    "\t	added to all messages incorporated by \"notmuch new\".\n";
+    "\t	added to all messages incorporated by \"notmuch new\".\n"
+    "\tprehook	A command to be executed before \"notmuch new\" starts\n"
+    "\t	incorporating new messages. For example, this could be used to fetch\n"
+    "\t	and deliver new messages to the mail directory.\n"
+    "\tposthook	A command to be executed after \"notmuch new\" has\n"
+    "\t	incorporated and tagged all new messages. For example, this could\n"
+    "\t	be used to perform further tagging on new messages.\n";
 
 static const char user_config_comment[] =
     " User configuration\n"
@@ -615,6 +621,14 @@ notmuch_config_get_hook (notmuch_config_t *config, notmuch_hook_t hook)
     const char *group, *key;
 
     switch (hook) {
+    case NOTMUCH_HOOK_PRE_NEW:
+	group = "new";
+	key = "prehook";
+	break;
+    case NOTMUCH_HOOK_POST_NEW:
+	group = "new";
+	key = "posthook";
+	break;
     default:
 	INTERNAL_ERROR ("Unknown hook %d\n.", hook);
     }
diff --git a/notmuch-new.c b/notmuch-new.c
index 0c70e64..09cc3f2 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -836,6 +836,7 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
     _filename_node_t *f;
     int i;
     notmuch_bool_t timer_is_active = FALSE;
+    int run_hooks = 1;
 
     add_files_state.verbose = 0;
     add_files_state.output_is_a_tty = isatty (fileno (stdout));
@@ -845,6 +846,8 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
     for (i = 0; i < argc && argv[i][0] == '-'; i++) {
 	if (STRNCMP_LITERAL (argv[i], "--verbose") == 0) {
 	    add_files_state.verbose = 1;
+	} else if (STRNCMP_LITERAL (argv[i], "--no-hooks") == 0) {
+	    run_hooks = 0;
 	} else {
 	    fprintf (stderr, "Unrecognized option: %s\n", argv[i]);
 	    return 1;
@@ -854,6 +857,9 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
     if (config == NULL)
 	return 1;
 
+    if (run_hooks && notmuch_run_hook (config, NOTMUCH_HOOK_PRE_NEW))
+	return 1;
+
     add_files_state.new_tags = notmuch_config_get_new_tags (config, &add_files_state.new_tags_length);
     add_files_state.synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config);
     db_path = notmuch_config_get_database_path (config);
@@ -1006,5 +1012,8 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
 
     notmuch_database_close (notmuch);
 
+    if (run_hooks && !interrupted)
+	ret |= notmuch_run_hook (config, NOTMUCH_HOOK_POST_NEW);
+
     return ret || interrupted;
 }
-- 
1.7.5.4

Re: [PATCH 1/2] cli: add mechanism for running user configurable hooks

[Jani Nikula]

On Fri,  2 Dec 2011 23:00:05 +0200, Jani Nikula <jani@nikula.org> wrote:
> TODO:

I meant these as RFC, but forgot to add the subject prefix.

Jani.

[PATCH v2 1/2] cli: introduce the concept of user defined hooks

[Jani Nikula]

Add mechanism for running user defined hooks. Hooks are executables or
symlinks to executables stored under the new notmuch hooks directory,
<database-path>/.notmuch/hooks.

No hooks are introduced here, but adding support for a hook is now a simple
matter of calling the new notmuch_run_hook() function at an appropriate
location with the hook name.

Signed-off-by: Jani Nikula <jani@nikula.org>

---

v2: Switch to git style hooks in a hook directory as suggested by Austin
Clements in IRC. Update manpage and add polish.
---
 Makefile.local   |    1 +
 notmuch-client.h |    3 ++
 notmuch-hook.c   |   63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+), 0 deletions(-)
 create mode 100644 notmuch-hook.c

diff --git a/Makefile.local b/Makefile.local
index c94402b..a1665e1 100644
--- a/Makefile.local
+++ b/Makefile.local
@@ -302,6 +302,7 @@ notmuch_client_srcs =		\
 	notmuch-config.c	\
 	notmuch-count.c		\
 	notmuch-dump.c		\
+	notmuch-hook.c		\
 	notmuch-new.c		\
 	notmuch-reply.c		\
 	notmuch-restore.c	\
diff --git a/notmuch-client.h b/notmuch-client.h
index b50cb38..a91ad6c 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -235,6 +235,9 @@ void
 notmuch_config_set_maildir_synchronize_flags (notmuch_config_t *config,
 					      notmuch_bool_t synchronize_flags);
 
+int
+notmuch_run_hook (const char *db_path, const char *hook);
+
 notmuch_bool_t
 debugger_is_active (void);
 
diff --git a/notmuch-hook.c b/notmuch-hook.c
new file mode 100644
index 0000000..fc32044
--- /dev/null
+++ b/notmuch-hook.c
@@ -0,0 +1,63 @@
+/* notmuch - Not much of an email program, (just index and search)
+ *
+ * This file is part of notmuch.
+ *
+ * Copyright © 2011 Jani Nikula
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see http://www.gnu.org/licenses/ .
+ *
+ * Author: Jani Nikula <jani@nikula.org>
+ */
+
+#include "notmuch-client.h"
+
+int
+notmuch_run_hook (const char *db_path, const char *hook)
+{
+    char *hook_path;
+    int status = 0;
+
+    if (asprintf (&hook_path, "%s/%s/%s/%s", db_path, ".notmuch", "hooks",
+		  hook) == -1)
+	return NOTMUCH_STATUS_OUT_OF_MEMORY;
+
+    if (access (hook_path, X_OK) == -1) {
+	/* Ignore ENOENT. It's okay not to have a hook, hook dir, or even
+	 * notmuch dir. Dangling symbolic links also result in ENOENT, but
+	 * we'll ignore that too for simplicity. */
+	if (errno != ENOENT) {
+	    fprintf (stderr, "Error: %s hook access failed: %s\n", hook,
+		     strerror (errno));
+	    status = NOTMUCH_STATUS_FILE_ERROR;
+	}
+	goto DONE;
+    }
+
+    status = system (hook_path);
+    if (status) {
+	if (WIFSIGNALED(status))
+	    fprintf(stderr, "Error: %s hook terminated with signal %d\n", hook,
+		    WTERMSIG(status));
+	else
+	    fprintf(stderr, "Error: %s hook failed with status %d\n", hook,
+		    WEXITSTATUS(status));
+
+	status = NOTMUCH_STATUS_FILE_ERROR; /* Close enough */
+    }
+
+  DONE:
+    free (hook_path);
+
+    return status;
+}
-- 
1.7.5.4

[PATCH v2 2/2] cli: add support for pre and post notmuch new hooks

[Jani Nikula]

Run notmuch new pre and post hooks, named "pre-new" and "post-new", if
present in the notmuch hooks directory. The hooks will be run before and
after incorporating new messages to the database.

Typical use cases for pre-new and post-new hooks are fetching or delivering
new mail to the maildir, and custom tagging of the mail incorporated to the
database.

Also add command line option --no-hooks to notmuch new to bypass the hooks.

Signed-off-by: Jani Nikula <jani@nikula.org>
---
 notmuch-new.c |   12 ++++++++++++
 notmuch.1     |   50 +++++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 61 insertions(+), 1 deletions(-)

diff --git a/notmuch-new.c b/notmuch-new.c
index 81a9350..27dde0c 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -811,6 +811,7 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
     _filename_node_t *f;
     int i;
     notmuch_bool_t timer_is_active = FALSE;
+    int run_hooks = 1;
 
     add_files_state.verbose = 0;
     add_files_state.output_is_a_tty = isatty (fileno (stdout));
@@ -820,6 +821,8 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
     for (i = 0; i < argc && argv[i][0] == '-'; i++) {
 	if (STRNCMP_LITERAL (argv[i], "--verbose") == 0) {
 	    add_files_state.verbose = 1;
+	} else if (STRNCMP_LITERAL (argv[i], "--no-hooks") == 0) {
+	    run_hooks = 0;
 	} else {
 	    fprintf (stderr, "Unrecognized option: %s\n", argv[i]);
 	    return 1;
@@ -833,6 +836,12 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
     add_files_state.synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config);
     db_path = notmuch_config_get_database_path (config);
 
+    if (run_hooks) {
+	ret = notmuch_run_hook (db_path, "pre-new");
+	if (ret)
+	    return ret;
+    }
+
     dot_notmuch_path = talloc_asprintf (ctx, "%s/%s", db_path, ".notmuch");
 
     if (stat (dot_notmuch_path, &st)) {
@@ -981,5 +990,8 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
 
     notmuch_database_close (notmuch);
 
+    if (run_hooks && !ret && !interrupted)
+	ret = notmuch_run_hook (db_path, "post-new");
+
     return ret || interrupted;
 }
diff --git a/notmuch.1 b/notmuch.1
index 92931d7..66f82e9 100644
--- a/notmuch.1
+++ b/notmuch.1
@@ -85,7 +85,7 @@ The
 command is used to incorporate new mail into the notmuch database.
 .RS 4
 .TP 4
-.B new
+.BR new " [options...]"
 
 Find and import any new messages to the database.
 
@@ -118,6 +118,22 @@ if
 has previously been completed, but
 .B "notmuch new"
 has not previously been run.
+
+The
+.B new
+command supports hooks. See the
+.B "HOOKS"
+section below for more details on hooks.
+
+Supported options for
+.B new
+include
+.RS 4
+.TP 4
+.BR \-\-no\-hooks
+
+Prevents hooks from being run.
+.RE
 .RE
 
 Several of the notmuch commands accept search terms with a common
@@ -705,6 +721,38 @@ specify a date range to return messages from 2009\-10\-01 until the
 current time:
 
 	$(date +%s \-d 2009\-10\-01)..$(date +%s)
+.SH HOOKS
+Hooks are scripts (or arbitrary executables or symlinks to such) you can place
+in the notmuch hooks directory to trigger action at certain points. The hooks
+directory is .notmuch/hooks within the database directory. The user must have
+executable permission set on the scripts.
+
+The currently available hooks are described below.
+.RS 4
+.TP 4
+.B pre\-new
+This hook is invoked by the
+.B new
+command before scanning or importing new messages into the database. Any errors
+in running the hook will abort further processing of the
+.B new
+command.
+
+Typical use case for this hook is fetching or delivering new mail to be imported
+into the database.
+.RE
+.RS 4
+.TP 4
+.B post\-new
+This hook is invoked by the
+.B new
+command after new messages have been imported into the database and initial tags
+have been applied. The hook will not be run if there have been any errors during
+the scan or import.
+
+Typical use case for this hook is performing additional query based tagging on
+the imported messages.
+.RE
 .SH ENVIRONMENT
 The following environment variables can be used to control the
 behavior of notmuch.
-- 
1.7.5.4

Re: [PATCH v2 1/2] cli: introduce the concept of user defined hooks

[Austin Clements]

I like it.  See below for some nits.

Quoth Jani Nikula on Dec 04 at  1:16 am:
> Add mechanism for running user defined hooks. Hooks are executables or
> symlinks to executables stored under the new notmuch hooks directory,
> <database-path>/.notmuch/hooks.
> 
> No hooks are introduced here, but adding support for a hook is now a simple
> matter of calling the new notmuch_run_hook() function at an appropriate
> location with the hook name.
> 
> Signed-off-by: Jani Nikula <jani@nikula.org>
> 
> ---
> 
> v2: Switch to git style hooks in a hook directory as suggested by Austin
> Clements in IRC. Update manpage and add polish.
> ---
>  Makefile.local   |    1 +
>  notmuch-client.h |    3 ++
>  notmuch-hook.c   |   63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 67 insertions(+), 0 deletions(-)
>  create mode 100644 notmuch-hook.c
> 
> diff --git a/Makefile.local b/Makefile.local
> index c94402b..a1665e1 100644
> --- a/Makefile.local
> +++ b/Makefile.local
> @@ -302,6 +302,7 @@ notmuch_client_srcs =		\
>  	notmuch-config.c	\
>  	notmuch-count.c		\
>  	notmuch-dump.c		\
> +	notmuch-hook.c		\
>  	notmuch-new.c		\
>  	notmuch-reply.c		\
>  	notmuch-restore.c	\
> diff --git a/notmuch-client.h b/notmuch-client.h
> index b50cb38..a91ad6c 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -235,6 +235,9 @@ void
>  notmuch_config_set_maildir_synchronize_flags (notmuch_config_t *config,
>  					      notmuch_bool_t synchronize_flags);
>  
> +int
> +notmuch_run_hook (const char *db_path, const char *hook);
> +
>  notmuch_bool_t
>  debugger_is_active (void);
>  
> diff --git a/notmuch-hook.c b/notmuch-hook.c
> new file mode 100644
> index 0000000..fc32044
> --- /dev/null
> +++ b/notmuch-hook.c
> @@ -0,0 +1,63 @@
> +/* notmuch - Not much of an email program, (just index and search)
> + *
> + * This file is part of notmuch.
> + *
> + * Copyright © 2011 Jani Nikula
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 3 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see http://www.gnu.org/licenses/ .
> + *
> + * Author: Jani Nikula <jani@nikula.org>
> + */
> +
> +#include "notmuch-client.h"
> +
> +int
> +notmuch_run_hook (const char *db_path, const char *hook)
> +{
> +    char *hook_path;
> +    int status = 0;

You use status as both a notmuch_status_t and for generic C library
results.  This seems a little weird.  You may or may not want to do
anything about it.

> +
> +    if (asprintf (&hook_path, "%s/%s/%s/%s", db_path, ".notmuch", "hooks",
> +		  hook) == -1)

asprintf isn't very portable.  Perhaps talloc_asprintf would be
better?  (And more idiomatic.)

> +	return NOTMUCH_STATUS_OUT_OF_MEMORY;
> +
> +    if (access (hook_path, X_OK) == -1) {
> +	/* Ignore ENOENT. It's okay not to have a hook, hook dir, or even
> +	 * notmuch dir. Dangling symbolic links also result in ENOENT, but
> +	 * we'll ignore that too for simplicity. */
> +	if (errno != ENOENT) {
> +	    fprintf (stderr, "Error: %s hook access failed: %s\n", hook,
> +		     strerror (errno));
> +	    status = NOTMUCH_STATUS_FILE_ERROR;
> +	}
> +	goto DONE;
> +    }
> +
> +    status = system (hook_path);

It would probably be better to fork/execl.  system is sensitive to all
sorts of weird things because it invokes the shell (e.g., spaces or
funny characters in db_path) and it plays funny games with signals.

Really proper error handling with fork/exec is a pain, but I think you
can get away with something simpler and even get rid of the access
call in the process.  Something like

ret = fork();
if (ret < 0) ...
else if (ret == 0) {
  ret = execl(hook_path, hook_path, NULL);
  if (ret != ENOENT && ret != EACCESS)
    print a real error message
  exit(0);
} else {
  waitpid(ret, &status, 0);
  if (status) .. checks you do now ..
}

I guess this wastes a fork if there is no hook script, so maybe the
access call is worth doing anyway.

> +    if (status) {
> +	if (WIFSIGNALED(status))
> +	    fprintf(stderr, "Error: %s hook terminated with signal %d\n", hook,
> +		    WTERMSIG(status));
> +	else
> +	    fprintf(stderr, "Error: %s hook failed with status %d\n", hook,
> +		    WEXITSTATUS(status));
> +
> +	status = NOTMUCH_STATUS_FILE_ERROR; /* Close enough */
> +    }
> +
> +  DONE:
> +    free (hook_path);
> +
> +    return status;
> +}

Re: [PATCH v2 2/2] cli: add support for pre and post notmuch new hooks

[Austin Clements]

Quoth Jani Nikula on Dec 04 at  1:16 am:
> Run notmuch new pre and post hooks, named "pre-new" and "post-new", if
> present in the notmuch hooks directory. The hooks will be run before and
> after incorporating new messages to the database.
> 
> Typical use cases for pre-new and post-new hooks are fetching or delivering
> new mail to the maildir, and custom tagging of the mail incorporated to the
> database.
> 
> Also add command line option --no-hooks to notmuch new to bypass the hooks.
> 
> Signed-off-by: Jani Nikula <jani@nikula.org>
> ---
>  notmuch-new.c |   12 ++++++++++++
>  notmuch.1     |   50 +++++++++++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 61 insertions(+), 1 deletions(-)
> 
> diff --git a/notmuch-new.c b/notmuch-new.c
> index 81a9350..27dde0c 100644
> --- a/notmuch-new.c
> +++ b/notmuch-new.c
> @@ -811,6 +811,7 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
>      _filename_node_t *f;
>      int i;
>      notmuch_bool_t timer_is_active = FALSE;
> +    int run_hooks = 1;

notmuch_bool_t?

>  
>      add_files_state.verbose = 0;
>      add_files_state.output_is_a_tty = isatty (fileno (stdout));
> @@ -820,6 +821,8 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
>      for (i = 0; i < argc && argv[i][0] == '-'; i++) {
>  	if (STRNCMP_LITERAL (argv[i], "--verbose") == 0) {
>  	    add_files_state.verbose = 1;
> +	} else if (STRNCMP_LITERAL (argv[i], "--no-hooks") == 0) {

I see this mistake all over notmuch, so maybe it's better to
perpetuate it here and fix it everywhere in another patch, but this
should be strcmp, not STRNCMP_LITERAL.  STRNCMP_LITERAL is the right
thing for options that take values, but for boolean options like this,
it will accept
  notmuch new --no-hooks-just-kidding

> +	    run_hooks = 0;
>  	} else {
>  	    fprintf (stderr, "Unrecognized option: %s\n", argv[i]);
>  	    return 1;
> @@ -833,6 +836,12 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
>      add_files_state.synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config);
>      db_path = notmuch_config_get_database_path (config);
>  
> +    if (run_hooks) {
> +	ret = notmuch_run_hook (db_path, "pre-new");
> +	if (ret)
> +	    return ret;
> +    }
> +
>      dot_notmuch_path = talloc_asprintf (ctx, "%s/%s", db_path, ".notmuch");
>  
>      if (stat (dot_notmuch_path, &st)) {
> @@ -981,5 +990,8 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
>  
>      notmuch_database_close (notmuch);
>  
> +    if (run_hooks && !ret && !interrupted)
> +	ret = notmuch_run_hook (db_path, "post-new");

Does it matter at this point if the hook fails?  I'm not sure.

> +
>      return ret || interrupted;
>  }
> diff --git a/notmuch.1 b/notmuch.1
> index 92931d7..66f82e9 100644
> --- a/notmuch.1
> +++ b/notmuch.1

I am willfully ignorant of nroff, so somebody else will have to
comment if any of the nroff code/formatting is wrong.

> @@ -85,7 +85,7 @@ The
>  command is used to incorporate new mail into the notmuch database.
>  .RS 4
>  .TP 4
> -.B new
> +.BR new " [options...]"
>  
>  Find and import any new messages to the database.
>  
> @@ -118,6 +118,22 @@ if
>  has previously been completed, but
>  .B "notmuch new"
>  has not previously been run.
> +
> +The
> +.B new
> +command supports hooks. See the
> +.B "HOOKS"
> +section below for more details on hooks.
> +
> +Supported options for
> +.B new
> +include
> +.RS 4
> +.TP 4
> +.BR \-\-no\-hooks
> +
> +Prevents hooks from being run.
> +.RE
>  .RE
>  
>  Several of the notmuch commands accept search terms with a common
> @@ -705,6 +721,38 @@ specify a date range to return messages from 2009\-10\-01 until the
>  current time:
>  
>  	$(date +%s \-d 2009\-10\-01)..$(date +%s)
> +.SH HOOKS
> +Hooks are scripts (or arbitrary executables or symlinks to such) you can place
> +in the notmuch hooks directory to trigger action at certain points. The hooks
> +directory is .notmuch/hooks within the database directory. The user must have
> +executable permission set on the scripts.

Could be more concise.  Maybe something like "Hooks are scripts (or
arbitrary executables or symlinks to such) that notmuch invokes before
and after certain actions.  These scripts reside in the .notmuch/hooks
directory within the database directory and must have executable
permissions."

> +
> +The currently available hooks are described below.
> +.RS 4
> +.TP 4
> +.B pre\-new
> +This hook is invoked by the
> +.B new
> +command before scanning or importing new messages into the database. Any errors
> +in running the hook will abort further processing of the

"If this script exits with a non-zero status, notmuch will abort ..."?

> +.B new
> +command.
> +
> +Typical use case for this hook is fetching or delivering new mail to be imported
> +into the database.

Perhaps "Typically this hook is used for ..."?

> +.RE
> +.RS 4
> +.TP 4
> +.B post\-new
> +This hook is invoked by the
> +.B new
> +command after new messages have been imported into the database and initial tags
> +have been applied. The hook will not be run if there have been any errors during
> +the scan or import.
> +
> +Typical use case for this hook is performing additional query based tagging on
> +the imported messages.

Same thing.  "Typically this hook is used to perform ..."?  Also,
"query-based".

> +.RE
>  .SH ENVIRONMENT
>  The following environment variables can be used to control the
>  behavior of notmuch.

Re: [PATCH v2 1/2] cli: introduce the concept of user defined hooks

[Jani Nikula]

On Sat, 3 Dec 2011 22:42:10 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> I like it.  See below for some nits.

Thanks for the review!

> Quoth Jani Nikula on Dec 04 at  1:16 am:
> > Add mechanism for running user defined hooks. Hooks are executables or
> > symlinks to executables stored under the new notmuch hooks directory,
> > <database-path>/.notmuch/hooks.
> > 
> > No hooks are introduced here, but adding support for a hook is now a simple
> > matter of calling the new notmuch_run_hook() function at an appropriate
> > location with the hook name.
> > 
> > Signed-off-by: Jani Nikula <jani@nikula.org>
> > 
> > ---
> > 
> > v2: Switch to git style hooks in a hook directory as suggested by Austin
> > Clements in IRC. Update manpage and add polish.
> > ---
> >  Makefile.local   |    1 +
> >  notmuch-client.h |    3 ++
> >  notmuch-hook.c   |   63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 67 insertions(+), 0 deletions(-)
> >  create mode 100644 notmuch-hook.c
> > 
> > diff --git a/Makefile.local b/Makefile.local
> > index c94402b..a1665e1 100644
> > --- a/Makefile.local
> > +++ b/Makefile.local
> > @@ -302,6 +302,7 @@ notmuch_client_srcs =		\
> >  	notmuch-config.c	\
> >  	notmuch-count.c		\
> >  	notmuch-dump.c		\
> > +	notmuch-hook.c		\
> >  	notmuch-new.c		\
> >  	notmuch-reply.c		\
> >  	notmuch-restore.c	\
> > diff --git a/notmuch-client.h b/notmuch-client.h
> > index b50cb38..a91ad6c 100644
> > --- a/notmuch-client.h
> > +++ b/notmuch-client.h
> > @@ -235,6 +235,9 @@ void
> >  notmuch_config_set_maildir_synchronize_flags (notmuch_config_t *config,
> >  					      notmuch_bool_t synchronize_flags);
> >  
> > +int
> > +notmuch_run_hook (const char *db_path, const char *hook);
> > +
> >  notmuch_bool_t
> >  debugger_is_active (void);
> >  
> > diff --git a/notmuch-hook.c b/notmuch-hook.c
> > new file mode 100644
> > index 0000000..fc32044
> > --- /dev/null
> > +++ b/notmuch-hook.c
> > @@ -0,0 +1,63 @@
> > +/* notmuch - Not much of an email program, (just index and search)
> > + *
> > + * This file is part of notmuch.
> > + *
> > + * Copyright © 2011 Jani Nikula
> > + *
> > + * This program is free software: you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation, either version 3 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program.  If not, see http://www.gnu.org/licenses/ .
> > + *
> > + * Author: Jani Nikula <jani@nikula.org>
> > + */
> > +
> > +#include "notmuch-client.h"
> > +
> > +int
> > +notmuch_run_hook (const char *db_path, const char *hook)
> > +{
> > +    char *hook_path;
> > +    int status = 0;
> 
> You use status as both a notmuch_status_t and for generic C library
> results.  This seems a little weird.  You may or may not want to do
> anything about it.

True, it's not consistent. I'll want to do something about it. I wonder
if it's worth returning anything other than ok/fail from this function
anyway.

There seems to be some confusion in notmuch_status_t usage across
notmuch cli. Should notmuch cli return a notmuch_status_t as exit
status? It currently does at least in some cases, but it also returns
plain 1 too which is (unintentionally) NOTMUCH_STATUS_OUT_OF_MEMORY.

Does it make sense for the cli to use the lib statuses internally
anyway; you wouldn't want to add new status codes to the lib just to be
able to use them in cli.

> > +
> > +    if (asprintf (&hook_path, "%s/%s/%s/%s", db_path, ".notmuch", "hooks",
> > +		  hook) == -1)
> 
> asprintf isn't very portable.  Perhaps talloc_asprintf would be
> better?  (And more idiomatic.)

Agreed.

> > +	return NOTMUCH_STATUS_OUT_OF_MEMORY;
> > +
> > +    if (access (hook_path, X_OK) == -1) {
> > +	/* Ignore ENOENT. It's okay not to have a hook, hook dir, or even
> > +	 * notmuch dir. Dangling symbolic links also result in ENOENT, but
> > +	 * we'll ignore that too for simplicity. */
> > +	if (errno != ENOENT) {
> > +	    fprintf (stderr, "Error: %s hook access failed: %s\n", hook,
> > +		     strerror (errno));
> > +	    status = NOTMUCH_STATUS_FILE_ERROR;
> > +	}
> > +	goto DONE;
> > +    }
> > +
> > +    status = system (hook_path);
> 
> It would probably be better to fork/execl.  system is sensitive to all
> sorts of weird things because it invokes the shell (e.g., spaces or
> funny characters in db_path) and it plays funny games with signals.

Agreed. I initially chose system() because it is simple to use and
allowed shell expressions (pipes, redirects, etc.) within the config
file in v1 of the patches. But that's not applicable in v2 anyway.

> Really proper error handling with fork/exec is a pain, but I think you
> can get away with something simpler and even get rid of the access
> call in the process.  Something like
> 
> ret = fork();
> if (ret < 0) ...
> else if (ret == 0) {
>   ret = execl(hook_path, hook_path, NULL);
>   if (ret != ENOENT && ret != EACCESS)
>     print a real error message
>   exit(0);
> } else {
>   waitpid(ret, &status, 0);
>   if (status) .. checks you do now ..
> }
> 
> I guess this wastes a fork if there is no hook script, so maybe the
> access call is worth doing anyway.

I'll look into this.

> > +    if (status) {
> > +	if (WIFSIGNALED(status))
> > +	    fprintf(stderr, "Error: %s hook terminated with signal %d\n", hook,
> > +		    WTERMSIG(status));
> > +	else
> > +	    fprintf(stderr, "Error: %s hook failed with status %d\n", hook,
> > +		    WEXITSTATUS(status));
> > +
> > +	status = NOTMUCH_STATUS_FILE_ERROR; /* Close enough */
> > +    }
> > +
> > +  DONE:
> > +    free (hook_path);
> > +
> > +    return status;
> > +}

Re: [PATCH v2 1/2] cli: introduce the concept of user defined hooks

[Austin Clements]

Quoth Jani Nikula on Dec 04 at  2:35 pm:
> > > +int
> > > +notmuch_run_hook (const char *db_path, const char *hook)
> > > +{
> > > +    char *hook_path;
> > > +    int status = 0;
> > 
> > You use status as both a notmuch_status_t and for generic C library
> > results.  This seems a little weird.  You may or may not want to do
> > anything about it.
> 
> True, it's not consistent. I'll want to do something about it. I wonder
> if it's worth returning anything other than ok/fail from this function
> anyway.
> 
> There seems to be some confusion in notmuch_status_t usage across
> notmuch cli. Should notmuch cli return a notmuch_status_t as exit
> status? It currently does at least in some cases, but it also returns
> plain 1 too which is (unintentionally) NOTMUCH_STATUS_OUT_OF_MEMORY.
> 
> Does it make sense for the cli to use the lib statuses internally
> anyway; you wouldn't want to add new status codes to the lib just to be
> able to use them in cli.

I think the answer is that there's no good answer because error
handling in C is so lame.  In this particular case, I'd say there's
little point in distinguishing different errors (and especially no
point in returning not-quite-appropriate status codes), so perhaps it
should follow the 0/1 convention.

Re: [PATCH v2 2/2] cli: add support for pre and post notmuch new hooks

[Tom Prince]

On Sun,  4 Dec 2011 01:16:49 +0200, Jani Nikula <jani@nikula.org> wrote:
> Run notmuch new pre and post hooks, named "pre-new" and "post-new", if
> present in the notmuch hooks directory. The hooks will be run before and
> after incorporating new messages to the database.
> 
> Typical use cases for pre-new and post-new hooks are fetching or delivering
> new mail to the maildir, and custom tagging of the mail incorporated to the
> database.
> 
> Also add command line option --no-hooks to notmuch new to bypass the
> hooks.

A useful extension would be providing the list of added message-ids to the
post-new hook, one per line? Or maybe the script wants added, moved,
deleted ...

  Tom

Re: [PATCH v2 2/2] cli: add support for pre and post notmuch new hooks

[Jani Nikula]

On Sat, 3 Dec 2011 23:00:47 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> Quoth Jani Nikula on Dec 04 at  1:16 am:
> > Run notmuch new pre and post hooks, named "pre-new" and "post-new", if
> > present in the notmuch hooks directory. The hooks will be run before and
> > after incorporating new messages to the database.
> > 
> > Typical use cases for pre-new and post-new hooks are fetching or delivering
> > new mail to the maildir, and custom tagging of the mail incorporated to the
> > database.
> > 
> > Also add command line option --no-hooks to notmuch new to bypass the hooks.
> > 
> > Signed-off-by: Jani Nikula <jani@nikula.org>
> > ---
> >  notmuch-new.c |   12 ++++++++++++
> >  notmuch.1     |   50 +++++++++++++++++++++++++++++++++++++++++++++++++-
> >  2 files changed, 61 insertions(+), 1 deletions(-)
> > 
> > diff --git a/notmuch-new.c b/notmuch-new.c
> > index 81a9350..27dde0c 100644
> > --- a/notmuch-new.c
> > +++ b/notmuch-new.c
> > @@ -811,6 +811,7 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
> >      _filename_node_t *f;
> >      int i;
> >      notmuch_bool_t timer_is_active = FALSE;
> > +    int run_hooks = 1;
> 
> notmuch_bool_t?

Yes.

> >      add_files_state.verbose = 0;
> >      add_files_state.output_is_a_tty = isatty (fileno (stdout));
> > @@ -820,6 +821,8 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
> >      for (i = 0; i < argc && argv[i][0] == '-'; i++) {
> >  	if (STRNCMP_LITERAL (argv[i], "--verbose") == 0) {
> >  	    add_files_state.verbose = 1;
> > +	} else if (STRNCMP_LITERAL (argv[i], "--no-hooks") == 0) {
> 
> I see this mistake all over notmuch, so maybe it's better to
> perpetuate it here and fix it everywhere in another patch, but this
> should be strcmp, not STRNCMP_LITERAL.  STRNCMP_LITERAL is the right
> thing for options that take values, but for boolean options like this,
> it will accept
>   notmuch new --no-hooks-just-kidding

Oops. I just took it from the --verbose handling above without
checking. I'll fix this one, as I don't think everyone else making the
same mistake is a good reason to repeat it. The rest will be taken care
of in the Great Argument Parsing Overhaul which is in the works...

> > +	    run_hooks = 0;
> >  	} else {
> >  	    fprintf (stderr, "Unrecognized option: %s\n", argv[i]);
> >  	    return 1;
> > @@ -833,6 +836,12 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
> >      add_files_state.synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config);
> >      db_path = notmuch_config_get_database_path (config);
> >  
> > +    if (run_hooks) {
> > +	ret = notmuch_run_hook (db_path, "pre-new");
> > +	if (ret)
> > +	    return ret;
> > +    }
> > +
> >      dot_notmuch_path = talloc_asprintf (ctx, "%s/%s", db_path, ".notmuch");
> >  
> >      if (stat (dot_notmuch_path, &st)) {
> > @@ -981,5 +990,8 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
> >  
> >      notmuch_database_close (notmuch);
> >  
> > +    if (run_hooks && !ret && !interrupted)
> > +	ret = notmuch_run_hook (db_path, "post-new");
> 
> Does it matter at this point if the hook fails?  I'm not sure.

I wasn't sure either, but I ended up thinking that the hooks become part
of 'notmuch new' and claiming success when a hook fails is not quite
right. This might have importance if scripting 'notmuch new'.

> > +
> >      return ret || interrupted;
> >  }
> > diff --git a/notmuch.1 b/notmuch.1
> > index 92931d7..66f82e9 100644
> > --- a/notmuch.1
> > +++ b/notmuch.1
> 
> I am willfully ignorant of nroff, so somebody else will have to
> comment if any of the nroff code/formatting is wrong.

That makes two of us; I shamelessly admit I'm just following what
everyone else seems to be doing... cargo cult.

> > @@ -85,7 +85,7 @@ The
> >  command is used to incorporate new mail into the notmuch database.
> >  .RS 4
> >  .TP 4
> > -.B new
> > +.BR new " [options...]"
> >  
> >  Find and import any new messages to the database.
> >  
> > @@ -118,6 +118,22 @@ if
> >  has previously been completed, but
> >  .B "notmuch new"
> >  has not previously been run.
> > +
> > +The
> > +.B new
> > +command supports hooks. See the
> > +.B "HOOKS"
> > +section below for more details on hooks.
> > +
> > +Supported options for
> > +.B new
> > +include
> > +.RS 4
> > +.TP 4
> > +.BR \-\-no\-hooks
> > +
> > +Prevents hooks from being run.
> > +.RE
> >  .RE
> >  
> >  Several of the notmuch commands accept search terms with a common
> > @@ -705,6 +721,38 @@ specify a date range to return messages from 2009\-10\-01 until the
> >  current time:
> >  
> >  	$(date +%s \-d 2009\-10\-01)..$(date +%s)
> > +.SH HOOKS
> > +Hooks are scripts (or arbitrary executables or symlinks to such) you can place
> > +in the notmuch hooks directory to trigger action at certain points. The hooks
> > +directory is .notmuch/hooks within the database directory. The user must have
> > +executable permission set on the scripts.
> 
> Could be more concise.  Maybe something like "Hooks are scripts (or
> arbitrary executables or symlinks to such) that notmuch invokes before
> and after certain actions.  These scripts reside in the .notmuch/hooks
> directory within the database directory and must have executable
> permissions."

Better, thanks.

> > +
> > +The currently available hooks are described below.
> > +.RS 4
> > +.TP 4
> > +.B pre\-new
> > +This hook is invoked by the
> > +.B new
> > +command before scanning or importing new messages into the database. Any errors
> > +in running the hook will abort further processing of the
> 
> "If this script exits with a non-zero status, notmuch will abort ..."?

Yeah, I was trying to cover also the errors that may happen before the
script is actually run, but perhaps that's not important.

> > +.B new
> > +command.
> > +
> > +Typical use case for this hook is fetching or delivering new mail to be imported
> > +into the database.
> 
> Perhaps "Typically this hook is used for ..."?

Not being a native speaker, I'll take your word for it. :)

> > +.RE
> > +.RS 4
> > +.TP 4
> > +.B post\-new
> > +This hook is invoked by the
> > +.B new
> > +command after new messages have been imported into the database and initial tags
> > +have been applied. The hook will not be run if there have been any errors during
> > +the scan or import.
> > +
> > +Typical use case for this hook is performing additional query based tagging on
> > +the imported messages.
> 
> Same thing.  "Typically this hook is used to perform ..."?  Also,
> "query-based".

Ditto.

> 
> > +.RE
> >  .SH ENVIRONMENT
> >  The following environment variables can be used to control the
> >  behavior of notmuch.

Many thanks for your thorough review, as always!


BR,
Jani.

Re: [PATCH v2 2/2] cli: add support for pre and post notmuch new hooks

[Tom Prince]

On Sun, 04 Dec 2011 21:36:21 +0200, Jani Nikula <jani@nikula.org> wrote:
> > > +    if (run_hooks && !ret && !interrupted)
> > > +	ret = notmuch_run_hook (db_path, "post-new");
> > 
> > Does it matter at this point if the hook fails?  I'm not sure.
> 
> I wasn't sure either, but I ended up thinking that the hooks become part
> of 'notmuch new' and claiming success when a hook fails is not quite
> right. This might have importance if scripting 'notmuch new'.

One can always add 'exit 0' (perhaps with appropriate traps) to the hook
script, if you don't want failure to propagate to 'notmuch new', but not
the reverse.

  Tom

Re: [PATCH v2 2/2] cli: add support for pre and post notmuch new hooks

[Jani Nikula]

On Sun, 04 Dec 2011 11:46:38 -0500, Tom Prince <tom.prince@ualberta.net> wrote:
> On Sun,  4 Dec 2011 01:16:49 +0200, Jani Nikula <jani@nikula.org> wrote:
> > Run notmuch new pre and post hooks, named "pre-new" and "post-new", if
> > present in the notmuch hooks directory. The hooks will be run before and
> > after incorporating new messages to the database.
> > 
> > Typical use cases for pre-new and post-new hooks are fetching or delivering
> > new mail to the maildir, and custom tagging of the mail incorporated to the
> > database.
> > 
> > Also add command line option --no-hooks to notmuch new to bypass the
> > hooks.
> 
> A useful extension would be providing the list of added message-ids to the
> post-new hook, one per line? Or maybe the script wants added, moved,
> deleted ...

Hi Tom -

I have thought about passing arguments (or stdin) to the hooks, but IMHO
for now we're better off starting with plain hooks, gathering some
experience, and doing such extensions later on to get them right. It's
easier to add arguments than to change them.

I think it was you who told me of the idea of initial tagging with "new"
and then processing all those messages in a tagging script. I think this
approach is still very good and valid with hooks, and probably more
robust than message-id passing too (the hooks must be run before opening
and after closing the database, and the hooks might get
interrupted). Passing plenty of message-ids might be a bit heavy if they
end up not being used after all.

BR,
Jani.

[PATCH v3 0/2] notmuch hooks

[Jani Nikula]

Hi all, this is v3 of the notmuch hooks patches. I think this is nearing
completion apart from final review and, most notably, tests.

Changes in v3:

* Incorporate Austin's review comments (id:"20111204034210.GA16405@mit.edu" and
  id:"20111204040047.GB16405@mit.edu"), the biggest change being the switch from
  system() to fork()/execl().

* Rename notmuch-hook.c to hooks.c (reserving notmuch- prefixed files to notmuch
  commands).

* Add "notmuch help" documentation.

* Add NEWS.

I've been using this for some days now, and (subjective as it is) I have to say
I like offlineimap being run from notmuch new "pre-new" hook much better than
vice versa. Even more so for "post-new" tagging scripts.


BR,
Jani.


Jani Nikula (2):
  cli: introduce the concept of user defined hooks
  cli: add support for pre and post notmuch new hooks

 Makefile.local   |    1 +
 NEWS             |   10 ++++++
 hooks.c          |   93 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 notmuch-client.h |    3 ++
 notmuch-new.c    |   12 +++++++
 notmuch.1        |   50 ++++++++++++++++++++++++++++-
 notmuch.c        |   39 ++++++++++++++++++++++-
 7 files changed, 206 insertions(+), 2 deletions(-)
 create mode 100644 hooks.c

-- 
1.7.5.4

[PATCH v3 1/2] cli: introduce the concept of user defined hooks

[Jani Nikula]

Add mechanism for running user defined hooks. Hooks are executables or
symlinks to executables stored under the new notmuch hooks directory,
<database-path>/.notmuch/hooks.

No hooks are introduced here, but adding support for a hook is now a simple
matter of calling the new notmuch_run_hook() function at an appropriate
location with the hook name.

Signed-off-by: Jani Nikula <jani@nikula.org>
---
 Makefile.local   |    1 +
 hooks.c          |   93 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 notmuch-client.h |    3 ++
 3 files changed, 97 insertions(+), 0 deletions(-)
 create mode 100644 hooks.c

diff --git a/Makefile.local b/Makefile.local
index c94402b..60ae9cb 100644
--- a/Makefile.local
+++ b/Makefile.local
@@ -298,6 +298,7 @@ notmuch_client_srcs =		\
 	debugger.c		\
 	gmime-filter-reply.c	\
 	gmime-filter-headers.c	\
+	hooks.c			\
 	notmuch.c		\
 	notmuch-config.c	\
 	notmuch-count.c		\
diff --git a/hooks.c b/hooks.c
new file mode 100644
index 0000000..c2d59c1
--- /dev/null
+++ b/hooks.c
@@ -0,0 +1,93 @@
+/* notmuch - Not much of an email program, (just index and search)
+ *
+ * This file is part of notmuch.
+ *
+ * Copyright © 2011 Jani Nikula
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see http://www.gnu.org/licenses/ .
+ *
+ * Author: Jani Nikula <jani@nikula.org>
+ */
+
+#include "notmuch-client.h"
+#include <sys/wait.h>
+
+int
+notmuch_run_hook (const char *db_path, const char *hook)
+{
+    char *hook_path;
+    int status = 0;
+    pid_t pid;
+
+    hook_path = talloc_asprintf (NULL, "%s/%s/%s/%s", db_path, ".notmuch",
+				 "hooks", hook);
+    if (hook_path == NULL) {
+	fprintf (stderr, "Out of memory\n");
+	return 1;
+    }
+
+    /* Check access before fork() for speed and simplicity of error handling. */
+    if (access (hook_path, X_OK) == -1) {
+	/* Ignore ENOENT. It's okay not to have a hook, hook dir, or even
+	 * notmuch dir. Dangling symbolic links also result in ENOENT, but
+	 * we'll ignore that too for simplicity. */
+	if (errno != ENOENT) {
+	    fprintf (stderr, "Error: %s hook access failed: %s\n", hook,
+		     strerror (errno));
+	    status = 1;
+	}
+	goto DONE;
+    }
+
+    pid = fork();
+    if (pid == -1) {
+	fprintf (stderr, "Error: %s hook fork failed: %s\n", hook,
+		 strerror (errno));
+	status = 1;
+	goto DONE;
+    } else if (pid == 0) {
+	execl (hook_path, hook_path, NULL);
+	/* Same as above for ENOENT, but unlikely now. Indicate all other errors
+	 * to parent through non-zero exit status. */
+	if (errno != ENOENT) {
+	    fprintf (stderr, "Error: %s hook execution failed: %s\n", hook,
+		     strerror (errno));
+	    status = 1;
+	}
+	exit (status);
+    }
+
+    if (waitpid (pid, &status, 0) == -1) {
+	fprintf (stderr, "Error: %s hook wait failed: %s\n", hook,
+		 strerror (errno));
+	status = 1;
+	goto DONE;
+    }
+
+    if (!WIFEXITED (status) || WEXITSTATUS (status)) {
+	if (WEXITSTATUS (status)) {
+	    fprintf (stderr, "Error: %s hook failed with status %d\n",
+		     hook, WEXITSTATUS (status));
+	} else if (WIFSIGNALED (status)) {
+	    fprintf (stderr, "Error: %s hook terminated with signal %d\n",
+		     hook, WTERMSIG (status));
+	}
+	status = 1;
+    }
+
+  DONE:
+    talloc_free (hook_path);
+
+    return status;
+}
diff --git a/notmuch-client.h b/notmuch-client.h
index b50cb38..a91ad6c 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -235,6 +235,9 @@ void
 notmuch_config_set_maildir_synchronize_flags (notmuch_config_t *config,
 					      notmuch_bool_t synchronize_flags);
 
+int
+notmuch_run_hook (const char *db_path, const char *hook);
+
 notmuch_bool_t
 debugger_is_active (void);
 
-- 
1.7.5.4

[PATCH v3 2/2] cli: add support for pre and post notmuch new hooks

[Jani Nikula]

Run notmuch new pre and post hooks, named "pre-new" and "post-new", if
present in the notmuch hooks directory. The hooks will be run before and
after incorporating new messages to the database.

Typical use cases for pre-new and post-new hooks are fetching or delivering
new mail to the maildir, and custom tagging of the mail incorporated to the
database.

Also add command line option --no-hooks to notmuch new to bypass the hooks.

Signed-off-by: Jani Nikula <jani@nikula.org>
---
 NEWS          |   10 ++++++++++
 notmuch-new.c |   12 ++++++++++++
 notmuch.1     |   50 +++++++++++++++++++++++++++++++++++++++++++++++++-
 notmuch.c     |   39 ++++++++++++++++++++++++++++++++++++++-
 4 files changed, 109 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index 2b2f08a..a7c13a8 100644
--- a/NEWS
+++ b/NEWS
@@ -1,6 +1,16 @@
 Notmuch 0.11 (201x-xx-xx)
 =========================
 
+New command-line features
+-------------------------
+
+Hooks
+
+  Hooks have been introduced to notmuch. Hooks are scripts that notmuch
+  invokes before and after certain actions. Initially, "notmuch new"
+  supports "pre-new" and "post-new" hooks that are run before and after
+  importing new messages into the database.
+
 Performance
 -----------
 
diff --git a/notmuch-new.c b/notmuch-new.c
index 81a9350..bfb4600 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -811,6 +811,7 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
     _filename_node_t *f;
     int i;
     notmuch_bool_t timer_is_active = FALSE;
+    notmuch_bool_t run_hooks = TRUE;
 
     add_files_state.verbose = 0;
     add_files_state.output_is_a_tty = isatty (fileno (stdout));
@@ -820,6 +821,8 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
     for (i = 0; i < argc && argv[i][0] == '-'; i++) {
 	if (STRNCMP_LITERAL (argv[i], "--verbose") == 0) {
 	    add_files_state.verbose = 1;
+	} else if (strcmp (argv[i], "--no-hooks") == 0) {
+	    run_hooks = FALSE;
 	} else {
 	    fprintf (stderr, "Unrecognized option: %s\n", argv[i]);
 	    return 1;
@@ -833,6 +836,12 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
     add_files_state.synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config);
     db_path = notmuch_config_get_database_path (config);
 
+    if (run_hooks) {
+	ret = notmuch_run_hook (db_path, "pre-new");
+	if (ret)
+	    return ret;
+    }
+
     dot_notmuch_path = talloc_asprintf (ctx, "%s/%s", db_path, ".notmuch");
 
     if (stat (dot_notmuch_path, &st)) {
@@ -981,5 +990,8 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
 
     notmuch_database_close (notmuch);
 
+    if (run_hooks && !ret && !interrupted)
+	ret = notmuch_run_hook (db_path, "post-new");
+
     return ret || interrupted;
 }
diff --git a/notmuch.1 b/notmuch.1
index 92931d7..f631a5e 100644
--- a/notmuch.1
+++ b/notmuch.1
@@ -85,7 +85,7 @@ The
 command is used to incorporate new mail into the notmuch database.
 .RS 4
 .TP 4
-.B new
+.BR new " [options...]"
 
 Find and import any new messages to the database.
 
@@ -118,6 +118,22 @@ if
 has previously been completed, but
 .B "notmuch new"
 has not previously been run.
+
+The
+.B new
+command supports hooks. See the
+.B "HOOKS"
+section below for more details on hooks.
+
+Supported options for
+.B new
+include
+.RS 4
+.TP 4
+.BR \-\-no\-hooks
+
+Prevents hooks from being run.
+.RE
 .RE
 
 Several of the notmuch commands accept search terms with a common
@@ -705,6 +721,38 @@ specify a date range to return messages from 2009\-10\-01 until the
 current time:
 
 	$(date +%s \-d 2009\-10\-01)..$(date +%s)
+.SH HOOKS
+Hooks are scripts (or arbitrary executables or symlinks to such) that notmuch
+invokes before and after certain actions. These scripts reside in
+the .notmuch/hooks directory within the database directory and must have
+executable permissions.
+
+The currently available hooks are described below.
+.RS 4
+.TP 4
+.B pre\-new
+This hook is invoked by the
+.B new
+command before scanning or importing new messages into the database. If this
+hook exits with a non-zero status, notmuch will abort further processing of the
+.B new
+command.
+
+Typically this hook is used for fetching or delivering new mail to be imported
+into the database.
+.RE
+.RS 4
+.TP 4
+.B post\-new
+This hook is invoked by the
+.B new
+command after new messages have been imported into the database and initial tags
+have been applied. The hook will not be run if there have been any errors during
+the scan or import.
+
+Typically this hook is used to perform additional query\-based tagging on the
+imported messages.
+.RE
 .SH ENVIRONMENT
 The following environment variables can be used to control the
 behavior of notmuch.
diff --git a/notmuch.c b/notmuch.c
index d44ce9a..c0ce026 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -127,6 +127,32 @@ static const char search_terms_help[] =
     "\n"
     "\t\t$(date +%%s -d 2009-10-01)..$(date +%%s)\n\n";
 
+static const char hooks_help[] =
+    "\tHooks are scripts (or arbitrary executables or symlinks to such) that\n"
+    "\tnotmuch invokes before and after certain actions. These scripts reside\n"
+    "\tin the .notmuch/hooks directory within the database directory and must\n"
+    "\thave executable permissions.\n"
+    "\n"
+    "\tThe currently available hooks are described below.\n"
+    "\n"
+    "\tpre-new\n"
+    "\t\tThis hook is invoked by the new command before scanning or\n"
+    "\t\timporting new messages into the database. If this hook exits\n"
+    "\t\twith a non-zero status, notmuch will abort further processing\n"
+    "\t\tof the new command.\n"
+    "\n"
+    "\t\tTypically this hook is used for fetching or delivering new\n"
+    "\t\tmail to be imported into the database.\n"
+    "\n"
+    "\tpost-new\n"
+    "\t\tThis hook is invoked by the new command after new messages\n"
+    "\t\thave been imported into the database and initial tags have\n"
+    "\t\tbeen applied. The hook will not be run if there have been any\n"
+    "\t\terrors during the scan or import.\n"
+    "\n"
+    "\t\tTypically this hook is used to perform additional query-based\n"
+    "\t\ttagging on the imported messages.\n\n";
+
 static command_t commands[] = {
     { "setup", notmuch_setup_command,
       NULL,
@@ -144,7 +170,7 @@ static command_t commands[] = {
       "\tInvoking notmuch with no command argument will run setup if\n"
       "\tthe setup command has not previously been completed." },
     { "new", notmuch_new_command,
-      "[--verbose]",
+      "[options...]",
       "Find and import new messages to the notmuch database.",
       "\tScans all sub-directories of the mail directory, performing\n"
       "\tfull-text indexing on new messages that are found. Each new\n"
@@ -159,8 +185,15 @@ static command_t commands[] = {
       "\tis delivered and you wish to incorporate it into the database.\n"
       "\tThese subsequent runs will be much quicker than the initial run.\n"
       "\n"
+      "\tThe new command supports hooks. See \"notmuch help hooks\" for\n"
+      "\tmore details on hooks.\n"
+      "\n"
       "\tSupported options for new include:\n"
       "\n"
+      "\t--no-hooks\n"
+      "\n"
+      "\t\tPrevent hooks from being run.\n"
+      "\n"
       "\t--verbose\n"
       "\n"
       "\t\tVerbose operation. Shows paths of message files as\n"
@@ -529,6 +562,10 @@ notmuch_help_command (unused (void *ctx), int argc, char *argv[])
 	printf ("\n");
 	printf (search_terms_help);
 	return 0;
+    } else if (strcmp (argv[0], "hooks") == 0) {
+	printf ("Help for <%s>\n\n", argv[0]);
+	printf (hooks_help);
+	return 0;
     }
 
     fprintf (stderr,
-- 
1.7.5.4

Re: [PATCH v3 1/2] cli: introduce the concept of user defined hooks

[Jani Nikula]

On Tue,  6 Dec 2011 15:22:37 +0200, Jani Nikula <jani@nikula.org> wrote:
> +    if (!WIFEXITED (status) || WEXITSTATUS (status)) {
> +	if (WEXITSTATUS (status)) {

Grrh, the above should be "if (WIFEXITED (status))". Please review
otherwise.

Jani.

> +	    fprintf (stderr, "Error: %s hook failed with status %d\n",
> +		     hook, WEXITSTATUS (status));
> +	} else if (WIFSIGNALED (status)) {
> +	    fprintf (stderr, "Error: %s hook terminated with signal %d\n",
> +		     hook, WTERMSIG (status));
> +	}
> +	status = 1;
> +    }

Re: [PATCH v3 0/2] notmuch hooks

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 1599 bytes --]

On Tue,  6 Dec 2011 15:22:36 +0200, Jani Nikula <jani@nikula.org> wrote:
> Hi all, this is v3 of the notmuch hooks patches. I think this is nearing
> completion apart from final review and, most notably, tests.

Hey, Jani.  Thanks so much for these patches.  I think this is a really
neat idea and it will be very useful.  I already know that this will cut
out a lot of extraneous shell scripting that I'm relying on now.

> I've been using this for some days now, and (subjective as it is) I have to say
> I like offlineimap being run from notmuch new "pre-new" hook much better than
> vice versa. Even more so for "post-new" tagging scripts.

I think the only thing that I was not particularly clear about is that I
guess the "pre-new" and "post-new" scripts actually need to be scripts
named "pre-new" and "post-new" in the hooks directory.  For some reason
I didn't quite grok that from the help pages, and was confused if I
needed to make a new "pre-new" subdirectory in the hooks directory or
what.  I think just expecting a script at hook/pre-new is fine, but it
maybe could be made a little more explicit in the documentation.

In fact, I think the best thing would actually be for notmuch new to
make the .notmuch/hooks directory itself and to pre-fill it with "turned
off" hook scripts that contain some useful comments on how to use them.
This is what git does, and I think it's very handy, since it's all
fairly self documenting.  The notmuch help could then just refer to the
fact that they're there, and point people to the comments in the various
scripts for how to use them.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH v3 0/2] notmuch hooks

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 198 bytes --]

Also, what if we make it so that the post-new hook script only runs if
notmuch new processes new messages?  All of my post-new functions don't
need to be run at all if there is no new mail.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH v3 0/2] notmuch hooks

[Tom Prince]

On Tue, 06 Dec 2011 18:47:01 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> Also, what if we make it so that the post-new hook script only runs if
> notmuch new processes new messages?  All of my post-new functions don't
> need to be run at all if there is no new mail.

Or would it make sense to pass this information to the hook somehow?

I currently run 'git pull && notmuch new && git commit && git push" or
so ...

  Tom

Re: [PATCH v3 0/2] notmuch hooks

[Jani Nikula]

On Tue, 06 Dec 2011 22:16:37 -0500, Tom Prince <tom.prince@ualberta.net> wrote:
> On Tue, 06 Dec 2011 18:47:01 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> > Also, what if we make it so that the post-new hook script only runs if
> > notmuch new processes new messages?  All of my post-new functions don't
> > need to be run at all if there is no new mail.

I think the post-new hook should be run always (provided there have been
no errors). I think it might be surprising not to, and some users might
use the hook for something other than tagging.

> Or would it make sense to pass this information to the hook somehow?

It would, but as I wrote in id:"87mxb8kt5r.fsf@nikula.org", I think that
should come as another patch afterwards. I know I can't decide yet what
should be passed and how. Processed message counts (added, deleted,
renamed) could be passed on the command line, but how useful is that
really? The same can be easily achieved through initial tagging.

Message-ids could not be passed on the command line (there just can be
too many of them) so it would require setting up a pipe and feeding them
to stdin of the hook. The post-new hook should be run after the database
has been closed, but the message-ids are not saved during notmuch new
processing. Saving them for later gets complicated for not much extra
benefit in addition to creative use of initial tagging, as far as I can
see. Plus interrupting the post-new hook with this setup would screw up
your processing if it only depended on the message-ids.

All in all, I'd postpone all of this until later.


BR,
Jani.

Re: [PATCH v3 0/2] notmuch hooks

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 672 bytes --]

On Wed, 07 Dec 2011 20:05:22 +0200, Jani Nikula <jani@nikula.org> wrote:
> It would, but as I wrote in id:"87mxb8kt5r.fsf@nikula.org", I think that
> should come as another patch afterwards. I know I can't decide yet what
> should be passed and how. Processed message counts (added, deleted,
> renamed) could be passed on the command line, but how useful is that
> really? The same can be easily achieved through initial tagging.

I would say that probably the best way to pass information to the hook
scripts is via the environment.  For instance, notmuch could pass the
variable NOTMUCH_NEW_MESSAGES containing the number of new messages to
the post-new script.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH v3 0/2] notmuch hooks

[Jani Nikula]

On Tue, 06 Dec 2011 18:27:27 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> On Tue,  6 Dec 2011 15:22:36 +0200, Jani Nikula <jani@nikula.org> wrote:
> > Hi all, this is v3 of the notmuch hooks patches. I think this is nearing
> > completion apart from final review and, most notably, tests.
> 
> Hey, Jani.  Thanks so much for these patches.  I think this is a really
> neat idea and it will be very useful.  I already know that this will cut
> out a lot of extraneous shell scripting that I'm relying on now.

Thanks for your interest!

> > I've been using this for some days now, and (subjective as it is) I have to say
> > I like offlineimap being run from notmuch new "pre-new" hook much better than
> > vice versa. Even more so for "post-new" tagging scripts.
> 
> I think the only thing that I was not particularly clear about is that I
> guess the "pre-new" and "post-new" scripts actually need to be scripts
> named "pre-new" and "post-new" in the hooks directory.  For some reason
> I didn't quite grok that from the help pages, and was confused if I
> needed to make a new "pre-new" subdirectory in the hooks directory or
> what.  I think just expecting a script at hook/pre-new is fine, but it
> maybe could be made a little more explicit in the documentation.

It's of course difficult for me to see this from someone else's point of
view, but IMHO all the relevant details can be found in the man page and
help.

> In fact, I think the best thing would actually be for notmuch new to
> make the .notmuch/hooks directory itself and to pre-fill it with "turned
> off" hook scripts that contain some useful comments on how to use them.
> This is what git does, and I think it's very handy, since it's all
> fairly self documenting.  The notmuch help could then just refer to the
> fact that they're there, and point people to the comments in the various
> scripts for how to use them.

This is a good idea, and complimentary to the documentation. I'll add
this. (I'll just have to come up with good samples. :o)

I wonder, though, whether the samples should be created only when
creating the database, or whenever the hooks directory does not exist?
The latter, I guess. Otherwise existing notmuch users would hardly get
the sample hooks.


BR,
Jani.

Re: [PATCH v3 0/2] notmuch hooks

[Austin Clements]

Quoth Jani Nikula on Dec 07 at  8:05 pm:
> On Tue, 06 Dec 2011 22:16:37 -0500, Tom Prince <tom.prince@ualberta.net> wrote:
> > On Tue, 06 Dec 2011 18:47:01 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> > > Also, what if we make it so that the post-new hook script only runs if
> > > notmuch new processes new messages?  All of my post-new functions don't
> > > need to be run at all if there is no new mail.
> 
> I think the post-new hook should be run always (provided there have been
> no errors). I think it might be surprising not to, and some users might
> use the hook for something other than tagging.
> 
> > Or would it make sense to pass this information to the hook somehow?
> 
> It would, but as I wrote in id:"87mxb8kt5r.fsf@nikula.org", I think that
> should come as another patch afterwards. I know I can't decide yet what
> should be passed and how. Processed message counts (added, deleted,
> renamed) could be passed on the command line, but how useful is that
> really? The same can be easily achieved through initial tagging.

I would worry about creating any hook interface that's difficult to
use correctly.  For example, notmuch new could be interrupted right
between processing all new messages and calling the hook; then, on the
next run, it'll tell the hook that nothing happened.  More generally,
any message counts passed to a hook can't be better than lower bounds
and I don't see how you could use such information correctly in a
hook.

Using an initial "new" tag that you remove at the end of a hook,
though, is stable.  It guarantees that the hook is aware of any new
messages at least once.  If a hook needs a new message count, it
should run notmuch count tag:new.

> Message-ids could not be passed on the command line (there just can be
> too many of them) so it would require setting up a pipe and feeding them
> to stdin of the hook. The post-new hook should be run after the database
> has been closed, but the message-ids are not saved during notmuch new
> processing. Saving them for later gets complicated for not much extra
> benefit in addition to creative use of initial tagging, as far as I can
> see. Plus interrupting the post-new hook with this setup would screw up
> your processing if it only depended on the message-ids.

I think this would have to be a separate hook that runs concurrently
with new, both so that new doesn't have to buffer this information and
so that the majority of post-new hooks that don't need this
information don't have to deal with it.  Though, as with passing
message counts to post-new, I worry about actually do anything
correctly in such a hook in the presence of interruptions and
failures.

> All in all, I'd postpone all of this until later.

That sounds wise.

Re: [PATCH v3 0/2] notmuch hooks

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 598 bytes --]

On Wed, 07 Dec 2011 21:42:33 +0200, Jani Nikula <jani@nikula.org> wrote:
> I wonder, though, whether the samples should be created only when
> creating the database, or whenever the hooks directory does not exist?
> The latter, I guess. Otherwise existing notmuch users would hardly get
> the sample hooks.

I agree, the later would be better.  notmuch new should just create the
hooks directory (and example scripts) if they don't already exist.

I also think it's nice to have the scripts not executable to begin with,
and the hook runner only executes the script if they are executable.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH v3 0/2] notmuch hooks

[Tomi Ollila]

On Wed, 07 Dec 2011 14:13:13 -0800, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> On Wed, 07 Dec 2011 21:42:33 +0200, Jani Nikula <jani@nikula.org> wrote:
> > I wonder, though, whether the samples should be created only when
> > creating the database, or whenever the hooks directory does not exist?
> > The latter, I guess. Otherwise existing notmuch users would hardly get
> > the sample hooks.
> 
> I agree, the later would be better.  notmuch new should just create the
> hooks directory (and example scripts) if they don't already exist.

+1

> 
> I also think it's nice to have the scripts not executable to begin with,
> and the hook runner only executes the script if they are executable.

If the hooks are named as 'pre-new.sample' and 'post-new.sample' and
executable then users need just copy the file and start editing (and
not remember to do any chmod's)...

... this is the way it seems to be in .git/hooks -- I tested copy --
permission bits seems to follow (filtered by umask).

> jamie.

Tomi

Re: [PATCH v3 0/2] notmuch hooks

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 537 bytes --]

On Thu, 08 Dec 2011 10:49:43 +0200, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> If the hooks are named as 'pre-new.sample' and 'post-new.sample' and
> executable then users need just copy the file and start editing (and
> not remember to do any chmod's)...

It seems simpler and cleaner to me to just have notmuch new check for
executability, and give the example scripts the proper names.  That's
certainly what git does, so there's already precedent, and since we're
modeling this on git it seems good to keep things consistent.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

[PATCH v4 0/3] notmuch hooks

[Jani Nikula]

Hi all, this is v4 of the notmuch hooks patches. Changes:

* Fix WIFEXITED (id:"8739cxzv30.fsf@nikula.org").

* Add tests.

BR,
Jani.


Jani Nikula (3):
  cli: introduce the concept of user defined hooks
  cli: add support for pre and post notmuch new hooks
  test: add tests for hooks

 Makefile.local    |    1 +
 NEWS              |   10 +++++
 hooks.c           |   93 +++++++++++++++++++++++++++++++++++++++++++++++
 notmuch-client.h  |    3 ++
 notmuch-new.c     |   12 ++++++
 notmuch.1         |   50 +++++++++++++++++++++++++-
 notmuch.c         |   39 +++++++++++++++++++-
 test/hooks        |  104 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 test/notmuch-test |    1 +
 9 files changed, 311 insertions(+), 2 deletions(-)
 create mode 100644 hooks.c
 create mode 100755 test/hooks

-- 
1.7.5.4

[PATCH v4 1/3] cli: introduce the concept of user defined hooks

[Jani Nikula]

Add mechanism for running user defined hooks. Hooks are executables or
symlinks to executables stored under the new notmuch hooks directory,
<database-path>/.notmuch/hooks.

No hooks are introduced here, but adding support for a hook is now a simple
matter of calling the new notmuch_run_hook() function at an appropriate
location with the hook name.

Signed-off-by: Jani Nikula <jani@nikula.org>
---
 Makefile.local   |    1 +
 hooks.c          |   93 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 notmuch-client.h |    3 ++
 3 files changed, 97 insertions(+), 0 deletions(-)
 create mode 100644 hooks.c

diff --git a/Makefile.local b/Makefile.local
index 15e6d88..88365da 100644
--- a/Makefile.local
+++ b/Makefile.local
@@ -298,6 +298,7 @@ notmuch_client_srcs =		\
 	debugger.c		\
 	gmime-filter-reply.c	\
 	gmime-filter-headers.c	\
+	hooks.c			\
 	notmuch.c		\
 	notmuch-config.c	\
 	notmuch-count.c		\
diff --git a/hooks.c b/hooks.c
new file mode 100644
index 0000000..44ee419
--- /dev/null
+++ b/hooks.c
@@ -0,0 +1,93 @@
+/* notmuch - Not much of an email program, (just index and search)
+ *
+ * This file is part of notmuch.
+ *
+ * Copyright © 2011 Jani Nikula
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see http://www.gnu.org/licenses/ .
+ *
+ * Author: Jani Nikula <jani@nikula.org>
+ */
+
+#include "notmuch-client.h"
+#include <sys/wait.h>
+
+int
+notmuch_run_hook (const char *db_path, const char *hook)
+{
+    char *hook_path;
+    int status = 0;
+    pid_t pid;
+
+    hook_path = talloc_asprintf (NULL, "%s/%s/%s/%s", db_path, ".notmuch",
+				 "hooks", hook);
+    if (hook_path == NULL) {
+	fprintf (stderr, "Out of memory\n");
+	return 1;
+    }
+
+    /* Check access before fork() for speed and simplicity of error handling. */
+    if (access (hook_path, X_OK) == -1) {
+	/* Ignore ENOENT. It's okay not to have a hook, hook dir, or even
+	 * notmuch dir. Dangling symbolic links also result in ENOENT, but
+	 * we'll ignore that too for simplicity. */
+	if (errno != ENOENT) {
+	    fprintf (stderr, "Error: %s hook access failed: %s\n", hook,
+		     strerror (errno));
+	    status = 1;
+	}
+	goto DONE;
+    }
+
+    pid = fork();
+    if (pid == -1) {
+	fprintf (stderr, "Error: %s hook fork failed: %s\n", hook,
+		 strerror (errno));
+	status = 1;
+	goto DONE;
+    } else if (pid == 0) {
+	execl (hook_path, hook_path, NULL);
+	/* Same as above for ENOENT, but unlikely now. Indicate all other errors
+	 * to parent through non-zero exit status. */
+	if (errno != ENOENT) {
+	    fprintf (stderr, "Error: %s hook execution failed: %s\n", hook,
+		     strerror (errno));
+	    status = 1;
+	}
+	exit (status);
+    }
+
+    if (waitpid (pid, &status, 0) == -1) {
+	fprintf (stderr, "Error: %s hook wait failed: %s\n", hook,
+		 strerror (errno));
+	status = 1;
+	goto DONE;
+    }
+
+    if (!WIFEXITED (status) || WEXITSTATUS (status)) {
+	if (WIFEXITED (status)) {
+	    fprintf (stderr, "Error: %s hook failed with status %d\n",
+		     hook, WEXITSTATUS (status));
+	} else if (WIFSIGNALED (status)) {
+	    fprintf (stderr, "Error: %s hook terminated with signal %d\n",
+		     hook, WTERMSIG (status));
+	}
+	status = 1;
+    }
+
+  DONE:
+    talloc_free (hook_path);
+
+    return status;
+}
diff --git a/notmuch-client.h b/notmuch-client.h
index b50cb38..a91ad6c 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -235,6 +235,9 @@ void
 notmuch_config_set_maildir_synchronize_flags (notmuch_config_t *config,
 					      notmuch_bool_t synchronize_flags);
 
+int
+notmuch_run_hook (const char *db_path, const char *hook);
+
 notmuch_bool_t
 debugger_is_active (void);
 
-- 
1.7.5.4

[PATCH v4 2/3] cli: add support for pre and post notmuch new hooks

[Jani Nikula]

Run notmuch new pre and post hooks, named "pre-new" and "post-new", if
present in the notmuch hooks directory. The hooks will be run before and
after incorporating new messages to the database.

Typical use cases for pre-new and post-new hooks are fetching or delivering
new mail to the maildir, and custom tagging of the mail incorporated to the
database.

Also add command line option --no-hooks to notmuch new to bypass the hooks.

Signed-off-by: Jani Nikula <jani@nikula.org>
---
 NEWS          |   10 ++++++++++
 notmuch-new.c |   12 ++++++++++++
 notmuch.1     |   50 +++++++++++++++++++++++++++++++++++++++++++++++++-
 notmuch.c     |   39 ++++++++++++++++++++++++++++++++++++++-
 4 files changed, 109 insertions(+), 2 deletions(-)

diff --git a/NEWS b/NEWS
index bb5e4d5..eaed960 100644
--- a/NEWS
+++ b/NEWS
@@ -1,6 +1,16 @@
 Notmuch 0.11 (201x-xx-xx)
 =========================
 
+New command-line features
+-------------------------
+
+Hooks
+
+  Hooks have been introduced to notmuch. Hooks are scripts that notmuch
+  invokes before and after certain actions. Initially, "notmuch new"
+  supports "pre-new" and "post-new" hooks that are run before and after
+  importing new messages into the database.
+
 Performance
 -----------
 
diff --git a/notmuch-new.c b/notmuch-new.c
index 81a9350..bfb4600 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -811,6 +811,7 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
     _filename_node_t *f;
     int i;
     notmuch_bool_t timer_is_active = FALSE;
+    notmuch_bool_t run_hooks = TRUE;
 
     add_files_state.verbose = 0;
     add_files_state.output_is_a_tty = isatty (fileno (stdout));
@@ -820,6 +821,8 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
     for (i = 0; i < argc && argv[i][0] == '-'; i++) {
 	if (STRNCMP_LITERAL (argv[i], "--verbose") == 0) {
 	    add_files_state.verbose = 1;
+	} else if (strcmp (argv[i], "--no-hooks") == 0) {
+	    run_hooks = FALSE;
 	} else {
 	    fprintf (stderr, "Unrecognized option: %s\n", argv[i]);
 	    return 1;
@@ -833,6 +836,12 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
     add_files_state.synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config);
     db_path = notmuch_config_get_database_path (config);
 
+    if (run_hooks) {
+	ret = notmuch_run_hook (db_path, "pre-new");
+	if (ret)
+	    return ret;
+    }
+
     dot_notmuch_path = talloc_asprintf (ctx, "%s/%s", db_path, ".notmuch");
 
     if (stat (dot_notmuch_path, &st)) {
@@ -981,5 +990,8 @@ notmuch_new_command (void *ctx, int argc, char *argv[])
 
     notmuch_database_close (notmuch);
 
+    if (run_hooks && !ret && !interrupted)
+	ret = notmuch_run_hook (db_path, "post-new");
+
     return ret || interrupted;
 }
diff --git a/notmuch.1 b/notmuch.1
index 147319e..3dbd67e 100644
--- a/notmuch.1
+++ b/notmuch.1
@@ -85,7 +85,7 @@ The
 command is used to incorporate new mail into the notmuch database.
 .RS 4
 .TP 4
-.B new
+.BR new " [options...]"
 
 Find and import any new messages to the database.
 
@@ -118,6 +118,22 @@ if
 has previously been completed, but
 .B "notmuch new"
 has not previously been run.
+
+The
+.B new
+command supports hooks. See the
+.B "HOOKS"
+section below for more details on hooks.
+
+Supported options for
+.B new
+include
+.RS 4
+.TP 4
+.BR \-\-no\-hooks
+
+Prevents hooks from being run.
+.RE
 .RE
 
 Several of the notmuch commands accept search terms with a common
@@ -705,6 +721,38 @@ specify a date range to return messages from 2009\-10\-01 until the
 current time:
 
 	$(date +%s \-d 2009\-10\-01)..$(date +%s)
+.SH HOOKS
+Hooks are scripts (or arbitrary executables or symlinks to such) that notmuch
+invokes before and after certain actions. These scripts reside in
+the .notmuch/hooks directory within the database directory and must have
+executable permissions.
+
+The currently available hooks are described below.
+.RS 4
+.TP 4
+.B pre\-new
+This hook is invoked by the
+.B new
+command before scanning or importing new messages into the database. If this
+hook exits with a non-zero status, notmuch will abort further processing of the
+.B new
+command.
+
+Typically this hook is used for fetching or delivering new mail to be imported
+into the database.
+.RE
+.RS 4
+.TP 4
+.B post\-new
+This hook is invoked by the
+.B new
+command after new messages have been imported into the database and initial tags
+have been applied. The hook will not be run if there have been any errors during
+the scan or import.
+
+Typically this hook is used to perform additional query\-based tagging on the
+imported messages.
+.RE
 .SH ENVIRONMENT
 The following environment variables can be used to control the
 behavior of notmuch.
diff --git a/notmuch.c b/notmuch.c
index d44ce9a..c0ce026 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -127,6 +127,32 @@ static const char search_terms_help[] =
     "\n"
     "\t\t$(date +%%s -d 2009-10-01)..$(date +%%s)\n\n";
 
+static const char hooks_help[] =
+    "\tHooks are scripts (or arbitrary executables or symlinks to such) that\n"
+    "\tnotmuch invokes before and after certain actions. These scripts reside\n"
+    "\tin the .notmuch/hooks directory within the database directory and must\n"
+    "\thave executable permissions.\n"
+    "\n"
+    "\tThe currently available hooks are described below.\n"
+    "\n"
+    "\tpre-new\n"
+    "\t\tThis hook is invoked by the new command before scanning or\n"
+    "\t\timporting new messages into the database. If this hook exits\n"
+    "\t\twith a non-zero status, notmuch will abort further processing\n"
+    "\t\tof the new command.\n"
+    "\n"
+    "\t\tTypically this hook is used for fetching or delivering new\n"
+    "\t\tmail to be imported into the database.\n"
+    "\n"
+    "\tpost-new\n"
+    "\t\tThis hook is invoked by the new command after new messages\n"
+    "\t\thave been imported into the database and initial tags have\n"
+    "\t\tbeen applied. The hook will not be run if there have been any\n"
+    "\t\terrors during the scan or import.\n"
+    "\n"
+    "\t\tTypically this hook is used to perform additional query-based\n"
+    "\t\ttagging on the imported messages.\n\n";
+
 static command_t commands[] = {
     { "setup", notmuch_setup_command,
       NULL,
@@ -144,7 +170,7 @@ static command_t commands[] = {
       "\tInvoking notmuch with no command argument will run setup if\n"
       "\tthe setup command has not previously been completed." },
     { "new", notmuch_new_command,
-      "[--verbose]",
+      "[options...]",
       "Find and import new messages to the notmuch database.",
       "\tScans all sub-directories of the mail directory, performing\n"
       "\tfull-text indexing on new messages that are found. Each new\n"
@@ -159,8 +185,15 @@ static command_t commands[] = {
       "\tis delivered and you wish to incorporate it into the database.\n"
       "\tThese subsequent runs will be much quicker than the initial run.\n"
       "\n"
+      "\tThe new command supports hooks. See \"notmuch help hooks\" for\n"
+      "\tmore details on hooks.\n"
+      "\n"
       "\tSupported options for new include:\n"
       "\n"
+      "\t--no-hooks\n"
+      "\n"
+      "\t\tPrevent hooks from being run.\n"
+      "\n"
       "\t--verbose\n"
       "\n"
       "\t\tVerbose operation. Shows paths of message files as\n"
@@ -529,6 +562,10 @@ notmuch_help_command (unused (void *ctx), int argc, char *argv[])
 	printf ("\n");
 	printf (search_terms_help);
 	return 0;
+    } else if (strcmp (argv[0], "hooks") == 0) {
+	printf ("Help for <%s>\n\n", argv[0]);
+	printf (hooks_help);
+	return 0;
     }
 
     fprintf (stderr,
-- 
1.7.5.4

[PATCH v4 3/3] test: add tests for hooks

[Jani Nikula]

Signed-off-by: Jani Nikula <jani@nikula.org>
---
 test/hooks        |  104 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 test/notmuch-test |    1 +
 2 files changed, 105 insertions(+), 0 deletions(-)
 create mode 100755 test/hooks

diff --git a/test/hooks b/test/hooks
new file mode 100755
index 0000000..77e8569
--- /dev/null
+++ b/test/hooks
@@ -0,0 +1,104 @@
+#!/usr/bin/env bash
+test_description='hooks'
+. ./test-lib.sh
+
+HOOK_DIR=${MAIL_DIR}/.notmuch/hooks
+
+create_echo_hook () {
+    local TOKEN="${RANDOM}"
+    mkdir -p ${HOOK_DIR}
+    cat <<EOF >"${HOOK_DIR}/${1}"
+#!/bin/sh
+echo "${TOKEN}" > ${3}
+EOF
+    chmod +x "${HOOK_DIR}/${1}"
+    echo "${TOKEN}" > ${2}
+}
+
+create_failing_hook () {
+    mkdir -p ${HOOK_DIR}
+    cat <<EOF >"${HOOK_DIR}/${1}"
+#!/bin/sh
+exit 13
+EOF
+    chmod +x "${HOOK_DIR}/${1}"
+}
+
+rm_hooks () {
+    rm -rf ${HOOK_DIR}
+}
+
+# add a message to generate mail dir and database
+add_message
+
+test_begin_subtest "pre-new is run"
+rm_hooks
+generate_message
+create_echo_hook "pre-new" expected output
+notmuch new > /dev/null
+test_expect_equal_file expected output
+
+test_begin_subtest "post-new is run"
+rm_hooks
+generate_message
+create_echo_hook "post-new" expected output
+notmuch new > /dev/null
+test_expect_equal_file expected output
+
+test_begin_subtest "pre-new is run before post-new"
+rm_hooks
+generate_message
+create_echo_hook "pre-new" pre-new.expected pre-new.output
+create_echo_hook "post-new" post-new.expected post-new.output
+notmuch new > /dev/null
+test_expect_equal_file post-new.expected post-new.output
+
+test_begin_subtest "pre-new non-zero exit status (hook status)"
+rm_hooks
+generate_message
+create_failing_hook "pre-new"
+output=`notmuch new 2>&1`
+test_expect_equal "$output" "Error: pre-new hook failed with status 13"
+
+# depends on the previous subtest leaving broken hook behind
+test_expect_code 1 "pre-new non-zero exit status (notmuch status)" "notmuch new"
+
+# depends on the previous subtests leaving 1 new message behind
+test_begin_subtest "pre-new non-zero exit status aborts new"
+rm_hooks
+output=$(NOTMUCH_NEW)
+test_expect_equal "$output" "Added 1 new message to the database."
+
+test_begin_subtest "post-new non-zero exit status (hook status)"
+rm_hooks
+generate_message
+create_failing_hook "post-new"
+NOTMUCH_NEW 2>output.stderr >output
+cat output.stderr >> output
+echo "Added 1 new message to the database." > expected
+echo "Error: post-new hook failed with status 13" >> expected
+test_expect_equal_file expected output
+
+# depends on the previous subtest leaving broken hook behind
+test_expect_code 1 "post-new non-zero exit status (notmuch status)" "notmuch new"
+
+# test_begin_subtest "hook without executable permissions"
+rm_hooks
+mkdir -p ${HOOK_DIR}
+cat <<EOF >"${HOOK_DIR}/pre-new"
+#!/bin/sh
+echo foo
+EOF
+output=`notmuch new 2>&1`
+test_expect_code 1 "hook without executable permissions" "notmuch new"
+
+# test_begin_subtest "hook execution failure"
+rm_hooks
+mkdir -p ${HOOK_DIR}
+cat <<EOF >"${HOOK_DIR}/pre-new"
+no hashbang, execl fails
+EOF
+chmod +x "${HOOK_DIR}/pre-new"
+test_expect_code 1 "hook execution failure" "notmuch new"
+
+test_done
diff --git a/test/notmuch-test b/test/notmuch-test
index 53ce355..896f51f 100755
--- a/test/notmuch-test
+++ b/test/notmuch-test
@@ -48,6 +48,7 @@ TESTS="
   search-folder-coherence
   atomicity
   python
+  hooks
 "
 TESTS=${NOTMUCH_TESTS:=$TESTS}
 
-- 
1.7.5.4

Re: [PATCH v4 1/3] cli: introduce the concept of user defined hooks

[Austin Clements]

Quoth Jani Nikula on Dec 09 at 12:48 am:
> Add mechanism for running user defined hooks. Hooks are executables or
> symlinks to executables stored under the new notmuch hooks directory,
> <database-path>/.notmuch/hooks.
> 
> No hooks are introduced here, but adding support for a hook is now a simple
> matter of calling the new notmuch_run_hook() function at an appropriate
> location with the hook name.
> 
> Signed-off-by: Jani Nikula <jani@nikula.org>
> ---
>  Makefile.local   |    1 +
>  hooks.c          |   93 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  notmuch-client.h |    3 ++
>  3 files changed, 97 insertions(+), 0 deletions(-)
>  create mode 100644 hooks.c
> 
> diff --git a/Makefile.local b/Makefile.local
> index 15e6d88..88365da 100644
> --- a/Makefile.local
> +++ b/Makefile.local
> @@ -298,6 +298,7 @@ notmuch_client_srcs =		\
>  	debugger.c		\
>  	gmime-filter-reply.c	\
>  	gmime-filter-headers.c	\
> +	hooks.c			\
>  	notmuch.c		\
>  	notmuch-config.c	\
>  	notmuch-count.c		\
> diff --git a/hooks.c b/hooks.c
> new file mode 100644
> index 0000000..44ee419
> --- /dev/null
> +++ b/hooks.c
> @@ -0,0 +1,93 @@
> +/* notmuch - Not much of an email program, (just index and search)
> + *
> + * This file is part of notmuch.
> + *
> + * Copyright © 2011 Jani Nikula
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 3 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see http://www.gnu.org/licenses/ .
> + *
> + * Author: Jani Nikula <jani@nikula.org>
> + */
> +
> +#include "notmuch-client.h"
> +#include <sys/wait.h>
> +
> +int
> +notmuch_run_hook (const char *db_path, const char *hook)
> +{
> +    char *hook_path;
> +    int status = 0;
> +    pid_t pid;
> +
> +    hook_path = talloc_asprintf (NULL, "%s/%s/%s/%s", db_path, ".notmuch",
> +				 "hooks", hook);
> +    if (hook_path == NULL) {
> +	fprintf (stderr, "Out of memory\n");
> +	return 1;
> +    }
> +
> +    /* Check access before fork() for speed and simplicity of error handling. */
> +    if (access (hook_path, X_OK) == -1) {
> +	/* Ignore ENOENT. It's okay not to have a hook, hook dir, or even
> +	 * notmuch dir. Dangling symbolic links also result in ENOENT, but
> +	 * we'll ignore that too for simplicity. */
> +	if (errno != ENOENT) {
> +	    fprintf (stderr, "Error: %s hook access failed: %s\n", hook,
> +		     strerror (errno));
> +	    status = 1;
> +	}

Is it the intent that a present but non-executable hook (errno ==
EACCES) will print the above error message and return with a failure?
I'm pretty sure this differs from the behavior of git hooks.

Other than this, this patch looks good to me.

> +	goto DONE;
> +    }
> +
> +    pid = fork();
> +    if (pid == -1) {
> +	fprintf (stderr, "Error: %s hook fork failed: %s\n", hook,
> +		 strerror (errno));
> +	status = 1;
> +	goto DONE;
> +    } else if (pid == 0) {
> +	execl (hook_path, hook_path, NULL);
> +	/* Same as above for ENOENT, but unlikely now. Indicate all other errors
> +	 * to parent through non-zero exit status. */
> +	if (errno != ENOENT) {
> +	    fprintf (stderr, "Error: %s hook execution failed: %s\n", hook,
> +		     strerror (errno));
> +	    status = 1;
> +	}
> +	exit (status);
> +    }
> +
> +    if (waitpid (pid, &status, 0) == -1) {
> +	fprintf (stderr, "Error: %s hook wait failed: %s\n", hook,
> +		 strerror (errno));
> +	status = 1;
> +	goto DONE;
> +    }
> +
> +    if (!WIFEXITED (status) || WEXITSTATUS (status)) {
> +	if (WIFEXITED (status)) {
> +	    fprintf (stderr, "Error: %s hook failed with status %d\n",
> +		     hook, WEXITSTATUS (status));
> +	} else if (WIFSIGNALED (status)) {
> +	    fprintf (stderr, "Error: %s hook terminated with signal %d\n",
> +		     hook, WTERMSIG (status));
> +	}
> +	status = 1;
> +    }
> +
> +  DONE:
> +    talloc_free (hook_path);
> +
> +    return status;
> +}
> diff --git a/notmuch-client.h b/notmuch-client.h
> index b50cb38..a91ad6c 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -235,6 +235,9 @@ void
>  notmuch_config_set_maildir_synchronize_flags (notmuch_config_t *config,
>  					      notmuch_bool_t synchronize_flags);
>  
> +int
> +notmuch_run_hook (const char *db_path, const char *hook);
> +
>  notmuch_bool_t
>  debugger_is_active (void);

Re: [PATCH v4 1/3] cli: introduce the concept of user defined hooks

[Jani Nikula]

On Thu, 8 Dec 2011 18:34:29 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> Quoth Jani Nikula on Dec 09 at 12:48 am:
> > +    /* Check access before fork() for speed and simplicity of error handling. */
> > +    if (access (hook_path, X_OK) == -1) {
> > +	/* Ignore ENOENT. It's okay not to have a hook, hook dir, or even
> > +	 * notmuch dir. Dangling symbolic links also result in ENOENT, but
> > +	 * we'll ignore that too for simplicity. */
> > +	if (errno != ENOENT) {
> > +	    fprintf (stderr, "Error: %s hook access failed: %s\n", hook,
> > +		     strerror (errno));
> > +	    status = 1;
> > +	}
> 
> Is it the intent that a present but non-executable hook (errno ==
> EACCES) will print the above error message and return with a failure?
> I'm pretty sure this differs from the behavior of git hooks.

It differs from git, and it is intentional. Git bails out with success
status, without even a warning, for *all* access() failures. That may be
fine for git (which generally expects the user to know what he's doing)
but I'd argue notmuch should let the user know something is wrong.

Also for EACCES, IMHO failing is more useful to the user than silently
ignoring. If the hook exists, but isn't executable, I think it's way
more likely that the user forgot to chmod +x than intentionally dropped
x so the hook would not be run. (And I think we agreed on IRC that in
the future, sample hooks would be named hook.sample and have executable
bit set.)

Anyway, this is my opinion; it's not a big deal to change if there are
compelling reasons to ignore EACCES that I didn't think of.


BR,
Jani.

Re: [PATCH v4 1/3] cli: introduce the concept of user defined hooks

[Austin Clements]

Quoth Jani Nikula on Dec 09 at  1:55 pm:
> On Thu, 8 Dec 2011 18:34:29 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> > Quoth Jani Nikula on Dec 09 at 12:48 am:
> > > +    /* Check access before fork() for speed and simplicity of error handling. */
> > > +    if (access (hook_path, X_OK) == -1) {
> > > +	/* Ignore ENOENT. It's okay not to have a hook, hook dir, or even
> > > +	 * notmuch dir. Dangling symbolic links also result in ENOENT, but
> > > +	 * we'll ignore that too for simplicity. */
> > > +	if (errno != ENOENT) {
> > > +	    fprintf (stderr, "Error: %s hook access failed: %s\n", hook,
> > > +		     strerror (errno));
> > > +	    status = 1;
> > > +	}
> > 
> > Is it the intent that a present but non-executable hook (errno ==
> > EACCES) will print the above error message and return with a failure?
> > I'm pretty sure this differs from the behavior of git hooks.
> 
> It differs from git, and it is intentional. Git bails out with success
> status, without even a warning, for *all* access() failures. That may be
> fine for git (which generally expects the user to know what he's doing)
> but I'd argue notmuch should let the user know something is wrong.
> 
> Also for EACCES, IMHO failing is more useful to the user than silently
> ignoring. If the hook exists, but isn't executable, I think it's way
> more likely that the user forgot to chmod +x than intentionally dropped
> x so the hook would not be run. (And I think we agreed on IRC that in
> the future, sample hooks would be named hook.sample and have executable
> bit set.)
> 
> Anyway, this is my opinion; it's not a big deal to change if there are
> compelling reasons to ignore EACCES that I didn't think of.

Consider me convinced.

Re: [PATCH v4 0/3] notmuch hooks

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 418 bytes --]

Hi, Jani.  This is a nice patch series.  I've been using both the pre-
and post-new hooks for a couple days now without any problems.  It's a
nice bit of new functionality.

The patches have already had a good review, and it all looks good to me,
so I definitely supporting pushing them.  They conflict with Austin's
notmuch-show rewrite, but we can easily resolve one or the other after
the first is applied.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH v4 0/3] notmuch hooks

[David Bremner]

On Fri,  9 Dec 2011 00:48:28 +0200, Jani Nikula <jani@nikula.org> wrote:
> Hi all, this is v4 of the notmuch hooks patches. Changes:
> 
> * Fix WIFEXITED (id:"8739cxzv30.fsf@nikula.org").
> 
> * Add tests.

v4 is pushed, thanks everyone.

d