Re: [PATCH v5 0/6] Use Xapian query syntax for batch-tag dump/restore
authorJani Nikula <jani@nikula.org>
Sun, 6 Jan 2013 22:01:49 +0000 (00:01 +0200)
committerW. Trevor King <wking@tremily.us>
Fri, 7 Nov 2014 17:52:58 +0000 (09:52 -0800)
0a/034871d4ef5b229f5e5aac4fe4fcb467e9a288 [new file with mode: 0644]

diff --git a/0a/034871d4ef5b229f5e5aac4fe4fcb467e9a288 b/0a/034871d4ef5b229f5e5aac4fe4fcb467e9a288
new file mode 100644 (file)
index 0000000..d8c9916
--- /dev/null
@@ -0,0 +1,368 @@
+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