Re: [PATCH] cli: avoid non-zero exits in notmuch insert --keep

[David Bremner]

Maarten Aertsen  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




___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch

[PATCH] cli: avoid non-zero exits in notmuch insert --keep

[Maarten Aertsen]

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 
 #include 
 
+#include 
+
 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
___
notmuch mailing list
notmuch@notmuchmail.org
https://notmuchmail.org/mailman/listinfo/notmuch