From: Tomi Ollila <tomi.ollila@iki.fi>
To: David Bremner <david@tethera.net>, notmuch@notmuchmail.org
Cc: David Bremner <david@tethera.net>
Subject: Re: [PATCH 37/38] CLI: use configured hook directory
Date: Sun, 24 Jan 2021 14:59:38 +0200 [thread overview]
Message-ID: <m2wnw2jxs5.fsf@guru.guru-group.fi> (raw)
In-Reply-To: <20210116170406.842014-38-david@tethera.net>
On Sat, Jan 16 2021, David Bremner wrote:
> This enables support for hooks outside the database directory.
> It relies strongly on configuration information being usable between
> closing the database and destroying it.
> ---
> hooks.c | 7 +-
> notmuch-client.h | 2 +-
> notmuch-insert.c | 7 +-
> notmuch-new.c | 4 +-
> test/T400-hooks.sh | 197 +++++++++++++++++++++++++--------------------
> 5 files changed, 120 insertions(+), 97 deletions(-)
>
> diff --git a/hooks.c b/hooks.c
> index 59c58070..ec89b22e 100644
> --- a/hooks.c
> +++ b/hooks.c
> @@ -24,14 +24,15 @@
> #include <sys/wait.h>
>
> int
> -notmuch_run_hook (const char *db_path, const char *hook)
> +notmuch_run_hook (notmuch_database_t *notmuch, 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 (notmuch, "%s/%s",
> + notmuch_config_get (notmuch, NOTMUCH_CONFIG_HOOK_DIR),
> + hook);
> if (hook_path == NULL) {
> fprintf (stderr, "Out of memory\n");
> return 1;
> diff --git a/notmuch-client.h b/notmuch-client.h
> index 9e09c36a..f60f5406 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -339,7 +339,7 @@ const char *
> _notmuch_config_get_path (notmuch_config_t *config);
>
> int
> -notmuch_run_hook (const char *db_path, const char *hook);
> +notmuch_run_hook (notmuch_database_t *notmuch, const char *hook);
>
> bool
> debugger_is_active (void);
> diff --git a/notmuch-insert.c b/notmuch-insert.c
> index e483b949..0f272e2e 100644
> --- a/notmuch-insert.c
> +++ b/notmuch-insert.c
> @@ -481,7 +481,6 @@ notmuch_insert_command (unused(notmuch_config_t *config),notmuch_database_t *not
> notmuch_process_shared_options (argv[0]);
>
>
> - /* XXX TODO replace this use of DATABASE_PATH with something specific to hooks */
> db_path = notmuch_config_get (notmuch, NOTMUCH_CONFIG_DATABASE_PATH);
>
> if (! db_path)
> @@ -570,7 +569,7 @@ notmuch_insert_command (unused(notmuch_config_t *config),notmuch_database_t *not
> status = add_file (notmuch, newpath, tag_ops, synchronize_flags, keep, indexing_cli_choices.opts);
>
> /* Commit changes. */
> - close_status = notmuch_database_destroy (notmuch);
> + close_status = notmuch_database_close (notmuch);
> if (close_status) {
> /* Hold on to the first error, if any. */
> if (! status)
> @@ -595,9 +594,11 @@ notmuch_insert_command (unused(notmuch_config_t *config),notmuch_database_t *not
>
> if (hooks && status == NOTMUCH_STATUS_SUCCESS) {
> /* Ignore hook failures. */
> - notmuch_run_hook (db_path, "post-insert");
> + notmuch_run_hook (notmuch, "post-insert");
> }
>
> + notmuch_database_destroy (notmuch);
> +
> talloc_free (local);
>
> return status_to_exit (status);
> diff --git a/notmuch-new.c b/notmuch-new.c
> index 0f416939..2fc34e2c 100644
> --- a/notmuch-new.c
> +++ b/notmuch-new.c
> @@ -1167,7 +1167,7 @@ notmuch_new_command (unused(notmuch_config_t *config), notmuch_database_t *notmu
> }
>
> if (hooks) {
> - ret = notmuch_run_hook (db_path, "pre-new");
> + ret = notmuch_run_hook (notmuch, "pre-new");
> if (ret)
> return EXIT_FAILURE;
> }
> @@ -1284,7 +1284,7 @@ notmuch_new_command (unused(notmuch_config_t *config), notmuch_database_t *notmu
> notmuch_database_close (notmuch);
>
> if (hooks && ! ret && ! interrupted)
> - ret = notmuch_run_hook (db_path, "post-new");
> + ret = notmuch_run_hook (notmuch, "post-new");
>
> notmuch_database_destroy (notmuch);
>
> diff --git a/test/T400-hooks.sh b/test/T400-hooks.sh
> index 49c690eb..b9894853 100755
> --- a/test/T400-hooks.sh
> +++ b/test/T400-hooks.sh
> @@ -2,8 +2,6 @@
> test_description='hooks'
> . $(dirname "$0")/test-lib.sh || exit 1
>
> -HOOK_DIR=${MAIL_DIR}/.notmuch/hooks
> -
> create_echo_hook () {
> local TOKEN="${RANDOM}"
> mkdir -p ${HOOK_DIR}
> @@ -16,6 +14,7 @@ EOF
> }
>
> create_failing_hook () {
> + local HOOK_DIR=${2}
> mkdir -p ${HOOK_DIR}
> cat <<EOF >"${HOOK_DIR}/${1}"
> #!/bin/sh
> @@ -24,98 +23,120 @@ EOF
> chmod +x "${HOOK_DIR}/${1}"
> }
>
> -rm_hooks () {
> - rm -rf ${HOOK_DIR}
> -}
> -
> # add a message to generate mail dir and database
> add_message
> # create maildir structure for notmuch-insert
> mkdir -p "$MAIL_DIR"/{cur,new,tmp}
>
> -test_begin_subtest "pre-new is run"
> -rm_hooks
> -generate_message
> -create_echo_hook "pre-new" expected output
> -notmuch new > /dev/null
> -test_expect_equal_file expected output
> -
> -test_begin_subtest "post-new is run"
> -rm_hooks
> -generate_message
> -create_echo_hook "post-new" expected output
> -notmuch new > /dev/null
> -test_expect_equal_file expected output
> -
> -test_begin_subtest "post-insert hook is run"
> -rm_hooks
> -generate_message
> -create_echo_hook "post-insert" expected output
> -notmuch insert < "$gen_msg_filename"
> -test_expect_equal_file expected output
> -
> -test_begin_subtest "pre-new is run before post-new"
> -rm_hooks
> -generate_message
> -create_echo_hook "pre-new" pre-new.expected pre-new.output
> -create_echo_hook "post-new" post-new.expected post-new.output
> -notmuch new > /dev/null
> -test_expect_equal_file post-new.expected post-new.output
> -
> -test_begin_subtest "pre-new non-zero exit status (hook status)"
> -rm_hooks
> -generate_message
> -create_failing_hook "pre-new"
> -output=`notmuch new 2>&1`
> -test_expect_equal "$output" "Error: pre-new hook failed with status 13"
> -
> -# depends on the previous subtest leaving broken hook behind
> -test_begin_subtest "pre-new non-zero exit status (notmuch status)"
> -test_expect_code 1 "notmuch new"
> -
> -# depends on the previous subtests leaving 1 new message behind
> -test_begin_subtest "pre-new non-zero exit status aborts new"
> -rm_hooks
> -output=$(NOTMUCH_NEW)
> -test_expect_equal "$output" "Added 1 new message to the database."
> -
> -test_begin_subtest "post-new non-zero exit status (hook status)"
> -rm_hooks
> -generate_message
> -create_failing_hook "post-new"
> -NOTMUCH_NEW 2>output.stderr >output
> -cat output.stderr >> output
> -echo "Added 1 new message to the database." > expected
> -echo "Error: post-new hook failed with status 13" >> expected
> -test_expect_equal_file expected output
> -
> -# depends on the previous subtest leaving broken hook behind
> -test_begin_subtest "post-new non-zero exit status (notmuch status)"
> -test_expect_code 1 "notmuch new"
> -
> -test_begin_subtest "post-insert hook does not affect insert status"
> -rm_hooks
> -generate_message
> -create_failing_hook "post-insert"
> -test_expect_success "notmuch insert < \"$gen_msg_filename\" > /dev/null"
> -
> -test_begin_subtest "hook without executable permissions"
> -rm_hooks
> -mkdir -p ${HOOK_DIR}
> -cat <<EOF >"${HOOK_DIR}/pre-new"
> -#!/bin/sh
> -echo foo
> +for config in traditional profile explicit XDG; do
> + unset NOTMUCH_PROFILE
> + notmuch config set database.hook_dir
> + case $config in
> + traditional)
> + HOOK_DIR=${MAIL_DIR}/.notmuch/hooks
> + ;;
> + profile)
> + dir=${HOME}/.config/notmuch/other
> + mkdir -p ${dir}
> + HOOK_DIR=${dir}/hooks
> + cp ${NOTMUCH_CONFIG} ${dir}/config
> + export NOTMUCH_PROFILE=other
> + ;;
> + explicit)
> + HOOK_DIR=${HOME}/.notmuch-hooks
> + mkdir -p $HOOK_DIR
> + notmuch config set database.hook_dir $HOOK_DIR
> + ;;
> + *)
This '*' would better be 'XDG', to match the loop value.
As this is not user input, '*' (with error message)
can be left out.
This concludes this review; I left out some comments about
long lines as those were consisten w/ each other, and
there were inconsistensiens in source files there. Also
there were one '{' at the end of a function definition,
but that file had one other file like that so...
Like I said before I did not see anything that could break
things (I suppose current configs work (for now)), but there
are so many changes it is hard to figure out everything...
Tomi
> + HOOK_DIR=${HOME}/.config/notmuch/default/hooks
> + ;;
> + esac
> +
> + test_begin_subtest "pre-new is run [${config}]"
> + rm -rf ${HOOK_DIR}
> + generate_message
> + create_echo_hook "pre-new" expected output $HOOK_DIR
> + notmuch new > /dev/null
> + test_expect_equal_file expected output
> +
> + test_begin_subtest "post-new is run [${config}]"
> + rm -rf ${HOOK_DIR}
> + generate_message
> + create_echo_hook "post-new" expected output $HOOK_DIR
> + notmuch new > /dev/null
> + test_expect_equal_file expected output
> +
> + test_begin_subtest "post-insert hook is run [${config}]"
> + rm -rf ${HOOK_DIR}
> + generate_message
> + create_echo_hook "post-insert" expected output $HOOK_DIR
> + notmuch insert < "$gen_msg_filename"
> + test_expect_equal_file expected output
> +
> + test_begin_subtest "pre-new is run before post-new [${config}]"
> + rm -rf ${HOOK_DIR}
> + generate_message
> + create_echo_hook "pre-new" pre-new.expected pre-new.output $HOOK_DIR
> + create_echo_hook "post-new" post-new.expected post-new.output $HOOK_DIR
> + notmuch new > /dev/null
> + test_expect_equal_file post-new.expected post-new.output
> +
> + test_begin_subtest "pre-new non-zero exit status (hook status) [${config}]"
> + rm -rf ${HOOK_DIR}
> + generate_message
> + create_failing_hook "pre-new" $HOOK_DIR
> + output=`notmuch new 2>&1`
> + test_expect_equal "$output" "Error: pre-new hook failed with status 13"
> +
> + # depends on the previous subtest leaving broken hook behind
> + test_begin_subtest "pre-new non-zero exit status (notmuch status) [${config}]"
> + test_expect_code 1 "notmuch new"
> +
> + # depends on the previous subtests leaving 1 new message behind
> + test_begin_subtest "pre-new non-zero exit status aborts new [${config}]"
> + rm -rf ${HOOK_DIR}
> + output=$(NOTMUCH_NEW)
> + test_expect_equal "$output" "Added 1 new message to the database."
> +
> + test_begin_subtest "post-new non-zero exit status (hook status) [${config}]"
> + rm -rf ${HOOK_DIR}
> + generate_message
> + create_failing_hook "post-new" $HOOK_DIR
> + NOTMUCH_NEW 2>output.stderr >output
> + cat output.stderr >> output
> + echo "Added 1 new message to the database." > expected
> + echo "Error: post-new hook failed with status 13" >> expected
> + test_expect_equal_file expected output
> +
> + # depends on the previous subtest leaving broken hook behind
> + test_begin_subtest "post-new non-zero exit status (notmuch status) [${config}]"
> + test_expect_code 1 "notmuch new"
> +
> + test_begin_subtest "post-insert hook does not affect insert status [${config}]"
> + rm -rf ${HOOK_DIR}
> + generate_message
> + create_failing_hook "post-insert" $HOOK_DIR
> + test_expect_success "notmuch insert < \"$gen_msg_filename\" > /dev/null"
> +
> + test_begin_subtest "hook without executable permissions [${config}]"
> + rm -rf ${HOOK_DIR}
> + mkdir -p ${HOOK_DIR}
> + cat <<EOF >"${HOOK_DIR}/pre-new"
> + #!/bin/sh
> + echo foo
> EOF
> -output=`notmuch new 2>&1`
> -test_expect_code 1 "notmuch new"
> -
> -test_begin_subtest "hook execution failure"
> -rm_hooks
> -mkdir -p ${HOOK_DIR}
> -cat <<EOF >"${HOOK_DIR}/pre-new"
> -no hashbang, execl fails
> + output=`notmuch new 2>&1`
> + test_expect_code 1 "notmuch new"
> +
> + test_begin_subtest "hook execution failure [${config}]"
> + rm -rf ${HOOK_DIR}
> + mkdir -p ${HOOK_DIR}
> + cat <<EOF >"${HOOK_DIR}/pre-new"
> + no hashbang, execl fails
> EOF
> -chmod +x "${HOOK_DIR}/pre-new"
> -test_expect_code 1 "notmuch new"
> + chmod +x "${HOOK_DIR}/pre-new"
> + test_expect_code 1 "notmuch new"
>
> + rm -rf ${HOOK_DIR}
> +done
> test_done
> --
> 2.29.2
next prev parent reply other threads:[~2021-01-24 12:59 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-16 17:03 v4 merged config David Bremner
2021-01-16 17:03 ` [PATCH 01/38] lib: add _notmuch_string_map_set David Bremner
2021-01-24 11:28 ` Tomi Ollila
2021-01-24 12:06 ` David Bremner
2021-01-16 17:03 ` [PATCH 02/38] lib: cache configuration information from database David Bremner
2021-01-16 17:03 ` [PATCH 03/38] lib: add stub for notmuch_database_open_with_config David Bremner
2021-01-16 17:03 ` [PATCH 04/38] lib/open: add support for config profiles and default locations David Bremner
2021-01-24 11:58 ` Tomi Ollila
2021-01-16 17:03 ` [PATCH 05/38] CLI: generalize notmuch_config_mode_t David Bremner
2021-01-16 17:03 ` [PATCH 06/38] lib/config: add notmuch_config_key_{get,set} David Bremner
2021-01-16 17:03 ` [PATCH 07/38] lib/open: load default values for known configuration keys David Bremner
2021-01-16 17:03 ` [PATCH 08/38] CLI: add (unused) database argument to subcommands David Bremner
2021-01-16 17:03 ` [PATCH 09/38] util: add strsplit_len: simplified strtok with delimiter escaping David Bremner
2021-01-16 17:03 ` [PATCH 10/38] lib/config: add config values iterator David Bremner
2021-01-16 17:03 ` [PATCH 11/38] lib/config: make values iterators restartable David Bremner
2021-01-16 17:03 ` [PATCH 12/38] CLI/count: switch to new configuration framework David Bremner
2021-01-16 17:03 ` [PATCH 13/38] cli/dump: convert to new config framework David Bremner
2021-01-16 17:03 ` [PATCH 14/38] lib: add notmuch_config_get_bool David Bremner
2021-01-16 17:03 ` [PATCH 15/38] CLI/restore: convert to new config framework David Bremner
2021-01-16 17:03 ` [PATCH 16/38] CLI/insert: " David Bremner
2021-01-16 17:03 ` [PATCH 17/38] cli/reindex: convert " David Bremner
2021-01-16 17:03 ` [PATCH 18/38] CLI/reply: convert to " David Bremner
2021-01-16 17:03 ` [PATCH 19/38] CLI/{search,address}: convert to new configuration framework David Bremner
2021-01-16 17:03 ` [PATCH 20/38] cli/config: add accessor for config file name David Bremner
2021-01-16 17:03 ` [PATCH 21/38] CLI/show: mostly switch show to new config framework David Bremner
2021-01-16 17:03 ` [PATCH 22/38] cli/tag: convert " David Bremner
2021-01-16 17:03 ` [PATCH 23/38] lib/config: add _notmuch_config_cache David Bremner
2021-01-16 17:03 ` [PATCH 24/38] lib: split notmuch_database_compact David Bremner
2021-01-16 17:03 ` [PATCH 25/38] cli/compact: convert to new configuration framework David Bremner
2021-01-16 17:03 ` [PATCH 26/38] bindings/notmuch2: add missing crypto error status codes David Bremner
2021-01-16 17:03 ` [PATCH 27/38] lib/config: add NOTMUCH_CONFIG_NEW_IGNORE David Bremner
2021-01-16 17:03 ` [PATCH 28/38] lib/open: factor out first part of open David Bremner
2021-01-16 17:03 ` [PATCH 29/38] lib: add NOTMUCH_STATUS_NO_CONFIG David Bremner
2021-01-16 17:03 ` [PATCH 30/38] lib/database: move n_d_create* to open.cc David Bremner
2021-01-16 17:03 ` [PATCH 31/38] lib: add NOTMUCH_STATUS_DATABASE_EXISTS David Bremner
2021-01-16 17:04 ` [PATCH 32/38] lib: introduce notmuch_database_create_with_config David Bremner
2021-01-16 17:04 ` [PATCH 33/38] cli/new: refactor database upgrade code David Bremner
2021-01-16 17:04 ` [PATCH 34/38] cli/new: convert to new config framework David Bremner
2021-01-16 17:04 ` [PATCH 35/38] lib/config: add HOOK_DIR David Bremner
2021-01-16 17:04 ` [PATCH 36/38] lib/open: set HOOK_DIR on open David Bremner
2021-01-16 17:04 ` [PATCH 37/38] CLI: use configured hook directory David Bremner
2021-01-24 12:59 ` Tomi Ollila [this message]
2021-01-16 17:04 ` [PATCH 38/38] doc: describe new config framework David Bremner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://notmuchmail.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=m2wnw2jxs5.fsf@guru.guru-group.fi \
--to=tomi.ollila@iki.fi \
--cc=david@tethera.net \
--cc=notmuch@notmuchmail.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).