From: Jani Nikula Date: Sun, 30 Mar 2014 21:21:47 +0000 (+0300) Subject: [PATCH v5 0/2] lib: drop mbox support, replace header parser with gmime X-Git-Url: http://git.tremily.us/?a=commitdiff_plain;h=c8b0cdad63f1a1db800d9d284fcd1048b50e74d3;p=notmuch-archives.git [PATCH v5 0/2] lib: drop mbox support, replace header parser with gmime --- diff --git a/34/d1c9573170f0da095bc2bc89a28a7057d9e755 b/34/d1c9573170f0da095bc2bc89a28a7057d9e755 new file mode 100644 index 000000000..b30c77971 --- /dev/null +++ b/34/d1c9573170f0da095bc2bc89a28a7057d9e755 @@ -0,0 +1,368 @@ +Return-Path: +X-Original-To: notmuch@notmuchmail.org +Delivered-To: notmuch@notmuchmail.org +Received: from localhost (localhost [127.0.0.1]) + by olra.theworths.org (Postfix) with ESMTP id 1E975431FBF + for ; Sun, 30 Mar 2014 14:22:04 -0700 (PDT) +X-Virus-Scanned: Debian amavisd-new at olra.theworths.org +X-Spam-Flag: NO +X-Spam-Score: -0.7 +X-Spam-Level: +X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5 + tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled +Received: from olra.theworths.org ([127.0.0.1]) + by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) + with ESMTP id gDdtV7UPwzA5 for ; + Sun, 30 Mar 2014 14:21:56 -0700 (PDT) +Received: from mail-wg0-f52.google.com (mail-wg0-f52.google.com + [74.125.82.52]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) (No client + certificate requested) by olra.theworths.org (Postfix) with ESMTPS id + BA5AD431FB6 for ; Sun, 30 Mar 2014 14:21:55 -0700 + (PDT) +Received: by mail-wg0-f52.google.com with SMTP id k14so5033638wgh.23 + for ; Sun, 30 Mar 2014 14:21:53 -0700 (PDT) +X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; + d=1e100.net; s=20130820; + h=x-gm-message-state:from:to:cc:subject:date:message-id; + bh=pBCeRAcggjGIj292YIYqsT5KoGDdVo79fFjgJbv+mV8=; + b=aoEkkW2y9oe2lwyyW0BZS8ERg+WKXBde6C6BtL6xeqWBG0gus50e5ggZq9c21iIemx + aTV4cQqUJiTbgBjW3vDkaovzzTRlWVwmsUoABwFLyZLXP+QlciYS6TqjgpOtaQMBG1i3 + gRx28lTjxWKw/2AlDgxqyvtdAAQD8h/cIK+MU6sm3o5BHIT12R4+oudFzo3b1KaQCcZu + fQ+DaDQd6ih9uFDsB7zViFiKEmn5RJ1eT/kCntNxKLYw81AXPKRxj7H9l0xuIyMgF4V2 + WrmoWnHiFML2OXxeanJSFSrma+6KPL4x/RGukr6Ec2jGPYBr2TOocpAB9EcUC5ixMzVB + KITg== +X-Gm-Message-State: + ALoCoQmQIDhvUlKpB2bHe8/D8sSErpwGy1rL3CVHc0iW42pCjbCN3l7Xy6FOEGOCtJ5chUNe0xEe +X-Received: by 10.194.63.46 with SMTP id d14mr9296656wjs.24.1396214512954; + Sun, 30 Mar 2014 14:21:52 -0700 (PDT) +Received: from localhost (dsl-hkibrasgw2-58c36f-91.dhcp.inet.fi. + [88.195.111.91]) + by mx.google.com with ESMTPSA id 48sm28357929eee.2.2014.03.30.14.21.50 + for + (version=TLSv1.2 cipher=RC4-SHA bits=128/128); + Sun, 30 Mar 2014 14:21:52 -0700 (PDT) +From: Jani Nikula +To: notmuch@notmuchmail.org +Subject: [PATCH v5 0/2] lib: drop mbox support, + replace header parser with gmime +Date: Mon, 31 Mar 2014 00:21:47 +0300 +Message-Id: +X-Mailer: git-send-email 1.9.0 +X-BeenThere: notmuch@notmuchmail.org +X-Mailman-Version: 2.1.13 +Precedence: list +List-Id: "Use and development of the notmuch mail system." + +List-Unsubscribe: , + +List-Archive: +List-Post: +List-Help: +List-Subscribe: , + +X-List-Received-Date: Sun, 30 Mar 2014 21:22:04 -0000 + +This is v5 of id:1395604866-19188-1-git-send-email-jani@nikula.org +addressing Austin's review. The most significant change is the new patch +dropping support for single-message mbox files. Diff between the +versions is at the end of this cover letter. + +BR, +Jani. + + +Jani Nikula (2): + lib: drop support for single-message mbox files + lib: replace the header parser with gmime + + lib/database.cc | 15 +- + lib/index.cc | 72 +-------- + lib/message-file.c | 413 ++++++++++++++++++++------------------------------ + lib/notmuch-private.h | 55 +++---- + test/T050-new.sh | 26 ++-- + 5 files changed, 216 insertions(+), 365 deletions(-) + +-- +1.9.0 + +diff --git a/lib/database.cc b/lib/database.cc +index 4750f40cf0fb..1efb14d4a0bd 100644 +--- a/lib/database.cc ++++ b/lib/database.cc +@@ -1972,7 +1972,7 @@ notmuch_database_add_message (notmuch_database_t *notmuch, + goto DONE; + + /* Parse message up front to get better error status. */ +- ret = notmuch_message_file_parse (message_file); ++ ret = _notmuch_message_file_parse (message_file); + if (ret) + goto DONE; + +diff --git a/lib/index.cc b/lib/index.cc +index 46a019325454..e1e2a3828f02 100644 +--- a/lib/index.cc ++++ b/lib/index.cc +@@ -430,10 +430,12 @@ _notmuch_message_index_file (notmuch_message_t *message, + GMimeMessage *mime_message; + InternetAddressList *addresses; + const char *from, *subject; ++ notmuch_status_t status; + +- mime_message = notmuch_message_file_get_mime_message (message_file); +- if (! mime_message) +- return NOTMUCH_STATUS_FILE_NOT_EMAIL; ++ status = _notmuch_message_file_get_mime_message (message_file, ++ &mime_message); ++ if (status) ++ return status; + + from = g_mime_message_get_sender (mime_message); + +diff --git a/lib/message-file.c b/lib/message-file.c +index 88662608d319..67828827e61d 100644 +--- a/lib/message-file.c ++++ b/lib/message-file.c +@@ -116,20 +116,37 @@ notmuch_message_file_close (notmuch_message_file_t *message) + talloc_free (message); + } + ++static notmuch_bool_t ++is_mbox (FILE *file) ++{ ++ char from_buf[5]; ++ notmuch_bool_t ret = FALSE; ++ ++ /* Is this mbox? */ ++ if (fread (from_buf, sizeof (from_buf), 1, file) == 1 && ++ strncmp (from_buf, "From ", 5) == 0) ++ ret = TRUE; ++ ++ rewind (file); ++ ++ return ret; ++} ++ + notmuch_status_t +-notmuch_message_file_parse (notmuch_message_file_t *message) ++_notmuch_message_file_parse (notmuch_message_file_t *message) + { + GMimeStream *stream; + GMimeParser *parser; + notmuch_status_t status = NOTMUCH_STATUS_SUCCESS; + static int initialized = 0; +- char from_buf[5]; +- notmuch_bool_t is_mbox = FALSE; +- static notmuch_bool_t mbox_warning = FALSE; + + if (message->message) + return NOTMUCH_STATUS_SUCCESS; + ++ /* We no longer support mboxes at all. */ ++ if (is_mbox (message->file)) ++ return NOTMUCH_STATUS_FILE_NOT_EMAIL; ++ + if (! initialized) { + g_mime_init (GMIME_ENABLE_RFC2047_WORKAROUNDS); + initialized = 1; +@@ -140,19 +157,13 @@ notmuch_message_file_parse (notmuch_message_file_t *message) + if (! message->headers) + return NOTMUCH_STATUS_OUT_OF_MEMORY; + +- /* Is this mbox? */ +- if (fread (from_buf, sizeof (from_buf), 1, message->file) == 1 && +- strncmp (from_buf, "From ", 5) == 0) +- is_mbox = TRUE; +- rewind (message->file); +- + stream = g_mime_stream_file_new (message->file); + + /* We'll own and fclose the FILE* ourselves. */ + g_mime_stream_file_set_owner (GMIME_STREAM_FILE (stream), FALSE); + + parser = g_mime_parser_new_with_stream (stream); +- g_mime_parser_set_scan_from (parser, is_mbox); ++ g_mime_parser_set_scan_from (parser, FALSE); + + message->message = g_mime_parser_construct_message (parser); + if (! message->message) { +@@ -160,26 +171,6 @@ notmuch_message_file_parse (notmuch_message_file_t *message) + goto DONE; + } + +- if (is_mbox) { +- if (! g_mime_parser_eos (parser)) { +- /* This is a multi-message mbox. */ +- status = NOTMUCH_STATUS_FILE_NOT_EMAIL; +- goto DONE; +- } +- /* +- * For historical reasons, we support single-message mboxes, +- * but this behavior is likely to change in the future, so +- * warn. +- */ +- if (! mbox_warning) { +- mbox_warning = TRUE; +- fprintf (stderr, "\ +-Warning: %s is an mbox containing a single message,\n\ +-likely caused by misconfigured mail delivery. Support for single-message\n\ +-mboxes is deprecated and may be removed in the future.\n", message->filename); +- } +- } +- + DONE: + g_object_unref (stream); + g_object_unref (parser); +@@ -199,13 +190,19 @@ mboxes is deprecated and may be removed in the future.\n", message->filename); + return status; + } + +-GMimeMessage * +-notmuch_message_file_get_mime_message (notmuch_message_file_t *message) ++notmuch_status_t ++_notmuch_message_file_get_mime_message (notmuch_message_file_t *message, ++ GMimeMessage **mime_message) + { +- if (notmuch_message_file_parse (message)) +- return NULL; ++ notmuch_status_t status; ++ ++ status = _notmuch_message_file_parse (message); ++ if (status) ++ return status; ++ ++ *mime_message = message->message; + +- return message->message; ++ return NOTMUCH_STATUS_SUCCESS; + } + + /* +@@ -235,13 +232,16 @@ _notmuch_message_file_get_combined_header (notmuch_message_file_t *message, + goto DONE; + + do { +- const char *value; ++ const char *value; + char *decoded; + + if (strcasecmp (g_mime_header_iter_get_name (iter), header) != 0) + continue; + ++ /* Note that GMime retains ownership of value... */ + value = g_mime_header_iter_get_value (iter); ++ ++ /* ... while decoded needs to be freed with g_free(). */ + decoded = g_mime_utils_header_decode_text (value); + if (! decoded) { + if (combined) { +@@ -276,23 +276,20 @@ _notmuch_message_file_get_combined_header (notmuch_message_file_t *message, + return combined; + } + +-/* Return NULL on errors, empty string for non-existing headers. */ + const char * + notmuch_message_file_get_header (notmuch_message_file_t *message, + const char *header) + { +- const char *value = NULL; ++ const char *value; + char *decoded; +- notmuch_bool_t found; + +- if (notmuch_message_file_parse (message)) ++ if (_notmuch_message_file_parse (message)) + return NULL; + + /* If we have a cached decoded value, use it. */ +- found = g_hash_table_lookup_extended (message->headers, header, +- NULL, (gpointer *) &value); +- if (found) +- return value ? value : ""; ++ value = g_hash_table_lookup (message->headers, header); ++ if (value) ++ return value; + + if (strcasecmp (header, "received") == 0) { + /* +diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h +index 734a4e338554..703ae7bb7a01 100644 +--- a/lib/notmuch-private.h ++++ b/lib/notmuch-private.h +@@ -354,19 +354,20 @@ notmuch_message_file_close (notmuch_message_file_t *message); + * status reporting. + */ + notmuch_status_t +-notmuch_message_file_parse (notmuch_message_file_t *message); ++_notmuch_message_file_parse (notmuch_message_file_t *message); + + /* Get the gmime message of a message file. + * + * The message file is parsed as necessary. + * +- * Returns GMimeMessage* on success (which the caller must not unref), +- * NULL if the message file parsing fails. ++ * The GMimeMessage* is set to *mime_message on success (which the ++ * caller must not unref). + * + * XXX: Would be nice to not have to expose GMimeMessage here. + */ +-GMimeMessage * +-notmuch_message_file_get_mime_message (notmuch_message_file_t *message); ++notmuch_status_t ++_notmuch_message_file_get_mime_message (notmuch_message_file_t *message, ++ GMimeMessage **mime_message); + + /* Get the value of the specified header from the message as a UTF-8 string. + * +diff --git a/test/T050-new.sh b/test/T050-new.sh +index ad46ee6d51b6..3c3195428223 100755 +--- a/test/T050-new.sh ++++ b/test/T050-new.sh +@@ -163,22 +163,6 @@ rm -rf "${MAIL_DIR}"/two + output=$(NOTMUCH_NEW) + test_expect_equal "$output" "No new mail. Removed 3 messages." + +-test_begin_subtest "Support single-message mbox (deprecated)" +-cat > "${MAIL_DIR}"/mbox_file1 < +-To: Notmuch Test Suite +-Subject: Test mbox message 1 +- +-Body. +-EOF +-output=$(NOTMUCH_NEW 2>&1) +-test_expect_equal "$output" \ +-"Warning: ${MAIL_DIR}/mbox_file1 is an mbox containing a single message, +-likely caused by misconfigured mail delivery. Support for single-message +-mboxes is deprecated and may be removed in the future. +-Added 1 new message to the database." +- + # This test requires that notmuch new has been run at least once. + test_begin_subtest "Skip and report non-mail files" + generate_message +@@ -200,14 +184,24 @@ Subject: Test mbox message 2 + + Body 2. + EOF ++cat > "${MAIL_DIR}"/mbox_file1 < ++To: Notmuch Test Suite ++Subject: Test mbox message 1 ++ ++Body. ++EOF + output=$(NOTMUCH_NEW 2>&1) + test_expect_equal "$output" \ + "Note: Ignoring non-mail file: ${MAIL_DIR}/.git/config + Note: Ignoring non-mail file: ${MAIL_DIR}/.ignored_hidden_file + Note: Ignoring non-mail file: ${MAIL_DIR}/ignored_file + Note: Ignoring non-mail file: ${MAIL_DIR}/mbox_file ++Note: Ignoring non-mail file: ${MAIL_DIR}/mbox_file1 + Added 1 new message to the database." + rm "${MAIL_DIR}"/mbox_file ++rm "${MAIL_DIR}"/mbox_file1 + + test_begin_subtest "Ignore files and directories specified in new.ignore" + generate_message