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 ABFA3431FB6 for ; Thu, 6 Dec 2012 17:27:10 -0800 (PST) X-Virus-Scanned: Debian amavisd-new at olra.theworths.org X-Spam-Flag: NO X-Spam-Score: 0 X-Spam-Level: X-Spam-Status: No, score=0 tagged_above=-999 required=5 tests=[none] 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 u-Cm-Q3sqx-V for ; Thu, 6 Dec 2012 17:27:09 -0800 (PST) Received: from tesseract.cs.unb.ca (tesseract.cs.unb.ca [131.202.240.238]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by olra.theworths.org (Postfix) with ESMTPS id 3550A431FAE for ; Thu, 6 Dec 2012 17:27:09 -0800 (PST) Received: from fctnnbsc30w-142167090129.dhcp-dynamic.fibreop.nb.bellaliant.net ([142.167.90.129] helo=zancas.localnet) by tesseract.cs.unb.ca with esmtpsa (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:16) (Exim 4.72) (envelope-from ) id 1TgmiZ-0003Np-7E for notmuch@notmuchmail.org; Thu, 06 Dec 2012 21:27:08 -0400 Received: from bremner by zancas.localnet with local (Exim 4.80) (envelope-from ) id 1TgmiT-0004kE-LV for notmuch@notmuchmail.org; Thu, 06 Dec 2012 21:27:01 -0400 From: david@tethera.net To: notmuch@notmuchmail.org Subject: V3b of batch tagging/dump/restore patches Date: Thu, 6 Dec 2012 21:26:38 -0400 Message-Id: <1354843607-17980-1-git-send-email-david@tethera.net> X-Mailer: git-send-email 1.7.10.4 X-Spam_bar: - 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: Fri, 07 Dec 2012 01:27:10 -0000 Here is a second piece of the tagging/dump/restore series. it obsoletes 8 of the patches in the series id:1353792017-31459-1-git-send-email-david@tethera.net [Patch v3b 1/9] notmuch-dump: add --format=(batch-tag|sup) [Patch v3b 3/9] util: add string-util.[ch] [Patch v3b 4/9] tag-util.[ch]: New files for common tagging routines [Patch v3b 5/9] notmuch-restore: add support for input format [Patch v3b 6/9] test: update dump-restore roundtripping test for [Patch v3b 7/9] test: second set of dump/restore --format=batch-tag [Patch v3b 8/9] notmuch-{dump,restore}.1: document new format [Patch v3b 9/9] tag-util: optimization of tag application It adds one new patch [Patch v3b 2/9] test: add sanity check for dump --format=batch-tag. I still have to work through some of the comments on the batch tagging; I still intend for that to follow fairly shortly, I just want to break the series at a logical point. Most of the changes are detailed in the following log (of changes before I squashed them) commit 5045d2f58beb4c3bc8e10f9419341e1c1b7748f2 Author: David Bremner Date: Tue Dec 4 13:39:48 2012 -0400 fixup for tag-util error messages diff --git a/tag-util.c b/tag-util.c index 2bb8355..ea05ee5 100644 --- a/tag-util.c +++ b/tag-util.c @@ -86,9 +86,8 @@ parse_tag_line (void *ctx, char *line, /* tok now points to the query string */ if (hex_decode_inplace (tok) != HEX_SUCCESS) { - /* FIXME: line has been modified! */ - fprintf (stderr, "Warning: Ignoring invalid input line: %s\n", - line); + fprintf (stderr, "Hex decoding of %s failed\n", + tok); return 1; } commit 5a1d697dc408c67424d586b6377976fdfb86d4ed Author: David Bremner Date: Tue Dec 4 13:40:09 2012 -0400 fixup for restore error messages diff --git a/notmuch-restore.c b/notmuch-restore.c index 22fcd2d..e7584bb 100644 --- a/notmuch-restore.c +++ b/notmuch-restore.c @@ -77,7 +77,7 @@ parse_sup_line (void *ctx, char *line, rerr = xregexec (®ex, line, 3, match, 0); if (rerr == REG_NOMATCH) { - fprintf (stderr, "Warning: Ignoring invalid input line: %s\n", + fprintf (stderr, "Warning: Ignoring invalid sup format line: %s\n", line); return 1; } commit 7136d3b4e974a4ba8e247f328c71362efd0c9e11 Author: David Bremner Date: Tue Dec 4 22:35:30 2012 -0400 fixup for tag-util error handling and permit the null set of tag operations diff --git a/tag-util.c b/tag-util.c index ea05ee5..de7ecc8 100644 --- a/tag-util.c +++ b/tag-util.c @@ -21,6 +21,8 @@ parse_tag_line (void *ctx, char *line, { char *tok = line; size_t tok_len = 0; + char *line_for_error=talloc_strdup (ctx, line); + int ret=0; chomp_newline (line); @@ -29,8 +31,10 @@ parse_tag_line (void *ctx, char *line, tok++; /* Skip empty and comment lines. */ - if (*tok == '\0' || *tok == '#') - return 1; + if (*tok == '\0' || *tok == '#') { + ret=1; + goto DONE; + } tag_op_list_reset (tag_ops); @@ -51,8 +55,9 @@ parse_tag_line (void *ctx, char *line, /* If tag is terminated by NUL, there's no query string. */ if (*(tok + tok_len) == '\0') { - tok = NULL; - break; + fprintf (stderr, "no query string: %s\n", line_for_error); + ret = 1; + goto DONE; } /* Terminate, and start next token after terminator. */ @@ -63,37 +68,43 @@ parse_tag_line (void *ctx, char *line, /* Maybe refuse empty tags. */ if (!(flags & TAG_FLAG_BE_GENEROUS) && *tag == '\0') { - tok = NULL; - break; + fprintf (stderr, "Error: empty tag: %s\n", line_for_error); + goto DONE; } /* Decode tag. */ if (hex_decode_inplace (tag) != HEX_SUCCESS) { - tok = NULL; - break; + fprintf (stderr, "Hex decoding of tag %s failed\n", + tag); + ret = 1; + goto DONE; } - if (tag_op_list_append (ctx, tag_ops, tag, remove)) - return -1; + if (tag_op_list_append (ctx, tag_ops, tag, remove)) { + ret = -1; + goto DONE; + } } - if (tok == NULL || tag_ops->count == 0) { - /* FIXME: line has been modified! */ + if (tok == NULL) { fprintf (stderr, "Warning: Ignoring invalid input line: %s\n", - line); - return 1; + line_for_error); + ret = 1; + goto DONE; } /* tok now points to the query string */ if (hex_decode_inplace (tok) != HEX_SUCCESS) { - fprintf (stderr, "Hex decoding of %s failed\n", + fprintf (stderr, "Hex decoding of query %s failed\n", tok); - return 1; + ret = 1; + goto DONE; } *query_string = tok; - - return 0; + DONE: + talloc_free (line_for_error); + return ret; } static inline void commit 5912c738d3683aae24ca5529839eb0513520d190 Author: David Bremner Date: Thu Dec 6 07:49:15 2012 -0400 fixup for id:87wqx1qrmq.fsf@nikula.org; use size_t for tag_op_list count diff --git a/tag-util.c b/tag-util.c index de7ecc8..1a0cf53 100644 --- a/tag-util.c +++ b/tag-util.c @@ -1,6 +1,7 @@ #include "string-util.h" #include "tag-util.h" #include "hex-escape.h" +#include struct _tag_operation_t { const char *tag; @@ -9,8 +10,8 @@ struct _tag_operation_t { struct _tag_op_list_t { tag_operation_t *ops; - int count; - int size; + size_t count; + size_t size; }; int @@ -44,7 +45,7 @@ parse_tag_line (void *ctx, char *line, char *tag; /* Optional explicit end of tags marker. */ - if (strncmp (tok, "--", tok_len) == 0) { + if (tok_len == 2 && strncmp (tok, "--", tok_len) == 0) { tok = strtok_len (tok + tok_len, " ", &tok_len); break; } @@ -126,17 +127,16 @@ makes_changes (notmuch_message_t *message, tag_op_list_t *list, tag_op_flag_t flags) { - - int i; - notmuch_tags_t *tags; notmuch_bool_t changes = FALSE; + size_t i; /* First, do we delete an existing tag? */ changes = FALSE; for (tags = notmuch_message_get_tags (message); ! changes && notmuch_tags_valid (tags); notmuch_tags_move_to_next (tags)) { + const char *cur_tag = notmuch_tags_get (tags); int last_op = (flags & TAG_FLAG_REMOVE_ALL) ? -1 : 0; @@ -182,8 +182,7 @@ tag_op_list_apply (notmuch_message_t *message, tag_op_list_t *list, tag_op_flag_t flags) { - int i; - + size_t i; notmuch_status_t status = 0; tag_operation_t *tag_ops = list->ops; @@ -199,7 +198,7 @@ tag_op_list_apply (notmuch_message_t *message, if (flags & TAG_FLAG_REMOVE_ALL) { status = notmuch_message_remove_all_tags (message); if (status) { - message_error (message, status, "removing all tags" ); + message_error (message, status, "removing all tags"); return status; } } @@ -241,8 +240,8 @@ tag_op_list_apply (notmuch_message_t *message, } -/* Array of tagging operations (add or remove), terminated with an - * empty element. Size will be increased as necessary. */ +/* Array of tagging operations (add or remove. Size will be increased + * as necessary. */ tag_op_list_t * tag_op_list_create (void *ctx) @@ -299,6 +298,7 @@ tag_op_list_append (void *ctx, notmuch_bool_t tag_op_list_isremove (const tag_op_list_t *list, size_t i) { + assert (i < list->count); return list->ops[i].remove; } @@ -329,5 +329,6 @@ tag_op_list_size (const tag_op_list_t *list) const char * tag_op_list_tag (const tag_op_list_t *list, size_t i) { + assert (i < list->count); return list->ops[i].tag; } commit e1af69d57854d5c6e927b0870be97e9d2e2f28ea Author: David Bremner Date: Thu Dec 6 07:51:43 2012 -0400 changes for id:87wqx1qrmq.fsf@nikula.org. tag_op_list_t.count -> size_t diff --git a/tag-util.c b/tag-util.c index 1a0cf53..ad13147 100644 --- a/tag-util.c +++ b/tag-util.c @@ -22,8 +22,8 @@ parse_tag_line (void *ctx, char *line, { char *tok = line; size_t tok_len = 0; - char *line_for_error=talloc_strdup (ctx, line); - int ret=0; + char *line_for_error = talloc_strdup (ctx, line); + int ret = 0; chomp_newline (line); @@ -33,7 +33,7 @@ parse_tag_line (void *ctx, char *line, /* Skip empty and comment lines. */ if (*tok == '\0' || *tok == '#') { - ret=1; + ret = 1; goto DONE; } @@ -68,7 +68,7 @@ parse_tag_line (void *ctx, char *line, tag = tok + 1; /* Maybe refuse empty tags. */ - if (!(flags & TAG_FLAG_BE_GENEROUS) && *tag == '\0') { + if (! (flags & TAG_FLAG_BE_GENEROUS) && *tag == '\0') { fprintf (stderr, "Error: empty tag: %s\n", line_for_error); goto DONE; } @@ -76,7 +76,7 @@ parse_tag_line (void *ctx, char *line, /* Decode tag. */ if (hex_decode_inplace (tag) != HEX_SUCCESS) { fprintf (stderr, "Hex decoding of tag %s failed\n", - tag); + tag); ret = 1; goto DONE; } @@ -103,7 +103,7 @@ parse_tag_line (void *ctx, char *line, } *query_string = tok; - DONE: + DONE: talloc_free (line_for_error); return ret; } commit 3f00fa4eba68876635df86ea54f60b68d172f580 Author: David Bremner Date: Thu Dec 6 08:30:58 2012 -0400 changes for id:87zk1wd1ko.fsf@nikula.org diff --git a/notmuch-restore.c b/notmuch-restore.c index e7584bb..41b742f 100644 --- a/notmuch-restore.c +++ b/notmuch-restore.c @@ -48,11 +48,10 @@ tag_message (unused (void *ctx), /* In order to detect missing messages, this check/optimization is * intentionally done *after* first finding the message. */ - if ( (flags & TAG_FLAG_REMOVE_ALL) || (tag_op_list_size (tag_ops))) + if ((flags & TAG_FLAG_REMOVE_ALL) || tag_op_list_size (tag_ops)) tag_op_list_apply (message, tag_ops, flags); - if (message) - notmuch_message_destroy (message); + notmuch_message_destroy (message); return ret; } @@ -184,6 +183,12 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) if (line_len == 0) return 0; + tag_ops = tag_op_list_create (ctx); + if (tag_ops == NULL) { + fprintf (stderr, "Out of memory.\n"); + return 1; + } + for (p = line; *p; p++) { if (*p == '(') input_format = DUMP_FORMAT_SUP; @@ -198,28 +203,28 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) REG_EXTENDED) ) INTERNAL_ERROR ("compile time constant regex failed."); - tag_ops = tag_op_list_create (ctx); - if (tag_ops == NULL) { - fprintf (stderr, "Out of memory.\n"); - return 1; - } - do { char *query_string; if (input_format == DUMP_FORMAT_SUP) { - ret = parse_sup_line (ctx, line, &query_string, tag_ops); + ret = parse_sup_line (ctx, line, &query_string, tag_ops); } else { - ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS, - &query_string, tag_ops); + ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS, + &query_string, tag_ops); if (ret == 0) { - if ( strncmp ("id:", query_string, 3) != 0) { + if (strncmp ("id:", query_string, 3) != 0) { fprintf (stderr, "Unsupported query: %s\n", query_string); continue; } - /* delete id: from front of string; tag_message expects a - * raw message-id */ + /* delete id: from front of string; tag_message + * expects a raw message-id. + * + * XXX: Note that query string id:foo and bar will be + * interpreted as a message id "foo and bar". This + * should eventually be fixed to give a better error + * message. + */ query_string = query_string + 3; } } @@ -233,8 +238,8 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) } while ((line_len = getline (&line, &line_size, input)) != -1); - - regfree (®ex); + if (input_format == DUMP_FORMAT_SUP) + regfree (®ex); if (line) free (line); commit 473fa928931080004706cff169c7cc9337601172 Author: David Bremner Date: Thu Dec 6 13:33:42 2012 -0400 fixup: notmuch-restore only auto-detect in auto mode diff --git a/notmuch-restore.c b/notmuch-restore.c index 41b742f..ceec2d3 100644 --- a/notmuch-restore.c +++ b/notmuch-restore.c @@ -189,7 +189,7 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) return 1; } - for (p = line; *p; p++) { + for (p = line; (input_format == DUMP_FORMAT_AUTO) && *p; p++) { if (*p == '(') input_format = DUMP_FORMAT_SUP; } commit ee7d25521e3f6f70b3f4fb79f586a11d39efec15 Author: David Bremner Date: Thu Dec 6 19:39:37 2012 -0400 changes for id:87wqx0d124.fsf@nikula.org; no deprecation for the moment diff --git a/man/man1/notmuch-dump.1 b/man/man1/notmuch-dump.1 index 9f59905..770b00f 100644 --- a/man/man1/notmuch-dump.1 +++ b/man/man1/notmuch-dump.1 @@ -64,15 +64,16 @@ and tags containing whitespace or non-\fBascii\fR(7) characters. Each line has the form .RS 4 -.RI "+<" "encoded-tag" "> " "" "+<" "encoded-tag" "> ... -- " "" " <" encoded-message-id > +.RI "+<" "encoded-tag" "> " "" "+<" "encoded-tag" "> ... -- " "" " id:<" encoded-message-id > where encoded means that every byte not matching the regex -.B [A-Za-z0-9+-_@=.:,] +.B [A-Za-z0-9@=.,_+-] is replace by .B %nn where nn is the two digit hex encoding. The astute reader will notice this is a special case of the batch input -format for \fBnotmuch-tag\fR(1). +format for \fBnotmuch-tag\fR(1); note that the single message-id query is +mandatory for \fBnotmuch-restore\fR(1). .RE diff --git a/man/man1/notmuch-restore.1 b/man/man1/notmuch-restore.1 index 3860829..6bba628 100644 --- a/man/man1/notmuch-restore.1 +++ b/man/man1/notmuch-restore.1 @@ -32,8 +32,8 @@ replacing each message's tags as they are read in from the dump file. .TP 4 .B \-\-format=(sup|batch-tag|auto) -Notmuch restore supports two plain text dump formats, with one message-id -per line, and a list of tags. +Notmuch restore supports two plain text dump formats, with each line +specifying a message-id and a set of tags. For details of the actual formats, see \fBnotmuch-dump\fR(1). .RS 4 commit 96c383be46cdb8ceaf7ed15590ef876799d6357e Author: David Bremner Date: Thu Dec 6 20:40:55 2012 -0400 Changes for id:87txs4cy7v.fsf@nikula.org diff --git a/tag-util.c b/tag-util.c index ad13147..9ab07e9 100644 --- a/tag-util.c +++ b/tag-util.c @@ -140,9 +140,11 @@ makes_changes (notmuch_message_t *message, const char *cur_tag = notmuch_tags_get (tags); int last_op = (flags & TAG_FLAG_REMOVE_ALL) ? -1 : 0; - for (i = 0; i < list->count; i++) { + /* slight contortions to count down with an unsigned index */ + for (i = list->count; i-- > 0; /*nothing*/) { if (strcmp (cur_tag, list->ops[i].tag) == 0) { last_op = list->ops[i].remove ? -1 : 1; + break; } } @@ -157,6 +159,9 @@ makes_changes (notmuch_message_t *message, for (i = 0; i < list->count; i++) { notmuch_bool_t exists = FALSE; + if (list->ops[i].remove) + continue; + for (tags = notmuch_message_get_tags (message); notmuch_tags_valid (tags); notmuch_tags_move_to_next (tags)) { @@ -168,9 +173,11 @@ makes_changes (notmuch_message_t *message, } notmuch_tags_destroy (tags); - /* the following test is conservative, it's ok to think we - * make changes when we don't */ - if ( ! exists && ! list->ops[i].remove ) + /* the following test is conservative, + * in the sense it ignores cases like +foo ... -foo + * but this is OK from a correctness point of view + */ + if (! exists) return TRUE; } return FALSE;