--- /dev/null
+Return-Path: <jani@nikula.org>\r
+X-Original-To: notmuch@notmuchmail.org\r
+Delivered-To: notmuch@notmuchmail.org\r
+Received: from localhost (localhost [127.0.0.1])\r
+ by olra.theworths.org (Postfix) with ESMTP id 1CDDE429E27\r
+ for <notmuch@notmuchmail.org>; Sun, 6 Jan 2013 14:01:58 -0800 (PST)\r
+X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
+X-Spam-Flag: NO\r
+X-Spam-Score: -0.7\r
+X-Spam-Level: \r
+X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5\r
+ tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled\r
+Received: from olra.theworths.org ([127.0.0.1])\r
+ by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
+ with ESMTP id ocpeZrbEiqWz for <notmuch@notmuchmail.org>;\r
+ Sun, 6 Jan 2013 14:01:57 -0800 (PST)\r
+Received: from mail-la0-f46.google.com (mail-la0-f46.google.com\r
+ [209.85.215.46]) (using TLSv1 with cipher RC4-SHA (128/128 bits))\r
+ (No client certificate requested)\r
+ by olra.theworths.org (Postfix) with ESMTPS id 95E71431FD4\r
+ for <notmuch@notmuchmail.org>; Sun, 6 Jan 2013 14:01:56 -0800 (PST)\r
+Received: by mail-la0-f46.google.com with SMTP id fq13so13835701lab.33\r
+ for <notmuch@notmuchmail.org>; Sun, 06 Jan 2013 14:01:55 -0800 (PST)\r
+X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;\r
+ d=google.com; s=20120113;\r
+ h=x-received:from:to:cc:subject:in-reply-to:references:user-agent\r
+ :date:message-id:mime-version:content-type:x-gm-message-state;\r
+ bh=IsubdIZfd12c/o4EToqCdeMHNvFBT8KXku/GFaePlXo=;\r
+ b=MIpdMxW9unb4Q6rMwBX7aoVkeS5ZxsRl01a27cU0g+867A9mU4IvhTVVZY4PO8NvEL\r
+ ye5R1NuypqsQTNzkdhVTpFWJcOmDducgDPLE3t5R/pZvWn3YxBcrsJwvwJ92FTbZb+Nm\r
+ +ztV4Zf0fFbDsj2ttriO+6nslaGJ2IhNW+I3G6htYOwI038RqUTkFsfpC5BAdSUGqeDm\r
+ zboySlorKA/8T5kj66Ys5OCG23b01V1R8bg0B+EAM+SWhyDYtnaXTKzSP5LI0RLti0C3\r
+ NT/GlBm5ORFSBjDc4xBhTNGkBpKYUfqNK6hP21t4aJgBtOTzgVwLf9wKst7fcOx0HqFa\r
+ 8gkA==\r
+X-Received: by 10.152.134.243 with SMTP id pn19mr55707468lab.11.1357509713659;\r
+ Sun, 06 Jan 2013 14:01:53 -0800 (PST)\r
+Received: from localhost (dsl-hkibrasgw4-50df51-27.dhcp.inet.fi.\r
+ [80.223.81.27])\r
+ by mx.google.com with ESMTPS id sv4sm3356399lab.0.2013.01.06.14.01.51\r
+ (version=SSLv3 cipher=OTHER); Sun, 06 Jan 2013 14:01:52 -0800 (PST)\r
+From: Jani Nikula <jani@nikula.org>\r
+To: Austin Clements <amdragon@MIT.EDU>, notmuch@notmuchmail.org\r
+Subject: Re: [PATCH v5 0/6] Use Xapian query syntax for batch-tag dump/restore\r
+In-Reply-To: <1357503762-28759-1-git-send-email-amdragon@mit.edu>\r
+References: <1357503762-28759-1-git-send-email-amdragon@mit.edu>\r
+User-Agent: Notmuch/0.14+211~gc8d6546 (http://notmuchmail.org) Emacs/24.2.1\r
+ (x86_64-pc-linux-gnu)\r
+Date: Mon, 07 Jan 2013 00:01:49 +0200\r
+Message-ID: <87d2xi7yya.fsf@nikula.org>\r
+MIME-Version: 1.0\r
+Content-Type: text/plain\r
+X-Gm-Message-State:\r
+ ALoCoQkSxZAmyqFc8xKDJje1l1C1ogd2zhyIz0EweqDmwDKtu2LwU9mzHEPAdMiTn1iyE4avufzX\r
+Cc: tomi.ollila@iki.fi\r
+X-BeenThere: notmuch@notmuchmail.org\r
+X-Mailman-Version: 2.1.13\r
+Precedence: list\r
+List-Id: "Use and development of the notmuch mail system."\r
+ <notmuch.notmuchmail.org>\r
+List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
+ <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
+List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
+List-Post: <mailto:notmuch@notmuchmail.org>\r
+List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
+List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
+ <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
+X-List-Received-Date: Sun, 06 Jan 2013 22:01:58 -0000\r
+\r
+On Sun, 06 Jan 2013, Austin Clements <amdragon@MIT.EDU> wrote:\r
+> This obsoletes\r
+>\r
+> id:1356936162-2589-1-git-send-email-amdragon@mit.edu\r
+>\r
+> v5 should address all of the comments on v4 except those I\r
+> specifically replied to (via the ML or IRC). It also adds a new patch\r
+> at the beginning that makes missing message IDs non-fatal in restore,\r
+> like they were in 0.14. This patch can be pushed separately; it's in\r
+> this series because later tests rely on it.\r
+\r
+The series LGMT,\r
+Jani.\r
+\r
+>\r
+> The diff from v4 follows.\r
+>\r
+> diff --git a/notmuch-dump.c b/notmuch-dump.c\r
+> index bf01a39..a3244e0 100644\r
+> --- a/notmuch-dump.c\r
+> +++ b/notmuch-dump.c\r
+> @@ -103,6 +103,18 @@ notmuch_dump_command (unused (void *ctx), int argc, char *argv[])\r
+> message = notmuch_messages_get (messages);\r
+> message_id = notmuch_message_get_message_id (message);\r
+> \r
+> + if (output_format == DUMP_FORMAT_BATCH_TAG &&\r
+> + strchr (message_id, '\n')) {\r
+> + /* This will produce a line break in the output, which\r
+> + * would be difficult to handle in tools. However, it's\r
+> + * also impossible to produce an email containing a line\r
+> + * break in a message ID because of unfolding, so we can\r
+> + * safely disallow it. */\r
+> + fprintf (stderr, "Warning: skipping message id containing line break: \"%s\"\n", message_id);\r
+> + notmuch_message_destroy (message);\r
+> + continue;\r
+> + }\r
+> +\r
+> if (output_format == DUMP_FORMAT_SUP) {\r
+> fprintf (output, "%s (", message_id);\r
+> }\r
+> @@ -133,19 +145,10 @@ notmuch_dump_command (unused (void *ctx), int argc, char *argv[])\r
+> if (output_format == DUMP_FORMAT_SUP) {\r
+> fputs (")\n", output);\r
+> } else {\r
+> - if (strchr (message_id, '\n')) {\r
+> - /* This will produce a line break in the output, which\r
+> - * would be difficult to handle in tools. However,\r
+> - * it's also impossible to produce an email containing\r
+> - * a line break in a message ID because of unfolding,\r
+> - * so we can safely disallow it. */\r
+> - fprintf (stderr, "Error: cannot dump message id containing line break: %s\n", message_id);\r
+> - return 1;\r
+> - }\r
+> if (make_boolean_term (notmuch, "id", message_id,\r
+> &buffer, &buffer_size)) {\r
+> - fprintf (stderr, "Error: failed to quote message id %s\n",\r
+> - message_id);\r
+> + fprintf (stderr, "Error quoting message id %s: %s\n",\r
+> + message_id, strerror (errno));\r
+> return 1;\r
+> }\r
+> fprintf (output, " -- %s\n", buffer);\r
+> diff --git a/notmuch-restore.c b/notmuch-restore.c\r
+> index 77a4c27..81d4d98 100644\r
+> --- a/notmuch-restore.c\r
+> +++ b/notmuch-restore.c\r
+> @@ -26,7 +26,8 @@\r
+> static regex_t regex;\r
+> \r
+> /* Non-zero return indicates an error in retrieving the message,\r
+> - * or in applying the tags.\r
+> + * or in applying the tags. Missing messages are reported, but not\r
+> + * considered errors.\r
+> */\r
+> static int\r
+> tag_message (unused (void *ctx),\r
+> @@ -40,13 +41,17 @@ tag_message (unused (void *ctx),\r
+> int ret = 0;\r
+> \r
+> status = notmuch_database_find_message (notmuch, message_id, &message);\r
+> - if (status || message == NULL) {\r
+> - fprintf (stderr, "Warning: cannot apply tags to %smessage: %s\n",\r
+> - message ? "" : "missing ", message_id);\r
+> - if (status)\r
+> - fprintf (stderr, "%s\n", notmuch_status_to_string (status));\r
+> + if (status) {\r
+> + fprintf (stderr, "Error applying tags to message %s: %s\n",\r
+> + message_id, notmuch_status_to_string (status));\r
+> return 1;\r
+> }\r
+> + if (message == NULL) {\r
+> + fprintf (stderr, "Warning: cannot apply tags to missing message: %s\n",\r
+> + message_id);\r
+> + /* We consider this a non-fatal error. */\r
+> + return 0;\r
+> + }\r
+> \r
+> /* In order to detect missing messages, this check/optimization is\r
+> * intentionally done *after* first finding the message. */\r
+> @@ -222,12 +227,17 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[])\r
+> if (ret == 0) {\r
+> ret = parse_boolean_term (line_ctx, query_string,\r
+> &prefix, &term);\r
+> - if (ret) {\r
+> - fprintf (stderr, "Warning: cannot parse query: %s\n",\r
+> - query_string);\r
+> + if (ret && errno == EINVAL) {\r
+> + fprintf (stderr, "Warning: cannot parse query: %s (skipping)\n", query_string);\r
+> continue;\r
+> + } else if (ret) {\r
+> + /* This is more fatal (e.g., out of memory) */\r
+> + fprintf (stderr, "Error parsing query: %s\n",\r
+> + strerror (errno));\r
+> + ret = 1;\r
+> + break;\r
+> } else if (strcmp ("id", prefix) != 0) {\r
+> - fprintf (stderr, "Warning: not an id query: %s\n", query_string);\r
+> + fprintf (stderr, "Warning: not an id query: %s (skipping)\n", query_string);\r
+> continue;\r
+> }\r
+> query_string = term;\r
+> diff --git a/test/dump-restore b/test/dump-restore\r
+> index f9ae5b3..f076c12 100755\r
+> --- a/test/dump-restore\r
+> +++ b/test/dump-restore\r
+> @@ -202,18 +202,32 @@ a\r
+> + +e -- id:20091117232137.GA7669@griffis1.net\r
+> # valid id, but warning about missing message\r
+> +e id:missing_message_id\r
+> +# exercise parser\r
+> ++e -- id:some)stuff\r
+> ++e -- id:some stuff\r
+> ++e -- id:some"stuff\r
+> ++e -- id:"a_message_id_with""_a_quote"\r
+> ++e -- id:"a message id with spaces"\r
+> ++e -- id:an_id_with_leading_and_trailing_ws \\r
+> +\r
+> EOF\r
+> \r
+> cat <<EOF > EXPECTED\r
+> -Warning: cannot parse query: a\r
+> +Warning: cannot parse query: a (skipping)\r
+> Warning: no query string [+0]\r
+> Warning: no query string [+a +b]\r
+> Warning: missing query string [+a +b ]\r
+> Warning: no query string after -- [+c +d --]\r
+> Warning: hex decoding of tag %zz failed [+%zz -- id:whatever]\r
+> -Warning: cannot parse query: id:"\r
+> -Warning: not an id query: tag:abc\r
+> +Warning: cannot parse query: id:" (skipping)\r
+> +Warning: not an id query: tag:abc (skipping)\r
+> Warning: cannot apply tags to missing message: missing_message_id\r
+> +Warning: cannot parse query: id:some)stuff (skipping)\r
+> +Warning: cannot parse query: id:some stuff (skipping)\r
+> +Warning: cannot apply tags to missing message: some"stuff\r
+> +Warning: cannot apply tags to missing message: a_message_id_with"_a_quote\r
+> +Warning: cannot apply tags to missing message: a message id with spaces\r
+> +Warning: cannot apply tags to missing message: an_id_with_leading_and_trailing_ws\r
+> EOF\r
+> \r
+> test_expect_equal_file EXPECTED OUTPUT\r
+> diff --git a/util/string-util.c b/util/string-util.c\r
+> index 52c7781..aba9aa8 100644\r
+> --- a/util/string-util.c\r
+> +++ b/util/string-util.c\r
+> @@ -23,6 +23,7 @@\r
+> #include "talloc.h"\r
+> \r
+> #include <ctype.h>\r
+> +#include <errno.h>\r
+> \r
+> char *\r
+> strtok_len (char *s, const char *delim, size_t *len)\r
+> @@ -36,6 +37,12 @@ strtok_len (char *s, const char *delim, size_t *len)\r
+> return *len ? s : NULL;\r
+> }\r
+> \r
+> +static int\r
+> +is_unquoted_terminator (unsigned char c)\r
+> +{\r
+> + return c == 0 || c <= ' ' || c == ')';\r
+> +}\r
+> +\r
+> int\r
+> make_boolean_term (void *ctx, const char *prefix, const char *term,\r
+> char **buf, size_t *len)\r
+> @@ -49,7 +56,8 @@ make_boolean_term (void *ctx, const char *prefix, const char *term,\r
+> * containing a quote, even though it only matters at the\r
+> * beginning, and anything containing non-ASCII text. */\r
+> for (in = term; *in && !need_quoting; in++)\r
+> - if (*in <= ' ' || *in == ')' || *in == '"' || (unsigned char)*in > 127)\r
+> + if (is_unquoted_terminator (*in) || *in == '"'\r
+> + || (unsigned char)*in > 127)\r
+> need_quoting = 1;\r
+> \r
+> if (need_quoting)\r
+> @@ -67,8 +75,10 @@ make_boolean_term (void *ctx, const char *prefix, const char *term,\r
+> *buf = talloc_realloc (ctx, *buf, char, *len);\r
+> }\r
+> \r
+> - if (! *buf)\r
+> - return 1;\r
+> + if (! *buf) {\r
+> + errno = ENOMEM;\r
+> + return -1;\r
+> + }\r
+> \r
+> out = *buf;\r
+> \r
+> @@ -102,7 +112,7 @@ make_boolean_term (void *ctx, const char *prefix, const char *term,\r
+> static const char*\r
+> skip_space (const char *str)\r
+> {\r
+> - while (*str && isspace (*str))\r
+> + while (*str && isspace ((unsigned char) *str))\r
+> ++str;\r
+> return str;\r
+> }\r
+> @@ -111,6 +121,7 @@ int\r
+> parse_boolean_term (void *ctx, const char *str,\r
+> char **prefix_out, char **term_out)\r
+> {\r
+> + int err = EINVAL;\r
+> *prefix_out = *term_out = NULL;\r
+> \r
+> /* Parse prefix */\r
+> @@ -119,12 +130,20 @@ parse_boolean_term (void *ctx, const char *str,\r
+> if (! pos)\r
+> goto FAIL;\r
+> *prefix_out = talloc_strndup (ctx, str, pos - str);\r
+> + if (! *prefix_out) {\r
+> + err = ENOMEM;\r
+> + goto FAIL;\r
+> + }\r
+> ++pos;\r
+> \r
+> /* Implement de-quoting compatible with make_boolean_term. */\r
+> if (*pos == '"') {\r
+> char *out = talloc_array (ctx, char, strlen (pos));\r
+> int closed = 0;\r
+> + if (! out) {\r
+> + err = ENOMEM;\r
+> + goto FAIL;\r
+> + }\r
+> *term_out = out;\r
+> /* Skip the opening quote, find the closing quote, and\r
+> * un-double doubled internal quotes. */\r
+> @@ -148,18 +167,25 @@ parse_boolean_term (void *ctx, const char *str,\r
+> } else {\r
+> const char *start = pos;\r
+> /* Check for text after the boolean term. */\r
+> - while (*pos > ' ' && *pos != ')')\r
+> + while (! is_unquoted_terminator (*pos))\r
+> ++pos;\r
+> - if (*skip_space (pos))\r
+> + if (*skip_space (pos)) {\r
+> + err = EINVAL;\r
+> goto FAIL;\r
+> + }\r
+> /* No trailing text; dup the string so the caller can free\r
+> * it. */\r
+> *term_out = talloc_strndup (ctx, start, pos - start);\r
+> + if (! *term_out) {\r
+> + err = ENOMEM;\r
+> + goto FAIL;\r
+> + }\r
+> }\r
+> return 0;\r
+> \r
+> FAIL:\r
+> talloc_free (*prefix_out);\r
+> talloc_free (*term_out);\r
+> - return 1;\r
+> + errno = err;\r
+> + return -1;\r
+> }\r
+> diff --git a/util/string-util.h b/util/string-util.h\r
+> index 8b9fe50..0194607 100644\r
+> --- a/util/string-util.h\r
+> +++ b/util/string-util.h\r
+> @@ -28,7 +28,8 @@ char *strtok_len (char *s, const char *delim, size_t *len);\r
+> * can be parsed by parse_boolean_term.\r
+> *\r
+> * Output is into buf; it may be talloc_realloced.\r
+> - * Return: 0 on success, non-zero on memory allocation failure.\r
+> + * Return: 0 on success, -1 on error. errno will be set to ENOMEM if\r
+> + * there is an allocation failure.\r
+> */\r
+> int make_boolean_term (void *talloc_ctx, const char *prefix, const char *term,\r
+> char **buf, size_t *len);\r
+> @@ -42,7 +43,8 @@ int make_boolean_term (void *talloc_ctx, const char *prefix, const char *term,\r
+> * of the quoting styles supported by Xapian (and hence notmuch).\r
+> * *prefix_out and *term_out will be talloc'd with context ctx.\r
+> *\r
+> - * Return: 0 on success, non-zero on parse error.\r
+> + * Return: 0 on success, -1 on error. errno will be set to EINVAL if\r
+> + * there is a parse error or ENOMEM if there is an allocation failure.\r
+> */\r
+> int\r
+> parse_boolean_term (void *ctx, const char *str,\r