v3 of pre-gmime-3.0 cleanup

v3 of pre-gmime-3.0 cleanup

[David Bremner]

This obsoletes [1]. Since that series, a bit of the actual port to
gmime-3.0 has crept in, in order to be able to mark tests as broken
with respect a particular gmime version.

The series in progress [2] has grown to 21 patches, and this is
roughly the first half. Currently there about 7 failing tests (not
counting the 5 marked broken) all crypto related. Basically my
quick-and-dirty translation of crypto status is not really close
enough, and probably can't really be perfect because the old version
reported raw error numbers in json/sexp output. That's probably wrong,
and certainly unportable between gmime major versions.

There are 3 sub-series here.

1) eliminate mixing gmime stream output with raw printfs

[Patch v3 01/11] util: convenience function to create gmime stream
[Patch v3 02/11] cli/reply: direct all output for text format to
[Patch v3 03/11] cli/show: use single stream for printf / gmime

2) Fix some memory leaks

[Patch v3 04/11] perf-test: add memory test for reply
[Patch v3 05/11] cli/reply: fix two memory leaks, document a third

3) Update test suite
[Patch v3 06/11] test/thread-naming: remove excess escaping from
...
[Patch v3 11/11] test: mark test as broken in gmime 3.0


[1]: id:20170523005351.15431-1-david@tethera.net
[2]: http://pivot.cs.unb.ca/git?p=notmuch.git;a=shortlog;h=refs/heads/gmime-3.0

[Patch v3 01/11] util: convenience function to create gmime stream for stdout

[David Bremner]

It turns out that our use of GMimeStreamPipe has only succeeded
because gmime has been ignoring some seek failures; this will no
longer be the case in gmime 3.0, so we use a GMimeStreamPipe, which
does not assume seekability, wrapped in a buffering stream.
---
 util/Makefile.local |  2 +-
 util/gmime-extra.c  | 20 ++++++++++++++++++++
 util/gmime-extra.h  |  6 ++++++
 3 files changed, 27 insertions(+), 1 deletion(-)
 create mode 100644 util/gmime-extra.c
 create mode 100644 util/gmime-extra.h

diff --git a/util/Makefile.local b/util/Makefile.local
index a6962d49..3027880b 100644
--- a/util/Makefile.local
+++ b/util/Makefile.local
@@ -5,7 +5,7 @@ extra_cflags += -I$(srcdir)/$(dir)
 
 libnotmuch_util_c_srcs := $(dir)/xutil.c $(dir)/error_util.c $(dir)/hex-escape.c \
 		  $(dir)/string-util.c $(dir)/talloc-extra.c $(dir)/zlib-extra.c \
-		$(dir)/util.c
+		$(dir)/util.c $(dir)/gmime-extra.c
 
 libnotmuch_util_modules := $(libnotmuch_util_c_srcs:.c=.o)
 
diff --git a/util/gmime-extra.c b/util/gmime-extra.c
new file mode 100644
index 00000000..f1538587
--- /dev/null
+++ b/util/gmime-extra.c
@@ -0,0 +1,20 @@
+#include "gmime-extra.h"
+
+GMimeStream *
+g_mime_stream_stdout_new()
+{
+    GMimeStream *stream_stdout = NULL;
+    GMimeStream *stream_buffered = NULL;
+
+    stream_stdout = g_mime_stream_pipe_new (STDOUT_FILENO);
+    if (!stream_stdout)
+	return NULL;
+
+    g_mime_stream_pipe_set_owner (GMIME_STREAM_PIPE (stream_stdout), FALSE);
+
+    stream_buffered = g_mime_stream_buffer_new (stream_stdout, GMIME_STREAM_BUFFER_BLOCK_WRITE);
+
+    g_object_unref (stream_stdout);
+
+    return stream_buffered;
+}
diff --git a/util/gmime-extra.h b/util/gmime-extra.h
new file mode 100644
index 00000000..e0432a94
--- /dev/null
+++ b/util/gmime-extra.h
@@ -0,0 +1,6 @@
+#ifndef _GMIME_EXTRA_H
+#define _GMIME_EXTRA_H
+#include <gmime/gmime.h>
+
+GMimeStream *g_mime_stream_stdout_new(void);
+#endif
-- 
2.11.0

[Patch v3 02/11] cli/reply: direct all output for text format to gmime stream

[David Bremner]

Interleaving printfs with writes to the gmime stream worked when the
gmime stream was backed by the FILE *stdout, but that is no longer the
case.  Create one stream and pass it into the two functions where
needed, as well well as replacing printfs with g_mime_stream_printf.
---
 notmuch-client.h |  2 +-
 notmuch-reply.c  | 63 +++++++++++++++++++++++++++-----------------------------
 2 files changed, 31 insertions(+), 34 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index a6f70eae..5692caf3 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -29,7 +29,7 @@
 
 #include "compat.h"
 
-#include <gmime/gmime.h>
+#include "gmime-extra.h"
 
 typedef GMimeCryptoContext notmuch_crypto_context_t;
 /* This is automatically included only since gmime 2.6.10 */
diff --git a/notmuch-reply.c b/notmuch-reply.c
index 2fa6e5a3..9239aac2 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -25,47 +25,42 @@
 #include "sprinter.h"
 
 static void
-show_reply_headers (GMimeMessage *message)
+show_reply_headers (GMimeStream *stream, GMimeMessage *message)
 {
-    GMimeStream *stream_stdout = NULL;
-
-    stream_stdout = g_mime_stream_file_new (stdout);
-    if (stream_stdout) {
-	g_mime_stream_file_set_owner (GMIME_STREAM_FILE (stream_stdout), FALSE);
-	/* Output RFC 2822 formatted (and RFC 2047 encoded) headers. */
-	g_mime_object_write_to_stream (GMIME_OBJECT(message), stream_stdout);
-	g_object_unref(stream_stdout);
+    /* Output RFC 2822 formatted (and RFC 2047 encoded) headers. */
+    if (g_mime_object_write_to_stream (GMIME_OBJECT(message), stream) < 0) {
+	INTERNAL_ERROR("failed to write headers to stdout\n");
     }
 }
 
 static void
-format_part_reply (mime_node_t *node)
+format_part_reply (GMimeStream *stream, mime_node_t *node)
 {
     int i;
 
     if (node->envelope_file) {
-	printf ("On %s, %s wrote:\n",
-		notmuch_message_get_header (node->envelope_file, "date"),
-		notmuch_message_get_header (node->envelope_file, "from"));
+	g_mime_stream_printf (stream, "On %s, %s wrote:\n",
+			      notmuch_message_get_header (node->envelope_file, "date"),
+			      notmuch_message_get_header (node->envelope_file, "from"));
     } else if (GMIME_IS_MESSAGE (node->part)) {
 	GMimeMessage *message = GMIME_MESSAGE (node->part);
 	InternetAddressList *recipients;
 	const char *recipients_string;
 
-	printf ("> From: %s\n", g_mime_message_get_sender (message));
+	g_mime_stream_printf (stream, "> From: %s\n", g_mime_message_get_sender (message));
 	recipients = g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_TO);
 	recipients_string = internet_address_list_to_string (recipients, 0);
 	if (recipients_string)
-	    printf ("> To: %s\n",
-		    recipients_string);
+	    g_mime_stream_printf (stream, "> To: %s\n",
+				  recipients_string);
 	recipients = g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_CC);
 	recipients_string = internet_address_list_to_string (recipients, 0);
 	if (recipients_string)
-	    printf ("> Cc: %s\n",
-		    recipients_string);
-	printf ("> Subject: %s\n", g_mime_message_get_subject (message));
-	printf ("> Date: %s\n", g_mime_message_get_date_as_string (message));
-	printf (">\n");
+	    g_mime_stream_printf (stream, "> Cc: %s\n",
+				  recipients_string);
+	g_mime_stream_printf (stream, "> Subject: %s\n", g_mime_message_get_subject (message));
+	g_mime_stream_printf (stream, "> Date: %s\n", g_mime_message_get_date_as_string (message));
+	g_mime_stream_printf (stream, ">\n");
     } else if (GMIME_IS_PART (node->part)) {
 	GMimeContentType *content_type = g_mime_object_get_content_type (node->part);
 	GMimeContentDisposition *disposition = g_mime_object_get_content_disposition (node->part);
@@ -75,24 +70,21 @@ format_part_reply (mime_node_t *node)
 	    /* Ignore PGP/MIME cruft parts */
 	} else if (g_mime_content_type_is_type (content_type, "text", "*") &&
 		   !g_mime_content_type_is_type (content_type, "text", "html")) {
-	    GMimeStream *stream_stdout = g_mime_stream_file_new (stdout);
-	    g_mime_stream_file_set_owner (GMIME_STREAM_FILE (stream_stdout), FALSE);
-	    show_text_part_content (node->part, stream_stdout, NOTMUCH_SHOW_TEXT_PART_REPLY);
-	    g_object_unref(stream_stdout);
+	    show_text_part_content (node->part, stream, NOTMUCH_SHOW_TEXT_PART_REPLY);
 	} else if (disposition &&
 		   strcasecmp (g_mime_content_disposition_get_disposition (disposition),
 			       GMIME_DISPOSITION_ATTACHMENT) == 0) {
 	    const char *filename = g_mime_part_get_filename (GMIME_PART (node->part));
-	    printf ("Attachment: %s (%s)\n", filename,
-		    g_mime_content_type_to_string (content_type));
+	    g_mime_stream_printf (stream, "Attachment: %s (%s)\n", filename,
+				  g_mime_content_type_to_string (content_type));
 	} else {
-	    printf ("Non-text part: %s\n",
-		    g_mime_content_type_to_string (content_type));
+	    g_mime_stream_printf (stream, "Non-text part: %s\n",
+				  g_mime_content_type_to_string (content_type));
 	}
     }
 
     for (i = 0; i < node->nchildren; i++)
-	format_part_reply (mime_node_child (node, i));
+	format_part_reply (stream, mime_node_child (node, i));
 }
 
 typedef enum {
@@ -678,9 +670,14 @@ static int do_reply(notmuch_config_t *config,
 	    /* End */
 	    sp->end (sp);
 	} else {
-	    show_reply_headers (reply);
-	    if (format == FORMAT_DEFAULT)
-		format_part_reply (node);
+	    GMimeStream *stream_stdout = stream_stdout = g_mime_stream_stdout_new ();
+	    if (stream_stdout) {
+		show_reply_headers (stream_stdout, reply);
+		if (format == FORMAT_DEFAULT)
+		    format_part_reply (stream_stdout, node);
+	    }
+	    g_mime_stream_flush (stream_stdout);
+	    g_object_unref(stream_stdout);
 	}
 
 	g_object_unref (G_OBJECT (reply));
-- 
2.11.0

[Patch v3 03/11] cli/show: use single stream for printf / gmime object output

[David Bremner]

This is again motivated by the need to transition away from
GMimeStreamFile for output to stdout.

format_part_mbox is left alone for now, as this cannot be mixed in
with output using gmime object output.
---
 notmuch-client.h |  1 +
 notmuch-show.c   | 70 ++++++++++++++++++++++++++------------------------------
 2 files changed, 33 insertions(+), 38 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index 5692caf3..62d4bcec 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -86,6 +86,7 @@ typedef struct notmuch_show_params {
     int part;
     notmuch_crypto_t crypto;
     notmuch_bool_t include_html;
+    GMimeStream *out_stream;
 } notmuch_show_params_t;
 
 /* There's no point in continuing when we've detected that we've done
diff --git a/notmuch-show.c b/notmuch-show.c
index 7021008e..accea48a 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -426,6 +426,7 @@ format_part_text (const void *ctx, sprinter_t *sp, mime_node_t *node,
 	GMIME_OBJECT (node->envelope_part) : node->part;
     GMimeContentType *content_type = g_mime_object_get_content_type (meta);
     const notmuch_bool_t leaf = GMIME_IS_PART (node->part);
+    GMimeStream *stream = params->out_stream;
     const char *part_type;
     int i;
 
@@ -433,13 +434,13 @@ format_part_text (const void *ctx, sprinter_t *sp, mime_node_t *node,
 	notmuch_message_t *message = node->envelope_file;
 
 	part_type = "message";
-	printf ("\f%s{ id:%s depth:%d match:%d excluded:%d filename:%s\n",
-		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,
-		notmuch_message_get_filename (message));
+	g_mime_stream_printf (stream, "\f%s{ id:%s depth:%d match:%d excluded:%d filename:%s\n",
+			      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,
+			      notmuch_message_get_filename (message));
     } else {
 	char *content_string;
 	const char *disposition = _get_disposition (meta);
@@ -453,14 +454,14 @@ format_part_text (const void *ctx, sprinter_t *sp, mime_node_t *node,
 	else
 	    part_type = "part";
 
-	printf ("\f%s{ ID: %d", part_type, node->part_num);
+	g_mime_stream_printf (stream, "\f%s{ ID: %d", part_type, node->part_num);
 	if (filename)
-	    printf (", Filename: %s", filename);
+	    g_mime_stream_printf (stream, ", Filename: %s", filename);
 	if (cid)
-	    printf (", Content-id: %s", cid);
+	    g_mime_stream_printf (stream, ", Content-id: %s", cid);
 
 	content_string = g_mime_content_type_to_string (content_type);
-	printf (", Content-type: %s\n", content_string);
+	g_mime_stream_printf (stream, ", Content-type: %s\n", content_string);
 	g_free (content_string);
     }
 
@@ -470,40 +471,37 @@ format_part_text (const void *ctx, sprinter_t *sp, mime_node_t *node,
 	char *recipients_string;
 	char *date_string;
 
-	printf ("\fheader{\n");
+	g_mime_stream_printf (stream, "\fheader{\n");
 	if (node->envelope_file)
-	    printf ("%s\n", _get_one_line_summary (ctx, node->envelope_file));
-	printf ("Subject: %s\n", g_mime_message_get_subject (message));
-	printf ("From: %s\n", g_mime_message_get_sender (message));
+	    g_mime_stream_printf (stream, "%s\n", _get_one_line_summary (ctx, node->envelope_file));
+	g_mime_stream_printf (stream, "Subject: %s\n", g_mime_message_get_subject (message));
+	g_mime_stream_printf (stream, "From: %s\n", g_mime_message_get_sender (message));
 	recipients = g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_TO);
 	recipients_string = internet_address_list_to_string (recipients, 0);
 	if (recipients_string)
-	    printf ("To: %s\n", recipients_string);
+	    g_mime_stream_printf (stream, "To: %s\n", recipients_string);
 	g_free (recipients_string);
 	recipients = g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_CC);
 	recipients_string = internet_address_list_to_string (recipients, 0);
 	if (recipients_string)
-	    printf ("Cc: %s\n", recipients_string);
+	    g_mime_stream_printf (stream, "Cc: %s\n", recipients_string);
 	g_free (recipients_string);
 	date_string = g_mime_message_get_date_as_string (message);
-	printf ("Date: %s\n", date_string);
+	g_mime_stream_printf (stream, "Date: %s\n", date_string);
 	g_free (date_string);
-	printf ("\fheader}\n");
+	g_mime_stream_printf (stream, "\fheader}\n");
 
-	printf ("\fbody{\n");
+	g_mime_stream_printf (stream, "\fbody{\n");
     }
 
     if (leaf) {
 	if (g_mime_content_type_is_type (content_type, "text", "*") &&
 	    !g_mime_content_type_is_type (content_type, "text", "html"))
 	{
-	    GMimeStream *stream_stdout = g_mime_stream_file_new (stdout);
-	    g_mime_stream_file_set_owner (GMIME_STREAM_FILE (stream_stdout), FALSE);
-	    show_text_part_content (node->part, stream_stdout, 0);
-	    g_object_unref(stream_stdout);
+	    show_text_part_content (node->part, stream, 0);
 	} else {
 	    char *content_string = g_mime_content_type_to_string (content_type);
-	    printf ("Non-text part: %s\n", content_string);
+	    g_mime_stream_printf (stream, "Non-text part: %s\n", content_string);
 	    g_free (content_string);
 	}
     }
@@ -512,9 +510,9 @@ format_part_text (const void *ctx, sprinter_t *sp, mime_node_t *node,
 	format_part_text (ctx, sp, mime_node_child (node, i), indent, params);
 
     if (GMIME_IS_MESSAGE (node->part))
-	printf ("\fbody}\n");
+	g_mime_stream_printf (stream, "\fbody}\n");
 
-    printf ("\f%s}\n", part_type);
+    g_mime_stream_printf (stream, "\f%s}\n", part_type);
 
     return NOTMUCH_STATUS_SUCCESS;
 }
@@ -741,7 +739,7 @@ format_part_mbox (const void *ctx, unused (sprinter_t *sp), mime_node_t *node,
 static notmuch_status_t
 format_part_raw (unused (const void *ctx), unused (sprinter_t *sp),
 		 mime_node_t *node, unused (int indent),
-		 unused (const notmuch_show_params_t *params))
+		 const notmuch_show_params_t *params)
 {
     if (node->envelope_file) {
 	/* Special case the entire message to avoid MIME parsing. */
@@ -781,13 +779,7 @@ format_part_raw (unused (const void *ctx), unused (sprinter_t *sp),
 	return NOTMUCH_STATUS_SUCCESS;
     }
 
-    GMimeStream *stream_stdout;
-    GMimeStream *stream_filter = NULL;
-
-    stream_stdout = g_mime_stream_file_new (stdout);
-    g_mime_stream_file_set_owner (GMIME_STREAM_FILE (stream_stdout), FALSE);
-
-    stream_filter = g_mime_stream_filter_new (stream_stdout);
+    GMimeStream *stream_filter = g_mime_stream_filter_new (params->out_stream);
 
     if (GMIME_IS_PART (node->part)) {
 	/* For leaf parts, we emit only the transfer-decoded
@@ -810,9 +802,6 @@ format_part_raw (unused (const void *ctx), unused (sprinter_t *sp),
     if (stream_filter)
 	g_object_unref (stream_filter);
 
-    if (stream_stdout)
-	g_object_unref(stream_stdout);
-
     return NOTMUCH_STATUS_SUCCESS;
 }
 
@@ -1167,6 +1156,8 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[])
     formatter = formatters[format];
     sprinter = formatter->new_sprinter(config, stdout);
 
+    params.out_stream = g_mime_stream_stdout_new ();
+
     /* If a single message is requested we do not use search_excludes. */
     if (single_message) {
 	ret = do_show_single (config, query, formatter, sprinter, &params);
@@ -1200,6 +1191,9 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[])
     }
 
  DONE:
+    g_mime_stream_flush (params.out_stream);
+    g_object_unref (params.out_stream);
+
     notmuch_crypto_cleanup (&params.crypto);
     notmuch_query_destroy (query);
     notmuch_database_destroy (notmuch);
-- 
2.11.0

[Patch v3 04/11] perf-test: add memory test for reply

[David Bremner]

Looking at the code for notmuch-reply, there seems to be several gmime
related memory leaks. This test is supposed to help eliminate those.
---
 performance-test/M04-reply.sh | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
 create mode 100755 performance-test/M04-reply.sh

diff --git a/performance-test/M04-reply.sh b/performance-test/M04-reply.sh
new file mode 100755
index 00000000..0e1ce087
--- /dev/null
+++ b/performance-test/M04-reply.sh
@@ -0,0 +1,15 @@
+#!/bin/bash
+
+test_description='search'
+
+. ./perf-test-lib.sh || exit 1
+
+memory_start
+
+for id in $(notmuch search --output=messages '*' | shuf -n 5); do
+    memory_run "reply $id" "notmuch reply \"$id\" 1>/dev/null"
+    memory_run "reply --format=json $id" "notmuch reply --format=json \"$id\" 1>/dev/null"
+    memory_run "reply --format=sexp $id" "notmuch reply --format=sexp \"$id\" 1>/dev/null"
+done
+
+memory_done
-- 
2.11.0

[Patch v3 05/11] cli/reply: fix two memory leaks, document a third

[David Bremner]

internet_address_list_to_string returns an allocated string, which
needs to be freed with g_free. g_free can handle a NULL argument, so
we follow the usage elsewhere of calling it unconditionally.

The third leak we leave as it would require restructuring of
add_recipients_from_message, and is fixed by later gmime-3.0 porting.
---
 notmuch-reply.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/notmuch-reply.c b/notmuch-reply.c
index 9239aac2..b88f1d31 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -45,7 +45,7 @@ format_part_reply (GMimeStream *stream, mime_node_t *node)
     } else if (GMIME_IS_MESSAGE (node->part)) {
 	GMimeMessage *message = GMIME_MESSAGE (node->part);
 	InternetAddressList *recipients;
-	const char *recipients_string;
+	char *recipients_string;
 
 	g_mime_stream_printf (stream, "> From: %s\n", g_mime_message_get_sender (message));
 	recipients = g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_TO);
@@ -53,11 +53,13 @@ format_part_reply (GMimeStream *stream, mime_node_t *node)
 	if (recipients_string)
 	    g_mime_stream_printf (stream, "> To: %s\n",
 				  recipients_string);
+	g_free (recipients_string);
 	recipients = g_mime_message_get_recipients (message, GMIME_RECIPIENT_TYPE_CC);
 	recipients_string = internet_address_list_to_string (recipients, 0);
 	if (recipients_string)
 	    g_mime_stream_printf (stream, "> Cc: %s\n",
 				  recipients_string);
+	g_free (recipients_string);
 	g_mime_stream_printf (stream, "> Subject: %s\n", g_mime_message_get_subject (message));
 	g_mime_stream_printf (stream, "> Date: %s\n", g_mime_message_get_date_as_string (message));
 	g_mime_stream_printf (stream, ">\n");
@@ -329,6 +331,12 @@ add_recipients_from_message (GMimeMessage *reply,
 			     GMimeMessage *message,
 			     notmuch_bool_t reply_all)
 {
+
+    /* There is a memory leak here with gmime-2.6 because get_sender
+     * returns a newly allocated list, while the others return
+     * references to libgmime owned data. This leak will be fixed with
+     * the transition to gmime-3.0.
+     */
     struct {
 	InternetAddressList * (*get_header)(GMimeMessage *message);
 	GMimeRecipientType recipient_type;
-- 
2.11.0

[Patch v3 06/11] test/thread-naming: remove excess escaping from sender address.

[David Bremner]

This is another case where the behaviour of gmime-2.6 and gmime-3.0
seems to differ. It may be that we prefer the more lax parsing of the
previous version, but that should be tested seperately.
---
 test/T200-thread-naming.sh | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/test/T200-thread-naming.sh b/test/T200-thread-naming.sh
index 132c1d77..2167ba8e 100755
--- a/test/T200-thread-naming.sh
+++ b/test/T200-thread-naming.sh
@@ -66,11 +66,11 @@ test_expect_equal "$output" "thread:XXX   2001-01-12 [6/8] Notmuch Test Suite; t
 test_begin_subtest "Use empty subjects if necessary."
 add_message '[subject]="@FORCE_EMPTY"' \
 	    '[date]="Sat, 13 Jan 2001 15:43:45 -0000"' \
-            '[from]="Empty Sender \<empty_test@notmuchmail.org\>"'
+            '[from]="Empty Sender <empty_test@notmuchmail.org>"'
 empty_parent=${gen_msg_id}
 add_message '[subject]="@FORCE_EMPTY"' \
 	    '[date]="Sun, 14 Jan 2001 15:43:45 -0000"' \
-            '[from]="Empty Sender \<empty_test@notmuchmail.org\>"' \
+            '[from]="Empty Sender <empty_test@notmuchmail.org>"' \
             "[in-reply-to]=\<$empty_parent\>"
 output=$(notmuch search --sort=newest-first from:empty_test@notmuchmail.org | notmuch_search_sanitize)
 test_expect_equal "$output" "thread:XXX   2001-01-14 [2/2] Empty Sender;  (inbox unread)"
@@ -78,15 +78,15 @@ test_expect_equal "$output" "thread:XXX   2001-01-14 [2/2] Empty Sender;  (inbox
 test_begin_subtest "Avoid empty subjects if possible (newest-first)."
 add_message '[subject]="Non-empty subject (1)"' \
 	    '[date]="Mon, 15 Jan 2001 15:43:45 -0000"' \
-            '[from]="Empty Sender \<empty_test@notmuchmail.org\>"' \
+            '[from]="Empty Sender <empty_test@notmuchmail.org>"' \
             "[in-reply-to]=\<$empty_parent\>"
 add_message '[subject]="Non-empty subject (2)"' \
 	    '[date]="Mon, 16 Jan 2001 15:43:45 -0000"' \
-            '[from]="Empty Sender \<empty_test@notmuchmail.org\>"' \
+            '[from]="Empty Sender <empty_test@notmuchmail.org>"' \
             "[in-reply-to]=\<$empty_parent\>"
 add_message '[subject]="@FORCE_EMPTY"' \
 	    '[date]="Tue, 17 Jan 2001 15:43:45 -0000"' \
-            '[from]="Empty Sender \<empty_test@notmuchmail.org\>"' \
+            '[from]="Empty Sender <empty_test@notmuchmail.org>"' \
             "[in-reply-to]=\<$empty_parent\>"
 output=$(notmuch search --sort=newest-first from:Empty | notmuch_search_sanitize)
 test_expect_equal "$output" "thread:XXX   2001-01-17 [5/5] Empty Sender; Non-empty subject (2) (inbox unread)"
-- 
2.11.0

Re: [Patch v3 06/11] test/thread-naming: remove excess escaping from sender address.

[Tomi Ollila]

On Sat, May 27 2017, David Bremner wrote:

> This is another case where the behaviour of gmime-2.6 and gmime-3.0
> seems to differ. It may be that we prefer the more lax parsing of the
> previous version, but that should be tested seperately.

This series lgtm (afaiu) so far, but s/seper/separ/ above. In 04/11 `shuf`
adds to coreutils dependencies, but that is in perf test...

> ---
>  test/T200-thread-naming.sh | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/test/T200-thread-naming.sh b/test/T200-thread-naming.sh
> index 132c1d77..2167ba8e 100755
> --- a/test/T200-thread-naming.sh
> +++ b/test/T200-thread-naming.sh
> @@ -66,11 +66,11 @@ test_expect_equal "$output" "thread:XXX   2001-01-12 [6/8] Notmuch Test Suite; t
>  test_begin_subtest "Use empty subjects if necessary."
>  add_message '[subject]="@FORCE_EMPTY"' \
>  	    '[date]="Sat, 13 Jan 2001 15:43:45 -0000"' \
> -            '[from]="Empty Sender \<empty_test@notmuchmail.org\>"'
> +            '[from]="Empty Sender <empty_test@notmuchmail.org>"'
>  empty_parent=${gen_msg_id}
>  add_message '[subject]="@FORCE_EMPTY"' \
>  	    '[date]="Sun, 14 Jan 2001 15:43:45 -0000"' \
> -            '[from]="Empty Sender \<empty_test@notmuchmail.org\>"' \
> +            '[from]="Empty Sender <empty_test@notmuchmail.org>"' \
>              "[in-reply-to]=\<$empty_parent\>"
>  output=$(notmuch search --sort=newest-first from:empty_test@notmuchmail.org | notmuch_search_sanitize)
>  test_expect_equal "$output" "thread:XXX   2001-01-14 [2/2] Empty Sender;  (inbox unread)"
> @@ -78,15 +78,15 @@ test_expect_equal "$output" "thread:XXX   2001-01-14 [2/2] Empty Sender;  (inbox
>  test_begin_subtest "Avoid empty subjects if possible (newest-first)."
>  add_message '[subject]="Non-empty subject (1)"' \
>  	    '[date]="Mon, 15 Jan 2001 15:43:45 -0000"' \
> -            '[from]="Empty Sender \<empty_test@notmuchmail.org\>"' \
> +            '[from]="Empty Sender <empty_test@notmuchmail.org>"' \
>              "[in-reply-to]=\<$empty_parent\>"
>  add_message '[subject]="Non-empty subject (2)"' \
>  	    '[date]="Mon, 16 Jan 2001 15:43:45 -0000"' \
> -            '[from]="Empty Sender \<empty_test@notmuchmail.org\>"' \
> +            '[from]="Empty Sender <empty_test@notmuchmail.org>"' \
>              "[in-reply-to]=\<$empty_parent\>"
>  add_message '[subject]="@FORCE_EMPTY"' \
>  	    '[date]="Tue, 17 Jan 2001 15:43:45 -0000"' \
> -            '[from]="Empty Sender \<empty_test@notmuchmail.org\>"' \
> +            '[from]="Empty Sender <empty_test@notmuchmail.org>"' \
>              "[in-reply-to]=\<$empty_parent\>"
>  output=$(notmuch search --sort=newest-first from:Empty | notmuch_search_sanitize)
>  test_expect_equal "$output" "thread:XXX   2001-01-17 [5/5] Empty Sender; Non-empty subject (2) (inbox unread)"
> -- 
> 2.11.0

Re: [Patch v3 06/11] test/thread-naming: remove excess escaping from sender address.

[David Bremner]

Tomi Ollila <tomi.ollila@iki.fi> writes:

> On Sat, May 27 2017, David Bremner wrote:
>
>> This is another case where the behaviour of gmime-2.6 and gmime-3.0
>> seems to differ. It may be that we prefer the more lax parsing of the
>> previous version, but that should be tested seperately.
>
> This series lgtm (afaiu) so far, but s/seper/separ/ above. In 04/11 `shuf`
> adds to coreutils dependencies, but that is in perf test...

pushed first 6 to master, with spelliñ fiks

d

[Patch v3 07/11] configure: add optional support for gmime-3.0

[David Bremner]

This is only the changes to make configure work; it won't compile with
gmime-3.0 yet.
---
 configure | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/configure b/configure
index ed7c2280..91aeba51 100755
--- a/configure
+++ b/configure
@@ -484,8 +484,13 @@ fi
 GMIME_MINVER=2.6.7
 
 printf "Checking for GMime development files... "
-if pkg-config --exists "gmime-2.6 >= $GMIME_MINVER"; then
-    printf "Yes.\n"
+if pkg-config --exists "gmime-3.0"; then
+    printf "Yes (3.0).\n"
+    have_gmime=1
+    gmime_cflags=$(pkg-config --cflags gmime-3.0)
+    gmime_ldflags=$(pkg-config --libs gmime-3.0)
+elif pkg-config --exists "gmime-2.6 >= $GMIME_MINVER"; then
+    printf "Yes (2.6).\n"
     have_gmime=1
     gmime_cflags=$(pkg-config --cflags gmime-2.6)
     gmime_ldflags=$(pkg-config --libs gmime-2.6)
-- 
2.11.0

[Patch v3 08/11] test: define GMime version dependant breakage

[David Bremner]

We have some tests where the gmime 3 behaviour seems like a bug fix,
others where it's less clear, so we allow both possibilities.
---
 configure        |  5 +++++
 test/test-lib.sh | 16 ++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/configure b/configure
index 91aeba51..c5e2ffed 100755
--- a/configure
+++ b/configure
@@ -489,11 +489,13 @@ if pkg-config --exists "gmime-3.0"; then
     have_gmime=1
     gmime_cflags=$(pkg-config --cflags gmime-3.0)
     gmime_ldflags=$(pkg-config --libs gmime-3.0)
+    gmime_major=3
 elif pkg-config --exists "gmime-2.6 >= $GMIME_MINVER"; then
     printf "Yes (2.6).\n"
     have_gmime=1
     gmime_cflags=$(pkg-config --cflags gmime-2.6)
     gmime_ldflags=$(pkg-config --libs gmime-2.6)
+    gmime_major=2
 else
     have_gmime=0
     printf "No.\n"
@@ -1212,6 +1214,9 @@ NOTMUCH_PYTHON=${python}
 # building/testing ruby bindings.
 NOTMUCH_HAVE_RUBY_DEV=${have_ruby_dev}
 
+# Major version of gmime
+NOTMUCH_GMIME_MAJOR=${gmime_major}
+
 # Platform we are run on
 PLATFORM=${platform}
 EOF
diff --git a/test/test-lib.sh b/test/test-lib.sh
index 37f8ddfa..093481c3 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -1202,6 +1202,22 @@ test_init_ () {
 
 . ./test-lib-common.sh || exit 1
 
+if [ "${NOTMUCH_GMIME_MAJOR}" = 3 ]; then
+    function test_subtest_broken_gmime_3 () {
+	test_subtest_known_broken
+    }
+    function test_subtest_broken_gmime_2 () {
+	/bin/true
+    }
+else
+    function test_subtest_broken_gmime_3 () {
+	/bin/true
+    }
+    function test_subtest_broken_gmime_2 () {
+	test_subtest_known_broken
+    }
+fi
+
 emacs_generate_script
 
 
-- 
2.11.0

Re: [Patch v3 08/11] test: define GMime version dependant breakage

[Tomi Ollila]

On Sat, May 27 2017, David Bremner wrote:

> We have some tests where the gmime 3 behaviour seems like a bug fix,
> others where it's less clear, so we allow both possibilities.
> ---
>  configure        |  5 +++++
>  test/test-lib.sh | 16 ++++++++++++++++
>  2 files changed, 21 insertions(+)
>
> diff --git a/configure b/configure
> index 91aeba51..c5e2ffed 100755
> --- a/configure
> +++ b/configure
> @@ -489,11 +489,13 @@ if pkg-config --exists "gmime-3.0"; then
>      have_gmime=1
>      gmime_cflags=$(pkg-config --cflags gmime-3.0)
>      gmime_ldflags=$(pkg-config --libs gmime-3.0)
> +    gmime_major=3
>  elif pkg-config --exists "gmime-2.6 >= $GMIME_MINVER"; then
>      printf "Yes (2.6).\n"
>      have_gmime=1
>      gmime_cflags=$(pkg-config --cflags gmime-2.6)
>      gmime_ldflags=$(pkg-config --libs gmime-2.6)
> +    gmime_major=2
>  else
>      have_gmime=0
>      printf "No.\n"
> @@ -1212,6 +1214,9 @@ NOTMUCH_PYTHON=${python}
>  # building/testing ruby bindings.
>  NOTMUCH_HAVE_RUBY_DEV=${have_ruby_dev}
>  
> +# Major version of gmime
> +NOTMUCH_GMIME_MAJOR=${gmime_major}
> +
>  # Platform we are run on
>  PLATFORM=${platform}
>  EOF
> diff --git a/test/test-lib.sh b/test/test-lib.sh
> index 37f8ddfa..093481c3 100644
> --- a/test/test-lib.sh
> +++ b/test/test-lib.sh
> @@ -1202,6 +1202,22 @@ test_init_ () {
>  
>  . ./test-lib-common.sh || exit 1

series lgtm so far (afaiu)

>  
> +if [ "${NOTMUCH_GMIME_MAJOR}" = 3 ]; then
> +    function test_subtest_broken_gmime_3 () {
> +	
> +    }

s/function // i.e. no `function` keyword (unnecessary and we don't use
those elsewhere)

Too bad bash(1) is picky enough not supporting 
test_subtest_broken_gmime_3 () test_subtest_known_broken
(i.e. w/o {}s like other shells do)

> +    function test_subtest_broken_gmime_2 () {
> +	/bin/true

/bin/true runs external command unnecessarily, plain `true` would
execute internal shell command.

> +    }
> +else
> +    function test_subtest_broken_gmime_3 () {

> +	/bin/true
> +    }
> +    function test_subtest_broken_gmime_2 () {
> +	test_subtest_known_broken
> +    }
> +fi
> +
>  emacs_generate_script
>  
>  
> -- 
> 2.11.0

Re: [Patch v3 08/11] test: define GMime version dependant breakage

[David Bremner]

Tomi Ollila <tomi.ollila@iki.fi> writes:

> On Sat, May 27 2017, David Bremner wrote:
>
>> We have some tests where the gmime 3 behaviour seems like a bug fix,
>> others where it's less clear, so we allow both possibilities.

pushed the next two, with suggested amendments

[Patch v3 09/11] test/multipart: reorganize creation of multipart message

[David Bremner]

We want to have the bodies of the multipart available in a file on
their own for planned modifications to tests.
---
 test/T190-multipart.sh | 40 +++++++++++++++++++++++++---------------
 1 file changed, 25 insertions(+), 15 deletions(-)

diff --git a/test/T190-multipart.sh b/test/T190-multipart.sh
index 12a10451..91a632c6 100755
--- a/test/T190-multipart.sh
+++ b/test/T190-multipart.sh
@@ -2,14 +2,7 @@
 test_description="output of multipart message"
 . ./test-lib.sh || exit 1
 
-cat <<EOF > embedded_message
-From: Carl Worth <cworth@cworth.org>
-To: cworth@cworth.org
-Subject: html message
-Date: Fri, 05 Jan 2001 15:42:57 +0000
-User-Agent: Notmuch/0.5 (http://notmuchmail.org) Emacs/23.3.1 (i486-pc-linux-gnu)
-Message-ID: <87liy5ap01.fsf@yoom.home.cworth.org>
-MIME-Version: 1.0
+cat <<EOF > embedded_message_body
 Content-Type: multipart/alternative; boundary="==-=-=="
 
 --==-=-==
@@ -24,15 +17,19 @@ This is an embedded message, with a multipart/alternative part.
 
 --==-=-==--
 EOF
-
-cat <<EOF > ${MAIL_DIR}/multipart
+cat <<EOF > embedded_message
 From: Carl Worth <cworth@cworth.org>
 To: cworth@cworth.org
-Subject: Multipart message
-Date: Fri, 05 Jan 2001 15:43:57 +0000
+Subject: html message
+Date: Fri, 05 Jan 2001 15:42:57 +0000
 User-Agent: Notmuch/0.5 (http://notmuchmail.org) Emacs/23.3.1 (i486-pc-linux-gnu)
-Message-ID: <87liy5ap00.fsf@yoom.home.cworth.org>
+Message-ID: <87liy5ap01.fsf@yoom.home.cworth.org>
 MIME-Version: 1.0
+EOF
+
+cat embedded_message_body >> embedded_message
+
+cat <<EOF > multipart_body
 Content-Type: multipart/signed; boundary="==-=-=";
 	micalg=pgp-sha1; protocol="application/pgp-signature"
 
@@ -44,8 +41,9 @@ Content-Type: message/rfc822
 Content-Disposition: inline
 
 EOF
-cat embedded_message >> ${MAIL_DIR}/multipart
-cat <<EOF >> ${MAIL_DIR}/multipart
+
+cat embedded_message >> multipart_body
+cat <<EOF >> multipart_body
 
 --=-=-=
 Content-Disposition: attachment; filename=attachment
@@ -73,6 +71,18 @@ W6cAmQE4dcYrx/LPLtYLZm1jsGauE5hE
 --==-=-=--
 EOF
 
+cat <<EOF > ${MAIL_DIR}/multipart
+From: Carl Worth <cworth@cworth.org>
+To: cworth@cworth.org
+Subject: Multipart message
+Date: Fri, 05 Jan 2001 15:43:57 +0000
+User-Agent: Notmuch/0.5 (http://notmuchmail.org) Emacs/23.3.1 (i486-pc-linux-gnu)
+Message-ID: <87liy5ap00.fsf@yoom.home.cworth.org>
+MIME-Version: 1.0
+EOF
+
+cat multipart_body >> ${MAIL_DIR}/multipart
+
 cat <<EOF > ${MAIL_DIR}/base64-part-with-crlf
 From: Carl Worth <cworth@cworth.org>
 To: cworth@cworth.org
-- 
2.11.0

[Patch v3 10/11] test: mark inclusion of headers as broken in gmime-2.x

[David Bremner]

The output of headers here reflects an underlying bug / quirk of
gmime-2.6.
---
 test/T190-multipart.sh | 28 ++++------------------------
 1 file changed, 4 insertions(+), 24 deletions(-)

diff --git a/test/T190-multipart.sh b/test/T190-multipart.sh
index 91a632c6..94bb0570 100755
--- a/test/T190-multipart.sh
+++ b/test/T190-multipart.sh
@@ -465,8 +465,9 @@ notmuch show --format=raw --part=0 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUT
 test_expect_equal_file "${MAIL_DIR}"/multipart OUTPUT
 
 test_begin_subtest "--format=raw --part=1, message body"
+test_subtest_broken_gmime_2
 notmuch show --format=raw --part=1 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
-test_expect_equal_file "${MAIL_DIR}"/multipart OUTPUT
+test_expect_equal_file multipart_body OUTPUT
 
 test_begin_subtest "--format=raw --part=2, multipart/mixed"
 notmuch show --format=raw --part=2 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
@@ -518,30 +519,9 @@ notmuch show --format=raw --part=3 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUT
 test_expect_equal_file embedded_message OUTPUT
 
 test_begin_subtest "--format=raw --part=4, rfc822's multipart"
+test_subtest_broken_gmime_2
 notmuch show --format=raw --part=4 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
-cat <<EOF >EXPECTED
-From: Carl Worth <cworth@cworth.org>
-To: cworth@cworth.org
-Subject: html message
-Date: Fri, 05 Jan 2001 15:42:57 +0000
-User-Agent: Notmuch/0.5 (http://notmuchmail.org) Emacs/23.3.1 (i486-pc-linux-gnu)
-Message-ID: <87liy5ap01.fsf@yoom.home.cworth.org>
-MIME-Version: 1.0
-Content-Type: multipart/alternative; boundary="==-=-=="
-
---==-=-==
-Content-Type: text/html
-
-<p>This is an embedded message, with a multipart/alternative part.</p>
-
---==-=-==
-Content-Type: text/plain
-
-This is an embedded message, with a multipart/alternative part.
-
---==-=-==--
-EOF
-test_expect_equal_file EXPECTED OUTPUT
+test_expect_equal_file embedded_message_body OUTPUT
 
 test_begin_subtest "--format=raw --part=5, rfc822's html part"
 notmuch show --format=raw --part=5 'id:87liy5ap00.fsf@yoom.home.cworth.org' >OUTPUT
-- 
2.11.0

[Patch v3 11/11] test: mark test as broken in gmime 3.0

[David Bremner]

Currently I'm not sure what the intent of this test is, so it's not
clear if the new answer is better or worse than the old one.
---
 test/T310-emacs.sh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/test/T310-emacs.sh b/test/T310-emacs.sh
index ef599849..2ef43925 100755
--- a/test/T310-emacs.sh
+++ b/test/T310-emacs.sh
@@ -86,6 +86,7 @@ test_emacs "(let ((notmuch-show-indent-messages-width 4))
 test_expect_equal_file $EXPECTED/notmuch-show-thread-maildir-storage-with-fourfold-indentation OUTPUT
 
 test_begin_subtest "notmuch-show for message with invalid From"
+test_subtest_broken_gmime_3
 add_message "[subject]=\"message-with-invalid-from\"" \
 	    "[from]=\"\\\"Invalid \\\" From\\\" <test_suite@notmuchmail.org>\""
 thread=$(notmuch search --output=threads subject:message-with-invalid-from)
-- 
2.11.0