From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by arlo.cworth.org (Postfix) with ESMTP id 02B316DE02AF for ; Mon, 29 Aug 2016 22:43:02 -0700 (PDT) X-Virus-Scanned: Debian amavisd-new at cworth.org X-Spam-Flag: NO X-Spam-Score: 0.555 X-Spam-Level: X-Spam-Status: No, score=0.555 tagged_above=-999 required=5 tests=[AWL=-0.097, SPF_NEUTRAL=0.652] autolearn=disabled Received: from arlo.cworth.org ([127.0.0.1]) by localhost (arlo.cworth.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 859te2W-jDX6 for ; Mon, 29 Aug 2016 22:43:00 -0700 (PDT) Received: from guru.guru-group.fi (guru.guru-group.fi [46.183.73.34]) by arlo.cworth.org (Postfix) with ESMTP id 352726DE0130 for ; Mon, 29 Aug 2016 22:42:59 -0700 (PDT) Received: from guru.guru-group.fi (localhost [IPv6:::1]) by guru.guru-group.fi (Postfix) with ESMTP id E530F100104; Tue, 30 Aug 2016 08:43:20 +0300 (EEST) From: Tomi Ollila To: Erik Rybakken Cc: Jani Nikula , notmuch@notmuchmail.org, David Bremner Subject: Re: [PATCH] Add option `hooks.path` for setting the directory of hooks. In-Reply-To: <20160827132543.xlqxwu5owhc5x546@dell> References: <20160824163006.dzdvzjvkg5uxtrnh@dell> <87vayqnjr3.fsf@nikula.org> <20160824215430.ljugc3vevx4ve2gq@dell> <20160827132543.xlqxwu5owhc5x546@dell> User-Agent: Notmuch/0.22+61~geeecb9e (https://notmuchmail.org) Emacs/24.5.1 (x86_64-unknown-linux-gnu) X-Face: HhBM'cA~ MIME-Version: 1.0 Content-Type: text/plain X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 30 Aug 2016 05:43:02 -0000 On Sat, Aug 27 2016, Erik Rybakken 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 > 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 > > 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