From: Jani Nikula <jani@nikula.org>
To: notmuch@notmuchmail.org
Subject: [PATCH v2 2/3] cli/insert: require succesful message indexing for
	success status
Date: Sun, 28 Sep 2014 17:40:58 +0300
Message-Id:
 <627e789a0574d93b66b3e935f36328661df4e99d.1411914914.git.jani@nikula.org>
In-Reply-To: <cover.1411914914.git.jani@nikula.org>
References: <cover.1411914914.git.jani@nikula.org>
In-Reply-To: <cover.1411914914.git.jani@nikula.org>
References: <cover.1411914914.git.jani@nikula.org>
Precedence: list

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         |  2 +-
 3 files changed, 43 insertions(+), 14 deletions(-)

diff --git a/doc/man1/notmuch-insert.rst b/doc/man1/notmuch-insert.rst
index 2be1a7b8841c..e396f6cf2279 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 0ea438015dbe..7074077ca699 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 ea9db07e2fa2..aacc643b7288 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"
-- 
2.1.0