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
_______________________________________________
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-leave@notmuchmail.org