From: David Bremner <david@tethera.net>
To: notmuch@notmuchmail.org
Subject: [Patch v2.5 3/4] cli/insert: require succesful message indexing for
	success status
Date: Fri,  3 Oct 2014 23:18:59 +0200
Message-Id: <1412371140-21051-4-git-send-email-david@tethera.net>
In-Reply-To: <1412371140-21051-1-git-send-email-david@tethera.net>
References: <1412371140-21051-1-git-send-email-david@tethera.net>
Precedence: list

From: Jani Nikula <jani@nikula.org>

Add --keep option to keep any remaining stuff in index or file. We
could distinguish between failures to index and failures to apply tags
or maildir sync, but for simplicity just have one.
---
 doc/man1/notmuch-insert.rst | 19 ++++++++++++-------
 notmuch-insert.c            | 36 ++++++++++++++++++++++++++++++------
 test/T070-insert.sh         |  3 +--
 3 files changed, 43 insertions(+), 15 deletions(-)

diff --git a/doc/man1/notmuch-insert.rst b/doc/man1/notmuch-insert.rst
index 2be1a7b..e396f6c 100644
--- a/doc/man1/notmuch-insert.rst
+++ b/doc/man1/notmuch-insert.rst
@@ -38,16 +38,21 @@ Supported options for **insert** include
         does not exist. Otherwise the folder must already exist for mail
         delivery to succeed.
 
+    ``--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.
+
 EXIT STATUS
 ===========
 
-This command returns exit status 0 if the message was successfully added
-to the mail directory, even if the message could not be indexed and
-added to the notmuch database. In the latter case, a warning will be
-printed to standard error but the message file will be left on disk.
-
-If the message could not be written to disk then a non-zero exit status
-is returned.
+This command returns exit status 0 on succesful mail delivery,
+non-zero otherwise. The default is to indicate failed mail delivery on
+any errors, including message file delivery to the filesystem, message
+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.
 
 SEE ALSO
 ========
diff --git a/notmuch-insert.c b/notmuch-insert.c
index 0ea4380..7074077 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -443,6 +443,7 @@ add_file (notmuch_database_t *notmuch, const char *path, tag_op_list_t *tag_ops,
 int
 notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
 {
+    notmuch_status_t status, close_status;
     notmuch_database_t *notmuch;
     struct sigaction action;
     const char *db_path;
@@ -452,6 +453,7 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
     char *query_string = NULL;
     const char *folder = NULL;
     notmuch_bool_t create_folder = FALSE;
+    notmuch_bool_t keep = FALSE;
     notmuch_bool_t synchronize_flags;
     const char *maildir;
     char *newpath;
@@ -461,6 +463,7 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
     notmuch_opt_desc_t options[] = {
 	{ NOTMUCH_OPT_STRING, &folder, "folder", 0, 0 },
 	{ NOTMUCH_OPT_BOOLEAN, &create_folder, "create-folder", 0, 0 },
+	{ NOTMUCH_OPT_BOOLEAN, &keep, "keep", 0, 0 },
 	{ NOTMUCH_OPT_END, 0, 0, 0, 0 }
     };
 
@@ -535,11 +538,32 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
 	return EXIT_FAILURE;
     }
 
-    /* Add the message to the index.
-     * Even if adding the message to the notmuch database fails,
-     * the message is on disk and we consider the delivery completed. */
-    add_file (notmuch, newpath, tag_ops, synchronize_flags, TRUE);
+    /* 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) {
+	if (keep) {
+	    status = NOTMUCH_STATUS_SUCCESS;
+	} else {
+	    /* If maildir flag sync failed, this might fail. */
+	    if (unlink (newpath)) {
+		fprintf (stderr, "Warning: failed to remove '%s' from maildir "
+			 "after errors: %s. Please run 'notmuch new' to fix.\n",
+			 newpath, strerror (errno));
+	    }
+	}
+    }
 
-    notmuch_database_destroy (notmuch);
-    return EXIT_SUCCESS;
+    return status ? EXIT_FAILURE : EXIT_SUCCESS;
 }
diff --git a/test/T070-insert.sh b/test/T070-insert.sh
index 40a7636..be8060e 100755
--- a/test/T070-insert.sh
+++ b/test/T070-insert.sh
@@ -23,7 +23,7 @@ test_expect_code 1 "Insert zero-length file" \
 
 # This test is a proxy for other errors that may occur while trying to
 # add a message to the notmuch database, e.g. database locked.
-test_expect_code 0 "Insert non-message" \
+test_expect_code 1 "Insert non-message" \
     "echo bad_message | notmuch insert"
 
 test_begin_subtest "Database empty so far"
@@ -199,7 +199,6 @@ end
 run
 EOF
 test_begin_subtest "error exit when add_message returns $code"
-test_subtest_known_broken
 gdb --batch-silent --return-child-result -x index-file-$code.gdb \
     --args notmuch insert  < $gen_msg_filename
 test_expect_equal $? 1
-- 
2.1.0