[PATCH 1/3] cli: add utility routine to print error status.

[PATCH 1/3] cli: add utility routine to print error status.

[David Bremner]

No attention to formatting here, initially just focus on getting the
relevant strings out of the library.
---
 Makefile.local   |  1 +
 notmuch-client.h |  9 +++++++++
 status.c         | 21 +++++++++++++++++++++
 3 files changed, 31 insertions(+)
 create mode 100644 status.c

diff --git a/Makefile.local b/Makefile.local
index 61a9c4c..d58cea0 100644
--- a/Makefile.local
+++ b/Makefile.local
@@ -271,6 +271,7 @@ dataclean: distclean
 notmuch_client_srcs =		\
 	command-line-arguments.c\
 	debugger.c		\
+	status.c		\
 	gmime-filter-reply.c	\
 	hooks.c			\
 	notmuch.c		\
diff --git a/notmuch-client.h b/notmuch-client.h
index 882aa30..de8a3b1 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -449,6 +449,15 @@ notmuch_database_dump (notmuch_database_t *notmuch,
 		       dump_format_t output_format,
 		       notmuch_bool_t gzip_output);
 
+/* If status is non-zero (i.e. error) print appropriate
+   messages to stderr.
+*/
+
+notmuch_status_t
+print_status_query (const char *loc,
+		    const notmuch_query_t *query,
+		    notmuch_status_t status);
+
 #include "command-line-arguments.h"
 
 extern char *notmuch_requested_db_uuid;
diff --git a/status.c b/status.c
new file mode 100644
index 0000000..8fa81cb
--- /dev/null
+++ b/status.c
@@ -0,0 +1,21 @@
+#include "notmuch-client.h"
+
+notmuch_status_t
+print_status_query (const char *loc,
+		    const notmuch_query_t *query,
+		    notmuch_status_t status)
+{
+    if (status) {
+	const char *msg;
+	notmuch_database_t *notmuch;
+
+	fprintf (stderr, "%s: %s\n", loc,
+		 notmuch_status_to_string (status));
+
+	notmuch = notmuch_query_get_database (query);
+	msg = notmuch_database_status_string (notmuch);
+	if (msg)
+	    fputs (msg, stderr);
+    }
+    return status;
+}
-- 
2.5.1

[PATCH 2/3] cli/count: update to use notmuch_query_search_messages_st

[David Bremner]

This brings back status information that may have been hidden by the
great library logging conversion.

Note the change of the internal API / return-value for count_files The
other count calls to the lib will also get error handling when that
API is updated in the lib.
---
 notmuch-count.c    | 22 ++++++++++++++++------
 test/T060-count.sh | 30 ++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 6 deletions(-)

diff --git a/notmuch-count.c b/notmuch-count.c
index f26e726..65e1f4e 100644
--- a/notmuch-count.c
+++ b/notmuch-count.c
@@ -34,17 +34,19 @@ enum {
     EXCLUDE_FALSE,
 };
 
-static unsigned int
+/* Return the number of files matching the query, or -1 for an error */
+static int
 count_files (notmuch_query_t *query)
 {
     notmuch_messages_t *messages;
     notmuch_message_t *message;
     notmuch_filenames_t *filenames;
-    unsigned int count = 0;
+    notmuch_status_t status;
+    int count = 0;
 
-    messages = notmuch_query_search_messages (query);
-    if (messages == NULL)
-	return 0;
+    status = notmuch_query_search_messages_st (query, &messages);
+    if (print_status_query ("notmuch count", query, status))
+	return -1;
 
     for (;
 	 notmuch_messages_valid (messages);
@@ -72,6 +74,7 @@ print_count (notmuch_database_t *notmuch, const char *query_str,
 {
     notmuch_query_t *query;
     size_t i;
+    int count;
     unsigned long revision;
     const char *uuid;
     int ret = 0;
@@ -93,7 +96,13 @@ print_count (notmuch_database_t *notmuch, const char *query_str,
 	printf ("%u", notmuch_query_count_threads (query));
 	break;
     case OUTPUT_FILES:
-	printf ("%u", count_files (query));
+	count = count_files (query);
+	if (count >= 0) {
+	    printf ("%u", count);
+	} else {
+	    ret = -1;
+	    goto DONE;
+	}
 	break;
     }
 
@@ -104,6 +113,7 @@ print_count (notmuch_database_t *notmuch, const char *query_str,
 	fputs ("\n", stdout);
     }
 
+  DONE:
     notmuch_query_destroy (query);
 
     return ret;
diff --git a/test/T060-count.sh b/test/T060-count.sh
index 5ef3879..3fec94e 100755
--- a/test/T060-count.sh
+++ b/test/T060-count.sh
@@ -93,5 +93,35 @@ notmuch count --output=messages >>EXPECTED
 notmuch count --output=messages tag:inbox >>EXPECTED
 test_expect_equal_file EXPECTED OUTPUT
 
+backup_database
+test_begin_subtest "error message for database open"
+dd if=/dev/zero of="${MAIL_DIR}/.notmuch/xapian/postlist.DB" count=3
+notmuch count '*' 2>OUTPUT 1>/dev/null
+output=$(sed 's/^\(A Xapian exception [^:]*\):.*$/\1/' OUTPUT)
+test_expect_equal "${output}" "A Xapian exception occurred opening database"
+restore_database
+
+cat <<EOF > count-files.gdb
+set breakpoint pending on
+break count_files
+commands
+shell cp /dev/null ${MAIL_DIR}/.notmuch/xapian/postlist.DB
+continue
+end
+run
+EOF
+
+backup_database
+test_begin_subtest "error message from query_search_messages"
+gdb --batch-silent --return-child-result -x count-files.gdb \
+    --args notmuch count --output=files '*' 2>OUTPUT 1>/dev/null
+cat <<EOF > EXPECTED
+notmuch count: A Xapian exception occurred
+A Xapian exception occurred performing query
+Query string was: *
+EOF
+sed 's/^\(A Xapian exception [^:]*\):.*$/\1/' < OUTPUT > OUTPUT.clean
+test_expect_equal_file EXPECTED OUTPUT.clean
+restore_database
 
 test_done
-- 
2.5.1

[PATCH 3/3] cli: convert remainder of CLI to n_q_search_{messages,threads}_st

[David Bremner]

I think it would be no real problem to cut and paste the gdb based
error message test from count to the other clients modified here, but
I'm not currently convinced it's worth the trouble since the code path
being tested is almost the the same, and the tests are relatively
heavyweight.
---
 notmuch-dump.c   |  7 ++++++-
 notmuch-reply.c  | 20 +++++++++++++++++---
 notmuch-search.c | 15 +++++++++------
 notmuch-show.c   | 10 +++++++---
 notmuch-tag.c    |  8 +++++++-
 5 files changed, 46 insertions(+), 14 deletions(-)

diff --git a/notmuch-dump.c b/notmuch-dump.c
index 24fc2f2..829781f 100644
--- a/notmuch-dump.c
+++ b/notmuch-dump.c
@@ -48,8 +48,13 @@ database_dump_file (notmuch_database_t *notmuch, gzFile output,
 
     char *buffer = NULL;
     size_t buffer_size = 0;
+    notmuch_status_t status;
 
-    for (messages = notmuch_query_search_messages (query);
+    status = notmuch_query_search_messages_st (query, &messages);
+    if (print_status_query ("notmuch dump", query, status))
+	return EXIT_FAILURE;
+
+    for (;
 	 notmuch_messages_valid (messages);
 	 notmuch_messages_move_to_next (messages)) {
 	int first = 1;
diff --git a/notmuch-reply.c b/notmuch-reply.c
index 7c5c28f..fd6a1ec 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -606,8 +606,13 @@ notmuch_reply_format_default(void *ctx,
     notmuch_messages_t *messages;
     notmuch_message_t *message;
     mime_node_t *root;
+    notmuch_status_t status;
 
-    for (messages = notmuch_query_search_messages (query);
+    status = notmuch_query_search_messages_st (query, &messages);
+    if (print_status_query ("notmuch reply", query, status))
+	return 1;
+
+    for (;
 	 notmuch_messages_valid (messages);
 	 notmuch_messages_move_to_next (messages))
     {
@@ -650,13 +655,17 @@ notmuch_reply_format_sprinter(void *ctx,
     notmuch_messages_t *messages;
     notmuch_message_t *message;
     mime_node_t *node;
+    notmuch_status_t status;
 
     if (notmuch_query_count_messages (query) != 1) {
 	fprintf (stderr, "Error: search term did not match precisely one message.\n");
 	return 1;
     }
 
-    messages = notmuch_query_search_messages (query);
+    status = notmuch_query_search_messages_st (query, &messages);
+    if (print_status_query ("notmuch reply", query, status))
+	return 1;
+
     message = notmuch_messages_get (messages);
     if (mime_node_open (ctx, message, &(params->crypto), &node) != NOTMUCH_STATUS_SUCCESS)
 	return 1;
@@ -698,8 +707,13 @@ notmuch_reply_format_headers_only(void *ctx,
     notmuch_message_t *message;
     const char *in_reply_to, *orig_references, *references;
     char *reply_headers;
+    notmuch_status_t status;
+
+    status = notmuch_query_search_messages_st (query, &messages);
+    if (print_status_query ("notmuch reply", query, status))
+	return 1;
 
-    for (messages = notmuch_query_search_messages (query);
+    for (;
 	 notmuch_messages_valid (messages);
 	 notmuch_messages_move_to_next (messages))
     {
diff --git a/notmuch-search.c b/notmuch-search.c
index 3076c3f..bade114 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -111,6 +111,7 @@ do_search_threads (search_context_t *ctx)
     sprinter_t *format = ctx->format;
     time_t date;
     int i;
+    notmuch_status_t status;
 
     if (ctx->offset < 0) {
 	ctx->offset += notmuch_query_count_threads (ctx->query);
@@ -118,8 +119,8 @@ do_search_threads (search_context_t *ctx)
 	    ctx->offset = 0;
     }
 
-    threads = notmuch_query_search_threads (ctx->query);
-    if (threads == NULL)
+    status = notmuch_query_search_threads_st (ctx->query, &threads);
+    if (print_status_query("notmuch search", ctx->query, status))
 	return 1;
 
     format->begin_list (format);
@@ -412,6 +413,7 @@ do_search_messages (search_context_t *ctx)
     notmuch_filenames_t *filenames;
     sprinter_t *format = ctx->format;
     int i;
+    notmuch_status_t status;
 
     if (ctx->offset < 0) {
 	ctx->offset += notmuch_query_count_messages (ctx->query);
@@ -419,8 +421,8 @@ do_search_messages (search_context_t *ctx)
 	    ctx->offset = 0;
     }
 
-    messages = notmuch_query_search_messages (ctx->query);
-    if (messages == NULL)
+    status = notmuch_query_search_messages_st (ctx->query, &messages);
+    if (print_status_query ("notmuch search", ctx->query, status))
 	return 1;
 
     format->begin_list (format);
@@ -508,8 +510,9 @@ do_search_tags (const search_context_t *ctx)
     if (strcmp (notmuch_query_get_query_string (query), "*") == 0) {
 	tags = notmuch_database_get_all_tags (notmuch);
     } else {
-	messages = notmuch_query_search_messages (query);
-	if (messages == NULL)
+	notmuch_status_t status;
+	status = notmuch_query_search_messages_st (query, &messages);
+	if (print_status_query ("notmuch search", query, status))
 	    return 1;
 
 	tags = notmuch_messages_collect_tags (messages);
diff --git a/notmuch-show.c b/notmuch-show.c
index 4356025..e054808 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -895,13 +895,17 @@ do_show_single (void *ctx,
 {
     notmuch_messages_t *messages;
     notmuch_message_t *message;
+    notmuch_status_t status;
 
     if (notmuch_query_count_messages (query) != 1) {
 	fprintf (stderr, "Error: search term did not match precisely one message.\n");
 	return 1;
     }
 
-    messages = notmuch_query_search_messages (query);
+    status = notmuch_query_search_messages_st (query, &messages);
+    if (print_status_query ("notmuch show", query, status))
+	return 1;
+
     message = notmuch_messages_get (messages);
 
     if (message == NULL) {
@@ -928,8 +932,8 @@ do_show (void *ctx,
     notmuch_messages_t *messages;
     notmuch_status_t status, res = NOTMUCH_STATUS_SUCCESS;
 
-    threads = notmuch_query_search_threads (query);
-    if (! threads)
+    status= notmuch_query_search_threads_st (query, &threads);
+    if (print_status_query ("notmuch show", query, status))
 	return 1;
 
     sp->begin_list (sp);
diff --git a/notmuch-tag.c b/notmuch-tag.c
index 7ae98f6..c020cb6 100644
--- a/notmuch-tag.c
+++ b/notmuch-tag.c
@@ -97,6 +97,8 @@ tag_query (void *ctx, notmuch_database_t *notmuch, const char *query_string,
     notmuch_query_t *query;
     notmuch_messages_t *messages;
     notmuch_message_t *message;
+    notmuch_status_t status;
+
     int ret = NOTMUCH_STATUS_SUCCESS;
 
     if (! (flags & TAG_FLAG_REMOVE_ALL)) {
@@ -119,7 +121,11 @@ tag_query (void *ctx, notmuch_database_t *notmuch, const char *query_string,
     /* tagging is not interested in any special sort order */
     notmuch_query_set_sort (query, NOTMUCH_SORT_UNSORTED);
 
-    for (messages = notmuch_query_search_messages (query);
+    status = notmuch_query_search_messages_st (query, &messages);
+    if (print_status_query ("notmuch tag", query, status))
+	return status;
+
+    for (;
 	 notmuch_messages_valid (messages) && ! interrupted;
 	 notmuch_messages_move_to_next (messages)) {
 	message = notmuch_messages_get (messages);
-- 
2.5.1

Re: [PATCH 2/3] cli/count: update to use notmuch_query_search_messages_st

[David Bremner]

David Bremner <david@tethera.net> writes:

> This brings back status information that may have been hidden by the
> great library logging conversion.

pushed 1 and 2

Re: [PATCH 3/3] cli: convert remainder of CLI to n_q_search_{messages,threads}_st

[David Bremner]

David Bremner <david@tethera.net> writes:

> I think it would be no real problem to cut and paste the gdb based
> error message test from count to the other clients modified here, but
> I'm not currently convinced it's worth the trouble since the code path
> being tested is almost the the same, and the tests are relatively
> heavyweight.

I have pushed this commit to master. The changes are hopefully pretty
straightforward.

d