fix deprecation warnings for notmuch_message_get_flag

fix deprecation warnings for notmuch_message_get_flag

[David Bremner]

I decided before commiting to a style of error return for existing
Boolean API functions I should try it out. I eliminated all of the
deprecation warnings for notmuch_message_get_flag. Honestly the worst
part was dealing with interprocedural error propagation, I don't thing
the choice of error return matters that much.

This is intended to apply on top of the series

     id:20200704151805.3717715-1-david@tethera.net

[PATCH 1/5] cli/search: replace deprecated notmuch_message_get_flag

[David Bremner]

Our handling of errors is all or nothing here, but it's an improvement
on the status quo, and it avoids rippling internal API changes.
---
 notmuch-search.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/notmuch-search.c b/notmuch-search.c
index fd0b58c5..2805d960 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -90,9 +90,13 @@ get_thread_query (notmuch_thread_t *thread,
 	 notmuch_messages_move_to_next (messages)) {
 	notmuch_message_t *message = notmuch_messages_get (messages);
 	const char *mid = notmuch_message_get_message_id (message);
+	notmuch_bool_t is_set;
+	char **buf;
+
+	if (notmuch_message_get_flag_st (message, NOTMUCH_MESSAGE_FLAG_MATCH, &is_set))
+	    return -1;
 	/* Determine which query buffer to extend */
-	char **buf = notmuch_message_get_flag (
-	    message, NOTMUCH_MESSAGE_FLAG_MATCH) ? matched_out : unmatched_out;
+	buf = is_set ? matched_out : unmatched_out;
 	/* Add this message's id: query.  Since "id" is an exclusive
 	 * prefix, it is implicitly 'or'd together, so we only need to
 	 * join queries with a space. */
-- 
2.27.0

[PATCH 2/5] cli/show: replace deprecated notmuch_message_get_flag

[David Bremner]

This can be seen as moving an abort out of the library, into the CLI
where we can both print to stderr and shut the process down without
ill effect.
---
 notmuch-show.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index 36265043..dd836add 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -80,6 +80,18 @@ _get_disposition (GMimeObject *meta)
     return g_mime_content_disposition_get_disposition (disposition);
 }
 
+static bool _get_message_flag (notmuch_message_t *message, notmuch_message_flag_t flag) {
+    notmuch_bool_t is_set;
+    notmuch_status_t status;
+
+    status = notmuch_message_get_flag_st (message, flag, &is_set);
+
+    if (print_status_message ("notmuch show", message, status))
+	INTERNAL_ERROR("unexpected error getting message flag\n");
+
+    return is_set;
+}
+
 /* Emit a sequence of key/value pairs for the metadata of message.
  * The caller should begin a map before calling this. */
 static void
@@ -97,10 +109,10 @@ format_message_sprinter (sprinter_t *sp, notmuch_message_t *message)
     sp->string (sp, notmuch_message_get_message_id (message));
 
     sp->map_key (sp, "match");
-    sp->boolean (sp, notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH));
+    sp->boolean (sp, _get_message_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH));
 
     sp->map_key (sp, "excluded");
-    sp->boolean (sp, notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED));
+    sp->boolean (sp, _get_message_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED));
 
     sp->map_key (sp, "filename");
     if (notmuch_format_version >= 3) {
@@ -507,8 +519,8 @@ format_part_text (const void *ctx, sprinter_t *sp, mime_node_t *node,
 			      part_type,
 			      notmuch_message_get_message_id (message),
 			      indent,
-			      notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH) ? 1 : 0,
-			      notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED) ? 1 : 0,
+			      _get_message_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH) ? 1 : 0,
+			      _get_message_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED) ? 1 : 0,
 			      notmuch_message_get_filename (message));
     } else {
 	char *content_string;
@@ -1002,8 +1014,8 @@ show_messages (void *ctx,
 
 	message = notmuch_messages_get (messages);
 
-	match = notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH);
-	excluded = notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED);
+	match = _get_message_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH);
+	excluded = _get_message_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED);
 
 	next_indent = indent;
 
@@ -1143,7 +1155,7 @@ do_show_unthreaded (void *ctx,
 	message = notmuch_messages_get (messages);
 
 	notmuch_message_set_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH, TRUE);
-	excluded = notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED);
+	excluded = _get_message_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED);
 
 	if (!excluded || !params->omit_excluded) {
 	    status = show_message (ctx, format, sp, message, 0, params);
-- 
2.27.0

[PATCH 3/5] lib/add-message: drop use of deprecated notmuch_message_get_flag.

[David Bremner]

As a side effect, we revert the switch from notmuch_bool_t to bool
here. This is because those two types are not actually compatible when
passing by reference.
---
 lib/add-message.cc | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/add-message.cc b/lib/add-message.cc
index 8c92689b..9dd4b697 100644
--- a/lib/add-message.cc
+++ b/lib/add-message.cc
@@ -477,7 +477,7 @@ notmuch_database_index_file (notmuch_database_t *notmuch,
     notmuch_message_t *message = NULL;
     notmuch_status_t ret = NOTMUCH_STATUS_SUCCESS, ret2;
     notmuch_private_status_t private_status;
-    bool is_ghost = false, is_new = false;
+    notmuch_bool_t is_ghost = false, is_new = false;
     notmuch_indexopts_t *def_indexopts = NULL;
 
     const char *date;
@@ -525,7 +525,9 @@ notmuch_database_index_file (notmuch_database_t *notmuch,
 	    is_new = true;
 	    break;
 	case NOTMUCH_PRIVATE_STATUS_SUCCESS:
-	    is_ghost = notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_GHOST);
+	    ret = notmuch_message_get_flag_st (message, NOTMUCH_MESSAGE_FLAG_GHOST, &is_ghost);
+	    if (ret)
+		goto DONE;
 	    is_new = false;
 	    break;
 	default:
-- 
2.27.0

[PATCH 4/5] lib/thread: replace use of deprecated notmuch_message_get_flag

[David Bremner]

This adds one more reason why _notmuch_thread_create might return
NULL, but those were not previously enumerated, so no promises are
broken.
---
 lib/thread.cc | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/lib/thread.cc b/lib/thread.cc
index 6073e45c..17346008 100644
--- a/lib/thread.cc
+++ b/lib/thread.cc
@@ -351,14 +351,16 @@ _thread_set_subject_from_message (notmuch_thread_t *thread,
 /* Add a message to this thread which is known to match the original
  * search specification. The 'sort' parameter controls whether the
  * oldest or newest matching subject is applied to the thread as a
- * whole. */
-static void
+ * whole. Returns 0 on success.
+ */
+static int
 _thread_add_matched_message (notmuch_thread_t *thread,
 			     notmuch_message_t *message,
 			     notmuch_sort_t sort)
 {
     time_t date;
     notmuch_message_t *hashed_message;
+    notmuch_bool_t is_set;
 
     date = notmuch_message_get_date (message);
 
@@ -375,7 +377,9 @@ _thread_add_matched_message (notmuch_thread_t *thread,
 	    _thread_set_subject_from_message (thread, message);
     }
 
-    if (! notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED))
+    if (notmuch_message_get_flag_st (message, NOTMUCH_MESSAGE_FLAG_EXCLUDED, &is_set))
+	return -1;
+    if (! is_set)
 	thread->matched_messages++;
 
     if (g_hash_table_lookup_extended (thread->message_hash,
@@ -386,6 +390,7 @@ _thread_add_matched_message (notmuch_thread_t *thread,
     }
 
     _thread_add_matched_author (thread, _notmuch_message_get_author (hashed_message));
+    return 0;
 }
 
 static bool
@@ -625,7 +630,10 @@ _notmuch_thread_create (void *ctx,
 
 	if ( _notmuch_doc_id_set_contains (match_set, doc_id)) {
 	    _notmuch_doc_id_set_remove (match_set, doc_id);
-	    _thread_add_matched_message (thread, message, sort);
+	    if (_thread_add_matched_message (thread, message, sort)) {
+		thread = NULL;
+		goto DONE;
+	    }
 	}
 
 	_notmuch_message_close (message);
-- 
2.27.0

[PATCH 5/5] bindings/ruby: replacy use of deprecated notmuch_message_get_flag

[David Bremner]

Depending on the flag, this actually can return an errror, so raise a
ruby exception if so.
---
 bindings/ruby/message.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/bindings/ruby/message.c b/bindings/ruby/message.c
index c55cf6e2..6ea82afa 100644
--- a/bindings/ruby/message.c
+++ b/bindings/ruby/message.c
@@ -137,13 +137,18 @@ VALUE
 notmuch_rb_message_get_flag (VALUE self, VALUE flagv)
 {
     notmuch_message_t *message;
+    notmuch_bool_t is_set;
+    notmuch_status_t status;
 
     Data_Get_Notmuch_Message (self, message);
 
     if (!FIXNUM_P (flagv))
 	rb_raise (rb_eTypeError, "Flag not a Fixnum");
 
-    return notmuch_message_get_flag (message, FIX2INT (flagv)) ? Qtrue : Qfalse;
+    status = notmuch_message_get_flag_st (message, FIX2INT (flagv), &is_set);
+    notmuch_rb_status_raise (status);
+
+    return is_set ? Qtrue : Qfalse;
 }
 
 /*
-- 
2.27.0

[PATCH] cli/new: replace newly deprecated n_m_has_maildir_flag

[David Bremner]

Boolean return values have no out-of-band-values to signal errors. The
change here is that a (somewhat unlikely) fatal error after indexing
will now be caught.
---
 notmuch-new.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/notmuch-new.c b/notmuch-new.c
index f079f62a..4075d395 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -388,8 +388,11 @@ add_file (notmuch_database_t *notmuch, const char *filename,
 	    notmuch_message_maildir_flags_to_tags (message);
 
 	for (tag = state->new_tags; *tag != NULL; tag++) {
-	    if (strcmp ("unread", *tag) != 0 ||
-		! notmuch_message_has_maildir_flag (message, 'S')) {
+	    notmuch_bool_t is_set;
+	    /* Currently all errors from has_maildir_flag are fatal */
+	    if ((status = notmuch_message_has_maildir_flag_st (message, 'S', &is_set)))
+		goto DONE;
+	    if (strcmp ("unread", *tag) != 0 || ! is_set) {
 		notmuch_message_add_tag (message, *tag);
 	    }
 	}
-- 
2.27.0

Re: [PATCH] cli/new: replace newly deprecated n_m_has_maildir_flag

[David Bremner]

David Bremner <david@tethera.net> writes:

> Boolean return values have no out-of-band-values to signal errors. The
> change here is that a (somewhat unlikely) fatal error after indexing
> will now be caught.

applied to master.

d

Re: fix deprecation warnings for notmuch_message_get_flag

[David Bremner]

David Bremner <david@tethera.net> writes:

> I decided before commiting to a style of error return for existing
> Boolean API functions I should try it out. I eliminated all of the
> deprecation warnings for notmuch_message_get_flag. Honestly the worst
> part was dealing with interprocedural error propagation, I don't thing
> the choice of error return matters that much.
>
> This is intended to apply on top of the series
>
>      id:20200704151805.3717715-1-david@tethera.net

Patches 1-5 merged to master. Note that this does generate a couple of
deprecation warnings in bindings/python-cffi. I didn't yet figure out
all the changes needed to introduce and use a new function in the new
bindings.