* notmuch insert --keep fails in parallel w/ notmuch new
@ 2016-01-24 15:48 Maarten Aertsen
2016-01-26 15:09 ` David Bremner
0 siblings, 1 reply; 18+ messages in thread
From: Maarten Aertsen @ 2016-01-24 15:48 UTC (permalink / raw)
To: Notmuch Mail
Hello list,
I'd like to report a bug against notmuch 0.21, packaged for Debian
jessie-backports as 0.21-3~bpo8+1.
# Steps to reproduce:
1. start a notmuch new mail w/ lots of stuff to process (e.g. a
migration)
2. try notmuch insert --keep
# Expected result:
Given the current text in the manpage, I'd expect notmuch insert --keep
to save the message to the Maildir even if no db interaction is possible:
"--keep Keep the message file if indexing fails, and keep the message
indexed if applying tags or maildir flag synchronization fails.
Ignore these errors and return exit status 0 to indicate
succesful mail delivery."
That way, I can cron notmuch new to pick up the occasional message that
can't get indexed right away.
# Actual result:
notmuch insert --keep fails with exit code 1 due to the inability to get
db access.
# Context:
I've switched to a setup with postfix, where notmuch insert --keep is
called by /local/, postfix's local delivery daemon, via a pipe in
.forward. Local treats a non-zero exit code as a 5.3.x status code and
sends a bounce to the sender.
I had a short chat with j4ni on #notmuch, where he said:
< j4ni> Sagi: oh bummer, we try to open the db first, and bail out
early. we should change the order of that (at least with --keep).
Thanks for developing notmuch.
best regards, Maarten Aertsen
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: notmuch insert --keep fails in parallel w/ notmuch new
2016-01-24 15:48 notmuch insert --keep fails in parallel w/ notmuch new Maarten Aertsen
@ 2016-01-26 15:09 ` David Bremner
2016-02-07 21:13 ` [PATCH] cli: avoid non-zero exits in notmuch insert --keep Maarten Aertsen
0 siblings, 1 reply; 18+ messages in thread
From: David Bremner @ 2016-01-26 15:09 UTC (permalink / raw)
To: Maarten Aertsen, Notmuch Mail
Maarten Aertsen <sagi-notmuch@rtsn.nl> writes:
>
> I had a short chat with j4ni on #notmuch, where he said:
> < j4ni> Sagi: oh bummer, we try to open the db first, and bail out
> early. we should change the order of that (at least with --keep).
>
FWIW, I agree it's a bug, and it should be not too hard to fix by moving
the open after the call to maildir write new.
d
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH] cli: avoid non-zero exits in notmuch insert --keep
2016-01-26 15:09 ` David Bremner
@ 2016-02-07 21:13 ` Maarten Aertsen
2016-02-08 11:56 ` David Bremner
0 siblings, 1 reply; 18+ messages in thread
From: Maarten Aertsen @ 2016-02-07 21:13 UTC (permalink / raw)
To: Notmuch Mail
Re-ordered code that touches the database to try and deliver e-mail to
at least try to deliver to the Maildir (which, with --keep should return
success).
In the case of any failure, we now return EX_TEMPFAIL (a sendmail
convention, defined in sysexits.h) to signal to the LDA that it should
retry later. This prevents a direct reject or bounce of e-mail.
---
notmuch-client.h | 1 +
notmuch-insert.c | 42 +++++++++++++++++++++++-------------------
notmuch.c | 17 +++++++++++++----
3 files changed, 37 insertions(+), 23 deletions(-)
diff --git a/notmuch-client.h b/notmuch-client.h
index 18e6c60..e3d6a46 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -466,6 +466,7 @@ print_status_query (const char *loc,
extern char *notmuch_requested_db_uuid;
extern const notmuch_opt_desc_t notmuch_shared_options [];
+notmuch_bool_t notmuch_has_unmatched_db_uuid (notmuch_database_t *notmuch);
void notmuch_exit_if_unmatched_db_uuid (notmuch_database_t *notmuch);
void notmuch_process_shared_options (const char* subcommand_name);
diff --git a/notmuch-insert.c b/notmuch-insert.c
index 5205c17..35b6779 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -28,6 +28,8 @@
#include <sys/stat.h>
#include <fcntl.h>
+#include <sysexits.h>
+
static volatile sig_atomic_t interrupted;
static void
@@ -532,31 +534,33 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
action.sa_flags = 0;
sigaction (SIGINT, &action, NULL);
- if (notmuch_database_open (notmuch_config_get_database_path (config),
- NOTMUCH_DATABASE_MODE_READ_WRITE, ¬much))
- return EXIT_FAILURE;
-
- notmuch_exit_if_unmatched_db_uuid (notmuch);
-
/* Write the message to the Maildir new directory. */
newpath = maildir_write_new (config, STDIN_FILENO, maildir);
if (! newpath) {
- notmuch_database_destroy (notmuch);
return EXIT_FAILURE;
}
- /* Index the message. */
- status = add_file (notmuch, newpath, tag_ops, synchronize_flags, keep);
-
- /* Commit changes. */
- close_status = notmuch_database_destroy (notmuch);
- if (close_status) {
- /* Hold on to the first error, if any. */
- if (! status)
- status = close_status;
- fprintf (stderr, "%s: failed to commit database changes: %s\n",
- keep ? "Warning" : "Error",
- notmuch_status_to_string (close_status));
+ status = notmuch_database_open (notmuch_config_get_database_path (config),
+ NOTMUCH_DATABASE_MODE_READ_WRITE, ¬much);
+ if (! status) {
+ /* with keep, send EX_TEMPFAIL per sysexits.h to invite the caller to
+ * retry at some later point and avoid permanent failure */
+ if (notmuch_has_unmatched_db_uuid(notmuch))
+ exit (keep ? EX_TEMPFAIL : EXIT_FAILURE);
+
+ /* Index the message. */
+ status = add_file (notmuch, newpath, tag_ops, synchronize_flags, keep);
+
+ /* Commit changes. */
+ close_status = notmuch_database_destroy (notmuch);
+ if (close_status) {
+ /* Hold on to the first error, if any. */
+ if (! status)
+ status = close_status;
+ fprintf (stderr, "%s: failed to commit database changes: %s\n",
+ keep ? "Warning" : "Error",
+ notmuch_status_to_string (close_status));
+ }
}
if (status) {
diff --git a/notmuch.c b/notmuch.c
index ce6c575..783bb2a 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -220,20 +220,29 @@ be supported in the future.\n", notmuch_format_version);
}
}
-void
-notmuch_exit_if_unmatched_db_uuid (notmuch_database_t *notmuch)
+notmuch_bool_t
+notmuch_has_unmatched_db_uuid (notmuch_database_t *notmuch)
{
const char *uuid = NULL;
if (!notmuch_requested_db_uuid)
- return;
+ return FALSE;
IGNORE_RESULT (notmuch_database_get_revision (notmuch, &uuid));
if (strcmp (notmuch_requested_db_uuid, uuid) != 0){
fprintf (stderr, "Error: requested database revision %s does not match %s\n",
notmuch_requested_db_uuid, uuid);
- exit (1);
+ return TRUE;
}
+
+ return FALSE;
+}
+
+void
+notmuch_exit_if_unmatched_db_uuid (notmuch_database_t *notmuch)
+{
+ if (notmuch_has_unmatched_db_uuid(notmuch))
+ exit (1);
}
static void
--
2.7.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH] cli: avoid non-zero exits in notmuch insert --keep
2016-02-07 21:13 ` [PATCH] cli: avoid non-zero exits in notmuch insert --keep Maarten Aertsen
@ 2016-02-08 11:56 ` David Bremner
2016-11-28 12:16 ` [PATCH 1/2] cli/insert: delay database open until after writing mail file David Bremner
0 siblings, 1 reply; 18+ messages in thread
From: David Bremner @ 2016-02-08 11:56 UTC (permalink / raw)
To: Maarten Aertsen, Notmuch Mail
Maarten Aertsen <sagi-notmuch@rtsn.nl> writes:
> In the case of any failure, we now return EX_TEMPFAIL (a sendmail
> convention, defined in sysexits.h) to signal to the LDA that it should
> retry later. This prevents a direct reject or bounce of e-mail.
We talked a bit on IRC about the portability of sysexits.h. Afaik, it's
available in GNU systems, on *BSD systems, and on OS/X. We can also
borrow the gnulib copy if we need a compatability version.
> +notmuch_bool_t notmuch_has_unmatched_db_uuid (notmuch_database_t *notmuch);
> void notmuch_exit_if_unmatched_db_uuid (notmuch_database_t *notmuch);
It's a bit unfortunate to end up with two such functions, but I guess we
could migrate everything to has_unmatched_db_uuid. The double negative
is a bit confusing too, it seems like it would be more natural to have a
_has_matching_db_uuid
> + status = notmuch_database_open (notmuch_config_get_database_path (config),
> + NOTMUCH_DATABASE_MODE_READ_WRITE, ¬much);
> + if (! status) {
> + /* with keep, send EX_TEMPFAIL per sysexits.h to invite the caller to
> + * retry at some later point and avoid permanent failure */
> + if (notmuch_has_unmatched_db_uuid(notmuch))
> + exit (keep ? EX_TEMPFAIL : EXIT_FAILURE);
We're a bit fussy about spaces; see devel/STYLE
I'd invite brainstorming about whether mismatched UUID is really a
temporary failure. Unlike a lock, I don't see it going away without
user/admin intervention. Actually, someone even specifying a UUID when
calling notmuch insert would be a bit surprising.
I suspect some of the tests in T070-insert.sh could/should be updated
for changed exit values.
d
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/2] cli/insert: delay database open until after writing mail file
2016-02-08 11:56 ` David Bremner
@ 2016-11-28 12:16 ` David Bremner
2016-11-28 12:16 ` [PATCH 2/2] cli/insert: return EX_TEMPFAIL for some errors David Bremner
2016-11-28 12:23 ` [PATCH 1/2] cli/insert: delay database open until after writing mail file David Bremner
0 siblings, 2 replies; 18+ messages in thread
From: David Bremner @ 2016-11-28 12:16 UTC (permalink / raw)
To: David Bremner, Maarten Aertsen, Notmuch Mail
The idea is to get the mail written to disk, even if we can't open the
database (e.g. because some other process has a write lock, and notmuch
is compiled for non-blocking opens).
---
notmuch-insert.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/notmuch-insert.c b/notmuch-insert.c
index 131f09e..862da88 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -532,19 +532,19 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
action.sa_flags = 0;
sigaction (SIGINT, &action, NULL);
- if (notmuch_database_open (notmuch_config_get_database_path (config),
- NOTMUCH_DATABASE_MODE_READ_WRITE, ¬much))
- return EXIT_FAILURE;
-
- notmuch_exit_if_unmatched_db_uuid (notmuch);
-
/* Write the message to the Maildir new directory. */
newpath = maildir_write_new (config, STDIN_FILENO, maildir);
if (! newpath) {
- notmuch_database_destroy (notmuch);
return EXIT_FAILURE;
}
+ if (notmuch_database_open (notmuch_config_get_database_path (config),
+ NOTMUCH_DATABASE_MODE_READ_WRITE, ¬much))
+ return EXIT_FAILURE;
+
+ notmuch_exit_if_unmatched_db_uuid (notmuch);
+
+
/* Index the message. */
status = add_file (notmuch, newpath, tag_ops, synchronize_flags, keep);
--
2.10.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/2] cli/insert: return EX_TEMPFAIL for some errors
2016-11-28 12:16 ` [PATCH 1/2] cli/insert: delay database open until after writing mail file David Bremner
@ 2016-11-28 12:16 ` David Bremner
2016-11-28 12:22 ` David Bremner
2016-11-28 22:12 ` v2 of insert tempfail series David Bremner
2016-11-28 12:23 ` [PATCH 1/2] cli/insert: delay database open until after writing mail file David Bremner
1 sibling, 2 replies; 18+ messages in thread
From: David Bremner @ 2016-11-28 12:16 UTC (permalink / raw)
To: David Bremner, Maarten Aertsen, Notmuch Mail
Attempt to distinguish between errors indicating misconfiguration or
programmer error, which we consider "permanant", in the sense that
automatic retries are unlikely to be useful, and those indicating
transient error conditions. We consider XAPIAN_EXCEPTION transient
because it covers the important special case of locking failure.
---
notmuch-client.h | 3 +++
notmuch-insert.c | 9 +++++----
status.c | 16 ++++++++++++++++
test/T070-insert.sh | 24 ++++++++++++++----------
4 files changed, 38 insertions(+), 14 deletions(-)
diff --git a/notmuch-client.h b/notmuch-client.h
index 793f32e..d026e60 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -489,6 +489,9 @@ print_status_database (const char *loc,
const notmuch_database_t *database,
notmuch_status_t status);
+int
+status_to_exit (notmuch_status_t status);
+
#include "command-line-arguments.h"
extern char *notmuch_requested_db_uuid;
diff --git a/notmuch-insert.c b/notmuch-insert.c
index 862da88..a152f15 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -538,9 +538,10 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
return EXIT_FAILURE;
}
- if (notmuch_database_open (notmuch_config_get_database_path (config),
- NOTMUCH_DATABASE_MODE_READ_WRITE, ¬much))
- return EXIT_FAILURE;
+ status = notmuch_database_open (notmuch_config_get_database_path (config),
+ NOTMUCH_DATABASE_MODE_READ_WRITE, ¬much);
+ if (status)
+ return status_to_exit(status);
notmuch_exit_if_unmatched_db_uuid (notmuch);
@@ -577,5 +578,5 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
notmuch_run_hook (db_path, "post-insert");
}
- return status ? EXIT_FAILURE : EXIT_SUCCESS;
+ return status ? status_to_exit(status) : EXIT_SUCCESS;
}
diff --git a/status.c b/status.c
index 45d3fb4..8bc2fe4 100644
--- a/status.c
+++ b/status.c
@@ -36,3 +36,19 @@ print_status_database (const char *loc,
}
return status;
}
+
+int
+status_to_exit (notmuch_status_t status)
+{
+ switch (status) {
+ case NOTMUCH_STATUS_SUCCESS:
+ return EXIT_SUCCESS;
+ case NOTMUCH_STATUS_OUT_OF_MEMORY:
+ case NOTMUCH_STATUS_XAPIAN_EXCEPTION:
+ case NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID:
+ case NOTMUCH_STATUS_FILE_ERROR:
+ return EX_TEMPFAIL;
+ default:
+ return EXIT_FAILURE;
+ }
+}
diff --git a/test/T070-insert.sh b/test/T070-insert.sh
index c2485bb..fd620e5 100755
--- a/test/T070-insert.sh
+++ b/test/T070-insert.sh
@@ -189,7 +189,6 @@ notmuch config set new.tags $OLDCONFIG
for code in OUT_OF_MEMORY XAPIAN_EXCEPTION FILE_NOT_EMAIL \
READ_ONLY_DATABASE UPGRADE_REQUIRED PATH_ERROR; do
-gen_insert_msg
cat <<EOF > index-file-$code.gdb
set breakpoint pending on
set logging file index-file-$code.log
@@ -201,15 +200,20 @@ continue
end
run
EOF
-test_begin_subtest "error exit when add_message returns $code"
-gdb --batch-silent --return-child-result -x index-file-$code.gdb \
- --args notmuch insert < $gen_msg_filename
-test_expect_equal $? 1
-
-test_begin_subtest "success exit with --keep when add_message returns $code"
-gdb --batch-silent --return-child-result -x index-file-$code.gdb \
- --args notmuch insert --keep < $gen_msg_filename
-test_expect_equal $? 0
+done
+
+gen_insert_msg
+
+for code in FILE_NOT_EMAIL READ_ONLY_DATABASE UPGRADE_REQUIRED PATH_ERROR; do
+ test_expect_code 1 "EXIT_FAILURE when add_message returns $code" \
+ "gdb --batch-silent --return-child-result -x index-file-$code.gdb \
+ --args notmuch insert < $gen_msg_filename"
+done
+
+for code in OUT_OF_MEMORY XAPIAN_EXCEPTION ; do
+ test_expect_code 75 "EX_TEMPFAIL when add_message returns $code" \
+ "gdb --batch-silent --return-child-result -x index-file-$code.gdb \
+ --args notmuch insert < $gen_msg_filename"
done
test_done
--
2.10.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] cli/insert: return EX_TEMPFAIL for some errors
2016-11-28 12:16 ` [PATCH 2/2] cli/insert: return EX_TEMPFAIL for some errors David Bremner
@ 2016-11-28 12:22 ` David Bremner
2016-11-28 22:12 ` v2 of insert tempfail series David Bremner
1 sibling, 0 replies; 18+ messages in thread
From: David Bremner @ 2016-11-28 12:22 UTC (permalink / raw)
To: Maarten Aertsen, Notmuch Mail
David Bremner <david@tethera.net> writes:
> Attempt to distinguish between errors indicating misconfiguration or
> programmer error, which we consider "permanant", in the sense that
> automatic retries are unlikely to be useful, and those indicating
> transient error conditions. We consider XAPIAN_EXCEPTION transient
> because it covers the important special case of locking failure.
> ---
> notmuch-client.h | 3 +++
> notmuch-insert.c | 9 +++++----
> status.c | 16 ++++++++++++++++
> test/T070-insert.sh | 24 ++++++++++++++----------
> 4 files changed, 38 insertions(+), 14 deletions(-)
>
> diff --git a/notmuch-client.h b/notmuch-client.h
> index 793f32e..d026e60 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -489,6 +489,9 @@ print_status_database (const char *loc,
> const notmuch_database_t *database,
> notmuch_status_t status);
>
> +int
> +status_to_exit (notmuch_status_t status);
> +
> #include "command-line-arguments.h"
>
> extern char *notmuch_requested_db_uuid;
> diff --git a/notmuch-insert.c b/notmuch-insert.c
> index 862da88..a152f15 100644
> --- a/notmuch-insert.c
> +++ b/notmuch-insert.c
> @@ -538,9 +538,10 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
> return EXIT_FAILURE;
> }
>
> - if (notmuch_database_open (notmuch_config_get_database_path (config),
> - NOTMUCH_DATABASE_MODE_READ_WRITE, ¬much))
> - return EXIT_FAILURE;
> + status = notmuch_database_open (notmuch_config_get_database_path (config),
> + NOTMUCH_DATABASE_MODE_READ_WRITE, ¬much);
> + if (status)
> + return status_to_exit(status);
I guess this should return NOTMUCH_STATUS_SUCCESS if --keep is passed.
> notmuch_exit_if_unmatched_db_uuid (notmuch);
>
> @@ -577,5 +578,5 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
> notmuch_run_hook (db_path, "post-insert");
> }
>
> - return status ? EXIT_FAILURE : EXIT_SUCCESS;
> + return status ? status_to_exit(status) : EXIT_SUCCESS;
This can be simplified to just "return status_to_exit (status)".
And whitespace fixes for both calls to status_to_exit.
> +int
> +status_to_exit (notmuch_status_t status)
> +{
> + switch (status) {
> + case NOTMUCH_STATUS_SUCCESS:
> + return EXIT_SUCCESS;
> + case NOTMUCH_STATUS_OUT_OF_MEMORY:
> + case NOTMUCH_STATUS_XAPIAN_EXCEPTION:
> + case NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID:
> + case NOTMUCH_STATUS_FILE_ERROR:
> + return EX_TEMPFAIL;
> + default:
> + return EXIT_FAILURE;
> + }
This classification is pretty arbitrary. The main goal is have locking
errors, which are currently NOTMUCH_STATUS_XAPIAN_EXCEPTION treated as
TEMPFAIL.
d
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] cli/insert: delay database open until after writing mail file
2016-11-28 12:16 ` [PATCH 1/2] cli/insert: delay database open until after writing mail file David Bremner
2016-11-28 12:16 ` [PATCH 2/2] cli/insert: return EX_TEMPFAIL for some errors David Bremner
@ 2016-11-28 12:23 ` David Bremner
1 sibling, 0 replies; 18+ messages in thread
From: David Bremner @ 2016-11-28 12:23 UTC (permalink / raw)
To: Maarten Aertsen, Notmuch Mail
David Bremner <david@tethera.net> writes:
> The idea is to get the mail written to disk, even if we can't open the
> database (e.g. because some other process has a write lock, and notmuch
> is compiled for non-blocking opens).
> ---
> notmuch-insert.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/notmuch-insert.c b/notmuch-insert.c
> index 131f09e..862da88 100644
> --- a/notmuch-insert.c
> +++ b/notmuch-insert.c
> @@ -532,19 +532,19 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
> action.sa_flags = 0;
> sigaction (SIGINT, &action, NULL);
>
> - if (notmuch_database_open (notmuch_config_get_database_path (config),
> - NOTMUCH_DATABASE_MODE_READ_WRITE, ¬much))
> - return EXIT_FAILURE;
> -
> - notmuch_exit_if_unmatched_db_uuid (notmuch);
> -
> /* Write the message to the Maildir new directory. */
> newpath = maildir_write_new (config, STDIN_FILENO, maildir);
> if (! newpath) {
> - notmuch_database_destroy (notmuch);
> return EXIT_FAILURE;
> }
>
> + if (notmuch_database_open (notmuch_config_get_database_path (config),
> + NOTMUCH_DATABASE_MODE_READ_WRITE, ¬much))
> + return EXIT_FAILURE;
> +
> + notmuch_exit_if_unmatched_db_uuid (notmuch);
> +
> +
One extra newline here.
^ permalink raw reply [flat|nested] 18+ messages in thread
* v2 of insert tempfail series
2016-11-28 12:16 ` [PATCH 2/2] cli/insert: return EX_TEMPFAIL for some errors David Bremner
2016-11-28 12:22 ` David Bremner
@ 2016-11-28 22:12 ` David Bremner
2016-11-28 22:12 ` [PATCH 1/3] test: gdb insert: redirect input inside gdb script David Bremner
` (3 more replies)
1 sibling, 4 replies; 18+ messages in thread
From: David Bremner @ 2016-11-28 22:12 UTC (permalink / raw)
To: David Bremner, Maarten Aertsen, Notmuch Mail
This incorporates Tomi's patch of
id:1480367228-22183-1-git-send-email-tomi.ollila@iki.fi
verbatim, to sort out conflicts.
It fixes the issues I alread sent mail about, and puts back the --keep
tests for various error codes.
The interdiff follows; most of it is Tomi's fault :).
diff --git a/notmuch-insert.c b/notmuch-insert.c
index a152f15..bc96af0 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -541,7 +541,7 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
status = notmuch_database_open (notmuch_config_get_database_path (config),
NOTMUCH_DATABASE_MODE_READ_WRITE, ¬much);
if (status)
- return status_to_exit(status);
+ return keep ? NOTMUCH_STATUS_SUCCESS : status_to_exit (status);
notmuch_exit_if_unmatched_db_uuid (notmuch);
@@ -578,5 +578,5 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
notmuch_run_hook (db_path, "post-insert");
}
- return status ? status_to_exit(status) : EXIT_SUCCESS;
+ return status_to_exit (status);
}
diff --git a/test/T070-insert.sh b/test/T070-insert.sh
index fd620e5..3e7d582 100755
--- a/test/T070-insert.sh
+++ b/test/T070-insert.sh
@@ -206,14 +206,24 @@ gen_insert_msg
for code in FILE_NOT_EMAIL READ_ONLY_DATABASE UPGRADE_REQUIRED PATH_ERROR; do
test_expect_code 1 "EXIT_FAILURE when add_message returns $code" \
- "gdb --batch-silent --return-child-result -x index-file-$code.gdb \
- --args notmuch insert < $gen_msg_filename"
+ "gdb --batch-silent --return-child-result \
+ -ex \"set args insert < $gen_msg_filename\" \
+ -x index-file-$code.gdb notmuch"
+ test_expect_code 0 "success exit with --keep when add_message returns $code" \
+ "gdb --batch-silent --return-child-result \
+ -ex \"set args insert --keep < $gen_msg_filename\" \
+ -x index-file-$code.gdb notmuch"
done
for code in OUT_OF_MEMORY XAPIAN_EXCEPTION ; do
test_expect_code 75 "EX_TEMPFAIL when add_message returns $code" \
- "gdb --batch-silent --return-child-result -x index-file-$code.gdb \
- --args notmuch insert < $gen_msg_filename"
+ "gdb --batch-silent --return-child-result \
+ -ex \"set args insert < $gen_msg_filename\" \
+ -x index-file-$code.gdb notmuch"
+ test_expect_code 0 "success exit with --keep when add_message returns $code" \
+ "gdb --batch-silent --return-child-result \
+ -ex \"set args insert --keep < $gen_msg_filename\" \
+ -x index-file-$code.gdb notmuch"
done
test_done
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 1/3] test: gdb insert: redirect input inside gdb script
2016-11-28 22:12 ` v2 of insert tempfail series David Bremner
@ 2016-11-28 22:12 ` David Bremner
2016-11-28 22:12 ` [PATCH 2/3] cli/insert: delay database open until after writing mail file David Bremner
` (2 subsequent siblings)
3 siblings, 0 replies; 18+ messages in thread
From: David Bremner @ 2016-11-28 22:12 UTC (permalink / raw)
To: David Bremner, Maarten Aertsen, Notmuch Mail; +Cc: Tomi Ollila
From: Tomi Ollila <tomi.ollila@iki.fi>
Running `gdb command < input` is not as reliable way to give input
to the command (some installations of gdb consume it). Use "set args"
gdb command to have input redirected at gdb 'run' time.
---
test/T070-insert.sh | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/test/T070-insert.sh b/test/T070-insert.sh
index c2485bb..43b3abc 100755
--- a/test/T070-insert.sh
+++ b/test/T070-insert.sh
@@ -202,13 +202,15 @@ end
run
EOF
test_begin_subtest "error exit when add_message returns $code"
-gdb --batch-silent --return-child-result -x index-file-$code.gdb \
- --args notmuch insert < $gen_msg_filename
+gdb --batch-silent --return-child-result \
+ -ex "set args insert < $gen_msg_filename" \
+ -x index-file-$code.gdb notmuch
test_expect_equal $? 1
test_begin_subtest "success exit with --keep when add_message returns $code"
-gdb --batch-silent --return-child-result -x index-file-$code.gdb \
- --args notmuch insert --keep < $gen_msg_filename
+gdb --batch-silent --return-child-result \
+ -ex "set args insert --keep < $gen_msg_filename" \
+ -x index-file-$code.gdb notmuch
test_expect_equal $? 0
done
--
2.10.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/3] cli/insert: delay database open until after writing mail file
2016-11-28 22:12 ` v2 of insert tempfail series David Bremner
2016-11-28 22:12 ` [PATCH 1/3] test: gdb insert: redirect input inside gdb script David Bremner
@ 2016-11-28 22:12 ` David Bremner
2016-11-28 22:12 ` [PATCH 3/3] cli/insert: return EX_TEMPFAIL for some errors David Bremner
2016-12-04 20:51 ` v2 of insert tempfail series Tomi Ollila
3 siblings, 0 replies; 18+ messages in thread
From: David Bremner @ 2016-11-28 22:12 UTC (permalink / raw)
To: David Bremner, Maarten Aertsen, Notmuch Mail
The idea is to get the mail written to disk, even if we can't open the
database (e.g. because some other process has a write lock, and notmuch
is compiled for non-blocking opens).
---
notmuch-insert.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/notmuch-insert.c b/notmuch-insert.c
index 131f09e..862da88 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -532,19 +532,19 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
action.sa_flags = 0;
sigaction (SIGINT, &action, NULL);
- if (notmuch_database_open (notmuch_config_get_database_path (config),
- NOTMUCH_DATABASE_MODE_READ_WRITE, ¬much))
- return EXIT_FAILURE;
-
- notmuch_exit_if_unmatched_db_uuid (notmuch);
-
/* Write the message to the Maildir new directory. */
newpath = maildir_write_new (config, STDIN_FILENO, maildir);
if (! newpath) {
- notmuch_database_destroy (notmuch);
return EXIT_FAILURE;
}
+ if (notmuch_database_open (notmuch_config_get_database_path (config),
+ NOTMUCH_DATABASE_MODE_READ_WRITE, ¬much))
+ return EXIT_FAILURE;
+
+ notmuch_exit_if_unmatched_db_uuid (notmuch);
+
+
/* Index the message. */
status = add_file (notmuch, newpath, tag_ops, synchronize_flags, keep);
--
2.10.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 3/3] cli/insert: return EX_TEMPFAIL for some errors
2016-11-28 22:12 ` v2 of insert tempfail series David Bremner
2016-11-28 22:12 ` [PATCH 1/3] test: gdb insert: redirect input inside gdb script David Bremner
2016-11-28 22:12 ` [PATCH 2/3] cli/insert: delay database open until after writing mail file David Bremner
@ 2016-11-28 22:12 ` David Bremner
2016-12-04 20:51 ` v2 of insert tempfail series Tomi Ollila
3 siblings, 0 replies; 18+ messages in thread
From: David Bremner @ 2016-11-28 22:12 UTC (permalink / raw)
To: David Bremner, Maarten Aertsen, Notmuch Mail
Attempt to distinguish between errors indicating misconfiguration or
programmer error, which we consider "permanent", in the sense that
automatic retries are unlikely to be useful, and those indicating
transient error conditions. We consider XAPIAN_EXCEPTION transient
because it covers the important special case of locking failure.
---
notmuch-client.h | 3 +++
notmuch-insert.c | 9 +++++----
status.c | 16 ++++++++++++++++
test/T070-insert.sh | 36 ++++++++++++++++++++++++------------
4 files changed, 48 insertions(+), 16 deletions(-)
diff --git a/notmuch-client.h b/notmuch-client.h
index 793f32e..d026e60 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -489,6 +489,9 @@ print_status_database (const char *loc,
const notmuch_database_t *database,
notmuch_status_t status);
+int
+status_to_exit (notmuch_status_t status);
+
#include "command-line-arguments.h"
extern char *notmuch_requested_db_uuid;
diff --git a/notmuch-insert.c b/notmuch-insert.c
index 862da88..bc96af0 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -538,9 +538,10 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
return EXIT_FAILURE;
}
- if (notmuch_database_open (notmuch_config_get_database_path (config),
- NOTMUCH_DATABASE_MODE_READ_WRITE, ¬much))
- return EXIT_FAILURE;
+ status = notmuch_database_open (notmuch_config_get_database_path (config),
+ NOTMUCH_DATABASE_MODE_READ_WRITE, ¬much);
+ if (status)
+ return keep ? NOTMUCH_STATUS_SUCCESS : status_to_exit (status);
notmuch_exit_if_unmatched_db_uuid (notmuch);
@@ -577,5 +578,5 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
notmuch_run_hook (db_path, "post-insert");
}
- return status ? EXIT_FAILURE : EXIT_SUCCESS;
+ return status_to_exit (status);
}
diff --git a/status.c b/status.c
index 45d3fb4..8bc2fe4 100644
--- a/status.c
+++ b/status.c
@@ -36,3 +36,19 @@ print_status_database (const char *loc,
}
return status;
}
+
+int
+status_to_exit (notmuch_status_t status)
+{
+ switch (status) {
+ case NOTMUCH_STATUS_SUCCESS:
+ return EXIT_SUCCESS;
+ case NOTMUCH_STATUS_OUT_OF_MEMORY:
+ case NOTMUCH_STATUS_XAPIAN_EXCEPTION:
+ case NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID:
+ case NOTMUCH_STATUS_FILE_ERROR:
+ return EX_TEMPFAIL;
+ default:
+ return EXIT_FAILURE;
+ }
+}
diff --git a/test/T070-insert.sh b/test/T070-insert.sh
index 43b3abc..3e7d582 100755
--- a/test/T070-insert.sh
+++ b/test/T070-insert.sh
@@ -189,7 +189,6 @@ notmuch config set new.tags $OLDCONFIG
for code in OUT_OF_MEMORY XAPIAN_EXCEPTION FILE_NOT_EMAIL \
READ_ONLY_DATABASE UPGRADE_REQUIRED PATH_ERROR; do
-gen_insert_msg
cat <<EOF > index-file-$code.gdb
set breakpoint pending on
set logging file index-file-$code.log
@@ -201,17 +200,30 @@ continue
end
run
EOF
-test_begin_subtest "error exit when add_message returns $code"
-gdb --batch-silent --return-child-result \
- -ex "set args insert < $gen_msg_filename" \
- -x index-file-$code.gdb notmuch
-test_expect_equal $? 1
-
-test_begin_subtest "success exit with --keep when add_message returns $code"
-gdb --batch-silent --return-child-result \
- -ex "set args insert --keep < $gen_msg_filename" \
- -x index-file-$code.gdb notmuch
-test_expect_equal $? 0
+done
+
+gen_insert_msg
+
+for code in FILE_NOT_EMAIL READ_ONLY_DATABASE UPGRADE_REQUIRED PATH_ERROR; do
+ test_expect_code 1 "EXIT_FAILURE when add_message returns $code" \
+ "gdb --batch-silent --return-child-result \
+ -ex \"set args insert < $gen_msg_filename\" \
+ -x index-file-$code.gdb notmuch"
+ test_expect_code 0 "success exit with --keep when add_message returns $code" \
+ "gdb --batch-silent --return-child-result \
+ -ex \"set args insert --keep < $gen_msg_filename\" \
+ -x index-file-$code.gdb notmuch"
+done
+
+for code in OUT_OF_MEMORY XAPIAN_EXCEPTION ; do
+ test_expect_code 75 "EX_TEMPFAIL when add_message returns $code" \
+ "gdb --batch-silent --return-child-result \
+ -ex \"set args insert < $gen_msg_filename\" \
+ -x index-file-$code.gdb notmuch"
+ test_expect_code 0 "success exit with --keep when add_message returns $code" \
+ "gdb --batch-silent --return-child-result \
+ -ex \"set args insert --keep < $gen_msg_filename\" \
+ -x index-file-$code.gdb notmuch"
done
test_done
--
2.10.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: v2 of insert tempfail series
2016-11-28 22:12 ` v2 of insert tempfail series David Bremner
` (2 preceding siblings ...)
2016-11-28 22:12 ` [PATCH 3/3] cli/insert: return EX_TEMPFAIL for some errors David Bremner
@ 2016-12-04 20:51 ` Tomi Ollila
2016-12-07 11:27 ` NEWS/docs for insert tempfail changes David Bremner
3 siblings, 1 reply; 18+ messages in thread
From: Tomi Ollila @ 2016-12-04 20:51 UTC (permalink / raw)
To: David Bremner, Maarten Aertsen, Notmuch Mail
On Tue, Nov 29 2016, David Bremner <david@tethera.net> wrote:
> This incorporates Tomi's patch of
>
> id:1480367228-22183-1-git-send-email-tomi.ollila@iki.fi
>
> verbatim, to sort out conflicts.
>
> It fixes the issues I alread sent mail about, and puts back the --keep
> tests for various error codes.
>
> The interdiff follows; most of it is Tomi's fault :).
This series looks good to me -- also my part which I had a slight
afterthought when I looked what brew install gdb printed out -- it
suggested setting 'startup-with-shell off' which would have borken
my version (on macOS), but
http://apple.stackexchange.com/questions/246245/macos-sierra-gdb-not-codesigned
informed that setting startup-with-shell has so far not been necessary
(and it would be major pain everywhere if it were required; in addition to
everything else using `exec-wrapper` also requires shell...)
I.e. if it makes gdb use more robust in general that is good reason to use it.
Last (and least) (i.e. I could not resist the templation): the code block
below would be simpler in this format -- but perhaps what happends there is
less understandable -- although now the where the expansion is done is more
visible ;)
+ test_expect_code 75 "EX_TEMPFAIL when add_message returns $code" \
+ "gdb --batch-silent --return-child-result \
+ -ex 'set args insert < $gen_msg_filename' \
+ -x index-file-$code.gdb notmuch"
(all variable expansions are done here (in context of the outermost "gdb...")
and not in the `eval` that is done by test_expect_code -- unless any of the
variables expanded to yet another variable syntax ;)
Tomi
>
> diff --git a/notmuch-insert.c b/notmuch-insert.c
> index a152f15..bc96af0 100644
> --- a/notmuch-insert.c
> +++ b/notmuch-insert.c
> @@ -541,7 +541,7 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
> status = notmuch_database_open (notmuch_config_get_database_path (config),
> NOTMUCH_DATABASE_MODE_READ_WRITE, ¬much);
> if (status)
> - return status_to_exit(status);
> + return keep ? NOTMUCH_STATUS_SUCCESS : status_to_exit (status);
>
> notmuch_exit_if_unmatched_db_uuid (notmuch);
>
> @@ -578,5 +578,5 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
> notmuch_run_hook (db_path, "post-insert");
> }
>
> - return status ? status_to_exit(status) : EXIT_SUCCESS;
> + return status_to_exit (status);
> }
> diff --git a/test/T070-insert.sh b/test/T070-insert.sh
> index fd620e5..3e7d582 100755
> --- a/test/T070-insert.sh
> +++ b/test/T070-insert.sh
> @@ -206,14 +206,24 @@ gen_insert_msg
>
> for code in FILE_NOT_EMAIL READ_ONLY_DATABASE UPGRADE_REQUIRED PATH_ERROR; do
> test_expect_code 1 "EXIT_FAILURE when add_message returns $code" \
> - "gdb --batch-silent --return-child-result -x index-file-$code.gdb \
> - --args notmuch insert < $gen_msg_filename"
> + "gdb --batch-silent --return-child-result \
> + -ex \"set args insert < $gen_msg_filename\" \
> + -x index-file-$code.gdb notmuch"
> + test_expect_code 0 "success exit with --keep when add_message returns $code" \
> + "gdb --batch-silent --return-child-result \
> + -ex \"set args insert --keep < $gen_msg_filename\" \
> + -x index-file-$code.gdb notmuch"
> done
>
> for code in OUT_OF_MEMORY XAPIAN_EXCEPTION ; do
> test_expect_code 75 "EX_TEMPFAIL when add_message returns $code" \
> - "gdb --batch-silent --return-child-result -x index-file-$code.gdb \
> - --args notmuch insert < $gen_msg_filename"
> + "gdb --batch-silent --return-child-result \
> + -ex \"set args insert < $gen_msg_filename\" \
> + -x index-file-$code.gdb notmuch"
> + test_expect_code 0 "success exit with --keep when add_message returns $code" \
> + "gdb --batch-silent --return-child-result \
> + -ex \"set args insert --keep < $gen_msg_filename\" \
> + -x index-file-$code.gdb notmuch"
> done
>
> test_done
^ permalink raw reply [flat|nested] 18+ messages in thread
* NEWS/docs for insert tempfail changes
2016-12-04 20:51 ` v2 of insert tempfail series Tomi Ollila
@ 2016-12-07 11:27 ` David Bremner
2016-12-07 11:27 ` [PATCH 1/2] cli/insert: document the use of EX_TEMPFAIL David Bremner
` (2 more replies)
0 siblings, 3 replies; 18+ messages in thread
From: David Bremner @ 2016-12-07 11:27 UTC (permalink / raw)
To: Tomi Ollila, David Bremner, Maarten Aertsen, Notmuch Mail
I went back and forth few times, but eventually decided to go with
Tomi's version of the test quoting, the better to share the blame.
The patches will be merged to release and master, in prep for another
bugfix release. Here are some proposed news and manpage changes.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/2] cli/insert: document the use of EX_TEMPFAIL
2016-12-07 11:27 ` NEWS/docs for insert tempfail changes David Bremner
@ 2016-12-07 11:27 ` David Bremner
2016-12-15 13:01 ` David Bremner
2016-12-07 11:27 ` [PATCH 2/2] NEWS: news for notmuch-insert error handling David Bremner
2016-12-10 19:26 ` NEWS/docs for insert tempfail changes Tomi Ollila
2 siblings, 1 reply; 18+ messages in thread
From: David Bremner @ 2016-12-07 11:27 UTC (permalink / raw)
To: Tomi Ollila, David Bremner, Maarten Aertsen, Notmuch Mail
---
doc/man1/notmuch-insert.rst | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/doc/man1/notmuch-insert.rst b/doc/man1/notmuch-insert.rst
index 9e7afc3..9847e67 100644
--- a/doc/man1/notmuch-insert.rst
+++ b/doc/man1/notmuch-insert.rst
@@ -60,6 +60,13 @@ indexing to Notmuch database, changing tags, and synchronizing tags to
maildir flags. The ``--keep`` option may be used to settle for
successful message file delivery.
+This command supports the following special exit status code for
+errors most likely to be temporary in nature, e.g. failure to get a
+database write lock.
+
+``75 (EX_TEMPFAIL)``
+ A temporary failure occured; the user is invited to retry.
+
The exit status of the **post-insert** hook does not affect the exit
status of the **insert** command.
--
2.10.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/2] NEWS: news for notmuch-insert error handling
2016-12-07 11:27 ` NEWS/docs for insert tempfail changes David Bremner
2016-12-07 11:27 ` [PATCH 1/2] cli/insert: document the use of EX_TEMPFAIL David Bremner
@ 2016-12-07 11:27 ` David Bremner
2016-12-10 19:26 ` NEWS/docs for insert tempfail changes Tomi Ollila
2 siblings, 0 replies; 18+ messages in thread
From: David Bremner @ 2016-12-07 11:27 UTC (permalink / raw)
To: Tomi Ollila, David Bremner, Maarten Aertsen, Notmuch Mail
---
NEWS | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/NEWS b/NEWS
index 99ef277..c0aa479 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,15 @@
+Notmuch 0.23.4 (UNRELEASED)
+===========================
+
+Command Line Interface
+----------------------
+
+Improve error handling in notmuch insert
+
+ Database lock errors no longer prevent message file delivery to the
+ filesystem. Certain errors during `notmuch insert` most likely to
+ be temporary return EX_TEMPFAIL.
+
Notmuch 0.23.3 (2016-11-27)
===========================
--
2.10.2
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: NEWS/docs for insert tempfail changes
2016-12-07 11:27 ` NEWS/docs for insert tempfail changes David Bremner
2016-12-07 11:27 ` [PATCH 1/2] cli/insert: document the use of EX_TEMPFAIL David Bremner
2016-12-07 11:27 ` [PATCH 2/2] NEWS: news for notmuch-insert error handling David Bremner
@ 2016-12-10 19:26 ` Tomi Ollila
2 siblings, 0 replies; 18+ messages in thread
From: Tomi Ollila @ 2016-12-10 19:26 UTC (permalink / raw)
To: David Bremner, Maarten Aertsen, Notmuch Mail
On Wed, Dec 07 2016, David Bremner <david@tethera.net> wrote:
> I went back and forth few times, but eventually decided to go with
> Tomi's version of the test quoting, the better to share the blame.
> The patches will be merged to release and master, in prep for another
> bugfix release. Here are some proposed news and manpage changes.
From the (content and) formatting point of view these look good.
Tomi
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] cli/insert: document the use of EX_TEMPFAIL
2016-12-07 11:27 ` [PATCH 1/2] cli/insert: document the use of EX_TEMPFAIL David Bremner
@ 2016-12-15 13:01 ` David Bremner
0 siblings, 0 replies; 18+ messages in thread
From: David Bremner @ 2016-12-15 13:01 UTC (permalink / raw)
To: Tomi Ollila, Maarten Aertsen, Notmuch Mail
David Bremner <david@tethera.net> writes:
> ---
> doc/man1/notmuch-insert.rst | 7 +++++++
series pushed to release and master
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2016-12-15 13:01 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-01-24 15:48 notmuch insert --keep fails in parallel w/ notmuch new Maarten Aertsen
2016-01-26 15:09 ` David Bremner
2016-02-07 21:13 ` [PATCH] cli: avoid non-zero exits in notmuch insert --keep Maarten Aertsen
2016-02-08 11:56 ` David Bremner
2016-11-28 12:16 ` [PATCH 1/2] cli/insert: delay database open until after writing mail file David Bremner
2016-11-28 12:16 ` [PATCH 2/2] cli/insert: return EX_TEMPFAIL for some errors David Bremner
2016-11-28 12:22 ` David Bremner
2016-11-28 22:12 ` v2 of insert tempfail series David Bremner
2016-11-28 22:12 ` [PATCH 1/3] test: gdb insert: redirect input inside gdb script David Bremner
2016-11-28 22:12 ` [PATCH 2/3] cli/insert: delay database open until after writing mail file David Bremner
2016-11-28 22:12 ` [PATCH 3/3] cli/insert: return EX_TEMPFAIL for some errors David Bremner
2016-12-04 20:51 ` v2 of insert tempfail series Tomi Ollila
2016-12-07 11:27 ` NEWS/docs for insert tempfail changes David Bremner
2016-12-07 11:27 ` [PATCH 1/2] cli/insert: document the use of EX_TEMPFAIL David Bremner
2016-12-15 13:01 ` David Bremner
2016-12-07 11:27 ` [PATCH 2/2] NEWS: news for notmuch-insert error handling David Bremner
2016-12-10 19:26 ` NEWS/docs for insert tempfail changes Tomi Ollila
2016-11-28 12:23 ` [PATCH 1/2] cli/insert: delay database open until after writing mail file David Bremner
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).