From: Jani Nikula Date: Sun, 2 Dec 2012 13:29:43 +0000 (+0200) Subject: Re: [Patch v2 13/17] notmuch-restore: add support for input format 'batch-tag' X-Git-Url: http://git.tremily.us/gitweb.cgi?a=commitdiff_plain;h=875a6ed3163bb52c4cfafe7fe8d9395ec66c3ef3;p=notmuch-archives.git Re: [Patch v2 13/17] notmuch-restore: add support for input format 'batch-tag' --- diff --git a/a3/9602fe804fac1b1e019ee6b7218f5f2116ec99 b/a3/9602fe804fac1b1e019ee6b7218f5f2116ec99 new file mode 100644 index 000000000..4899f0935 --- /dev/null +++ b/a3/9602fe804fac1b1e019ee6b7218f5f2116ec99 @@ -0,0 +1,405 @@ +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 99958431FAF + for ; Sun, 2 Dec 2012 05:29:53 -0800 (PST) +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 1GhUazsQcveL for ; + Sun, 2 Dec 2012 05:29:49 -0800 (PST) +Received: from mail-lb0-f181.google.com (mail-lb0-f181.google.com + [209.85.217.181]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) + (No client certificate requested) + by olra.theworths.org (Postfix) with ESMTPS id AF574431FAE + for ; Sun, 2 Dec 2012 05:29:48 -0800 (PST) +Received: by mail-lb0-f181.google.com with SMTP id ge1so1763467lbb.26 + for ; Sun, 02 Dec 2012 05:29:47 -0800 (PST) +X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; + d=google.com; s=20120113; + h=from:to:cc:subject:in-reply-to:references:user-agent:date + :message-id:mime-version:content-type:x-gm-message-state; + bh=4pG3dsHLro2a6t1zGdq7KCFVB5k2FuL7Aixh8+mW97k=; + b=iOG8OULvDABOB7FJf7Wzj4oBmL8rG323NkNMRj+etCZsvLo9H+4gjNIptLqD25o34k + nBmw7naYTezYcET5v5HTojICOYoTbE01b7mHW3c2rN/OzcU1DpdwMMQSEdOZD5Gn/VJq + +16FGNtf3QBlUE44eAsx2m0jTWJRL66vXr9Olk+pn+m95Dug3OnMYScjSsbvJxNimj9x + G//hiLAcxNZxUNSkg5plYVT7RiamSkJpGSpJWseQfgqGMoOkWU/bAv3dXtMm2914uYuf + bzlgoyR4xVWoX+GVUKRykl6rYYhUb1HdQQCmAYZhlHLXjHLygYFnUrDm2vNQg+0ACfp5 + WYNg== +Received: by 10.152.114.65 with SMTP id je1mr6695753lab.33.1354454987058; + Sun, 02 Dec 2012 05:29:47 -0800 (PST) +Received: from localhost (dsl-hkibrasgw4-fe51df00-27.dhcp.inet.fi. + [80.223.81.27]) + by mx.google.com with ESMTPS id b4sm4150390lbi.0.2012.12.02.05.29.45 + (version=SSLv3 cipher=OTHER); Sun, 02 Dec 2012 05:29:46 -0800 (PST) +From: Jani Nikula +To: david@tethera.net, notmuch@notmuchmail.org +Subject: Re: [Patch v2 13/17] notmuch-restore: add support for input format + 'batch-tag' +In-Reply-To: <1353792017-31459-14-git-send-email-david@tethera.net> +References: <1353792017-31459-1-git-send-email-david@tethera.net> + <1353792017-31459-14-git-send-email-david@tethera.net> +User-Agent: Notmuch/0.14+124~g3b17402 (http://notmuchmail.org) Emacs/23.4.1 + (i686-pc-linux-gnu) +Date: Sun, 02 Dec 2012 15:29:43 +0200 +Message-ID: <87zk1wd1ko.fsf@nikula.org> +MIME-Version: 1.0 +Content-Type: text/plain; charset=us-ascii +X-Gm-Message-State: + ALoCoQnkuhDlQ10YCjDeEjMG8q7DRMmvK6smTHhMB09E9uuzdYSSICajMJXHt3UXjYtQLy0EcQ+N +Cc: David Bremner +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, 02 Dec 2012 13:29:53 -0000 + +On Sat, 24 Nov 2012, david@tethera.net wrote: +> From: David Bremner +> +> This is the same as the batch input for notmuch tag, except by default +> it removes all tags before modifying a given message id and only "id:" +> is supported. +> --- +> notmuch-restore.c | 199 +++++++++++++++++++++++++++++++++-------------------- +> 1 file changed, 125 insertions(+), 74 deletions(-) +> +> diff --git a/notmuch-restore.c b/notmuch-restore.c +> index f03dcac..22fcd2d 100644 +> --- a/notmuch-restore.c +> +++ b/notmuch-restore.c +> @@ -19,18 +19,22 @@ +> */ +> +> #include "notmuch-client.h" +> +#include "dump-restore-private.h" +> +#include "tag-util.h" +> +#include "string-util.h" +> + +> +static volatile sig_atomic_t interrupted; +> +static regex_t regex; +> +> static int +> -tag_message (notmuch_database_t *notmuch, const char *message_id, +> - char *file_tags, notmuch_bool_t remove_all, +> - notmuch_bool_t synchronize_flags) +> +tag_message (unused (void *ctx), +> + notmuch_database_t *notmuch, +> + const char *message_id, +> + tag_op_list_t *tag_ops, +> + tag_op_flag_t flags) +> { +> notmuch_status_t status; +> - notmuch_tags_t *db_tags; +> - char *db_tags_str; +> notmuch_message_t *message = NULL; +> - const char *tag; +> - char *next; +> int ret = 0; +> +> status = notmuch_database_find_message (notmuch, message_id, &message); +> @@ -44,55 +48,63 @@ tag_message (notmuch_database_t *notmuch, const char *message_id, +> +> /* In order to detect missing messages, this check/optimization is +> * intentionally done *after* first finding the message. */ +> - if (! remove_all && (file_tags == NULL || *file_tags == '\0')) +> - goto DONE; +> - +> - db_tags_str = NULL; +> - for (db_tags = notmuch_message_get_tags (message); +> - notmuch_tags_valid (db_tags); +> - notmuch_tags_move_to_next (db_tags)) { +> - tag = notmuch_tags_get (db_tags); +> - +> - if (db_tags_str) +> - db_tags_str = talloc_asprintf_append (db_tags_str, " %s", tag); +> - else +> - db_tags_str = talloc_strdup (message, tag); +> - } +> + if ( (flags & TAG_FLAG_REMOVE_ALL) || (tag_op_list_size (tag_ops))) + +Extra space between ('s, and no need to wrap tag_op_list_size call in +braces. + +> + tag_op_list_apply (message, tag_ops, flags); +> +> - if (((file_tags == NULL || *file_tags == '\0') && +> - (db_tags_str == NULL || *db_tags_str == '\0')) || +> - (file_tags && db_tags_str && strcmp (file_tags, db_tags_str) == 0)) + +This is a necessary optimization you're throwing away, but I'll get back +to this after checking the last patch in the series. + +> - goto DONE; +> + if (message) +> + notmuch_message_destroy (message); + +message != NULL always, you can remove the if. + +> +> - notmuch_message_freeze (message); +> + return ret; +> +} +> +> - if (remove_all) +> - notmuch_message_remove_all_tags (message); +> +static int +> +parse_sup_line (void *ctx, char *line, +> + char **query_str, tag_op_list_t *tag_ops) +> +{ +> +> - next = file_tags; +> - while (next) { +> - tag = strsep (&next, " "); +> - if (*tag == '\0') +> - continue; +> - status = notmuch_message_add_tag (message, tag); +> - if (status) { +> - fprintf (stderr, "Error applying tag %s to message %s:\n", +> - tag, message_id); +> - fprintf (stderr, "%s\n", notmuch_status_to_string (status)); +> - ret = 1; +> - } +> + regmatch_t match[3]; +> + char *file_tags; +> + int rerr; +> + +> + tag_op_list_reset (tag_ops); +> + +> + chomp_newline (line); +> + +> + /* Silently ignore blank lines */ +> + if (line[0] == '\0') { +> + return 1; +> + } +> + +> + rerr = xregexec (®ex, line, 3, match, 0); +> + if (rerr == REG_NOMATCH) { +> + fprintf (stderr, "Warning: Ignoring invalid input line: %s\n", +> + line); +> + return 1; +> } +> +> - notmuch_message_thaw (message); +> + *query_str = talloc_strndup (ctx, line + match[1].rm_so, +> + match[1].rm_eo - match[1].rm_so); +> + file_tags = talloc_strndup (ctx, line + match[2].rm_so, +> + match[2].rm_eo - match[2].rm_so); +> +> - if (synchronize_flags) +> - notmuch_message_tags_to_maildir_flags (message); +> + char *tok = file_tags; +> + size_t tok_len = 0; +> +> - DONE: +> - if (message) +> - notmuch_message_destroy (message); +> + tag_op_list_reset (tag_ops); +> + +> + while ((tok = strtok_len (tok + tok_len, " ", &tok_len)) != NULL) { +> + +> + if (*(tok + tok_len) != '\0') { +> + *(tok + tok_len) = '\0'; +> + tok_len++; +> + } +> + +> + if (tag_op_list_append (ctx, tag_ops, tok, FALSE)) +> + return -1; +> + } +> + +> + return 0; +> +> - return ret; +> } +> +> int +> @@ -100,16 +112,19 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) +> { +> notmuch_config_t *config; +> notmuch_database_t *notmuch; +> - notmuch_bool_t synchronize_flags; +> notmuch_bool_t accumulate = FALSE; +> + tag_op_flag_t flags = 0; +> + tag_op_list_t *tag_ops; +> + +> char *input_file_name = NULL; +> FILE *input = stdin; +> char *line = NULL; +> size_t line_size; +> ssize_t line_len; +> - regex_t regex; +> - int rerr; +> + +> + int ret = 0; +> int opt_index; +> + int input_format = DUMP_FORMAT_AUTO; +> +> config = notmuch_config_open (ctx, NULL, NULL); +> if (config == NULL) +> @@ -119,9 +134,15 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) +> NOTMUCH_DATABASE_MODE_READ_WRITE, ¬much)) +> return 1; +> +> - synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config); +> + if (notmuch_config_get_maildir_synchronize_flags (config)) +> + flags |= TAG_FLAG_MAILDIR_SYNC; +> +> notmuch_opt_desc_t options[] = { +> + { NOTMUCH_OPT_KEYWORD, &input_format, "format", 'f', +> + (notmuch_keyword_t []){ { "auto", DUMP_FORMAT_AUTO }, +> + { "batch-tag", DUMP_FORMAT_BATCH_TAG }, +> + { "sup", DUMP_FORMAT_SUP }, +> + { 0, 0 } } }, +> { NOTMUCH_OPT_STRING, &input_file_name, "input", 'i', 0 }, +> { NOTMUCH_OPT_BOOLEAN, &accumulate, "accumulate", 'a', 0 }, +> { 0, 0, 0, 0, 0 } +> @@ -134,6 +155,9 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) +> return 1; +> } +> +> + if (! accumulate) +> + flags |= TAG_FLAG_REMOVE_ALL; +> + +> if (input_file_name) { +> input = fopen (input_file_name, "r"); +> if (input == NULL) { +> @@ -154,35 +178,61 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) +> * non-space characters for the message-id, then one or more +> * spaces, then a list of space-separated tags as a sequence of +> * characters within literal '(' and ')'. */ +> - if ( xregcomp (®ex, +> - "^([^ ]+) \\(([^)]*)\\)$", +> - REG_EXTENDED) ) +> - INTERNAL_ERROR ("compile time constant regex failed."); +> + char *p; +> +> - while ((line_len = getline (&line, &line_size, input)) != -1) { +> - regmatch_t match[3]; +> - char *message_id, *file_tags; +> + line_len = getline (&line, &line_size, input); +> + if (line_len == 0) +> + return 0; +> +> - chomp_newline (line); +> + for (p = line; *p; p++) { +> + if (*p == '(') +> + input_format = DUMP_FORMAT_SUP; +> + } +> +> - rerr = xregexec (®ex, line, 3, match, 0); +> - if (rerr == REG_NOMATCH) { +> - fprintf (stderr, "Warning: Ignoring invalid input line: %s\n", +> - line); +> - continue; +> + if (input_format == DUMP_FORMAT_AUTO) +> + input_format = DUMP_FORMAT_BATCH_TAG; +> + +> + if (input_format == DUMP_FORMAT_SUP) +> + if ( xregcomp (®ex, +> + "^([^ ]+) \\(([^)]*)\\)$", +> + 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; +> + } + +Tag op list creation could be moved earlier to keep the parsing stuff +closer together. + +> + +> + do { +> + char *query_string; +> + +> + if (input_format == DUMP_FORMAT_SUP) { +> + 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); + +Extra spaces after each = above. + +> + +> + if (ret == 0) { +> + if ( strncmp ("id:", query_string, 3) != 0) { +> + fprintf (stderr, "Unsupported query: %s\n", query_string); +> + continue; +> + } + +There should probably be a comment somewhere here saying that everything +after id: is taken to be the message id, i.e. a query of "id:foo AND +tag:bar" will not do what you want. + +An option is to add a TAG_FLAG to accept only message id as the query in +parse_tag_line. It could trim trailing whitespace (if it doesn't +already) and look for spaces in between individual search terms *before* +hex decoding the query string, and fail if it finds any, and then check +for id: prefix after hex decode. + +> + /* delete id: from front of string; tag_message expects a +> + * raw message-id */ +> + query_string = query_string + 3; +> + } +> } +> +> - message_id = xstrndup (line + match[1].rm_so, +> - match[1].rm_eo - match[1].rm_so); +> - file_tags = xstrndup (line + match[2].rm_so, +> - match[2].rm_eo - match[2].rm_so); +> + if (ret > 0) +> + continue; +> +> - tag_message (notmuch, message_id, file_tags, ! accumulate, +> - synchronize_flags); +> + if (ret < 0 || tag_message (ctx, notmuch, query_string, +> + tag_ops, flags)) +> + break; +> + +> + } while ((line_len = getline (&line, &line_size, input)) != -1); +> +> - free (message_id); +> - free (file_tags); +> - } +> +> regfree (®ex); + +Only do this for sup format. + +BR, +Jani. + +> +> @@ -190,8 +240,9 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) +> free (line); +> +> notmuch_database_destroy (notmuch); +> + +> if (input != stdin) +> fclose (input); +> +> - return 0; +> + return ret; +> } +> -- +> 1.7.10.4 +> +> _______________________________________________ +> notmuch mailing list +> notmuch@notmuchmail.org +> http://notmuchmail.org/mailman/listinfo/notmuch