I like it. See below for some nits. Quoth Jani Nikula on Dec 04 at 1:16 am: > Add mechanism for running user defined hooks. Hooks are executables or > symlinks to executables stored under the new notmuch hooks directory, > <database-path>/.notmuch/hooks. > > No hooks are introduced here, but adding support for a hook is now a simple > matter of calling the new notmuch_run_hook() function at an appropriate > location with the hook name. > > Signed-off-by: Jani Nikula <jani@nikula.org> > > --- > > v2: Switch to git style hooks in a hook directory as suggested by Austin > Clements in IRC. Update manpage and add polish. > --- > Makefile.local | 1 + > notmuch-client.h | 3 ++ > notmuch-hook.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 67 insertions(+), 0 deletions(-) > create mode 100644 notmuch-hook.c > > diff --git a/Makefile.local b/Makefile.local > index c94402b..a1665e1 100644 > --- a/Makefile.local > +++ b/Makefile.local > @@ -302,6 +302,7 @@ notmuch_client_srcs = \ > notmuch-config.c \ > notmuch-count.c \ > notmuch-dump.c \ > + notmuch-hook.c \ > notmuch-new.c \ > notmuch-reply.c \ > notmuch-restore.c \ > diff --git a/notmuch-client.h b/notmuch-client.h > index b50cb38..a91ad6c 100644 > --- a/notmuch-client.h > +++ b/notmuch-client.h > @@ -235,6 +235,9 @@ void > notmuch_config_set_maildir_synchronize_flags (notmuch_config_t *config, > notmuch_bool_t synchronize_flags); > > +int > +notmuch_run_hook (const char *db_path, const char *hook); > + > notmuch_bool_t > debugger_is_active (void); > > diff --git a/notmuch-hook.c b/notmuch-hook.c > new file mode 100644 > index 0000000..fc32044 > --- /dev/null > +++ b/notmuch-hook.c > @@ -0,0 +1,63 @@ > +/* notmuch - Not much of an email program, (just index and search) > + * > + * This file is part of notmuch. > + * > + * Copyright © 2011 Jani Nikula > + * > + * This program is free software: you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation, either version 3 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program. If not, see http://www.gnu.org/licenses/ . > + * > + * Author: Jani Nikula <jani@nikula.org> > + */ > + > +#include "notmuch-client.h" > + > +int > +notmuch_run_hook (const char *db_path, const char *hook) > +{ > + char *hook_path; > + int status = 0; You use status as both a notmuch_status_t and for generic C library results. This seems a little weird. You may or may not want to do anything about it. > + > + if (asprintf (&hook_path, "%s/%s/%s/%s", db_path, ".notmuch", "hooks", > + hook) == -1) asprintf isn't very portable. Perhaps talloc_asprintf would be better? (And more idiomatic.) > + return NOTMUCH_STATUS_OUT_OF_MEMORY; > + > + if (access (hook_path, X_OK) == -1) { > + /* Ignore ENOENT. It's okay not to have a hook, hook dir, or even > + * notmuch dir. Dangling symbolic links also result in ENOENT, but > + * we'll ignore that too for simplicity. */ > + if (errno != ENOENT) { > + fprintf (stderr, "Error: %s hook access failed: %s\n", hook, > + strerror (errno)); > + status = NOTMUCH_STATUS_FILE_ERROR; > + } > + goto DONE; > + } > + > + status = system (hook_path); It would probably be better to fork/execl. system is sensitive to all sorts of weird things because it invokes the shell (e.g., spaces or funny characters in db_path) and it plays funny games with signals. Really proper error handling with fork/exec is a pain, but I think you can get away with something simpler and even get rid of the access call in the process. Something like ret = fork(); if (ret < 0) ... else if (ret == 0) { ret = execl(hook_path, hook_path, NULL); if (ret != ENOENT && ret != EACCESS) print a real error message exit(0); } else { waitpid(ret, &status, 0); if (status) .. checks you do now .. } I guess this wastes a fork if there is no hook script, so maybe the access call is worth doing anyway. > + if (status) { > + if (WIFSIGNALED(status)) > + fprintf(stderr, "Error: %s hook terminated with signal %d\n", hook, > + WTERMSIG(status)); > + else > + fprintf(stderr, "Error: %s hook failed with status %d\n", hook, > + WEXITSTATUS(status)); > + > + status = NOTMUCH_STATUS_FILE_ERROR; /* Close enough */ > + } > + > + DONE: > + free (hook_path); > + > + return status; > +}