Re: [PATCH] Add option `hooks.path` for setting the directory of hooks.

[Erik Rybakken <erik.rybakken@math.ntnu.no>]

Hi,

Thanks Tomi and David for the feedback!

On Fri, Aug 26, 2016 at 02:32:19PM +0300, Tomi Ollila wrote:

> ... but I can think of one problem there (if my memory server correctly)

Yeah, I didn't think of that. I have been thinking about how to make the
generated configuration show only the options that differ from the
default, and have the default options commented out, but it got a bit too
involved for me.

However, I think I have another solution, and I included a updated patch
for this. There is only one (*) difference from the first patch:

When reading the configuration file and "hooks.path" is either unset or
set to an empty string, we set config->hooks_path to be the expanded
path "database.path/.notmuch/hooks", but we set the value of hooks.path in
config->key_file to be an empty string. That way, when calling
"notmuch_config_get_hooks_path", the expanded path gets returned, but when
saving the config file, either from "notmuch setup" or "notmuch config set",
the value will still be an empty string, given that it wasn't changed.

I believe this is the easiest fix, and if this sounds good, I will start
working on the tests.

(*) Except for some small changes suggested by David Bremner.

Best,
Erik

From 9ddb73f8c373d3de8d54a50378653b44edf60ddc Mon Sep 17 00:00:00 2001
From: Erik Rybakken <erik.rybakken@math.ntnu.no>
Date: Sat, 27 Aug 2016 11:34:00 +0200
Subject: [PATCH] Add hooks.path option

---
 NEWS                        |  7 +++++++
 doc/man1/notmuch-config.rst |  8 ++++++++
 doc/man5/notmuch-hooks.rst  |  7 ++++---
 hooks.c                     |  5 ++---
 notmuch-client.h            |  9 ++++++++-
 notmuch-config.c            | 42 ++++++++++++++++++++++++++++++++++++++++++
 notmuch-insert.c            |  4 +++-
 notmuch-new.c               |  6 ++++--
 8 files changed, 78 insertions(+), 10 deletions(-)

diff --git a/NEWS b/NEWS
index 3a9c8d3..8ddb379 100644
--- a/NEWS
+++ b/NEWS
@@ -1,6 +1,13 @@
 Notmuch 0.23 (UNRELEASED)
 =========================
 
+Command Line Interface
+-------
+
+Add option `hooks.path` for setting the directory for hooks. If
+unset, or set to an empty string, it will default to the
+`.notmuch/hooks` sub-directory.
+
 Emacs
 -----
 
diff --git a/doc/man1/notmuch-config.rst b/doc/man1/notmuch-config.rst
index 5a517eb..21652d3 100644
--- a/doc/man1/notmuch-config.rst
+++ b/doc/man1/notmuch-config.rst
@@ -51,6 +51,14 @@ The available configuration items are described below.
 
         Default: ``$MAILDIR`` variable if set, otherwise ``$HOME/mail``.
 
+    **hooks.path**
+        The directory where your hooks exist. If this value is empty or
+        not set, it will be interpreted as the sub-directory
+        ``.notmuch/hooks`` of the path configured in the ``database.path``
+        option.
+
+        Default: empty string.
+
     **user.name**
         Your full name.
 
diff --git a/doc/man5/notmuch-hooks.rst b/doc/man5/notmuch-hooks.rst
index f96a923..fbbebe3 100644
--- a/doc/man5/notmuch-hooks.rst
+++ b/doc/man5/notmuch-hooks.rst
@@ -11,9 +11,10 @@ DESCRIPTION
 ===========
 
 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.
+notmuch invokes before and after certain actions. By default, these
+scripts reside in the .notmuch/hooks directory within the database
+directory, but this can be changed by setting the ``hooks.path``
+option. The hooks must have executable permissions.
 
 The currently available hooks are described below.
 
diff --git a/hooks.c b/hooks.c
index 7348d32..8bc7ca6 100644
--- a/hooks.c
+++ b/hooks.c
@@ -24,14 +24,13 @@
 #include <sys/wait.h>
 
 int
-notmuch_run_hook (const char *db_path, const char *hook)
+notmuch_run_hook (const char *hooks_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);
+    hook_path = talloc_asprintf (NULL, "%s/%s", hooks_path, hook);
     if (hook_path == NULL) {
 	fprintf (stderr, "Out of memory\n");
 	return 1;
diff --git a/notmuch-client.h b/notmuch-client.h
index ebc092b..2cbfc5b 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -274,6 +274,13 @@ notmuch_config_set_database_path (notmuch_config_t *config,
 				  const char *database_path);
 
 const char *
+notmuch_config_get_hooks_path (notmuch_config_t *config);
+
+void
+notmuch_config_set_hooks_path (notmuch_config_t *config,
+				  const char *hooks_path);
+
+const char *
 notmuch_config_get_crypto_gpg_path (notmuch_config_t *config);
 
 void
@@ -336,7 +343,7 @@ notmuch_config_set_search_exclude_tags (notmuch_config_t *config,
 				      size_t length);
 
 int
-notmuch_run_hook (const char *db_path, const char *hook);
+notmuch_run_hook (const char *hooks_path, const char *hook);
 
 notmuch_bool_t
 debugger_is_active (void);
diff --git a/notmuch-config.c b/notmuch-config.c
index e5d42a0..6b6833d 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -38,6 +38,14 @@ static const char database_config_comment[] =
     " Notmuch will store its database within a sub-directory of the path\n"
     " configured here named \".notmuch\".\n";
 
+static const char hooks_config_comment[] =
+    " Hook configuration\n"
+    "\n"
+    " The only value supported here is 'path' which should be the directory\n"
+    " where your hooks exist. If this value is empty, it will be interpreted\n"
+    " as the sub-directory \".notmuch/hooks\" of the path configured in the\n"
+    " \"database.path\" option.\n";
+
 static const char new_config_comment[] =
     " Configuration for \"notmuch new\"\n"
     "\n"
@@ -115,6 +123,7 @@ struct _notmuch_config {
     notmuch_bool_t is_new;
 
     char *database_path;
+    char *hooks_path;
     char *crypto_gpg_path;
     char *user_name;
     char *user_primary_email;
@@ -228,6 +237,8 @@ get_username_from_passwd_file (void *ctx)
  *
  *		database_path:		$MAILDIR, otherwise $HOME/mail
  *
+ *		hooks_path:		database_path/.notmuch/hooks
+ *
  *		user_name:		$NAME variable if set, otherwise
  *					read from /etc/passwd
  *
@@ -249,6 +260,7 @@ notmuch_config_open (void *ctx,
     size_t tmp;
     char *notmuch_config_env = NULL;
     int file_had_database_group;
+    int file_had_hooks_group;
     int file_had_new_group;
     int file_had_user_group;
     int file_had_maildir_group;
@@ -276,6 +288,7 @@ notmuch_config_open (void *ctx,
 
     config->is_new = FALSE;
     config->database_path = NULL;
+    config->hooks_path = NULL;
     config->user_name = NULL;
     config->user_primary_email = NULL;
     config->user_other_email = NULL;
@@ -333,6 +346,7 @@ notmuch_config_open (void *ctx,
      */
     file_had_database_group = g_key_file_has_group (config->key_file,
 						    "database");
+    file_had_hooks_group = g_key_file_has_group (config->key_file, "hooks");
     file_had_new_group = g_key_file_has_group (config->key_file, "new");
     file_had_user_group = g_key_file_has_group (config->key_file, "user");
     file_had_maildir_group = g_key_file_has_group (config->key_file, "maildir");
@@ -350,6 +364,15 @@ notmuch_config_open (void *ctx,
 	talloc_free (path);
     }
 
+    const char *hooks_path = notmuch_config_get_hooks_path (config);
+    if (hooks_path == NULL || strcmp(hooks_path, "") == 0) {
+	char *path = talloc_asprintf (config, "%s/.notmuch/hooks",
+				    notmuch_config_get_database_path (config));
+	config->hooks_path = talloc_strdup (config, path);
+	talloc_free (path);
+  g_key_file_set_string (config->key_file, "hooks", "path", "");
+    }
+
     if (notmuch_config_get_user_name (config) == NULL) {
 	char *name = getenv ("NAME");
 	if (name)
@@ -432,6 +455,10 @@ notmuch_config_open (void *ctx,
 	g_key_file_set_comment (config->key_file, "database", NULL,
 				database_config_comment, NULL);
 
+    if (! file_had_hooks_group)
+	g_key_file_set_comment (config->key_file, "hooks", NULL,
+				hooks_config_comment, NULL);
+
     if (! file_had_new_group)
 	g_key_file_set_comment (config->key_file, "new", NULL,
 				new_config_comment, NULL);
@@ -616,6 +643,19 @@ notmuch_config_set_database_path (notmuch_config_t *config,
 }
 
 const char *
+notmuch_config_get_hooks_path (notmuch_config_t *config)
+{
+    return _config_get (config, &config->hooks_path, "hooks", "path");
+}
+
+void
+notmuch_config_set_hooks_path (notmuch_config_t *config,
+				  const char *hooks_path)
+{
+    _config_set (config, &config->hooks_path, "hooks", "path", hooks_path);
+}
+
+const char *
 notmuch_config_get_user_name (notmuch_config_t *config)
 {
     return _config_get (config, &config->user_name, "user", "name");
@@ -779,6 +819,8 @@ notmuch_config_command_get (notmuch_config_t *config, char *item)
 {
     if (strcmp(item, "database.path") == 0) {
 	printf ("%s\n", notmuch_config_get_database_path (config));
+    } else if (strcmp(item, "hooks.path") == 0) {
+	printf ("%s\n", notmuch_config_get_hooks_path (config));
     } else if (strcmp(item, "user.name") == 0) {
 	printf ("%s\n", notmuch_config_get_user_name (config));
     } else if (strcmp(item, "user.primary_email") == 0) {
diff --git a/notmuch-insert.c b/notmuch-insert.c
index 131f09e..fe6d038 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -447,6 +447,7 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
     notmuch_database_t *notmuch;
     struct sigaction action;
     const char *db_path;
+    const char *hooks_path;
     const char **new_tags;
     size_t new_tags_length;
     tag_op_list_t *tag_ops;
@@ -477,6 +478,7 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
     notmuch_process_shared_options (argv[0]);
 
     db_path = notmuch_config_get_database_path (config);
+    hooks_path = notmuch_config_get_hooks_path (config);
     new_tags = notmuch_config_get_new_tags (config, &new_tags_length);
     synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config);
 
@@ -574,7 +576,7 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
 
     if (! no_hooks && status == NOTMUCH_STATUS_SUCCESS) {
 	/* Ignore hook failures. */
-	notmuch_run_hook (db_path, "post-insert");
+	notmuch_run_hook (hooks_path, "post-insert");
     }
 
     return status ? EXIT_FAILURE : EXIT_SUCCESS;
diff --git a/notmuch-new.c b/notmuch-new.c
index 799fec2..c71fd45 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -936,6 +936,7 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])
     int ret = 0;
     struct stat st;
     const char *db_path;
+    const char *hooks_path;
     char *dot_notmuch_path;
     struct sigaction action;
     _filename_node_t *f;
@@ -971,6 +972,7 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])
     add_files_state.new_ignore = notmuch_config_get_new_ignore (config, &add_files_state.new_ignore_length);
     add_files_state.synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config);
     db_path = notmuch_config_get_database_path (config);
+    hooks_path = notmuch_config_get_hooks_path (config);
 
     for (i = 0; i < add_files_state.new_tags_length; i++) {
 	const char *error_msg;
@@ -984,7 +986,7 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])
     }
 
     if (!no_hooks) {
-	ret = notmuch_run_hook (db_path, "pre-new");
+	ret = notmuch_run_hook (hooks_path, "pre-new");
 	if (ret)
 	    return EXIT_FAILURE;
     }
@@ -1149,7 +1151,7 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])
     notmuch_database_destroy (notmuch);
 
     if (!no_hooks && !ret && !interrupted)
-	ret = notmuch_run_hook (db_path, "post-new");
+	ret = notmuch_run_hook (hooks_path, "post-new");
 
     return ret || interrupted ? EXIT_FAILURE : EXIT_SUCCESS;
 }
-- 
2.9.3