Feature request: Finer control of paths

Feature request: Finer control of paths

[Erik Rybakken]

Hi everybody,

I would like to store hooks in a different directory from my mail.
It would be nice to have an option "hooks.path" with default being
"$DATABASEDIR/.notmuch/hooks/". Also, I think a similar option for
the xabian database would make sense.

Best,
Erik

Re: Feature request: Finer control of paths

[Jani Nikula]

On Wed, 24 Aug 2016, Erik Rybakken <erik.rybakken@math.ntnu.no> wrote:
> I would like to store hooks in a different directory from my mail.
> It would be nice to have an option "hooks.path" with default being
> "$DATABASEDIR/.notmuch/hooks/". Also, I think a similar option for
> the xabian database would make sense.

I think we agree it would be great to be able to configure the location
of .notmuch. Unfortunately, the notmuch library currently assumes it can
deduce the database location from the mail store location. The last I
checked, making it configurable was a bit complicated, and so far nobody
has thought it would be worth it. Making the hooks directory
configurable would be significantly simpler.

For the time being, you can work around the limitations by symlinking
.notmuch or .notmuch/hooks to your preferred location.

BR,
Jani.

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

[Erik Rybakken]

Hi again,

I implemented the option for hooks myself. The patch is included. Please
bear with me, this is my first contribution to notmuch (and my first
attempt to write C code). I tested the option, and it seems to work.

Best,
Erik

On Wed, Aug 24, 2016 at 08:45:36PM +0300, Jani Nikula wrote:
> On Wed, 24 Aug 2016, Erik Rybakken <erik.rybakken@math.ntnu.no> wrote:
> > I would like to store hooks in a different directory from my mail.
> > It would be nice to have an option "hooks.path" with default being
> > "$DATABASEDIR/.notmuch/hooks/". Also, I think a similar option for
> > the xabian database would make sense.
> 
> I think we agree it would be great to be able to configure the location
> of .notmuch. Unfortunately, the notmuch library currently assumes it can
> deduce the database location from the mail store location. The last I
> checked, making it configurable was a bit complicated, and so far nobody
> has thought it would be worth it. Making the hooks directory
> configurable would be significantly simpler.
> 
> For the time being, you can work around the limitations by symlinking
> .notmuch or .notmuch/hooks to your preferred location.
> 
> BR,
> Jani.

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

diff --git a/NEWS b/NEWS
index 3a9c8d3..b200bbf 100644
--- a/NEWS
+++ b/NEWS
@@ -1,6 +1,12 @@
 Notmuch 0.23 (UNRELEASED)
 =========================
 
+General
+-------
+
+Add option `hooks.path` for setting the directory for hooks. If
+unset, 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..b1e266c 100644
--- a/doc/man1/notmuch-config.rst
+++ b/doc/man1/notmuch-config.rst
@@ -51,6 +51,11 @@ The available configuration items are described below.
 
         Default: ``$MAILDIR`` variable if set, otherwise ``$HOME/mail``.
 
+    **hooks.path**
+        The directory where your hooks exists.
+
+        Default: ``database.path/.notmuch/hooks``.
+
     **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..7b5ac8b 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -38,6 +38,12 @@ 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 exists.\n";
+
 static const char new_config_comment[] =
     " Configuration for \"notmuch new\"\n"
     "\n"
@@ -115,6 +121,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 +235,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 +258,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 +286,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 +344,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 +362,13 @@ notmuch_config_open (void *ctx,
 	talloc_free (path);
     }
 
+    if (notmuch_config_get_hooks_path (config) == NULL) {
+	char *path = talloc_asprintf (config, "%s/.notmuch/hooks",
+				    notmuch_config_get_database_path (config));
+	notmuch_config_set_hooks_path (config, path);
+	talloc_free (path);
+    }
+
     if (notmuch_config_get_user_name (config) == NULL) {
 	char *name = getenv ("NAME");
 	if (name)
@@ -432,6 +451,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 +639,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 +815,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.2

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

[David Bremner]

Erik Rybakken <erik.rybakken@math.ntnu.no> writes:

> Hi again,
>
> I implemented the option for hooks myself. The patch is included. Please
> bear with me, this is my first contribution to notmuch (and my first
> attempt to write C code). I tested the option, and it seems to work.
>
> Best,
> Erik

Welcome. You may find https://notmuchmail.org/contributing/ has some
helpful hints, if you didn't already see it.  In particular, your patch
needs an "meaningful commit message". You seem to have mostly managed to
follow the notmuch coding style, so thanks for that.

> +General
> +-------
> +
> +Add option `hooks.path` for setting the directory for hooks. If
> +unset, it will default to the `.notmuch/hooks` sub-directory.
> +

As a minor point, this should go under "Command Line Interface", since
hooks are strictly a CLI feature.

> +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 exists.\n";
> +
"hooks exist."

No real objections to the code or the feature, but you'll need to update
the test suite. In particular T030-config and T040-setup are currently
broken by your patch (but easy to fix).  You should also add at least
test to T400-hooks.sh to ensure running hooks from a non-standard
location works. Feel free to ask for help with the test suite, if
copying existing tests isn't enough.

d

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

[Tomi Ollila]

On Thu, Aug 25 2016, Erik Rybakken <erik.rybakken@math.ntnu.no> wrote:

> Hi again,
>
> I implemented the option for hooks myself. The patch is included. Please
> bear with me, this is my first contribution to notmuch (and my first
> attempt to write C code). I tested the option, and it seems to work.

The implementation looks pretty good...

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

If this new option is added, and as (IIRC) our configuration system writes
all missing options to the configuration file we may have this
(hypothetical for me, at least (*)) "coupling" problem.


database.path points to some directory

hooks.path points to 'hooks/' subdirectory in database.path

this directory we're talking about is moved elsewhere

now just editing database.path in configuration gives user
a broken system where current hooks cannot be used.

If our configuration syste did not write nonexistent entries we could just
have [hooks]\n#path outcommented in the configuration by default. To
change this is just Simple Matter of Programming (SMOP), or course.

(*) I do not use hooks, nor I plan to move my directory (which means that
both of these are going to change some day in the future ;))


I hope someone(™) can counterargument this email...

Tomi


>
> Best,
> Erik
>
> On Wed, Aug 24, 2016 at 08:45:36PM +0300, Jani Nikula wrote:
>> On Wed, 24 Aug 2016, Erik Rybakken <erik.rybakken@math.ntnu.no> wrote:
>> > I would like to store hooks in a different directory from my mail.
>> > It would be nice to have an option "hooks.path" with default being
>> > "$DATABASEDIR/.notmuch/hooks/". Also, I think a similar option for
>> > the xabian database would make sense.
>> 
>> I think we agree it would be great to be able to configure the location
>> of .notmuch. Unfortunately, the notmuch library currently assumes it can
>> deduce the database location from the mail store location. The last I
>> checked, making it configurable was a bit complicated, and so far nobody
>> has thought it would be worth it. Making the hooks directory
>> configurable would be significantly simpler.
>> 
>> For the time being, you can work around the limitations by symlinking
>> .notmuch or .notmuch/hooks to your preferred location.
>> 
>> BR,
>> Jani.
>
> ---
>  NEWS                        |  6 ++++++
>  doc/man1/notmuch-config.rst |  5 +++++
>  doc/man5/notmuch-hooks.rst  |  7 ++++---
>  hooks.c                     |  5 ++---
>  notmuch-client.h            |  9 ++++++++-
>  notmuch-config.c            | 38 ++++++++++++++++++++++++++++++++++++++
>  notmuch-insert.c            |  4 +++-
>  notmuch-new.c               |  6 ++++--
>  8 files changed, 70 insertions(+), 10 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 3a9c8d3..b200bbf 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -1,6 +1,12 @@
>  Notmuch 0.23 (UNRELEASED)
>  =========================
>  
> +General
> +-------
> +
> +Add option `hooks.path` for setting the directory for hooks. If
> +unset, 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..b1e266c 100644
> --- a/doc/man1/notmuch-config.rst
> +++ b/doc/man1/notmuch-config.rst
> @@ -51,6 +51,11 @@ The available configuration items are described below.
>  
>          Default: ``$MAILDIR`` variable if set, otherwise ``$HOME/mail``.
>  
> +    **hooks.path**
> +        The directory where your hooks exists.
> +
> +        Default: ``database.path/.notmuch/hooks``.
> +
>      **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..7b5ac8b 100644
> --- a/notmuch-config.c
> +++ b/notmuch-config.c
> @@ -38,6 +38,12 @@ 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 exists.\n";
> +
>  static const char new_config_comment[] =
>      " Configuration for \"notmuch new\"\n"
>      "\n"
> @@ -115,6 +121,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 +235,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 +258,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 +286,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 +344,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 +362,13 @@ notmuch_config_open (void *ctx,
>  	talloc_free (path);
>      }
>  
> +    if (notmuch_config_get_hooks_path (config) == NULL) {
> +	char *path = talloc_asprintf (config, "%s/.notmuch/hooks",
> +				    notmuch_config_get_database_path (config));
> +	notmuch_config_set_hooks_path (config, path);
> +	talloc_free (path);
> +    }
> +
>      if (notmuch_config_get_user_name (config) == NULL) {
>  	char *name = getenv ("NAME");
>  	if (name)
> @@ -432,6 +451,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 +639,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 +815,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.2
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch

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

[Erik Rybakken]

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

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

[Tomi Ollila]

On Sat, Aug 27 2016, Erik Rybakken <erik.rybakken@math.ntnu.no> wrote:

> 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.

I kinda like how this would work...

The code looked pretty good -- when did I git am to the email content
I got all from the beginning of this email to the commit message --
so before next patches use git-format-patch and git-am... Check

https://notmuchmail.org/contributing/ for more information...

2 things that came up after quick view

1) there is one indentation mismatch ;/

2) I wonder whether calling notmuch_config_get_hooks_path() could
be "lazier".... ARGH no :( -- it would make notmuch_config_get_hooks_path()
have different code than others and ... (**)

3) I don't see any calls to notmuch_config_set_hooks_path()

Tomi


(**) pain tolerance exceeded in a hurry (already 20 minutes late)...

>
> (*) 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

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

[Erik Rybakken]

On Tue, Aug 30, 2016 at 08:43:20AM +0300, Tomi Ollila wrote:
> The code looked pretty good -- when did I git am to the email content
> I got all from the beginning of this email to the commit message --
> so before next patches use git-format-patch and git-am... Check
> 
> https://notmuchmail.org/contributing/ for more information...

Sorry about that. I will start checking my email patches.

> 2 things that came up after quick view
> 
> 1) there is one indentation mismatch ;/

I'm not sure what you refer to here. Could you point me to it?

> 3) I don't see any calls to notmuch_config_set_hooks_path()

Yes, that's true. The function is not needed now, but I kept it for
completeness.

So, there are only two places in the code where there are calls to
notmuch_config_set_*. One of them are in the procedure
"notmuch_config_open" in notmuch-config.c where the config file is read.
For hooks_path the reading is done differently as explained in the last
email.

The other place is in notmuch-setup.c, but the notmuch setup does not
(for now) set hooks.path.

The command "notmuch config set" does not use notmuch_config_set for
some reason, but changes the key file directly.

- Erik

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

[David Bremner]

Erik Rybakken <erik.rybakken@math.ntnu.no> writes:

>> 
>> 1) there is one indentation mismatch ;/
>
> I'm not sure what you refer to here. Could you point me to it?

I'm not sure either, but a good way to check is

% uncrustify [--replace] --config devel/uncrustify.cfg $file

Then git diff will tell you what uncrustify found. There are some false
positives, usually existing code that's not your fault.

d

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

[Tomi Ollila]

On Tue, Aug 30 2016, Tomi Ollila <tomi.ollila@iki.fi> wrote:

> On Sat, Aug 27 2016, Erik Rybakken <erik.rybakken@math.ntnu.no> wrote:
>
>> 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.
>
> I kinda like how this would work...
>
> The code looked pretty good -- when did I git am to the email content
> I got all from the beginning of this email to the commit message --
> so before next patches use git-format-patch and git-am... Check
>
> https://notmuchmail.org/contributing/ for more information...
>
> 2 things that came up after quick view
>
> 2) I wonder whether calling notmuch_config_get_hooks_path() could
> be "lazier".... ARGH no :( -- it would make notmuch_config_get_hooks_path()
> have different code than others and ... (**)

That said, perhaps 

const char *
notmuch_config_get_hooks_path (notmuch_config_t *config)
{
     const char * hooks_path =
         _config_get (config, &config->hooks_path, "hooks", "path");
     if (hooks_path == NULL || hooks_path[0] == '\0') {
        hooks_path = talloc_asprintf (config, "%s/.notmuch/hooks",
                                      notmuch_config_get_database_path (config));
        _config_set(config, &config->hooks_path, "hooks", "path", hooks_path);
     }
     return hooks_path;
}

But, it takes quite a bit of careful examination to check whether that works
as expected, and I always think whether some accidental fragileness there
causes that stored value to be dumped to the configuration file (now, or
in later changes).

But, the above may be useless crap -- just I don't have more time to check
that out now...

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

[Tomi Ollila]

On Tue, Aug 30 2016, Tomi Ollila <tomi.ollila@iki.fi> wrote:

> On Tue, Aug 30 2016, Tomi Ollila <tomi.ollila@iki.fi> wrote:
>
>> On Sat, Aug 27 2016, Erik Rybakken <erik.rybakken@math.ntnu.no> wrote:
>>
>>> 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.
>>
>> I kinda like how this would work...
>>
>> The code looked pretty good -- when did I git am to the email content
>> I got all from the beginning of this email to the commit message --
>> so before next patches use git-format-patch and git-am... Check
>>
>> https://notmuchmail.org/contributing/ for more information...
>>
>> 2 things that came up after quick view
>>
>> 2) I wonder whether calling notmuch_config_get_hooks_path() could
>> be "lazier".... ARGH no :( -- it would make notmuch_config_get_hooks_path()
>> have different code than others and ... (**)
>
> That said, perhaps 
>
> const char *
> notmuch_config_get_hooks_path (notmuch_config_t *config)
> {
>      const char * hooks_path =
>          _config_get (config, &config->hooks_path, "hooks", "path");
>      if (hooks_path == NULL || hooks_path[0] == '\0') {
>         hooks_path = talloc_asprintf (config, "%s/.notmuch/hooks",
>                                       notmuch_config_get_database_path (config));
>         _config_set(config, &config->hooks_path, "hooks", "path", hooks_path);
>      }
>      return hooks_path;
> }
>
> But, it takes quite a bit of careful examination to check whether that works
> as expected, and I always think whether some accidental fragileness there
> causes that stored value to be dumped to the configuration file (now, or
> in later changes).

I forgot that this risk can be mitigated by a set of suitable tests...

>
> But, the above may be useless crap -- just I don't have more time to check
> that out now...