From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp1 ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms11 with LMTPS id 4E/TEEpvDWBofgAA0tVLHw (envelope-from ) for ; Sun, 24 Jan 2021 12:59:54 +0000 Received: from aspmx1.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp1 with LMTPS id YF2LDEpvDWBDGgAAbx9fmQ (envelope-from ) for ; Sun, 24 Jan 2021 12:59:54 +0000 Received: from mail.notmuchmail.org (nmbug.tethera.net [144.217.243.247]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (2048 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id 7B440940484 for ; Sun, 24 Jan 2021 12:59:53 +0000 (UTC) Received: from nmbug.tethera.net (localhost [127.0.0.1]) by mail.notmuchmail.org (Postfix) with ESMTP id 11B922BF53; Sun, 24 Jan 2021 07:59:46 -0500 (EST) Received: from lahtoruutu.iki.fi (lahtoruutu.iki.fi [IPv6:2a0b:5c81:1c1::37]) by mail.notmuchmail.org (Postfix) with ESMTPS id 1381E1FF9D for ; Sun, 24 Jan 2021 07:59:43 -0500 (EST) Received: from guru.guru-group.fi (unknown [IPv6:2a02:2380:1:9:5054:ff:feb7:a4bc]) (using TLSv1.2 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) (Authenticated sender: too) by lahtoruutu.iki.fi (Postfix) with ESMTPSA id 566B41B00334; Sun, 24 Jan 2021 14:59:40 +0200 (EET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=iki.fi; s=lahtoruutu; t=1611493180; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=+GrWgj+u/pZlTXG25Ml/kAfK/gqWiX5oq0nEhMPtV4c=; b=n5SuNm1u5wJMvyLEah9BYAbcELaiBRDXKoxdxgfuytFpDBl6NSdXZKnmIC4d9JwmBojMEC 5lv/fH8cUVdCBGEA1rP6FDew4MnURK4OwkOVDKLGpbLNF+dT18J7SOpcpclE+5UEVQCktL WSrS8gOOuzPyutwyuXyrNZJ6qgMYIHFvyW5X/kA1+GIvS+fj5LD1ffVPTmwBcVbi7Use5l wxtdKEqAM7QZuovYgeYuEEnzk94sikKKZTpG8WuY9VS5uCOBpkmYBWpbResvRhDxSGLbkf XBJSn0/m8oB9hd2/0A0O00RVUmM2BHv6lE+KTdjY+lK/3FalWniq1HahEB2iFA== From: Tomi Ollila To: David Bremner , notmuch@notmuchmail.org Cc: David Bremner Subject: Re: [PATCH 37/38] CLI: use configured hook directory In-Reply-To: <20210116170406.842014-38-david@tethera.net> References: <20210116170406.842014-1-david@tethera.net> <20210116170406.842014-38-david@tethera.net> User-Agent: Notmuch/0.31.3+85~g92bd77f (https://notmuchmail.org) Emacs/27.1 X-Face: HhBM'cA~ MIME-Version: 1.0 ARC-Seal: i=1; s=lahtoruutu; d=iki.fi; t=1611493180; a=rsa-sha256; cv=none; b=h3XD3C/NYi3O6WQPO4nZPQGV+nNsYnBA40uqrP0Hnb+RmXEcOzZ7E26xieBkubV0A8resg r1OUIIR/UK7hULsSsAywfAymBvii3ZxCFzx1EeKhZcnv07t/mHZLRIun49hNcinss13jHG EeUhfKfIDoJGX9OyTF1ZnrNrJ66rRMdBEkSSish75auWAcS3R2ysdbf6C1O/f5ooGtzTwM WT7hbvF7jd5BxCH2vBviEYluO8lge7zSAuKbYc/aCtBTv7iUFi442Q+1ZmX5rFL9uP0ZF4 sC4k2RAXPj9fnng6lE7pNiK9bTwz9JUs3KdLMTPNo2dRQSlHUfnIkBA1+r1gzA== ARC-Authentication-Results: i=1; ORIGINATING; auth=pass smtp.auth=too smtp.mailfrom=tomi.ollila@iki.fi ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=iki.fi; s=lahtoruutu; t=1611493180; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=+GrWgj+u/pZlTXG25Ml/kAfK/gqWiX5oq0nEhMPtV4c=; b=iYpV1GkgalXBnEsAKOH6sy8FqCVbGRjhb+mh00aQXIrBaZpZaF/7k4CAK/ygF4/HmK0GLv JAKxwIsfgmZ/EYgXFXtud/voywcmXupEUGr8aQjav/Ya6lBhaNx52uGDv6GrKuZq8Ezj3G Ta1srilo9WrDEvGmLdWwTkWgok0RwQNUEyjIUt+PRFY1duPimpZyi8rRwnVPwcovhXn4Un VECowBQIKfZwEQY2vtSQYC+jmuvbjC+MfcsQVamzZqpHQkwlClAx7SZXGPIwGYebsYWv4g uVZHY7LBSQXeQ6qn5H49wd/4qaKVpMS9Stb7J9oPGOfZ0ACIJciF9bYLy7KAJw== Message-ID-Hash: RTZDV3EI25AHSXIJRM3ZIEXSOMV5ONIX X-Message-ID-Hash: RTZDV3EI25AHSXIJRM3ZIEXSOMV5ONIX X-MailFrom: tomi.ollila@iki.fi X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; header-match-notmuch.notmuchmail.org-0; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; suspicious-header X-Mailman-Version: 3.2.1 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Help: List-Post: List-Subscribe: List-Unsubscribe: Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit X-Migadu-Flow: FLOW_IN X-Migadu-Spam-Score: 0.93 Authentication-Results: aspmx1.migadu.com; dkim=fail ("body hash did not verify") header.d=iki.fi header.s=lahtoruutu header.b=n5SuNm1u; arc=reject ("signature check failed: fail, {[1] = sig:iki.fi:reject}"); dmarc=none; spf=pass (aspmx1.migadu.com: domain of notmuch-bounces@notmuchmail.org designates 144.217.243.247 as permitted sender) smtp.mailfrom=notmuch-bounces@notmuchmail.org X-Migadu-Queue-Id: 7B440940484 X-Spam-Score: 0.93 X-Migadu-Scanner: scn0.migadu.com X-TUID: qMEVznuH+mqy 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 > > 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 <"${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 <"${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 <"${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 <"${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 <"${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