* Feature request: Finer control of paths @ 2016-08-24 16:30 Erik Rybakken 2016-08-24 17:45 ` Jani Nikula 0 siblings, 1 reply; 11+ messages in thread From: Erik Rybakken @ 2016-08-24 16:30 UTC (permalink / raw) To: notmuch 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: Feature request: Finer control of paths 2016-08-24 16:30 Feature request: Finer control of paths Erik Rybakken @ 2016-08-24 17:45 ` Jani Nikula 2016-08-24 21:55 ` [PATCH] Add option `hooks.path` for setting the directory of hooks Erik Rybakken 0 siblings, 1 reply; 11+ messages in thread From: Jani Nikula @ 2016-08-24 17:45 UTC (permalink / raw) To: Erik Rybakken, notmuch 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. ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] Add option `hooks.path` for setting the directory of hooks. 2016-08-24 17:45 ` Jani Nikula @ 2016-08-24 21:55 ` Erik Rybakken 2016-08-26 10:57 ` David Bremner 2016-08-26 11:32 ` Tomi Ollila 0 siblings, 2 replies; 11+ messages in thread From: Erik Rybakken @ 2016-08-24 21:55 UTC (permalink / raw) To: Jani Nikula, notmuch 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] Add option `hooks.path` for setting the directory of hooks. 2016-08-24 21:55 ` [PATCH] Add option `hooks.path` for setting the directory of hooks Erik Rybakken @ 2016-08-26 10:57 ` David Bremner 2016-08-26 11:32 ` Tomi Ollila 1 sibling, 0 replies; 11+ messages in thread From: David Bremner @ 2016-08-26 10:57 UTC (permalink / raw) To: Erik Rybakken, Jani Nikula, notmuch 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Add option `hooks.path` for setting the directory of hooks. 2016-08-24 21:55 ` [PATCH] Add option `hooks.path` for setting the directory of hooks Erik Rybakken 2016-08-26 10:57 ` David Bremner @ 2016-08-26 11:32 ` Tomi Ollila 2016-08-27 13:28 ` Erik Rybakken 1 sibling, 1 reply; 11+ messages in thread From: Tomi Ollila @ 2016-08-26 11:32 UTC (permalink / raw) To: Erik Rybakken, Jani Nikula, notmuch 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Add option `hooks.path` for setting the directory of hooks. 2016-08-26 11:32 ` Tomi Ollila @ 2016-08-27 13:28 ` Erik Rybakken 2016-08-30 5:43 ` Tomi Ollila 0 siblings, 1 reply; 11+ messages in thread From: Erik Rybakken @ 2016-08-27 13:28 UTC (permalink / raw) To: Tomi Ollila; +Cc: Jani Nikula, notmuch, David Bremner 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 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] Add option `hooks.path` for setting the directory of hooks. 2016-08-27 13:28 ` Erik Rybakken @ 2016-08-30 5:43 ` Tomi Ollila 2016-08-30 8:49 ` Erik Rybakken 2016-08-30 13:35 ` Tomi Ollila 0 siblings, 2 replies; 11+ messages in thread From: Tomi Ollila @ 2016-08-30 5:43 UTC (permalink / raw) To: Erik Rybakken; +Cc: Jani Nikula, notmuch, David Bremner 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Add option `hooks.path` for setting the directory of hooks. 2016-08-30 5:43 ` Tomi Ollila @ 2016-08-30 8:49 ` Erik Rybakken 2016-08-30 11:43 ` David Bremner 2016-08-30 13:35 ` Tomi Ollila 1 sibling, 1 reply; 11+ messages in thread From: Erik Rybakken @ 2016-08-30 8:49 UTC (permalink / raw) To: Tomi Ollila; +Cc: Jani Nikula, notmuch, David Bremner 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Add option `hooks.path` for setting the directory of hooks. 2016-08-30 8:49 ` Erik Rybakken @ 2016-08-30 11:43 ` David Bremner 0 siblings, 0 replies; 11+ messages in thread From: David Bremner @ 2016-08-30 11:43 UTC (permalink / raw) To: Erik Rybakken, Tomi Ollila; +Cc: Jani Nikula, notmuch 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Add option `hooks.path` for setting the directory of hooks. 2016-08-30 5:43 ` Tomi Ollila 2016-08-30 8:49 ` Erik Rybakken @ 2016-08-30 13:35 ` Tomi Ollila 2016-08-30 17:51 ` Tomi Ollila 1 sibling, 1 reply; 11+ messages in thread From: Tomi Ollila @ 2016-08-30 13:35 UTC (permalink / raw) To: Erik Rybakken; +Cc: Jani Nikula, notmuch, David Bremner 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... ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Add option `hooks.path` for setting the directory of hooks. 2016-08-30 13:35 ` Tomi Ollila @ 2016-08-30 17:51 ` Tomi Ollila 0 siblings, 0 replies; 11+ messages in thread From: Tomi Ollila @ 2016-08-30 17:51 UTC (permalink / raw) To: Erik Rybakken; +Cc: Jani Nikula, notmuch, David Bremner 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... ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2016-08-30 17:51 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-08-24 16:30 Feature request: Finer control of paths Erik Rybakken 2016-08-24 17:45 ` Jani Nikula 2016-08-24 21:55 ` [PATCH] Add option `hooks.path` for setting the directory of hooks Erik Rybakken 2016-08-26 10:57 ` David Bremner 2016-08-26 11:32 ` Tomi Ollila 2016-08-27 13:28 ` Erik Rybakken 2016-08-30 5:43 ` Tomi Ollila 2016-08-30 8:49 ` Erik Rybakken 2016-08-30 11:43 ` David Bremner 2016-08-30 13:35 ` Tomi Ollila 2016-08-30 17:51 ` Tomi Ollila
Code repositories for project(s) associated with this public inbox https://yhetil.org/notmuch.git/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).