[PATCH 3/3] notmuch-restore: allocate a temporary talloc context for each line parsed.
From: David BremnerThis lets the high level code in notmuch restore be ignorant about what the lower level code is doing as far as allocating memory. --- notmuch-restore.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/notmuch-restore.c b/notmuch-restore.c index 665373f..9ed9b51 100644 --- a/notmuch-restore.c +++ b/notmuch-restore.c @@ -125,6 +125,7 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) char *input_file_name = NULL; FILE *input = stdin; char *line = NULL; +void *line_ctx = NULL; size_t line_size; ssize_t line_len; @@ -208,10 +209,14 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) do { char *query_string; + if (line_ctx != NULL) + talloc_free (line_ctx); + + line_ctx = talloc_new (ctx); if (input_format == DUMP_FORMAT_SUP) { - ret = parse_sup_line (ctx, line, _string, tag_ops); + ret = parse_sup_line (line_ctx, line, _string, tag_ops); } else { - ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS, + ret = parse_tag_line (line_ctx, line, TAG_FLAG_BE_GENEROUS, _string, tag_ops); if (ret == 0) { @@ -244,6 +249,9 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) } while ((line_len = getline (, _size, input)) != -1); +if (line_ctx != NULL) + talloc_free (line_ctx); + if (input_format == DUMP_FORMAT_SUP) regfree (); -- 1.7.10.4
[PATCH 2/3] tag-utils: use the tag_opt_list_t as talloc context, if possible.
From: David BremnerThis code is no less correct than the previous version, since it does not make sense for the array to live longer than the wrapping struct. By not relying on the context passed into tag_parse_line, we can allow tag_op_list_t structures to live longer than that context. --- notmuch-restore.c |2 +- tag-util.c|9 - tag-util.h|3 +-- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/notmuch-restore.c b/notmuch-restore.c index 5a02328..665373f 100644 --- a/notmuch-restore.c +++ b/notmuch-restore.c @@ -105,7 +105,7 @@ parse_sup_line (void *ctx, char *line, tok_len++; } - if (tag_op_list_append (ctx, tag_ops, tok, FALSE)) + if (tag_op_list_append (tag_ops, tok, FALSE)) return -1; } diff --git a/tag-util.c b/tag-util.c index eab482f..705b7ba 100644 --- a/tag-util.c +++ b/tag-util.c @@ -109,7 +109,7 @@ parse_tag_line (void *ctx, char *line, goto DONE; } - if (tag_op_list_append (ctx, tag_ops, tag, remove)) { + if (tag_op_list_append (tag_ops, tag, remove)) { ret = line_error (TAG_PARSE_OUT_OF_MEMORY, line_for_error, "aborting"); goto DONE; @@ -294,7 +294,7 @@ tag_op_list_create (void *ctx) list->size = TAG_OP_LIST_INITIAL_SIZE; list->count = 0; -list->ops = talloc_array (ctx, tag_operation_t, list->size); +list->ops = talloc_array (list, tag_operation_t, list->size); if (list->ops == NULL) return NULL; @@ -303,8 +303,7 @@ tag_op_list_create (void *ctx) int -tag_op_list_append (void *ctx, - tag_op_list_t *list, +tag_op_list_append (tag_op_list_t *list, const char *tag, notmuch_bool_t remove) { @@ -314,7 +313,7 @@ tag_op_list_append (void *ctx, if (list->count == list->size) { list->size *= 2; - list->ops = talloc_realloc (ctx, list->ops, tag_operation_t, + list->ops = talloc_realloc (list, list->ops, tag_operation_t, list->size); if (list->ops == NULL) { fprintf (stderr, "Out of memory.\n"); diff --git a/tag-util.h b/tag-util.h index 99b0fa0..c07bfde 100644 --- a/tag-util.h +++ b/tag-util.h @@ -87,8 +87,7 @@ tag_op_list_create (void *ctx); */ int -tag_op_list_append (void *ctx, - tag_op_list_t *list, +tag_op_list_append (tag_op_list_t *list, const char *tag, notmuch_bool_t remove); -- 1.7.10.4
[PATCH 1/3] notmuch-restore: fix return value propagation
From: David BremnerPreviously notmuch_restore_command returned 0 if tag_message returned a non-zero (failure) value. This is wrong, since non-zero status indicates something mysterious went wrong with retrieving the message, or applying it. There was also a failure to check or propagate the return value from tag_op_list_apply in tag_message. --- notmuch-restore.c | 13 ++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/notmuch-restore.c b/notmuch-restore.c index 40596a8..5a02328 100644 --- a/notmuch-restore.c +++ b/notmuch-restore.c @@ -25,6 +25,9 @@ static regex_t regex; +/* Non-zero return indicates an error in retrieving the message, + * or in applying the tags. + */ static int tag_message (unused (void *ctx), notmuch_database_t *notmuch, @@ -48,7 +51,7 @@ 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)) - tag_op_list_apply (message, tag_ops, flags); + ret = tag_op_list_apply (message, tag_ops, flags); notmuch_message_destroy (message); @@ -231,8 +234,12 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) if (ret > 0) continue; - if (ret < 0 || tag_message (ctx, notmuch, query_string, - tag_ops, flags)) + if (ret < 0) + break; + + ret = tag_message (line_ctx, notmuch, query_string, + tag_ops, flags); + if (ret) break; } while ((line_len = getline (, _size, input)) != -1); -- 1.7.10.4
v2 restore leak fix
This obsoletes id:1355688997-19164-1-git-send-email-david at tethera.net Actually the first patch in the series is now an unrelated bug fix, since it isn't needed anymore for 2 and 3.
[RFC PATCH] cli: add --remove-all option to "notmuch tag"
On Sun, 16 Dec 2012, Mark Walters wrote: > On Mon, 03 Dec 2012, Jani Nikula wrote: >> Add --remove-all option to "notmuch tag" to remove all tags matching >> query before applying the tag changes. This allows removal and >> unconditional setting of the tags of a message: >> >> $ notmuch tag --remove-all id:foo at example.com >> $ notmuch tag --remove-all +foo +bar id:foo at example.com >> >> without having to resort to the complicated (and still quoting broken): >> >> $ notmuch tag $(notmuch search --output=tags '*' | sed 's/^/-/') id:foo at >> example.com >> $ notmuch tag $(notmuch search --output=tags '*' | sed 's/^/-/') +foo +bar >> id:foo at example.com >> >> --- >> >> It's completely untested... >> >> This is on top of David's batch tagging branch new-tagging at >> git://pivot.cs.unb.ca/notmuch.git >> >> If there's interest, I'll spin a new version with tests and man page >> changes after David's stuff has been merged. > > I like this: the possibility of setting the tags to something seems > nice. Thanks for the feedback; I'll rebase once we're done with the batch tagging stuff. > I am not very keen on the option name but don't have anything much > better: maybe --remove-all-first or --set-to? I still like --remove-all, and, like you say, those aren't much better. But we can bikeshed this later. ;) > Incidentally, does this (or should this) give an error if the user calls > --remove-all with -some_tag (as opposed to +some_tag)? I think not. There are no errors if you do -foo -foo, or remove a tag that isn't in any of the messages matching query. BR, Jani. > > Best wishes > > Mark > > >> --- >> notmuch-tag.c | 32 >> 1 file changed, 20 insertions(+), 12 deletions(-) >> >> diff --git a/notmuch-tag.c b/notmuch-tag.c >> index e4fca67..67624cc 100644 >> --- a/notmuch-tag.c >> +++ b/notmuch-tag.c >> @@ -119,12 +119,15 @@ tag_query (void *ctx, notmuch_database_t *notmuch, >> const char *query_string, >> notmuch_messages_t *messages; >> notmuch_message_t *message; >> >> -/* Optimize the query so it excludes messages that already have >> - * the specified set of tags. */ >> -query_string = _optimize_tag_query (ctx, query_string, tag_ops); >> -if (query_string == NULL) { >> -fprintf (stderr, "Out of memory.\n"); >> -return 1; >> +if (! (flags & TAG_FLAG_REMOVE_ALL)) { >> +/* Optimize the query so it excludes messages that already >> + * have the specified set of tags. */ >> +query_string = _optimize_tag_query (ctx, query_string, tag_ops); >> +if (query_string == NULL) { >> +fprintf (stderr, "Out of memory.\n"); >> +return 1; >> +} >> +flags |= TAG_FLAG_PRE_OPTIMIZED; >> } >> >> query = notmuch_query_create (notmuch, query_string); >> @@ -140,7 +143,7 @@ tag_query (void *ctx, notmuch_database_t *notmuch, const >> char *query_string, >> notmuch_messages_valid (messages) && ! interrupted; >> notmuch_messages_move_to_next (messages)) { >> message = notmuch_messages_get (messages); >> -tag_op_list_apply (message, tag_ops, flags | TAG_FLAG_PRE_OPTIMIZED); >> +tag_op_list_apply (message, tag_ops, flags); >> notmuch_message_destroy (message); >> } >> >> @@ -157,8 +160,9 @@ notmuch_tag_command (void *ctx, int argc, char *argv[]) >> notmuch_config_t *config; >> notmuch_database_t *notmuch; >> struct sigaction action; >> -tag_op_flag_t synchronize_flags = TAG_FLAG_NONE; >> +tag_op_flag_t flags = TAG_FLAG_NONE; >> notmuch_bool_t batch = FALSE; >> +notmuch_bool_t remove_all = FALSE; >> FILE *input = stdin; >> char *input_file_name = NULL; >> int i, opt_index; >> @@ -174,6 +178,7 @@ notmuch_tag_command (void *ctx, int argc, char *argv[]) >> notmuch_opt_desc_t options[] = { >> { NOTMUCH_OPT_BOOLEAN, , "batch", 0, 0 }, >> { NOTMUCH_OPT_STRING, _file_name, "input", 'i', 0 }, >> +{ NOTMUCH_OPT_BOOLEAN, _all, "remove-all", 0, 0 }, >> { 0, 0, 0, 0, 0 } >> }; >> >> @@ -230,7 +235,7 @@ notmuch_tag_command (void *ctx, int argc, char *argv[]) >> } >> } >> >> -if (tag_op_list_size (tag_ops) == 0) { >> +if (tag_op_list_size (tag_ops) == 0 && !remove_all) { >> fprintf (stderr, "Error: 'notmuch tag' requires at least one tag to >> add or remove.\n"); >> return 1; >> } >> @@ -252,7 +257,10 @@ notmuch_tag_command (void *ctx, int argc, char *argv[]) >> return 1; >> >> if (notmuch_config_get_maildir_synchronize_flags (config)) >> -synchronize_flags = TAG_FLAG_MAILDIR_SYNC; >> +flags |= TAG_FLAG_MAILDIR_SYNC; >> + >> +if (remove_all) >> +flags |= TAG_FLAG_REMOVE_ALL; >> >> if (batch) { >> >> @@ -280,14 +288,14 @@ notmuch_tag_command (void *ctx, int argc, char *argv[]) >> continue; >> >> if (ret < 0 || tag_query (ctx, notmuch, query_string, >> -
[PATCH 3/3] notmuch-restore: use xtalloc version of strndup
From: David BremnerThis gives line numbers for better debugging. --- notmuch-restore.c |7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/notmuch-restore.c b/notmuch-restore.c index 40596a8..0cc9c9f 100644 --- a/notmuch-restore.c +++ b/notmuch-restore.c @@ -85,9 +85,10 @@ parse_sup_line (void *ctx, char *line, return 1; } -*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, +*query_str = xtalloc_strndup (ctx, line + match[1].rm_so, + match[1].rm_eo - match[1].rm_so); + +file_tags = xtalloc_strndup (ctx, line + match[2].rm_so, match[2].rm_eo - match[2].rm_so); char *tok = file_tags; -- 1.7.10.4
[PATCH 2/3] util: add xtalloc.[ch]
From: David BremnerThese are intended to be simple wrappers to provide slightly better debugging information than what talloc currently provides natively. --- notmuch-client.h|2 +- util/Makefile.local |2 +- util/xtalloc.c | 15 +++ util/xtalloc.h | 18 ++ 4 files changed, 35 insertions(+), 2 deletions(-) create mode 100644 util/xtalloc.c create mode 100644 util/xtalloc.h diff --git a/notmuch-client.h b/notmuch-client.h index d7b352e..60be030 100644 --- a/notmuch-client.h +++ b/notmuch-client.h @@ -58,7 +58,7 @@ typedef GMimeCipherContext notmuch_crypto_context_t; #include #include -#include +#include "xtalloc.h" #define unused(x) x __attribute__ ((unused)) diff --git a/util/Makefile.local b/util/Makefile.local index a11e35b..8a62c00 100644 --- a/util/Makefile.local +++ b/util/Makefile.local @@ -4,7 +4,7 @@ dir := util extra_cflags += -I$(srcdir)/$(dir) libutil_c_srcs := $(dir)/xutil.c $(dir)/error_util.c $(dir)/hex-escape.c \ - $(dir)/string-util.c + $(dir)/string-util.c $(dir)/xtalloc.c libutil_modules := $(libutil_c_srcs:.c=.o) diff --git a/util/xtalloc.c b/util/xtalloc.c new file mode 100644 index 000..22834bd --- /dev/null +++ b/util/xtalloc.c @@ -0,0 +1,15 @@ +#include +#include "xtalloc.h" + +char * +xtalloc_strndup_named_const (void *ctx, const char *str, +size_t len, const char *name) +{ +char *ptr = talloc_named_const (ctx, len + 1, name); + +if (ptr) { + memcpy (ptr, str, len); + *(ptr + len) = '\0'; +} +return ptr; +} diff --git a/util/xtalloc.h b/util/xtalloc.h new file mode 100644 index 000..3cc1179 --- /dev/null +++ b/util/xtalloc.h @@ -0,0 +1,18 @@ +#ifndef _XTALLOC_H +#define _XTALLOC_H + +#include + +/* Like talloc_strndup, but take an extra parameter for the internal talloc + * name (for debugging) */ + +char * +xtalloc_strndup_named_const (void *ctx, const char *str, +size_t len, const char *name); + +/* use the __location__ macro from talloc.h to name a string according to its + * source location */ + +#define xtalloc_strndup(ctx, str, len) xtalloc_strndup_named_const (ctx, str, len, __location__) + +#endif -- 1.7.10.4
[PATCH 1/3] CLI: add talloc leak report, controlled by an environment variable.
From: David BremnerThe argument handling in notmuch.c seems due for an overhaul, but until then use an environment variable to specify a location to write the talloc leak report to. This is only enabled for the (interesting) case where some notmuch subcommand is invoked. --- notmuch.c | 16 ++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/notmuch.c b/notmuch.c index 9516dfb..fb49c5a 100644 --- a/notmuch.c +++ b/notmuch.c @@ -322,8 +322,20 @@ main (int argc, char *argv[]) for (i = 0; i < ARRAY_SIZE (commands); i++) { command = [i]; - if (strcmp (argv[1], command->name) == 0) - return (command->function) (local, argc - 1, [1]); + if (strcmp (argv[1], command->name) == 0) { + int ret; + char *talloc_report; + + ret = (command->function) (local, argc - 1, [1]); + + talloc_report=getenv ("NOTMUCH_TALLOC_REPORT"); + if (talloc_report && strcmp(talloc_report, "") != 0) { + FILE *report = fopen (talloc_report, "w"); + talloc_report_full (NULL, report); + } + + return ret; + } } fprintf (stderr, "Error: Unknown command '%s' (see \"notmuch help\")\n", -- 1.7.10.4
talloc leak debugging
I was playing a bit with adding talloc leak debugging to notmuch (since it seems to be essentially free, performance-wise). I got a bit discouraged about modifying the argument handling in notmuch.c, so I hacked it in via an environment variable.
[PATCH v3 5/5] man: document notmuch search --format=text0
--- man/man1/notmuch-search.1 | 26 -- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/man/man1/notmuch-search.1 b/man/man1/notmuch-search.1 index 0aff348..22bcd0a 100644 --- a/man/man1/notmuch-search.1 +++ b/man/man1/notmuch-search.1 @@ -25,9 +25,11 @@ Supported options for include .RS 4 .TP 4 -.BR \-\-format= ( json | sexp | text ) +.BR \-\-format= ( json | sexp | text | text0 ) -Presents the results in either JSON, S-Expressions or plain-text (default). +Presents the results in either JSON, S-Expressions, newline character +separated plain-text (default), or null character separated plain-text +(compatible with \fBxargs\fR(1) -0 option where available). .RE .RS 4 @@ -48,32 +50,36 @@ the authors of the thread and the subject. .B threads Output the thread IDs of all threads with any message matching the -search terms, either one per line (\-\-format=text) or as a JSON array -(\-\-format=json) or an S-Expression list (\-\-format=sexp). +search terms, either one per line (\-\-format=text), separated by null +characters (\-\-format=text0), as a JSON array (\-\-format=json), or +an S-Expression list (\-\-format=sexp). .RE .RS 4 .TP 4 .B messages Output the message IDs of all messages matching the search terms, -either one per line (\-\-format=text) or as a JSON array -(\-\-format=json) or as an S-Expression list (\-\-format=sexp). +either one per line (\-\-format=text), separated by null characters +(\-\-format=text0), as a JSON array (\-\-format=json), or as an +S-Expression list (\-\-format=sexp). .RE .RS 4 .TP 4 .B files Output the filenames of all messages matching the search terms, either -one per line (\-\-format=text) or as a JSON array (\-\-format=json) or -as an S-Expression list (\-\-format=sexp). +one per line (\-\-format=text), separated by null characters +(\-\-format=text0), as a JSON array (\-\-format=json), or as an +S-Expression list (\-\-format=sexp). .RE .RS 4 .TP 4 .B tags Output all tags that appear on any message matching the search terms, -either one per line (\-\-format=text) or as a JSON array (\-\-format=json) -or as an S-Expression list (\-\-format=sexp). +either one per line (\-\-format=text), separated by null characters +(\-\-format=text0), as a JSON array (\-\-format=json), or as an +S-Expression list (\-\-format=sexp). .RE .RE -- 1.7.10.4
[PATCH v3 4/5] test: notmuch search --format=text0
--- test/text | 33 + 1 file changed, 33 insertions(+) diff --git a/test/text b/test/text index 428c89b..b5ccefc 100755 --- a/test/text +++ b/test/text @@ -52,4 +52,37 @@ output=$(notmuch search --format=text "t?xt-search-m?ssage" | notmuch_search_s test_expect_equal "$output" \ "thread:XXX 2000-01-01 [1/1] Notmuch Test Suite; text-search-utf8-body-s?bj?ct (inbox unread)" +add_email_corpus + +test_begin_subtest "Search message tags: text0" +cat < EXPECTED +attachment inbox signed unread +EOF +notmuch search --format=text0 --output=tags '*' | xargs -0 | notmuch_search_sanitize > OUTPUT +test_expect_equal_file EXPECTED OUTPUT + +# Use tr(1) to convert --output=text0 to --output=text for +# comparison. Also translate newlines to spaces to fail with more +# noise if they are present as delimiters instead of null +# characters. This assumes there are no newlines in the data. +test_begin_subtest "Compare text vs. text0 for threads" +notmuch search --format=text --output=threads '*' | notmuch_search_sanitize > EXPECTED +notmuch search --format=text0 --output=threads '*' | tr "\n\0" " \n" | notmuch_search_sanitize > OUTPUT +test_expect_equal_file EXPECTED OUTPUT + +test_begin_subtest "Compare text vs. text0 for messages" +notmuch search --format=text --output=messages '*' | notmuch_search_sanitize > EXPECTED +notmuch search --format=text0 --output=messages '*' | tr "\n\0" " \n" | notmuch_search_sanitize > OUTPUT +test_expect_equal_file EXPECTED OUTPUT + +test_begin_subtest "Compare text vs. text0 for files" +notmuch search --format=text --output=files '*' | notmuch_search_sanitize > EXPECTED +notmuch search --format=text0 --output=files '*' | tr "\n\0" " \n" | notmuch_search_sanitize > OUTPUT +test_expect_equal_file EXPECTED OUTPUT + +test_begin_subtest "Compare text vs. text0 for tags" +notmuch search --format=text --output=tags '*' | notmuch_search_sanitize > EXPECTED +notmuch search --format=text0 --output=tags '*' | tr "\n\0" " \n" | notmuch_search_sanitize > OUTPUT +test_expect_equal_file EXPECTED OUTPUT + test_done -- 1.7.10.4
[PATCH v3 3/5] cli: add --format=text0 to notmuch search
Add new format text0, which is otherwise the same as text, but use the null character as separator instead of the newline character. This is similar to find(1) -print0 option, and works together with the xargs(1) -0 option. --- notmuch-search.c | 16 ++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/notmuch-search.c b/notmuch-search.c index 6218622..627962b 100644 --- a/notmuch-search.c +++ b/notmuch-search.c @@ -305,8 +305,12 @@ notmuch_search_command (void *ctx, int argc, char *argv[]) int exclude = EXCLUDE_TRUE; unsigned int i; -enum { NOTMUCH_FORMAT_JSON, NOTMUCH_FORMAT_TEXT, NOTMUCH_FORMAT_SEXP } - format_sel = NOTMUCH_FORMAT_TEXT; +enum { + NOTMUCH_FORMAT_JSON, + NOTMUCH_FORMAT_TEXT, + NOTMUCH_FORMAT_TEXT0, + NOTMUCH_FORMAT_SEXP +} format_sel = NOTMUCH_FORMAT_TEXT; notmuch_opt_desc_t options[] = { { NOTMUCH_OPT_KEYWORD, , "sort", 's', @@ -317,6 +321,7 @@ notmuch_search_command (void *ctx, int argc, char *argv[]) (notmuch_keyword_t []){ { "json", NOTMUCH_FORMAT_JSON }, { "sexp", NOTMUCH_FORMAT_SEXP }, { "text", NOTMUCH_FORMAT_TEXT }, + { "text0", NOTMUCH_FORMAT_TEXT0 }, { 0, 0 } } }, { NOTMUCH_OPT_KEYWORD, , "output", 'o', (notmuch_keyword_t []){ { "summary", OUTPUT_SUMMARY }, @@ -345,6 +350,13 @@ notmuch_search_command (void *ctx, int argc, char *argv[]) case NOTMUCH_FORMAT_TEXT: format = sprinter_text_create (ctx, stdout); break; +case NOTMUCH_FORMAT_TEXT0: + if (output == OUTPUT_SUMMARY) { + fprintf (stderr, "Error: --format=text0 is not compatible with --output=summary.\n"); + return 1; + } + format = sprinter_text0_create (ctx, stdout); + break; case NOTMUCH_FORMAT_JSON: format = sprinter_json_create (ctx, stdout); break; -- 1.7.10.4
[PATCH v3 2/5] sprinter: add text0 formatter for null character separated text
Same as the text formatter, but with each field separated by a null character rather than a newline character. --- sprinter-text.c | 22 ++ sprinter.h |6 ++ 2 files changed, 28 insertions(+) diff --git a/sprinter-text.c b/sprinter-text.c index 10343be..7779488 100644 --- a/sprinter-text.c +++ b/sprinter-text.c @@ -68,6 +68,14 @@ text_separator (struct sprinter *sp) } static void +text0_separator (struct sprinter *sp) +{ +struct sprinter_text *sptxt = (struct sprinter_text *) sp; + +fputc ('\0', sptxt->stream); +} + +static void text_set_prefix (struct sprinter *sp, const char *prefix) { struct sprinter_text *sptxt = (struct sprinter_text *) sp; @@ -133,3 +141,17 @@ sprinter_text_create (const void *ctx, FILE *stream) res->stream = stream; return >vtable; } + +struct sprinter * +sprinter_text0_create (const void *ctx, FILE *stream) +{ +struct sprinter *sp; + +sp = sprinter_text_create (ctx, stream); +if (! sp) + return NULL; + +sp->separator = text0_separator; + +return sp; +} diff --git a/sprinter.h b/sprinter.h index f43a844..f859672 100644 --- a/sprinter.h +++ b/sprinter.h @@ -67,6 +67,12 @@ typedef struct sprinter { struct sprinter * sprinter_text_create (const void *ctx, FILE *stream); +/* Create a new unstructured printer that emits the text format for + * "notmuch search", with each field separated by a null character + * instead of the newline character. */ +struct sprinter * +sprinter_text0_create (const void *ctx, FILE *stream); + /* Create a new structure printer that emits JSON. */ struct sprinter * sprinter_json_create (const void *ctx, FILE *stream); -- 1.7.10.4
[PATCH v3 1/5] sprinter: clarify separator documentation
For text printers, the separator is a syntactic element. --- sprinter.h |9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/sprinter.h b/sprinter.h index 59776a9..f43a844 100644 --- a/sprinter.h +++ b/sprinter.h @@ -42,10 +42,11 @@ typedef struct sprinter { */ void (*map_key) (struct sprinter *, const char *); -/* Insert a separator (usually extra whitespace) for improved - * readability without affecting the abstract syntax of the - * structure being printed. - * For JSON, this could simply be a line break. +/* Insert a separator (usually extra whitespace). For the text + * printers, this is a syntactic separator. For the structured + * printers, this is for improved readability without affecting + * the abstract syntax of the structure being printed. For JSON, + * this could simply be a line break. */ void (*separator) (struct sprinter *); -- 1.7.10.4
gmail importer script
Quoting Jason A. Donenfeld (2012-12-16 20:44:04) > On Sat, Dec 15, 2012 at 11:41 AM, Patrick Totzke > wrote: > > Well, thats not the point.. the script shouldn't die like this. > I think it's be better if the script caught that exception, deleted the > file > and continued.. > > > Probably, but I suspect it's related to whatever mystery error you ran into > before with the corruption. > > Does deleting that file and trying again fix it?? I don't know. How would I know which file to remove? I just removed the newest files in new/cur/tmp but this doesnt solve the issue. What would really help here is some more informative error message! I tried with the `-d` option but did not see any local path in the output. > > Anyway, this is extremely stable for me and a few others at this point. I'm > going to wait for other users to report errors. Alternatively, send me patches > if you want things to happen. Personally, I don't really care if 'things are happening', as I'm kind of OK with my current offlineimap configuration and would seriously consider switching from OI only if it meant an improvement in speed or stability. I test this code because I believe that the notmuch ecosystem would benefit from a working solution and that reviews and bug-reports are important. Of course, I don't expect any miracles in terms of speed at this early stage. But the bug I reported above should be addressed at some point in order not to scare off potential users. Have you considered including your code in offlineimap? I'm asking because OI already solved some of the problems you might face later on if you want to continue working on this. * multi-threaded downloads * the ability to read passwords not as plaintext parameter.. best, /p -- next part -- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 198 bytes Desc: signature URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20121216/62774cac/attachment.pgp>
gmail importer script
On Sat, Dec 15, 2012 at 11:41 AM, Patrick Totzke wrote: > > Well, thats not the point.. the script shouldn't die like this. > I think it's be better if the script caught that exception, deleted the > file > and continued.. Probably, but I suspect it's related to whatever mystery error you ran into before with the corruption. Does deleting that file and trying again fix it? Anyway, this is extremely stable for me and a few others at this point. I'm going to wait for other users to report errors. Alternatively, send me patches if you want things to happen. -- next part -- An HTML attachment was scrubbed... URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20121216/e36a1be9/attachment-0001.html>
[PATCH v3 0/5] add --format=text0 to notmuch search
On Sun, 16 Dec 2012, Jani Nikula wrote: > Hi all, v3 of id:cover.1355064714.git.jani at nikula.org > > Changes since v2: > * add new patch 1/5 to clarify sprinter documentation > * fix the test patch 4/5 according to id:8738z6wguj.fsf at qmul.ac.uk and >id:87y5gyvvv7.fsf at awakening.csail.mit.edu > > Diff to v2 at the end of the cover letter. LGTM +1 Best wishes Mark > > > BR, > Jani. > > > Jani Nikula (5): > sprinter: clarify separator documentation > sprinter: add text0 formatter for null character separated text > cli: add --format=text0 to notmuch search > test: notmuch search --format=text0 > man: document notmuch search --format=text0 > > man/man1/notmuch-search.1 | 26 -- > notmuch-search.c | 16 ++-- > sprinter-text.c | 22 ++ > sprinter.h| 15 +++ > test/text | 33 + > 5 files changed, 96 insertions(+), 16 deletions(-) > > -- > 1.7.10.4 > > > > diff --git a/sprinter.h b/sprinter.h > index f36b999..f859672 100644 > --- a/sprinter.h > +++ b/sprinter.h > @@ -42,10 +42,11 @@ typedef struct sprinter { > */ > void (*map_key) (struct sprinter *, const char *); > > -/* Insert a separator (usually extra whitespace) for improved > - * readability without affecting the abstract syntax of the > - * structure being printed. > - * For JSON, this could simply be a line break. > +/* Insert a separator (usually extra whitespace). For the text > + * printers, this is a syntactic separator. For the structured > + * printers, this is for improved readability without affecting > + * the abstract syntax of the structure being printed. For JSON, > + * this could simply be a line break. > */ > void (*separator) (struct sprinter *); > > diff --git a/test/text b/test/text > index e003a66..b5ccefc 100755 > --- a/test/text > +++ b/test/text > @@ -55,30 +55,34 @@ test_expect_equal "$output" \ > add_email_corpus > > test_begin_subtest "Search message tags: text0" > -cat < EXPECTED.$test_count > +cat < EXPECTED > attachment inbox signed unread > EOF > -notmuch search --format=text0 --output=tags '*' | xargs -0 | > notmuch_search_sanitize > OUTPUT.$test_count > -test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count > +notmuch search --format=text0 --output=tags '*' | xargs -0 | > notmuch_search_sanitize > OUTPUT > +test_expect_equal_file EXPECTED OUTPUT > > +# Use tr(1) to convert --output=text0 to --output=text for > +# comparison. Also translate newlines to spaces to fail with more > +# noise if they are present as delimiters instead of null > +# characters. This assumes there are no newlines in the data. > test_begin_subtest "Compare text vs. text0 for threads" > -notmuch search --format=text --output=threads '*' | notmuch_search_sanitize > > EXPECTED.$test_count > -notmuch search --format=text0 --output=threads '*' | xargs -0 -L1 | > notmuch_search_sanitize > OUTPUT.$test_count > -test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count > +notmuch search --format=text --output=threads '*' | notmuch_search_sanitize > > EXPECTED > +notmuch search --format=text0 --output=threads '*' | tr "\n\0" " \n" | > notmuch_search_sanitize > OUTPUT > +test_expect_equal_file EXPECTED OUTPUT > > test_begin_subtest "Compare text vs. text0 for messages" > -notmuch search --format=text --output=messages '*' | notmuch_search_sanitize > > EXPECTED.$test_count > -notmuch search --format=text0 --output=messages '*' | xargs -0 -L1 | > notmuch_search_sanitize > OUTPUT.$test_count > -test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count > +notmuch search --format=text --output=messages '*' | notmuch_search_sanitize > > EXPECTED > +notmuch search --format=text0 --output=messages '*' | tr "\n\0" " \n" | > notmuch_search_sanitize > OUTPUT > +test_expect_equal_file EXPECTED OUTPUT > > test_begin_subtest "Compare text vs. text0 for files" > -notmuch search --format=text --output=files '*' | notmuch_search_sanitize > > EXPECTED.$test_count > -notmuch search --format=text0 --output=files '*' | xargs -0 -L1 | > notmuch_search_sanitize > OUTPUT.$test_count > -test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count > +notmuch search --format=text --output=files '*' | notmuch_search_sanitize > > EXPECTED > +notmuch search --format=text0 --output=files '*' | tr "\n\0" " \n" | > notmuch_search_sanitize > OUTPUT > +test_expect_equal_file EXPECTED OUTPUT > > test_begin_subtest "Compare text vs. text0 for tags" > -notmuch search --format=text --output=tags '*' | notmuch_search_sanitize > > EXPECTED.$test_count > -notmuch search --format=text0 --output=tags '*' | xargs -0 -L1 | > notmuch_search_sanitize > OUTPUT.$test_count > -test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count > +notmuch search --format=text --output=tags '*' |
[RFC PATCH] cli: add --remove-all option to "notmuch tag"
On Mon, 03 Dec 2012, Jani Nikula wrote: > Add --remove-all option to "notmuch tag" to remove all tags matching > query before applying the tag changes. This allows removal and > unconditional setting of the tags of a message: > > $ notmuch tag --remove-all id:foo at example.com > $ notmuch tag --remove-all +foo +bar id:foo at example.com > > without having to resort to the complicated (and still quoting broken): > > $ notmuch tag $(notmuch search --output=tags '*' | sed 's/^/-/') id:foo at > example.com > $ notmuch tag $(notmuch search --output=tags '*' | sed 's/^/-/') +foo +bar > id:foo at example.com > > --- > > It's completely untested... > > This is on top of David's batch tagging branch new-tagging at > git://pivot.cs.unb.ca/notmuch.git > > If there's interest, I'll spin a new version with tests and man page > changes after David's stuff has been merged. I like this: the possibility of setting the tags to something seems nice. I am not very keen on the option name but don't have anything much better: maybe --remove-all-first or --set-to? Incidentally, does this (or should this) give an error if the user calls --remove-all with -some_tag (as opposed to +some_tag)? Best wishes Mark > --- > notmuch-tag.c | 32 > 1 file changed, 20 insertions(+), 12 deletions(-) > > diff --git a/notmuch-tag.c b/notmuch-tag.c > index e4fca67..67624cc 100644 > --- a/notmuch-tag.c > +++ b/notmuch-tag.c > @@ -119,12 +119,15 @@ tag_query (void *ctx, notmuch_database_t *notmuch, > const char *query_string, > notmuch_messages_t *messages; > notmuch_message_t *message; > > -/* Optimize the query so it excludes messages that already have > - * the specified set of tags. */ > -query_string = _optimize_tag_query (ctx, query_string, tag_ops); > -if (query_string == NULL) { > - fprintf (stderr, "Out of memory.\n"); > - return 1; > +if (! (flags & TAG_FLAG_REMOVE_ALL)) { > + /* Optimize the query so it excludes messages that already > + * have the specified set of tags. */ > + query_string = _optimize_tag_query (ctx, query_string, tag_ops); > + if (query_string == NULL) { > + fprintf (stderr, "Out of memory.\n"); > + return 1; > + } > + flags |= TAG_FLAG_PRE_OPTIMIZED; > } > > query = notmuch_query_create (notmuch, query_string); > @@ -140,7 +143,7 @@ tag_query (void *ctx, notmuch_database_t *notmuch, const > char *query_string, >notmuch_messages_valid (messages) && ! interrupted; >notmuch_messages_move_to_next (messages)) { > message = notmuch_messages_get (messages); > - tag_op_list_apply (message, tag_ops, flags | TAG_FLAG_PRE_OPTIMIZED); > + tag_op_list_apply (message, tag_ops, flags); > notmuch_message_destroy (message); > } > > @@ -157,8 +160,9 @@ notmuch_tag_command (void *ctx, int argc, char *argv[]) > notmuch_config_t *config; > notmuch_database_t *notmuch; > struct sigaction action; > -tag_op_flag_t synchronize_flags = TAG_FLAG_NONE; > +tag_op_flag_t flags = TAG_FLAG_NONE; > notmuch_bool_t batch = FALSE; > +notmuch_bool_t remove_all = FALSE; > FILE *input = stdin; > char *input_file_name = NULL; > int i, opt_index; > @@ -174,6 +178,7 @@ notmuch_tag_command (void *ctx, int argc, char *argv[]) > notmuch_opt_desc_t options[] = { > { NOTMUCH_OPT_BOOLEAN, , "batch", 0, 0 }, > { NOTMUCH_OPT_STRING, _file_name, "input", 'i', 0 }, > + { NOTMUCH_OPT_BOOLEAN, _all, "remove-all", 0, 0 }, > { 0, 0, 0, 0, 0 } > }; > > @@ -230,7 +235,7 @@ notmuch_tag_command (void *ctx, int argc, char *argv[]) > } > } > > - if (tag_op_list_size (tag_ops) == 0) { > + if (tag_op_list_size (tag_ops) == 0 && !remove_all) { > fprintf (stderr, "Error: 'notmuch tag' requires at least one tag to > add or remove.\n"); > return 1; > } > @@ -252,7 +257,10 @@ notmuch_tag_command (void *ctx, int argc, char *argv[]) > return 1; > > if (notmuch_config_get_maildir_synchronize_flags (config)) > - synchronize_flags = TAG_FLAG_MAILDIR_SYNC; > + flags |= TAG_FLAG_MAILDIR_SYNC; > + > +if (remove_all) > + flags |= TAG_FLAG_REMOVE_ALL; > > if (batch) { > > @@ -280,14 +288,14 @@ notmuch_tag_command (void *ctx, int argc, char *argv[]) > continue; > > if (ret < 0 || tag_query (ctx, notmuch, query_string, > - tag_ops, synchronize_flags)) > + tag_ops, flags)) > break; > } > > if (line) > free (line); > } else > - ret = tag_query (ctx, notmuch, query_string, tag_ops, > synchronize_flags); > + ret = tag_query (ctx, notmuch, query_string, tag_ops, flags); > > notmuch_database_destroy (notmuch); > > -- > 1.7.10.4 > > ___ > notmuch mailing
[PATCH 4/4] perf-test: add memory leak test for dump restore
david at tethera.net writes: > + > +memory_run 'load nmbug tags' 'notmuch restore --accumulate > --input=corpus.tags/nmbug.sup-dump' > +memory_run 'dump *' 'notmuch dump --output=tags.sup' > +memory_run 'restore *' 'notmuch restore --input=tags.sup' > +memory_run 'dump --format=batch-tag *' 'notmuch dump --format=batch-tag > --output=tags.bt' > +memory_run 'restore --format=batch-tag *' 'notmuch restore > --format=batch-tag --input=tags.bt' > + We were talking on IRC about how/if valgrind would cope with talloc, and the possibility that chunks of memory are still reachable by talloc, but not by user code. Currently the talloc context "local" in main() is (slightly perversely) only freed in the case of "return 1", so all the memory allocated by talloc on that contex is shown as leaked: 3,005,500 (93 direct, 3,005,407 indirect) bytes in 1 blocks are definitely lost in loss record 553 of 553 at 0x4C2A26B: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) by 0x55F14C7: talloc_strndup (in /usr/lib/x86_64-linux-gnu/libtalloc.so.2.0.7) by 0x4115E8: parse_sup_line (notmuch-restore.c:90) by 0x411AD4: notmuch_restore_command (notmuch-restore.c:209) by 0x40B2A4: main (notmuch.c:294) Although this is probably a bug in main(), it does point valgrind to the right culprit. As our memory allocation is (alas) a mix of talloc, malloc, and g_malloc, we probably need both valgrind tests, and some way to toggle talloc memory debugging. ( http://talloc.samba.org/talloc/doc/html/group__talloc__debug.html ) d
[PATCH v2 1/7] emacs: Centralize notmuch command error handling
Austin Clements writes: > This provides library functions for unified handling of errors from > the notmuch CLI. Follow-up patches will convert some scattered error > handling to use this and add error handling where we currently ignore > errors. pushed, d
[PATCH 1/7] cli: Framework for structured output versioning
Austin Clements writes: > > This series of patches introduces a way for CLI callers to request a > specific format version on the command line and to determine if the > CLI does not supported the requested version pushed, d
[PATCH 3/3] notmuch-restore: allocate a temporary talloc context for each line parsed.
From: David BremnerThis lets the high level code in notmuch restore be ignorant about what the lower level code is doing as far as allocating memory. --- notmuch-restore.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/notmuch-restore.c b/notmuch-restore.c index 4b76d83..52e7ecb 100644 --- a/notmuch-restore.c +++ b/notmuch-restore.c @@ -122,6 +122,7 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) char *input_file_name = NULL; FILE *input = stdin; char *line = NULL; +void *line_ctx = NULL; size_t line_size; ssize_t line_len; @@ -205,10 +206,11 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) do { char *query_string; + line_ctx = talloc_new (ctx); if (input_format == DUMP_FORMAT_SUP) { - ret = parse_sup_line (ctx, line, _string, tag_ops); + ret = parse_sup_line (line_ctx, line, _string, tag_ops); } else { - ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS, + ret = parse_tag_line (line_ctx, line, TAG_FLAG_BE_GENEROUS, _string, tag_ops); if (ret == 0) { @@ -238,8 +240,14 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) break; } + talloc_free (line_ctx); + /* setting to NULL is important to avoid a double free */ + line_ctx = NULL; } while ((line_len = getline (, _size, input)) != -1); +if (line_ctx != NULL) + talloc_free (line_ctx); + if (input_format == DUMP_FORMAT_SUP) regfree (); -- 1.7.10.4
[PATCH 2/3] notmuch-restore: replace continue with if
From: David BremnerIt is maybe a bit counter-intuitive with the condition reversed like this, but it makes the memory handling fix in the next patch easier. --- notmuch-restore.c | 15 +-- 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/notmuch-restore.c b/notmuch-restore.c index ae0ef45..4b76d83 100644 --- a/notmuch-restore.c +++ b/notmuch-restore.c @@ -228,12 +228,15 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) } } - if (ret > 0) - continue; - - if (ret < 0 || tag_message (ctx, notmuch, query_string, - tag_ops, flags)) - break; + if (ret <= 0) { + if (ret < 0) + break; + + ret = tag_message (line_ctx, notmuch, query_string, + tag_ops, flags); + if (ret < 0) + break; + } } while ((line_len = getline (, _size, input)) != -1); -- 1.7.10.4
[PATCH 1/3] tag-utils: use the tag_opt_list_t as talloc context, if possible.
From: David BremnerThis code is no less correct than the previous version, since it does not make sense for the array to live longer than the wrapping struct. By not relying on the context passed into tag_parse_line, we can allow tag_op_list_t structures to live longer than that context. --- notmuch-restore.c |2 +- tag-util.c|9 - tag-util.h|3 +-- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/notmuch-restore.c b/notmuch-restore.c index 40596a8..ae0ef45 100644 --- a/notmuch-restore.c +++ b/notmuch-restore.c @@ -102,7 +102,7 @@ parse_sup_line (void *ctx, char *line, tok_len++; } - if (tag_op_list_append (ctx, tag_ops, tok, FALSE)) + if (tag_op_list_append (tag_ops, tok, FALSE)) return -1; } diff --git a/tag-util.c b/tag-util.c index eab482f..705b7ba 100644 --- a/tag-util.c +++ b/tag-util.c @@ -109,7 +109,7 @@ parse_tag_line (void *ctx, char *line, goto DONE; } - if (tag_op_list_append (ctx, tag_ops, tag, remove)) { + if (tag_op_list_append (tag_ops, tag, remove)) { ret = line_error (TAG_PARSE_OUT_OF_MEMORY, line_for_error, "aborting"); goto DONE; @@ -294,7 +294,7 @@ tag_op_list_create (void *ctx) list->size = TAG_OP_LIST_INITIAL_SIZE; list->count = 0; -list->ops = talloc_array (ctx, tag_operation_t, list->size); +list->ops = talloc_array (list, tag_operation_t, list->size); if (list->ops == NULL) return NULL; @@ -303,8 +303,7 @@ tag_op_list_create (void *ctx) int -tag_op_list_append (void *ctx, - tag_op_list_t *list, +tag_op_list_append (tag_op_list_t *list, const char *tag, notmuch_bool_t remove) { @@ -314,7 +313,7 @@ tag_op_list_append (void *ctx, if (list->count == list->size) { list->size *= 2; - list->ops = talloc_realloc (ctx, list->ops, tag_operation_t, + list->ops = talloc_realloc (list, list->ops, tag_operation_t, list->size); if (list->ops == NULL) { fprintf (stderr, "Out of memory.\n"); diff --git a/tag-util.h b/tag-util.h index 99b0fa0..c07bfde 100644 --- a/tag-util.h +++ b/tag-util.h @@ -87,8 +87,7 @@ tag_op_list_create (void *ctx); */ int -tag_op_list_append (void *ctx, - tag_op_list_t *list, +tag_op_list_append (tag_op_list_t *list, const char *tag, notmuch_bool_t remove); -- 1.7.10.4
proposed fix for memory leak in notmuch restore
In id:87vcc2q5n2.fsf at nikula.org, Jani points out a memory leak in the new restore code. This series is a proposed fix, namely do any allocation on a temporary talloc context that is destroyed after each line is processed.
[PATCH v3 0/5] add --format=text0 to notmuch search
On Sun, 16 Dec 2012, Jani Nikula wrote: > Hi all, v3 of id:cover.1355064714.git.jani at nikula.org > > Changes since v2: > * add new patch 1/5 to clarify sprinter documentation > * fix the test patch 4/5 according to id:8738z6wguj.fsf at qmul.ac.uk and >id:87y5gyvvv7.fsf at awakening.csail.mit.edu > > Diff to v2 at the end of the cover letter. > > > BR, > Jani. LGTM.
[PATCH 4/4] perf-test: add memory leak test for dump restore
From: David BremnerIn id:87vcc2q5n2.fsf at nikula.org, Jani points out a memory leak in the current version of the sup restore code. Among other things, this test is intended to verify a fix for that leak. --- performance-test/M01-dump-restore | 15 +++ 1 file changed, 15 insertions(+) create mode 100755 performance-test/M01-dump-restore diff --git a/performance-test/M01-dump-restore b/performance-test/M01-dump-restore new file mode 100755 index 000..be5894a --- /dev/null +++ b/performance-test/M01-dump-restore @@ -0,0 +1,15 @@ +#!/bin/bash + +test_description='dump and restore' + +. ./perf-test-lib.sh + +memory_start + +memory_run 'load nmbug tags' 'notmuch restore --accumulate --input=corpus.tags/nmbug.sup-dump' +memory_run 'dump *' 'notmuch dump --output=tags.sup' +memory_run 'restore *' 'notmuch restore --input=tags.sup' +memory_run 'dump --format=batch-tag *' 'notmuch dump --format=batch-tag --output=tags.bt' +memory_run 'restore --format=batch-tag *' 'notmuch restore --format=batch-tag --input=tags.bt' + +memory_done -- 1.7.10.4
[PATCH 3/4] perf-test: initial version of memory test infrastructure.
From: David BremnerThe idea is run some code under valgrind --leak-check=full and report a summary, leaving the user to peruse the log file if they want. We go to some lengths to preserve the log files from accidental overwriting; the full corpus takes about 3 hours to run under valgrind on my machine. The naming of the log directories is probably overkill; I find it nice to have them sequenced by time. Arguably the mktemp is then overkill, but I know people will be nervous if it looks like timestamps are being used for uniqueness. One new test is included, to check notmuch new for memory leaks. --- performance-test/.gitignore |1 + performance-test/M00-new | 14 + performance-test/Makefile.local | 17 +-- performance-test/README | 57 + performance-test/perf-test-lib.sh | 55 --- 5 files changed, 113 insertions(+), 31 deletions(-) create mode 100755 performance-test/M00-new diff --git a/performance-test/.gitignore b/performance-test/.gitignore index 6421a9a..f3f9be4 100644 --- a/performance-test/.gitignore +++ b/performance-test/.gitignore @@ -1,3 +1,4 @@ tmp.*/ +log.*/ corpus/ notmuch.cache.*/ diff --git a/performance-test/M00-new b/performance-test/M00-new new file mode 100755 index 000..733e9b0 --- /dev/null +++ b/performance-test/M00-new @@ -0,0 +1,14 @@ +#!/bin/bash + +test_description='notmuch new' + +. ./perf-test-lib.sh + +# ensure initial 'notmuch new' is run by memory_start +uncache_database + +memory_start + +memory_run "notmuch new" "notmuch new" + +memory_done diff --git a/performance-test/Makefile.local b/performance-test/Makefile.local index 57beb44..357d800 100644 --- a/performance-test/Makefile.local +++ b/performance-test/Makefile.local @@ -4,14 +4,25 @@ dir := performance-test include $(dir)/version.sh +# these two are just make sure dir is expanded at the right time. +TIME_TEST_SCRIPT := ${dir}/notmuch-time-test +MEMORY_TEST_SCRIPT := ${dir}/notmuch-memory-test + CORPUS_NAME := notmuch-email-corpus-$(PERFTEST_VERSION).tar.xz TXZFILE := ${dir}/download/${CORPUS_NAME} SIGFILE := ${TXZFILE}.asc -TEST_SCRIPT := ${dir}/notmuch-perf-test DEFAULT_URL := http://notmuchmail.org/releases/${CORPUS_NAME} +perf-test: time-test memory-test + time-test: setup-perf-test all - $(TEST_SCRIPT) $(OPTIONS) + @echo + $(TIME_TEST_SCRIPT) $(TEST_OPTIONS) + +memory-test: setup-perf-test all + @echo + $(MEMORY_TEST_SCRIPT) $(TEST_OPTIONS) + .PHONY: download-corpus setup-perf-test @@ -29,4 +40,4 @@ $(TXZFILE): download-corpus: wget -O ${TXZFILE} ${DEFAULT_URL} -CLEAN := $(CLEAN) $(dir)/tmp.* $(dir)/corpus $(dir)/notmuch.cache.* +CLEAN := $(CLEAN) $(dir)/tmp.* $(dir)/log.* $(dir)/corpus $(dir)/notmuch.cache.* diff --git a/performance-test/README b/performance-test/README index d1fb6de..7eaf5f7 100644 --- a/performance-test/README +++ b/performance-test/README @@ -1,3 +1,10 @@ +Performance Tests +- + +This directory contains two kinds of performance tests, time tests, +and memory tests. The former use gnu time, and the latter use +valgrind. + Pre-requisites -- @@ -5,9 +12,10 @@ In addition to having notmuch, you need: - gpg - gnu tar -- gnu time +- gnu time (for the time tests). - xz. Some speedup can be gotten by installing "pixz", but this is probably only worthwhile if you are debugging the tests. +- valgrind (for the memory tests) Getting set up to run tests: @@ -36,34 +44,47 @@ for a list of mirrors. Running tests - -The easiest way to run performance tests is to say "make time-test", (or -simply run the notmuch-time-test script). Either command will run all -available performance tests. - -Alternately, you can run a specific subset of tests by simply invoking -one of the executable scripts in this directory, (such as ./basic). -Each test script supports the following arguments +The easiest way to run performance tests is to say "make perf-test". +This will run all time and memory tests. Be aware that the memory +tests are quite time consuming when run on the full corpus, and that +depending on your interests it may be more sensible to run "make +time-test" or "make memory-test". You can also invoke one of the +scripts notmuch-time-test or notmuch-memory-test or run a more +specific subset of tests by simply invoking one of the executable +scripts in this directory, (such as ./T00-new). Each test script +supports the following arguments --small / --medium / --large Choose corpus size. --debugEnable debugging. In particular don't delete temporary directories. +When using the make targets, you can pass arguments to all test +scripts by defining the make variable TEST_OPTIONS. + Writing tests - -Have a look at "T01-dump-restore"
[PATCH 2/4] perf-test: rename current tests as "time tests"
From: David BremnerThis is almost entirely renaming files, except for updating a few references to those file names, and changing the makefile target. A new set of memory tests will be run separately because they take much longer. --- performance-test/00-new| 15 --- performance-test/01-dump-restore | 13 - performance-test/02-tag| 14 -- performance-test/Makefile.local|2 +- performance-test/README|9 + performance-test/T00-new | 15 +++ performance-test/T01-dump-restore | 13 + performance-test/T02-tag | 14 ++ performance-test/notmuch-perf-test | 27 --- performance-test/notmuch-time-test | 27 +++ 10 files changed, 75 insertions(+), 74 deletions(-) delete mode 100755 performance-test/00-new delete mode 100755 performance-test/01-dump-restore delete mode 100755 performance-test/02-tag create mode 100755 performance-test/T00-new create mode 100755 performance-test/T01-dump-restore create mode 100755 performance-test/T02-tag delete mode 100755 performance-test/notmuch-perf-test create mode 100755 performance-test/notmuch-time-test diff --git a/performance-test/00-new b/performance-test/00-new deleted file mode 100755 index 553bb8b..000 --- a/performance-test/00-new +++ /dev/null @@ -1,15 +0,0 @@ -#!/bin/bash - -test_description='notmuch new' - -. ./perf-test-lib.sh - -uncache_database - -time_start - -for i in $(seq 2 6); do -time_run "notmuch new #$i" 'notmuch new' -done - -time_done diff --git a/performance-test/01-dump-restore b/performance-test/01-dump-restore deleted file mode 100755 index b2ff940..000 --- a/performance-test/01-dump-restore +++ /dev/null @@ -1,13 +0,0 @@ -#!/bin/bash - -test_description='dump and restore' - -. ./perf-test-lib.sh - -time_start - -time_run 'load nmbug tags' 'notmuch restore --accumulate < corpus.tags/nmbug.sup-dump' -time_run 'dump *' 'notmuch dump > tags.out' -time_run 'restore *' 'notmuch restore < tags.out' - -time_done diff --git a/performance-test/02-tag b/performance-test/02-tag deleted file mode 100755 index 78ceccc..000 --- a/performance-test/02-tag +++ /dev/null @@ -1,14 +0,0 @@ -#!/bin/bash - -test_description='tagging' - -. ./perf-test-lib.sh - -time_start - -time_run 'tag * +new_tag' "notmuch tag +new_tag '*'" -time_run 'tag * +existing_tag' "notmuch tag +new_tag '*'" -time_run 'tag * -existing_tag' "notmuch tag -new_tag '*'" -time_run 'tag * -missing_tag' "notmuch tag -new_tag '*'" - -time_done diff --git a/performance-test/Makefile.local b/performance-test/Makefile.local index 3834e4d..57beb44 100644 --- a/performance-test/Makefile.local +++ b/performance-test/Makefile.local @@ -10,7 +10,7 @@ SIGFILE := ${TXZFILE}.asc TEST_SCRIPT := ${dir}/notmuch-perf-test DEFAULT_URL := http://notmuchmail.org/releases/${CORPUS_NAME} -perf-test: setup-perf-test all +time-test: setup-perf-test all $(TEST_SCRIPT) $(OPTIONS) .PHONY: download-corpus setup-perf-test diff --git a/performance-test/README b/performance-test/README index 1481660..d1fb6de 100644 --- a/performance-test/README +++ b/performance-test/README @@ -36,8 +36,8 @@ for a list of mirrors. Running tests - -The easiest way to run performance tests is to say "make perf-test", (or -simply run the notmuch-perf-test script). Either command will run all +The easiest way to run performance tests is to say "make time-test", (or +simply run the notmuch-time-test script). Either command will run all available performance tests. Alternately, you can run a specific subset of tests by simply invoking @@ -51,7 +51,7 @@ Each test script supports the following arguments Writing tests - -Have a look at "01-dump-restore" for an example. Sourcing +Have a look at "T01-dump-restore" for an example. Sourcing "perf-test-lib.sh" is mandatory. Utility functions include - 'add_email_corpus' unpacks a set of messages and adds them to the database. @@ -65,4 +65,5 @@ Have a look at "01-dump-restore" for an example. Sourcing Scripts are run in the order specified in notmuch-perf-test. In the future this order might be chosen automatically so please follow the -convention of starting the name with two digits to specify the order. +convention of starting the name with 'T' followed by two digits to +specify the order. diff --git a/performance-test/T00-new b/performance-test/T00-new new file mode 100755 index 000..553bb8b --- /dev/null +++ b/performance-test/T00-new @@ -0,0 +1,15 @@ +#!/bin/bash + +test_description='notmuch new' + +. ./perf-test-lib.sh + +uncache_database + +time_start + +for i in $(seq 2 6); do +time_run "notmuch new #$i" 'notmuch new' +done + +time_done diff --git a/performance-test/T01-dump-restore b/performance-test/T01-dump-restore new file mode 100755 index 000..b2ff940 --- /dev/null +++
[PATCH 1/4] perf-test: remove redunant "initial notmuch new"
From: David BremnerThe initial notmuch-new and caching are now done automatically by time_start --- performance-test/00-new |4 1 file changed, 4 deletions(-) diff --git a/performance-test/00-new b/performance-test/00-new index 6f0b50c..553bb8b 100755 --- a/performance-test/00-new +++ b/performance-test/00-new @@ -8,10 +8,6 @@ uncache_database time_start -time_run 'initial notmuch new' 'notmuch new' - -cache_database - for i in $(seq 2 6); do time_run "notmuch new #$i" 'notmuch new' done -- 1.7.10.4
memory leak tests
These are slightly rough around the edges, but I think they are useful. They already helped me track down a memory leak in notmuch new (id:1355234087-6886-1-git-send-email-david at tethera.net, id:1355196820-29734-1-git-send-email-david at tethera.net) They also would have caught the restore leak Jani pointed out in parse_sup_line, although that is a bit of hindsight since I didn't write the obvious test until Jani pointed out the leak.
[PATCH v2 3/4] test: notmuch search --format=text0
On Sun, 09 Dec 2012, Jani Nikula wrote: > --- > test/text | 29 + > 1 file changed, 29 insertions(+) > > diff --git a/test/text b/test/text > index 428c89b..e003a66 100755 > --- a/test/text > +++ b/test/text > @@ -52,4 +52,33 @@ output=$(notmuch search --format=text > "t?xt-search-m?ssage" | notmuch_search_s > test_expect_equal "$output" \ > "thread:XXX 2000-01-01 [1/1] Notmuch Test Suite; > text-search-utf8-body-s?bj?ct (inbox unread)" > > +add_email_corpus > + > +test_begin_subtest "Search message tags: text0" > +cat < EXPECTED.$test_count Other tests use just OUTPUT and EXPECTED. Why the $test_count? Is there a technical reason for it? > +attachment inbox signed unread > +EOF > +notmuch search --format=text0 --output=tags '*' | xargs -0 | > notmuch_search_sanitize > OUTPUT.$test_count > +test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count > + > +test_begin_subtest "Compare text vs. text0 for threads" > +notmuch search --format=text --output=threads '*' | notmuch_search_sanitize > > EXPECTED.$test_count > +notmuch search --format=text0 --output=threads '*' | xargs -0 -L1 | > notmuch_search_sanitize > OUTPUT.$test_count I think it would be worth "strengthening" these tests as Mark pointed out. It would be easy to accidentally include a literal \n in the output instead of calling the separator method, and this test wouldn't catch that. I think Mark's suggestion with tr is pretty good, since it directly disambiguates \0 and \n in the output, while producing a reasonable diff if things do go wrong. > +test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count > + > +test_begin_subtest "Compare text vs. text0 for messages" > +notmuch search --format=text --output=messages '*' | notmuch_search_sanitize > > EXPECTED.$test_count > +notmuch search --format=text0 --output=messages '*' | xargs -0 -L1 | > notmuch_search_sanitize > OUTPUT.$test_count > +test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count > + > +test_begin_subtest "Compare text vs. text0 for files" > +notmuch search --format=text --output=files '*' | notmuch_search_sanitize > > EXPECTED.$test_count > +notmuch search --format=text0 --output=files '*' | xargs -0 -L1 | > notmuch_search_sanitize > OUTPUT.$test_count > +test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count > + > +test_begin_subtest "Compare text vs. text0 for tags" > +notmuch search --format=text --output=tags '*' | notmuch_search_sanitize > > EXPECTED.$test_count > +notmuch search --format=text0 --output=tags '*' | xargs -0 -L1 | > notmuch_search_sanitize > OUTPUT.$test_count > +test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count > + > test_done > -- > 1.7.10.4 > > ___ > notmuch mailing list > notmuch at notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v5b] show: indicate length, encoding of omitted body content
If a leaf part's body content is omitted, return the encoded length and transfer encoding in --format=json output. This information may be used by the consumer, e.g. to decide whether to download a large attachment over a slow link. Returning the _encoded_ content length is more efficient than returning the _decoded_ content length. Returning the transfer encoding allows the consumer to estimate the decoded content length. --- devel/schemata | 9 - notmuch-show.c | 16 ++-- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/devel/schemata b/devel/schemata index d1ab983..7d167be 100644 --- a/devel/schemata +++ b/devel/schemata @@ -75,7 +75,14 @@ part = { # A leaf part's body content is optional, but may be included if # it can be correctly encoded as a string. Consumers should use # this in preference to fetching the part content separately. -content?: string +content?: string, +# If a leaf part's body content is not included, the length of +# the encoded content (in bytes) may be given instead. +content-length?: int, +# If a leaf part's body content is not included, its transfer encoding +# may be given. Using this and the encoded content length, it is +# possible for the consumer to estimate the decoded content length. +content-transfer-encoding?: string } # The headers of a message or part (format_headers_sprinter with reply = FALSE) diff --git a/notmuch-show.c b/notmuch-show.c index 6a9278c..2d59865 100644 --- a/notmuch-show.c +++ b/notmuch-show.c @@ -600,14 +600,26 @@ format_part_text (const void *ctx, sprinter_t *sp, mime_node_t *node, } static void -format_omitted_part_meta_sprinter (sprinter_t *sp, GMimeObject *meta) +format_omitted_part_meta_sprinter (sprinter_t *sp, GMimeObject *meta, GMimePart *part) { const char *content_charset = g_mime_object_get_content_type_parameter (meta, "charset"); +const char *cte = g_mime_object_get_header (meta, "content-transfer-encoding"); +GMimeDataWrapper *wrapper = g_mime_part_get_content_object (part); +GMimeStream *stream = g_mime_data_wrapper_get_stream (wrapper); +ssize_t content_length = g_mime_stream_length (stream); if (content_charset != NULL) { sp->map_key (sp, "content-charset"); sp->string (sp, content_charset); } +if (cte != NULL) { + sp->map_key (sp, "content-transfer-encoding"); + sp->string (sp, cte); +} +if (content_length >= 0) { + sp->map_key (sp, "content-length"); + sp->integer (sp, content_length); +} } void @@ -699,7 +711,7 @@ format_part_sprinter (const void *ctx, sprinter_t *sp, mime_node_t *node, sp->string_len (sp, (char *) part_content->data, part_content->len); g_object_unref (stream_memory); } else { - format_omitted_part_meta_sprinter (sp, meta); + format_omitted_part_meta_sprinter (sp, meta, GMIME_PART (node->part)); } } else if (GMIME_IS_MULTIPART (node->part)) { sp->map_key (sp, "content"); -- 1.7.12.1
[PATCH v5b] show: indicate charset for all omitted parts
Write a "charset" field for all omitted parts for which it is applicable, not only text/html parts. Factor out the code to a separate function. It will be extended with more fields next. --- notmuch-show.c | 24 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/notmuch-show.c b/notmuch-show.c index a83fef9..6a9278c 100644 --- a/notmuch-show.c +++ b/notmuch-show.c @@ -599,6 +599,17 @@ format_part_text (const void *ctx, sprinter_t *sp, mime_node_t *node, return NOTMUCH_STATUS_SUCCESS; } +static void +format_omitted_part_meta_sprinter (sprinter_t *sp, GMimeObject *meta) +{ +const char *content_charset = g_mime_object_get_content_type_parameter (meta, "charset"); + +if (content_charset != NULL) { + sp->map_key (sp, "content-charset"); + sp->string (sp, content_charset); +} +} + void format_part_sprinter (const void *ctx, sprinter_t *sp, mime_node_t *node, notmuch_bool_t first, notmuch_bool_t output_body) @@ -677,14 +688,9 @@ format_part_sprinter (const void *ctx, sprinter_t *sp, mime_node_t *node, * makes charset decoding the responsibility on the caller, we * report the charset for text/html parts. */ - if (g_mime_content_type_is_type (content_type, "text", "html")) { - const char *content_charset = g_mime_object_get_content_type_parameter (meta, "charset"); - - if (content_charset != NULL) { - sp->map_key (sp, "content-charset"); - sp->string (sp, content_charset); - } - } else if (g_mime_content_type_is_type (content_type, "text", "*")) { + if (g_mime_content_type_is_type (content_type, "text", "*") && + ! g_mime_content_type_is_type (content_type, "text", "html")) + { GMimeStream *stream_memory = g_mime_stream_mem_new (); GByteArray *part_content; show_text_part_content (node->part, stream_memory, 0); @@ -692,6 +698,8 @@ format_part_sprinter (const void *ctx, sprinter_t *sp, mime_node_t *node, sp->map_key (sp, "content"); sp->string_len (sp, (char *) part_content->data, part_content->len); g_object_unref (stream_memory); + } else { + format_omitted_part_meta_sprinter (sp, meta); } } else if (GMIME_IS_MULTIPART (node->part)) { sp->map_key (sp, "content"); -- 1.7.12.1
[PATCH] contrib: pick: close message pane when quitting from show in the message pane
We add a hook to the show buffer in the message window to close the message window when that buffer quits. It checks that the message-window is still displaying the show-message buffer and then closes it. --- This is (probably) a rather better version than the previous attempt. It uses hooks rather than redefining notmuch-kill-this-buffer and it avoids errors if the user has made the message pane the whole frame. Best wishes Mark contrib/notmuch-pick/notmuch-pick.el | 18 ++ 1 files changed, 18 insertions(+), 0 deletions(-) diff --git a/contrib/notmuch-pick/notmuch-pick.el b/contrib/notmuch-pick/notmuch-pick.el index 043e9e7..8383589 100644 --- a/contrib/notmuch-pick/notmuch-pick.el +++ b/contrib/notmuch-pick/notmuch-pick.el @@ -160,6 +160,9 @@ (defvar notmuch-pick-message-window nil) (make-variable-buffer-local 'notmuch-pick-message-window) (put 'notmuch-pick-message-window 'permanent-local t) +(defvar notmuch-show-message-window nil) +(make-variable-buffer-local 'notmuch-show-message-window) +(put 'notmuch-show-message-window 'permanent-local t) (defvar notmuch-pick-message-buffer nil) (make-variable-buffer-local 'notmuch-pick-message-buffer-name) (put 'notmuch-pick-message-buffer-name 'permanent-local t) @@ -389,6 +392,16 @@ Does NOT change the database." (notmuch-prettify-subject (notmuch-search-find-subject))) (notmuch-pick-show-match-message-with-wait)) +(defun notmuch-pick-message-window-kill-hook () + (let ((buffer (current-buffer))) +(when (and (window-live-p notmuch-show-message-window) + (eq (window-buffer notmuch-show-message-window) buffer)) + ;; We do not want an error if this is the sole window in the + ;; frame and I do not know how to test for that in emacs pre + ;; 24. Hence we just ignore-errors. + (ignore-errors + (delete-window notmuch-show-message-window) + (defun notmuch-pick-show-message () "Show the current message (in split-pane)." (interactive) @@ -406,6 +419,11 @@ Does NOT change the database." (let ((notmuch-show-indent-messages-width 0)) (setq current-prefix-arg '(4)) (setq buffer (notmuch-show id nil nil nil + ;; We need the `let' as notmuch-pick-message-window is buffer local. + (let ((window notmuch-pick-message-window)) + (with-current-buffer buffer + (setq notmuch-show-message-window window) + (add-hook 'kill-buffer-hook 'notmuch-pick-message-window-kill-hook))) (notmuch-pick-tag-update-display (list "-unread")) (setq notmuch-pick-message-buffer buffer -- 1.7.9.1
[PATCH v2 3/4] test: notmuch search --format=text0
On Sun, 09 Dec 2012, Jani Nikula wrote: > --- > test/text | 29 + > 1 file changed, 29 insertions(+) > > diff --git a/test/text b/test/text > index 428c89b..e003a66 100755 > --- a/test/text > +++ b/test/text > @@ -52,4 +52,33 @@ output=$(notmuch search --format=text > "t?xt-search-m?ssage" | notmuch_search_s > test_expect_equal "$output" \ > "thread:XXX 2000-01-01 [1/1] Notmuch Test Suite; > text-search-utf8-body-s?bj?ct (inbox unread)" > > +add_email_corpus > + > +test_begin_subtest "Search message tags: text0" > +cat < EXPECTED.$test_count > +attachment inbox signed unread > +EOF > +notmuch search --format=text0 --output=tags '*' | xargs -0 | > notmuch_search_sanitize > OUTPUT.$test_count > +test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count > + > +test_begin_subtest "Compare text vs. text0 for threads" > +notmuch search --format=text --output=threads '*' | notmuch_search_sanitize > > EXPECTED.$test_count > +notmuch search --format=text0 --output=threads '*' | xargs -0 -L1 | > notmuch_search_sanitize > OUTPUT.$test_count > +test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count Hi These xargs -0 -L1 tests almost pass with format=text (no zero) passed: the output only differs in one newline at the end. Would it be worth strengthening the test at all? I don't have any good suggestion but replacing the xargs with tr '\n\0' ' \n' seemed to give clearly different output in the two cases (and the test passes as it stands). OTOH maybe the test before is sufficient in that respect. Best wishes Mark > + > +test_begin_subtest "Compare text vs. text0 for messages" > +notmuch search --format=text --output=messages '*' | notmuch_search_sanitize > > EXPECTED.$test_count > +notmuch search --format=text0 --output=messages '*' | xargs -0 -L1 | > notmuch_search_sanitize > OUTPUT.$test_count > +test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count > + > +test_begin_subtest "Compare text vs. text0 for files" > +notmuch search --format=text --output=files '*' | notmuch_search_sanitize > > EXPECTED.$test_count > +notmuch search --format=text0 --output=files '*' | xargs -0 -L1 | > notmuch_search_sanitize > OUTPUT.$test_count > +test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count > + > +test_begin_subtest "Compare text vs. text0 for tags" > +notmuch search --format=text --output=tags '*' | notmuch_search_sanitize > > EXPECTED.$test_count > +notmuch search --format=text0 --output=tags '*' | xargs -0 -L1 | > notmuch_search_sanitize > OUTPUT.$test_count > +test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count > + > test_done > -- > 1.7.10.4
[PATCH v5 0/4] indicate length of omitted body content
The b versions of this look good to me. Best wishes Mark On Sat, 15 Dec 2012, David Bremner wrote: > Peter Wang writes: > >> On Sat, 15 Dec 2012 08:45:33 +, Mark Walters > gmail.com> wrote: >>> >>> This version looks good to me with one very minor comments: >>> >>> Perhaps the name format-omitted-part-meta should include sprinter (eg >>> format-omitted-part-meta-sprinter)? >> >> Seems so. I'll roll up another series if that's necessary. > > Maybe just post the one amended patch as a reply to the patch it's > amending? > > d > ___ > notmuch mailing list > notmuch at notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 0/7] Structed output versioning support
LGTM +1 Best wishes Mark On Sun, 16 Dec 2012, Austin Clements wrote: > This series obsoletes [0] and must be applied on top of the Emacs CLI > error handling series [1]. This is much simpler than the original > series because it no longer includes the Emacs CLI error handling. > This also switches to a CLI-wide minimum version instead of a > per-command version, prints a warning if an old but still supported > version is requested to ease client maintenance, and renames > --use-schema to --format-version (which parallels --format better and > simplifies error text). > > [0] id:1354416002-3557-1-git-send-email-amdragon at mit.edu > [1] id:1355601860-30121-1-git-send-email-amdragon at mit.edu
[PATCH v2 0/7] Improve Emacs CLI error handling
On Sat, Dec 15 2012, Austin Clements wrote: > This obsoletes id:1355548513-10085-1-git-send-email-amdragon at mit.edu > and fixes the things Mark and Tomi commented on. The interdiff is > below. Ok, new error messages appear at the end. Good. +1 Tomi > > diff --git a/emacs/notmuch-lib.el b/emacs/notmuch-lib.el > index cf61635..8f84087 100644 > --- a/emacs/notmuch-lib.el > +++ b/emacs/notmuch-lib.el > @@ -326,10 +326,12 @@ the user dismisses it." > (with-current-buffer buf >(view-mode-enter nil #'kill-buffer) >(let ((inhibit-read-only t)) > + (goto-char (point-max)) > + (unless (bobp) > + (insert "\n")) > (insert msg) > (unless (bolp) > - (insert "\n")) > - (goto-char (point-min > + (insert "\n" > (pop-to-buffer buf))) >
[Patch v7 05/14] quote_and_decode_query: new function to quote hex-decoded queries
On Fri, 14 Dec 2012, david at tethera.net wrote: > From: David Bremner > > The query is split into tokens, with ' ' and ':' as delimiters. Any > token containing some hex-escaped character is quoted according to > Xapian rules. This maps id:foo%20%22bar to id:"foo ""bar". > This intentionally does not quote prefixes, so they still work as prefixes. > --- > tag-util.c | 50 ++ > 1 file changed, 50 insertions(+) > > diff --git a/tag-util.c b/tag-util.c > index f89669a..e1181f8 100644 > --- a/tag-util.c > +++ b/tag-util.c > @@ -56,6 +56,56 @@ illegal_tag (const char *tag, notmuch_bool_t remove) > return NULL; > } > > +static tag_parse_status_t > +quote_and_decode_query (void *ctx, char *encoded, const char *line_for_error, > + char **query_string) > +{ > +char *tok = encoded; > +size_t tok_len = 0; > +char *buf = NULL; > +size_t buf_len = 0; > +tag_parse_status_t ret = TAG_PARSE_SUCCESS; > + > +*query_string = talloc_strdup (ctx, ""); > + > +while (*query_string && > +(tok = strtok_len (tok + tok_len, ": ", _len)) != NULL) { strtok_len() will eat all the leading delimiters at each call, and will not return a zero-length token if you have multiple consecutive delimiters. Which means you may end up losing stuff here. Whether that matters or not I'm too tired to tell... BR, Jani. > + char delim = tok[tok_len]; > + > + *(tok + tok_len++) = '\0'; > + > + if (strcspn (tok, "%") < tok_len - 1) { > + /* something to decode */ > + if (hex_decode_inplace (tok) != HEX_SUCCESS) { > + ret = line_error (TAG_PARSE_INVALID, line_for_error, > + "hex decoding of token '%s' failed", tok); > + goto DONE; > + } > + > + if (double_quote_str (ctx, tok, , _len)) { > + ret = line_error (TAG_PARSE_OUT_OF_MEMORY, > + line_for_error, "aborting"); > + goto DONE; > + } > + *query_string = talloc_asprintf_append_buffer ( > + *query_string, "%s%c", buf, delim); > + > + } else { > + /* This is not just an optimization, but used to preserve > + * prefixes like id:, which cannot be quoted. > + */ > + *query_string = talloc_asprintf_append_buffer ( > + *query_string, "%s%c", tok, delim); > + } > + > +} > + > + DONE: > +if (ret != TAG_PARSE_SUCCESS && *query_string) > + talloc_free (*query_string); > +return ret; > +} > + > tag_parse_status_t > parse_tag_line (void *ctx, char *line, > tag_op_flag_t flags, > -- > 1.7.10.4 > > ___ > notmuch mailing list > notmuch at notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch
[Patch v7 07/14] cli: add support for batch tagging operations to "notmuch tag"
On Fri, 14 Dec 2012, david at tethera.net wrote: > From: Jani Nikula > > Add support for batch tagging operations through stdin to "notmuch > tag". This can be enabled with the new --batch command line option to > "notmuch tag". The input must consist of lines of the format: > > +|- [...] [--] [...] > > Each line is interpreted similarly to "notmuch tag" command line > arguments. The delimiter is one or more spaces ' '. Any characters in > and MAY be hex encoded with %NN where NN is the > hexadecimal value of the character. Any ' ' and '%' characters in > and MUST be hex encoded (using %20 and %25, > respectively). Any characters that are not part of or > MUST NOT be hex encoded. > > Leading and trailing space ' ' is ignored. Empty lines and lines > beginning with '#' are ignored. > > Signed-off-by: Jani Nikula > > Hacked-like-crazy-by: David Bremner > --- > notmuch-tag.c | 88 > +++-- > 1 file changed, 80 insertions(+), 8 deletions(-) > > diff --git a/notmuch-tag.c b/notmuch-tag.c > index 13f2268..a81d911 100644 > --- a/notmuch-tag.c > +++ b/notmuch-tag.c > @@ -130,6 +130,43 @@ tag_query (void *ctx, notmuch_database_t *notmuch, const > char *query_string, > return interrupted; > } > > +static int > +tag_file (void *ctx, notmuch_database_t *notmuch, tag_op_flag_t flags, > + FILE *input) > +{ > +char *line = NULL; > +char *query_string = NULL; > +size_t line_size = 0; > +ssize_t line_len; > +int ret = 0; > +tag_op_list_t *tag_ops; > + > +tag_ops = tag_op_list_create (ctx); > +if (tag_ops == NULL) { > + fprintf (stderr, "Out of memory.\n"); > + return 1; > +} > + > +while ((line_len = getline (, _size, input)) != -1 && > +! interrupted) { > + > + ret = parse_tag_line (ctx, line, TAG_FLAG_NONE, > + _string, tag_ops); > + > + if (ret > 0) > + continue; > + > + if (ret < 0 || tag_query (ctx, notmuch, query_string, > + tag_ops, flags)) > + break; Hey, your changelog in the cover letter says you fixed the tag_query return value propagation, but it's not here? BR, Jani. > +} > + > +if (line) > + free (line); > + > +return ret; > +} > + > int > notmuch_tag_command (void *ctx, int argc, char *argv[]) > { > @@ -139,6 +176,10 @@ notmuch_tag_command (void *ctx, int argc, char *argv[]) > notmuch_database_t *notmuch; > struct sigaction action; > tag_op_flag_t tag_flags = TAG_FLAG_NONE; > +notmuch_bool_t batch = FALSE; > +FILE *input = stdin; > +char *input_file_name = NULL; > +int opt_index; > int ret = 0; > > /* Setup our handler for SIGINT */ > @@ -148,15 +189,43 @@ notmuch_tag_command (void *ctx, int argc, char *argv[]) > action.sa_flags = SA_RESTART; > sigaction (SIGINT, , NULL); > > -tag_ops = tag_op_list_create (ctx); > -if (tag_ops == NULL) { > - fprintf (stderr, "Out of memory.\n"); > +notmuch_opt_desc_t options[] = { > + { NOTMUCH_OPT_BOOLEAN, , "batch", 0, 0 }, > + { NOTMUCH_OPT_STRING, _file_name, "input", 'i', 0 }, > + { 0, 0, 0, 0, 0 } > +}; > + > +opt_index = parse_arguments (argc, argv, options, 1); > +if (opt_index < 0) > return 1; > + > +if (input_file_name) { > + batch = TRUE; > + input = fopen (input_file_name, "r"); > + if (input == NULL) { > + fprintf (stderr, "Error opening %s for reading: %s\n", > + input_file_name, strerror (errno)); > + return 1; > + } > } > > -if (parse_tag_command_line (ctx, argc - 1, argv + 1, > - _string, tag_ops)) > - return 1; > +if (batch) { > + if (opt_index != argc) { > + fprintf (stderr, "Can't specify both cmdline and stdin!\n"); > + return 1; > + } > +} else { > + > + tag_ops = tag_op_list_create (ctx); > + if (tag_ops == NULL) { > + fprintf (stderr, "Out of memory.\n"); > + return 1; > + } > + > + if (parse_tag_command_line (ctx, argc - opt_index, argv + opt_index, > + _string, tag_ops)) > + return 1; > +} > > config = notmuch_config_open (ctx, NULL, NULL); > if (config == NULL) > @@ -169,9 +238,12 @@ notmuch_tag_command (void *ctx, int argc, char *argv[]) > if (notmuch_config_get_maildir_synchronize_flags (config)) > tag_flags |= TAG_FLAG_MAILDIR_SYNC; > > -ret = tag_query (ctx, notmuch, query_string, tag_ops, tag_flags); > +if (batch) > + ret = tag_file (ctx, notmuch, tag_flags, input); > +else > + ret = tag_query (ctx, notmuch, query_string, tag_ops, tag_flags); > > notmuch_database_destroy (notmuch); > > -return ret; > +return ret || interrupted; > } > -- > 1.7.10.4
[PATCH] fixup for hex encoding desription in notmuch-tag.1
On Sat, 15 Dec 2012, david at tethera.net wrote: > From: David Bremner > > --- > and here is the updated description. > > man/man1/notmuch-tag.1 | 19 ++- > 1 file changed, 14 insertions(+), 5 deletions(-) > > diff --git a/man/man1/notmuch-tag.1 b/man/man1/notmuch-tag.1 > index ac2c8d7..a203f53 100644 > --- a/man/man1/notmuch-tag.1 > +++ b/man/man1/notmuch-tag.1 > @@ -67,13 +67,22 @@ The input must consist of lines of the format: > Each line is interpreted similarly to > .B notmuch tag > command line arguments. The delimiter is one or more spaces ' '. Any > -characters in and > +characters in > +.RI < tag > > +and > +.RI < search-term > > .B may > -be hex encoded with %NN where NN is the hexadecimal value of the > -character. Any ' ' and '%' characters in and > +be hex-encoded with %NN where NN is the hexadecimal value of the > +character (more precisely with %NN[%MM...] where NN, MM, etc... are > +the hexadecimal values of the bytes in the UTF-8 encoding of > +the character). Any characters in and not > +matching the regex > +.B [A-Za-z0-9@=.,_+-] And this actually means you can't have "id:foo" there, as : needs to be encoded. Not user friendly at all. If it should really mean : is okay un-encoded as prefix delimiter but not otherwise... it gets a bit messy to document. BR, Jani. > .B must > -be hex encoded (using %20 and %25, respectively). Any characters that > -are not part of or > +be hex-encoded. Any characters that are not part of > +.RI < tag > > +or > +.RI < search-term > > .B must not > be hex encoded. > > -- > 1.7.10.4 > > ___ > notmuch mailing list > notmuch at notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch
[Patch v7 06/14] notmuch-restore: move query handling for batch restore to parser
On Fri, 14 Dec 2012, david at tethera.net wrote: > From: David Bremner > > We are able to detect more errors by looking at the string before it > is hex-decoded. We also need this to avoid the query quoting for more > general queries (to be written) that will mess up raw message-ids. > --- > notmuch-restore.c | 18 +- > tag-util.c| 26 -- > tag-util.h|5 - > test/dump-restore |5 ++--- > 4 files changed, 27 insertions(+), 27 deletions(-) > > diff --git a/notmuch-restore.c b/notmuch-restore.c > index 40596a8..112f2f3 100644 > --- a/notmuch-restore.c > +++ b/notmuch-restore.c > @@ -208,24 +208,8 @@ notmuch_restore_command (unused (void *ctx), int argc, > char *argv[]) > if (input_format == DUMP_FORMAT_SUP) { > ret = parse_sup_line (ctx, line, _string, tag_ops); > } else { > - ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS, > + ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS | > TAG_FLAG_ID_ONLY, > _string, tag_ops); I realize that we've screwed up a bit here already in the restore series. parse_sup_line() allocates query_string for each input line, but doesn't free it during parsing. parse_tag_line() sets query_string to point to the query string within line. And now it gets worse when query_string is allocated vs. set to point to another buffer depending on TAG_FLAG_ID_ONLY, so it's not an easy interface to use. > - > - if (ret == 0) { > - if (strncmp ("id:", query_string, 3) != 0) { > - fprintf (stderr, "Warning: unsupported query: %s\n", > query_string); > - continue; > - } > - /* 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; > - } > } > > if (ret > 0) > diff --git a/tag-util.c b/tag-util.c > index e1181f8..8fea76c 100644 > --- a/tag-util.c > +++ b/tag-util.c > @@ -201,14 +201,28 @@ parse_tag_line (void *ctx, char *line, > } > > /* tok now points to the query string */ > -if (hex_decode_inplace (tok) != HEX_SUCCESS) { > - ret = line_error (TAG_PARSE_INVALID, line_for_error, > - "hex decoding of query %s failed", tok); > - goto DONE; > +if (flags & TAG_FLAG_ID_ONLY) { > + /* this is under the assumption that any whitespace in the > + * message-id must be hex-encoded. The check is probably not > + * perfect for exotic unicode whitespace; as fallback the > + * search for strange message-ids will fail */ > + if ((strncmp ("id:", tok, 3) != 0) || The current wording is, "Any characters in and MAY be hex encoded with %NN...", but the above no longer matches that, as "id:" must not be encoded. In the interest of keeping the documentation concise, I think you should move the above check after hex_decode_inplace(), but keep the below check here. That should do it, right? > + (strcspn (tok, " \t") < strlen (tok))) { > + ret = line_error (TAG_PARSE_INVALID, line_for_error, > + "query '%s' is not 'id:'", tok); > + goto DONE; > + } > + if (hex_decode_inplace (tok) != HEX_SUCCESS) { > + ret = line_error (TAG_PARSE_INVALID, line_for_error, > + "hex decoding of query %s failed", tok); > + goto DONE; > + } > + /* skip 'id:' */ > + *query_string = tok + 3; > +} else { > + ret = quote_and_decode_query (ctx, tok, line_for_error, query_string); > } It's not pretty that query_string gets allocated or not depending on a flag that doesn't seem to have anything to do with that. I don't have a good suggestion now, though, and I'd like to keep the restore path like it is, without allocation. > > -*query_string = tok; > - >DONE: > talloc_free (line_for_error); > return ret; > diff --git a/tag-util.h b/tag-util.h > index 2889736..7674051 100644 > --- a/tag-util.h > +++ b/tag-util.h > @@ -26,7 +26,10 @@ typedef enum { > /* Accept strange tags that might be user error; > * intended for use by notmuch-restore. > */ > -TAG_FLAG_BE_GENEROUS = (1 << 3) > +TAG_FLAG_BE_GENEROUS = (1 << 3), > + > +/* Query consists of a single id:$message-id */ > +TAG_FLAG_ID_ONLY = (1 << 4) > > } tag_op_flag_t; > > diff --git a/test/dump-restore b/test/dump-restore > index 6a989b6..eb7933a 100755 > --- a/test/dump-restore > +++ b/test/dump-restore > @@ -199,19 +199,18 @@ a > # the next non-comment line should report an an empty tag error for > # batch tagging, but not for restore >
[Patch v7 04/14] notmuch-tag: factor out double quoting routine
On Fri, 14 Dec 2012, david at tethera.net wrote: > From: David Bremner > > This could live in tag-util as well, but it is really nothing specific > to tags (although the conventions are arguable specific to Xapian). > > The API is changed from "caller-allocates" to "readline-like". The scan for > max tag length is pushed down into the double quoting routine. > --- > notmuch-tag.c | 50 -- > util/string-util.c | 34 ++ > util/string-util.h |8 > 3 files changed, 58 insertions(+), 34 deletions(-) > > diff --git a/notmuch-tag.c b/notmuch-tag.c > index 0965ee7..13f2268 100644 > --- a/notmuch-tag.c > +++ b/notmuch-tag.c > @@ -20,6 +20,7 @@ > > #include "notmuch-client.h" > #include "tag-util.h" > +#include "string-util.h" > > static volatile sig_atomic_t interrupted; > > @@ -37,25 +38,6 @@ handle_sigint (unused (int sig)) > } > > static char * > -_escape_tag (char *buf, const char *tag) > -{ > -const char *in = tag; > -char *out = buf; > - > -/* Boolean terms surrounded by double quotes can contain any > - * character. Double quotes are quoted by doubling them. */ > -*out++ = '"'; > -while (*in) { > - if (*in == '"') > - *out++ = '"'; > - *out++ = *in++; > -} > -*out++ = '"'; > -*out = 0; > -return buf; > -} > - > -static char * > _optimize_tag_query (void *ctx, const char *orig_query_string, >const tag_op_list_t *list) > { > @@ -67,44 +49,44 @@ _optimize_tag_query (void *ctx, const char > *orig_query_string, > * parenthesize and the exclusion part of the query must not use > * the '-' operator (though the NOT operator is fine). */ > > -char *escaped, *query_string; > +char *escaped = NULL; > +size_t escaped_len = 0; > +char *query_string; > const char *join = ""; > size_t i; > -unsigned int max_tag_len = 0; > > /* Don't optimize if there are no tag changes. */ > if (tag_op_list_size (list) == 0) > return talloc_strdup (ctx, orig_query_string); > > -/* Allocate a buffer for escaping tags. This is large enough to > - * hold a fully escaped tag with every character doubled plus > - * enclosing quotes and a NUL. */ > -for (i = 0; i < tag_op_list_size (list); i++) > - if (strlen (tag_op_list_tag (list, i)) > max_tag_len) > - max_tag_len = strlen (tag_op_list_tag (list, i)); > - > -escaped = talloc_array (ctx, char, max_tag_len * 2 + 3); > -if (! escaped) > - return NULL; > - > /* Build the new query string */ > if (strcmp (orig_query_string, "*") == 0) > query_string = talloc_strdup (ctx, "("); > else > query_string = talloc_asprintf (ctx, "( %s ) and (", orig_query_string); > > + > +/* Boolean terms surrounded by double quotes can contain any > + * character. Double quotes are quoted by doubling them. */ > + > for (i = 0; i < tag_op_list_size (list) && query_string; i++) { > + double_quote_str (ctx, > + tag_op_list_tag (list, i), > + , _len); Check return value? > + > query_string = talloc_asprintf_append_buffer ( > query_string, "%s%stag:%s", join, > tag_op_list_isremove (list, i) ? "" : "not ", > - _escape_tag (escaped, tag_op_list_tag (list, i))); > + escaped); > join = " or "; > } > > if (query_string) > query_string = talloc_strdup_append_buffer (query_string, ")"); > > -talloc_free (escaped); > +if (escaped) > + talloc_free (escaped); > + > return query_string; > } > > diff --git a/util/string-util.c b/util/string-util.c > index 44f8cd3..ea7c25b 100644 > --- a/util/string-util.c > +++ b/util/string-util.c > @@ -20,6 +20,7 @@ > > > #include "string-util.h" > +#include "talloc.h" > > char * > strtok_len (char *s, const char *delim, size_t *len) > @@ -32,3 +33,36 @@ strtok_len (char *s, const char *delim, size_t *len) > > return *len ? s : NULL; > } > + > + > +int > +double_quote_str (void *ctx, const char *str, > + char **buf, size_t *len) > +{ > +const char *in; > +char *out; > +size_t needed = 3; > + > +for (in = str; *in; in++) > + needed += (*in == '"') ? 2 : 1; > + > +if (needed > *len) > + *buf = talloc_realloc (ctx, *buf, char, 2*needed); You fail to set *len to 2*needed, leading to doing realloc every time. Also, I think you should follow the getline pattern like you did in hex_encode: if *buf == NULL, the input value of *len is ignored. BR, Jani. > + > +if (! *buf) > + return 1; > + > +out = *buf; > + > +*out++ = '"'; > +in = str; > +while (*in) { > + if (*in == '"') > + *out++ = '"'; > + *out++ = *in++; > +} > +*out++ = '"'; > +*out = 0; > + > +return 0; > +} > diff --git a/util/string-util.h
Re: [PATCH v5 0/4] indicate length of omitted body content
The b versions of this look good to me. Best wishes Mark On Sat, 15 Dec 2012, David Bremner da...@tethera.net wrote: Peter Wang noval...@gmail.com writes: On Sat, 15 Dec 2012 08:45:33 +, Mark Walters markwalters1...@gmail.com wrote: This version looks good to me with one very minor comments: Perhaps the name format-omitted-part-meta should include sprinter (eg format-omitted-part-meta-sprinter)? Seems so. I'll roll up another series if that's necessary. Maybe just post the one amended patch as a reply to the patch it's amending? d ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH] contrib: pick: close message pane when quitting from show in the message pane
We add a hook to the show buffer in the message window to close the message window when that buffer quits. It checks that the message-window is still displaying the show-message buffer and then closes it. --- This is (probably) a rather better version than the previous attempt. It uses hooks rather than redefining notmuch-kill-this-buffer and it avoids errors if the user has made the message pane the whole frame. Best wishes Mark contrib/notmuch-pick/notmuch-pick.el | 18 ++ 1 files changed, 18 insertions(+), 0 deletions(-) diff --git a/contrib/notmuch-pick/notmuch-pick.el b/contrib/notmuch-pick/notmuch-pick.el index 043e9e7..8383589 100644 --- a/contrib/notmuch-pick/notmuch-pick.el +++ b/contrib/notmuch-pick/notmuch-pick.el @@ -160,6 +160,9 @@ (defvar notmuch-pick-message-window nil) (make-variable-buffer-local 'notmuch-pick-message-window) (put 'notmuch-pick-message-window 'permanent-local t) +(defvar notmuch-show-message-window nil) +(make-variable-buffer-local 'notmuch-show-message-window) +(put 'notmuch-show-message-window 'permanent-local t) (defvar notmuch-pick-message-buffer nil) (make-variable-buffer-local 'notmuch-pick-message-buffer-name) (put 'notmuch-pick-message-buffer-name 'permanent-local t) @@ -389,6 +392,16 @@ Does NOT change the database. (notmuch-prettify-subject (notmuch-search-find-subject))) (notmuch-pick-show-match-message-with-wait)) +(defun notmuch-pick-message-window-kill-hook () + (let ((buffer (current-buffer))) +(when (and (window-live-p notmuch-show-message-window) + (eq (window-buffer notmuch-show-message-window) buffer)) + ;; We do not want an error if this is the sole window in the + ;; frame and I do not know how to test for that in emacs pre + ;; 24. Hence we just ignore-errors. + (ignore-errors + (delete-window notmuch-show-message-window) + (defun notmuch-pick-show-message () Show the current message (in split-pane). (interactive) @@ -406,6 +419,11 @@ Does NOT change the database. (let ((notmuch-show-indent-messages-width 0)) (setq current-prefix-arg '(4)) (setq buffer (notmuch-show id nil nil nil + ;; We need the `let' as notmuch-pick-message-window is buffer local. + (let ((window notmuch-pick-message-window)) + (with-current-buffer buffer + (setq notmuch-show-message-window window) + (add-hook 'kill-buffer-hook 'notmuch-pick-message-window-kill-hook))) (notmuch-pick-tag-update-display (list -unread)) (setq notmuch-pick-message-buffer buffer -- 1.7.9.1 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v2 3/4] test: notmuch search --format=text0
On Sun, 09 Dec 2012, Jani Nikula j...@nikula.org wrote: --- test/text | 29 + 1 file changed, 29 insertions(+) diff --git a/test/text b/test/text index 428c89b..e003a66 100755 --- a/test/text +++ b/test/text @@ -52,4 +52,33 @@ output=$(notmuch search --format=text tëxt-search-méssage | notmuch_search_s test_expect_equal $output \ thread:XXX 2000-01-01 [1/1] Notmuch Test Suite; text-search-utf8-body-sübjéct (inbox unread) +add_email_corpus + +test_begin_subtest Search message tags: text0 +cat EOF EXPECTED.$test_count Other tests use just OUTPUT and EXPECTED. Why the $test_count? Is there a technical reason for it? +attachment inbox signed unread +EOF +notmuch search --format=text0 --output=tags '*' | xargs -0 | notmuch_search_sanitize OUTPUT.$test_count +test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count + +test_begin_subtest Compare text vs. text0 for threads +notmuch search --format=text --output=threads '*' | notmuch_search_sanitize EXPECTED.$test_count +notmuch search --format=text0 --output=threads '*' | xargs -0 -L1 | notmuch_search_sanitize OUTPUT.$test_count I think it would be worth strengthening these tests as Mark pointed out. It would be easy to accidentally include a literal \n in the output instead of calling the separator method, and this test wouldn't catch that. I think Mark's suggestion with tr is pretty good, since it directly disambiguates \0 and \n in the output, while producing a reasonable diff if things do go wrong. +test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count + +test_begin_subtest Compare text vs. text0 for messages +notmuch search --format=text --output=messages '*' | notmuch_search_sanitize EXPECTED.$test_count +notmuch search --format=text0 --output=messages '*' | xargs -0 -L1 | notmuch_search_sanitize OUTPUT.$test_count +test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count + +test_begin_subtest Compare text vs. text0 for files +notmuch search --format=text --output=files '*' | notmuch_search_sanitize EXPECTED.$test_count +notmuch search --format=text0 --output=files '*' | xargs -0 -L1 | notmuch_search_sanitize OUTPUT.$test_count +test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count + +test_begin_subtest Compare text vs. text0 for tags +notmuch search --format=text --output=tags '*' | notmuch_search_sanitize EXPECTED.$test_count +notmuch search --format=text0 --output=tags '*' | xargs -0 -L1 | notmuch_search_sanitize OUTPUT.$test_count +test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count + test_done -- 1.7.10.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
memory leak tests
These are slightly rough around the edges, but I think they are useful. They already helped me track down a memory leak in notmuch new (id:1355234087-6886-1-git-send-email-da...@tethera.net, id:1355196820-29734-1-git-send-email-da...@tethera.net) They also would have caught the restore leak Jani pointed out in parse_sup_line, although that is a bit of hindsight since I didn't write the obvious test until Jani pointed out the leak. ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 1/4] perf-test: remove redunant initial notmuch new
From: David Bremner brem...@debian.org The initial notmuch-new and caching are now done automatically by time_start --- performance-test/00-new |4 1 file changed, 4 deletions(-) diff --git a/performance-test/00-new b/performance-test/00-new index 6f0b50c..553bb8b 100755 --- a/performance-test/00-new +++ b/performance-test/00-new @@ -8,10 +8,6 @@ uncache_database time_start -time_run 'initial notmuch new' 'notmuch new' - -cache_database - for i in $(seq 2 6); do time_run notmuch new #$i 'notmuch new' done -- 1.7.10.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 3/4] perf-test: initial version of memory test infrastructure.
From: David Bremner brem...@debian.org The idea is run some code under valgrind --leak-check=full and report a summary, leaving the user to peruse the log file if they want. We go to some lengths to preserve the log files from accidental overwriting; the full corpus takes about 3 hours to run under valgrind on my machine. The naming of the log directories is probably overkill; I find it nice to have them sequenced by time. Arguably the mktemp is then overkill, but I know people will be nervous if it looks like timestamps are being used for uniqueness. One new test is included, to check notmuch new for memory leaks. --- performance-test/.gitignore |1 + performance-test/M00-new | 14 + performance-test/Makefile.local | 17 +-- performance-test/README | 57 + performance-test/perf-test-lib.sh | 55 --- 5 files changed, 113 insertions(+), 31 deletions(-) create mode 100755 performance-test/M00-new diff --git a/performance-test/.gitignore b/performance-test/.gitignore index 6421a9a..f3f9be4 100644 --- a/performance-test/.gitignore +++ b/performance-test/.gitignore @@ -1,3 +1,4 @@ tmp.*/ +log.*/ corpus/ notmuch.cache.*/ diff --git a/performance-test/M00-new b/performance-test/M00-new new file mode 100755 index 000..733e9b0 --- /dev/null +++ b/performance-test/M00-new @@ -0,0 +1,14 @@ +#!/bin/bash + +test_description='notmuch new' + +. ./perf-test-lib.sh + +# ensure initial 'notmuch new' is run by memory_start +uncache_database + +memory_start + +memory_run notmuch new notmuch new + +memory_done diff --git a/performance-test/Makefile.local b/performance-test/Makefile.local index 57beb44..357d800 100644 --- a/performance-test/Makefile.local +++ b/performance-test/Makefile.local @@ -4,14 +4,25 @@ dir := performance-test include $(dir)/version.sh +# these two are just make sure dir is expanded at the right time. +TIME_TEST_SCRIPT := ${dir}/notmuch-time-test +MEMORY_TEST_SCRIPT := ${dir}/notmuch-memory-test + CORPUS_NAME := notmuch-email-corpus-$(PERFTEST_VERSION).tar.xz TXZFILE := ${dir}/download/${CORPUS_NAME} SIGFILE := ${TXZFILE}.asc -TEST_SCRIPT := ${dir}/notmuch-perf-test DEFAULT_URL := http://notmuchmail.org/releases/${CORPUS_NAME} +perf-test: time-test memory-test + time-test: setup-perf-test all - $(TEST_SCRIPT) $(OPTIONS) + @echo + $(TIME_TEST_SCRIPT) $(TEST_OPTIONS) + +memory-test: setup-perf-test all + @echo + $(MEMORY_TEST_SCRIPT) $(TEST_OPTIONS) + .PHONY: download-corpus setup-perf-test @@ -29,4 +40,4 @@ $(TXZFILE): download-corpus: wget -O ${TXZFILE} ${DEFAULT_URL} -CLEAN := $(CLEAN) $(dir)/tmp.* $(dir)/corpus $(dir)/notmuch.cache.* +CLEAN := $(CLEAN) $(dir)/tmp.* $(dir)/log.* $(dir)/corpus $(dir)/notmuch.cache.* diff --git a/performance-test/README b/performance-test/README index d1fb6de..7eaf5f7 100644 --- a/performance-test/README +++ b/performance-test/README @@ -1,3 +1,10 @@ +Performance Tests +- + +This directory contains two kinds of performance tests, time tests, +and memory tests. The former use gnu time, and the latter use +valgrind. + Pre-requisites -- @@ -5,9 +12,10 @@ In addition to having notmuch, you need: - gpg - gnu tar -- gnu time +- gnu time (for the time tests). - xz. Some speedup can be gotten by installing pixz, but this is probably only worthwhile if you are debugging the tests. +- valgrind (for the memory tests) Getting set up to run tests: @@ -36,34 +44,47 @@ for a list of mirrors. Running tests - -The easiest way to run performance tests is to say make time-test, (or -simply run the notmuch-time-test script). Either command will run all -available performance tests. - -Alternately, you can run a specific subset of tests by simply invoking -one of the executable scripts in this directory, (such as ./basic). -Each test script supports the following arguments +The easiest way to run performance tests is to say make perf-test. +This will run all time and memory tests. Be aware that the memory +tests are quite time consuming when run on the full corpus, and that +depending on your interests it may be more sensible to run make +time-test or make memory-test. You can also invoke one of the +scripts notmuch-time-test or notmuch-memory-test or run a more +specific subset of tests by simply invoking one of the executable +scripts in this directory, (such as ./T00-new). Each test script +supports the following arguments --small / --medium / --large Choose corpus size. --debugEnable debugging. In particular don't delete temporary directories. +When using the make targets, you can pass arguments to all test +scripts by defining the make variable TEST_OPTIONS. + Writing tests - -Have a look at T01-dump-restore for an
[PATCH 4/4] perf-test: add memory leak test for dump restore
From: David Bremner brem...@debian.org In id:87vcc2q5n2@nikula.org, Jani points out a memory leak in the current version of the sup restore code. Among other things, this test is intended to verify a fix for that leak. --- performance-test/M01-dump-restore | 15 +++ 1 file changed, 15 insertions(+) create mode 100755 performance-test/M01-dump-restore diff --git a/performance-test/M01-dump-restore b/performance-test/M01-dump-restore new file mode 100755 index 000..be5894a --- /dev/null +++ b/performance-test/M01-dump-restore @@ -0,0 +1,15 @@ +#!/bin/bash + +test_description='dump and restore' + +. ./perf-test-lib.sh + +memory_start + +memory_run 'load nmbug tags' 'notmuch restore --accumulate --input=corpus.tags/nmbug.sup-dump' +memory_run 'dump *' 'notmuch dump --output=tags.sup' +memory_run 'restore *' 'notmuch restore --input=tags.sup' +memory_run 'dump --format=batch-tag *' 'notmuch dump --format=batch-tag --output=tags.bt' +memory_run 'restore --format=batch-tag *' 'notmuch restore --format=batch-tag --input=tags.bt' + +memory_done -- 1.7.10.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 3/4] perf-test: initial version of memory test infrastructure.
Quoth da...@tethera.net on Dec 16 at 2:23 pm: From: David Bremner brem...@debian.org The idea is run some code under valgrind --leak-check=full and report a summary, leaving the user to peruse the log file if they want. We go to some lengths to preserve the log files from accidental overwriting; the full corpus takes about 3 hours to run under valgrind on my machine. The naming of the log directories is probably overkill; I find it nice to have them sequenced by time. Arguably the mktemp is then overkill, but I know people will be nervous if it looks like timestamps are being used for uniqueness. One new test is included, to check notmuch new for memory leaks. --- performance-test/.gitignore |1 + performance-test/M00-new | 14 + performance-test/Makefile.local | 17 +-- performance-test/README | 57 + performance-test/perf-test-lib.sh | 55 --- 5 files changed, 113 insertions(+), 31 deletions(-) create mode 100755 performance-test/M00-new diff --git a/performance-test/.gitignore b/performance-test/.gitignore index 6421a9a..f3f9be4 100644 --- a/performance-test/.gitignore +++ b/performance-test/.gitignore @@ -1,3 +1,4 @@ tmp.*/ +log.*/ corpus/ notmuch.cache.*/ diff --git a/performance-test/M00-new b/performance-test/M00-new new file mode 100755 index 000..733e9b0 --- /dev/null +++ b/performance-test/M00-new @@ -0,0 +1,14 @@ +#!/bin/bash + +test_description='notmuch new' + +. ./perf-test-lib.sh + +# ensure initial 'notmuch new' is run by memory_start +uncache_database + +memory_start + +memory_run notmuch new notmuch new This will run notmuch new twice. Is that intentional? If so, it might be worth a comment. + +memory_done diff --git a/performance-test/Makefile.local b/performance-test/Makefile.local index 57beb44..357d800 100644 --- a/performance-test/Makefile.local +++ b/performance-test/Makefile.local @@ -4,14 +4,25 @@ dir := performance-test include $(dir)/version.sh +# these two are just make sure dir is expanded at the right time. +TIME_TEST_SCRIPT := ${dir}/notmuch-time-test +MEMORY_TEST_SCRIPT := ${dir}/notmuch-memory-test This is obviously fine, but I don't understand the comment. There are all sorts of places where we have to := assign to get ${dir} right (including TXZFILE file below). Is there something different here? + CORPUS_NAME := notmuch-email-corpus-$(PERFTEST_VERSION).tar.xz TXZFILE := ${dir}/download/${CORPUS_NAME} SIGFILE := ${TXZFILE}.asc -TEST_SCRIPT := ${dir}/notmuch-perf-test DEFAULT_URL := http://notmuchmail.org/releases/${CORPUS_NAME} +perf-test: time-test memory-test + time-test: setup-perf-test all - $(TEST_SCRIPT) $(OPTIONS) + @echo + $(TIME_TEST_SCRIPT) $(TEST_OPTIONS) Why not use the same OPTIONS variable name as the correctness tests? + +memory-test: setup-perf-test all + @echo + $(MEMORY_TEST_SCRIPT) $(TEST_OPTIONS) + .PHONY: download-corpus setup-perf-test @@ -29,4 +40,4 @@ $(TXZFILE): download-corpus: wget -O ${TXZFILE} ${DEFAULT_URL} -CLEAN := $(CLEAN) $(dir)/tmp.* $(dir)/corpus $(dir)/notmuch.cache.* +CLEAN := $(CLEAN) $(dir)/tmp.* $(dir)/log.* $(dir)/corpus $(dir)/notmuch.cache.* diff --git a/performance-test/README b/performance-test/README index d1fb6de..7eaf5f7 100644 --- a/performance-test/README +++ b/performance-test/README @@ -1,3 +1,10 @@ +Performance Tests +- + +This directory contains two kinds of performance tests, time tests, s/performance tests,/performance tests:/ +and memory tests. The former use gnu time, and the latter use +valgrind. + Pre-requisites -- @@ -5,9 +12,10 @@ In addition to having notmuch, you need: - gpg - gnu tar -- gnu time +- gnu time (for the time tests). No period? - xz. Some speedup can be gotten by installing pixz, but this is probably only worthwhile if you are debugging the tests. +- valgrind (for the memory tests) Getting set up to run tests: @@ -36,34 +44,47 @@ for a list of mirrors. Running tests - -The easiest way to run performance tests is to say make time-test, (or -simply run the notmuch-time-test script). Either command will run all -available performance tests. - -Alternately, you can run a specific subset of tests by simply invoking -one of the executable scripts in this directory, (such as ./basic). -Each test script supports the following arguments +The easiest way to run performance tests is to say make perf-test. +This will run all time and memory tests. Be aware that the memory +tests are quite time consuming when run on the full corpus, and that +depending on your interests it may be more sensible to run make +time-test or make memory-test. You can also invoke one of the +scripts
proposed fix for memory leak in notmuch restore
In id:87vcc2q5n2@nikula.org, Jani points out a memory leak in the new restore code. This series is a proposed fix, namely do any allocation on a temporary talloc context that is destroyed after each line is processed. ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 3/3] notmuch-restore: allocate a temporary talloc context for each line parsed.
From: David Bremner brem...@debian.org This lets the high level code in notmuch restore be ignorant about what the lower level code is doing as far as allocating memory. --- notmuch-restore.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/notmuch-restore.c b/notmuch-restore.c index 4b76d83..52e7ecb 100644 --- a/notmuch-restore.c +++ b/notmuch-restore.c @@ -122,6 +122,7 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) char *input_file_name = NULL; FILE *input = stdin; char *line = NULL; +void *line_ctx = NULL; size_t line_size; ssize_t line_len; @@ -205,10 +206,11 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) do { char *query_string; + line_ctx = talloc_new (ctx); if (input_format == DUMP_FORMAT_SUP) { - ret = parse_sup_line (ctx, line, query_string, tag_ops); + ret = parse_sup_line (line_ctx, line, query_string, tag_ops); } else { - ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS, + ret = parse_tag_line (line_ctx, line, TAG_FLAG_BE_GENEROUS, query_string, tag_ops); if (ret == 0) { @@ -238,8 +240,14 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) break; } + talloc_free (line_ctx); + /* setting to NULL is important to avoid a double free */ + line_ctx = NULL; } while ((line_len = getline (line, line_size, input)) != -1); +if (line_ctx != NULL) + talloc_free (line_ctx); + if (input_format == DUMP_FORMAT_SUP) regfree (regex); -- 1.7.10.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 1/3] tag-utils: use the tag_opt_list_t as talloc context, if possible.
From: David Bremner brem...@debian.org This code is no less correct than the previous version, since it does not make sense for the array to live longer than the wrapping struct. By not relying on the context passed into tag_parse_line, we can allow tag_op_list_t structures to live longer than that context. --- notmuch-restore.c |2 +- tag-util.c|9 - tag-util.h|3 +-- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/notmuch-restore.c b/notmuch-restore.c index 40596a8..ae0ef45 100644 --- a/notmuch-restore.c +++ b/notmuch-restore.c @@ -102,7 +102,7 @@ parse_sup_line (void *ctx, char *line, tok_len++; } - if (tag_op_list_append (ctx, tag_ops, tok, FALSE)) + if (tag_op_list_append (tag_ops, tok, FALSE)) return -1; } diff --git a/tag-util.c b/tag-util.c index eab482f..705b7ba 100644 --- a/tag-util.c +++ b/tag-util.c @@ -109,7 +109,7 @@ parse_tag_line (void *ctx, char *line, goto DONE; } - if (tag_op_list_append (ctx, tag_ops, tag, remove)) { + if (tag_op_list_append (tag_ops, tag, remove)) { ret = line_error (TAG_PARSE_OUT_OF_MEMORY, line_for_error, aborting); goto DONE; @@ -294,7 +294,7 @@ tag_op_list_create (void *ctx) list-size = TAG_OP_LIST_INITIAL_SIZE; list-count = 0; -list-ops = talloc_array (ctx, tag_operation_t, list-size); +list-ops = talloc_array (list, tag_operation_t, list-size); if (list-ops == NULL) return NULL; @@ -303,8 +303,7 @@ tag_op_list_create (void *ctx) int -tag_op_list_append (void *ctx, - tag_op_list_t *list, +tag_op_list_append (tag_op_list_t *list, const char *tag, notmuch_bool_t remove) { @@ -314,7 +313,7 @@ tag_op_list_append (void *ctx, if (list-count == list-size) { list-size *= 2; - list-ops = talloc_realloc (ctx, list-ops, tag_operation_t, + list-ops = talloc_realloc (list, list-ops, tag_operation_t, list-size); if (list-ops == NULL) { fprintf (stderr, Out of memory.\n); diff --git a/tag-util.h b/tag-util.h index 99b0fa0..c07bfde 100644 --- a/tag-util.h +++ b/tag-util.h @@ -87,8 +87,7 @@ tag_op_list_create (void *ctx); */ int -tag_op_list_append (void *ctx, - tag_op_list_t *list, +tag_op_list_append (tag_op_list_t *list, const char *tag, notmuch_bool_t remove); -- 1.7.10.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [RFC PATCH] cli: add --remove-all option to notmuch tag
On Mon, 03 Dec 2012, Jani Nikula j...@nikula.org wrote: Add --remove-all option to notmuch tag to remove all tags matching query before applying the tag changes. This allows removal and unconditional setting of the tags of a message: $ notmuch tag --remove-all id:f...@example.com $ notmuch tag --remove-all +foo +bar id:f...@example.com without having to resort to the complicated (and still quoting broken): $ notmuch tag $(notmuch search --output=tags '*' | sed 's/^/-/') id:f...@example.com $ notmuch tag $(notmuch search --output=tags '*' | sed 's/^/-/') +foo +bar id:f...@example.com --- It's completely untested... This is on top of David's batch tagging branch new-tagging at git://pivot.cs.unb.ca/notmuch.git If there's interest, I'll spin a new version with tests and man page changes after David's stuff has been merged. I like this: the possibility of setting the tags to something seems nice. I am not very keen on the option name but don't have anything much better: maybe --remove-all-first or --set-to? Incidentally, does this (or should this) give an error if the user calls --remove-all with -some_tag (as opposed to +some_tag)? Best wishes Mark --- notmuch-tag.c | 32 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/notmuch-tag.c b/notmuch-tag.c index e4fca67..67624cc 100644 --- a/notmuch-tag.c +++ b/notmuch-tag.c @@ -119,12 +119,15 @@ tag_query (void *ctx, notmuch_database_t *notmuch, const char *query_string, notmuch_messages_t *messages; notmuch_message_t *message; -/* Optimize the query so it excludes messages that already have - * the specified set of tags. */ -query_string = _optimize_tag_query (ctx, query_string, tag_ops); -if (query_string == NULL) { - fprintf (stderr, Out of memory.\n); - return 1; +if (! (flags TAG_FLAG_REMOVE_ALL)) { + /* Optimize the query so it excludes messages that already + * have the specified set of tags. */ + query_string = _optimize_tag_query (ctx, query_string, tag_ops); + if (query_string == NULL) { + fprintf (stderr, Out of memory.\n); + return 1; + } + flags |= TAG_FLAG_PRE_OPTIMIZED; } query = notmuch_query_create (notmuch, query_string); @@ -140,7 +143,7 @@ tag_query (void *ctx, notmuch_database_t *notmuch, const char *query_string, notmuch_messages_valid (messages) ! interrupted; notmuch_messages_move_to_next (messages)) { message = notmuch_messages_get (messages); - tag_op_list_apply (message, tag_ops, flags | TAG_FLAG_PRE_OPTIMIZED); + tag_op_list_apply (message, tag_ops, flags); notmuch_message_destroy (message); } @@ -157,8 +160,9 @@ notmuch_tag_command (void *ctx, int argc, char *argv[]) notmuch_config_t *config; notmuch_database_t *notmuch; struct sigaction action; -tag_op_flag_t synchronize_flags = TAG_FLAG_NONE; +tag_op_flag_t flags = TAG_FLAG_NONE; notmuch_bool_t batch = FALSE; +notmuch_bool_t remove_all = FALSE; FILE *input = stdin; char *input_file_name = NULL; int i, opt_index; @@ -174,6 +178,7 @@ notmuch_tag_command (void *ctx, int argc, char *argv[]) notmuch_opt_desc_t options[] = { { NOTMUCH_OPT_BOOLEAN, batch, batch, 0, 0 }, { NOTMUCH_OPT_STRING, input_file_name, input, 'i', 0 }, + { NOTMUCH_OPT_BOOLEAN, remove_all, remove-all, 0, 0 }, { 0, 0, 0, 0, 0 } }; @@ -230,7 +235,7 @@ notmuch_tag_command (void *ctx, int argc, char *argv[]) } } - if (tag_op_list_size (tag_ops) == 0) { + if (tag_op_list_size (tag_ops) == 0 !remove_all) { fprintf (stderr, Error: 'notmuch tag' requires at least one tag to add or remove.\n); return 1; } @@ -252,7 +257,10 @@ notmuch_tag_command (void *ctx, int argc, char *argv[]) return 1; if (notmuch_config_get_maildir_synchronize_flags (config)) - synchronize_flags = TAG_FLAG_MAILDIR_SYNC; + flags |= TAG_FLAG_MAILDIR_SYNC; + +if (remove_all) + flags |= TAG_FLAG_REMOVE_ALL; if (batch) { @@ -280,14 +288,14 @@ notmuch_tag_command (void *ctx, int argc, char *argv[]) continue; if (ret 0 || tag_query (ctx, notmuch, query_string, - tag_ops, synchronize_flags)) + tag_ops, flags)) break; } if (line) free (line); } else - ret = tag_query (ctx, notmuch, query_string, tag_ops, synchronize_flags); + ret = tag_query (ctx, notmuch, query_string, tag_ops, flags); notmuch_database_destroy (notmuch); -- 1.7.10.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: gmail importer script
On Sat, Dec 15, 2012 at 11:41 AM, Patrick Totzke patricktot...@gmail.comwrote: Well, thats not the point.. the script shouldn't die like this. I think it's be better if the script caught that exception, deleted the file and continued.. Probably, but I suspect it's related to whatever mystery error you ran into before with the corruption. Does deleting that file and trying again fix it? Anyway, this is extremely stable for me and a few others at this point. I'm going to wait for other users to report errors. Alternatively, send me patches if you want things to happen. ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v3 0/5] add --format=text0 to notmuch search
Hi all, v3 of id:cover.1355064714.git.j...@nikula.org Changes since v2: * add new patch 1/5 to clarify sprinter documentation * fix the test patch 4/5 according to id:8738z6wguj@qmul.ac.uk and id:87y5gyvvv7@awakening.csail.mit.edu Diff to v2 at the end of the cover letter. BR, Jani. Jani Nikula (5): sprinter: clarify separator documentation sprinter: add text0 formatter for null character separated text cli: add --format=text0 to notmuch search test: notmuch search --format=text0 man: document notmuch search --format=text0 man/man1/notmuch-search.1 | 26 -- notmuch-search.c | 16 ++-- sprinter-text.c | 22 ++ sprinter.h| 15 +++ test/text | 33 + 5 files changed, 96 insertions(+), 16 deletions(-) -- 1.7.10.4 diff --git a/sprinter.h b/sprinter.h index f36b999..f859672 100644 --- a/sprinter.h +++ b/sprinter.h @@ -42,10 +42,11 @@ typedef struct sprinter { */ void (*map_key) (struct sprinter *, const char *); -/* Insert a separator (usually extra whitespace) for improved - * readability without affecting the abstract syntax of the - * structure being printed. - * For JSON, this could simply be a line break. +/* Insert a separator (usually extra whitespace). For the text + * printers, this is a syntactic separator. For the structured + * printers, this is for improved readability without affecting + * the abstract syntax of the structure being printed. For JSON, + * this could simply be a line break. */ void (*separator) (struct sprinter *); diff --git a/test/text b/test/text index e003a66..b5ccefc 100755 --- a/test/text +++ b/test/text @@ -55,30 +55,34 @@ test_expect_equal $output \ add_email_corpus test_begin_subtest Search message tags: text0 -cat EOF EXPECTED.$test_count +cat EOF EXPECTED attachment inbox signed unread EOF -notmuch search --format=text0 --output=tags '*' | xargs -0 | notmuch_search_sanitize OUTPUT.$test_count -test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count +notmuch search --format=text0 --output=tags '*' | xargs -0 | notmuch_search_sanitize OUTPUT +test_expect_equal_file EXPECTED OUTPUT +# Use tr(1) to convert --output=text0 to --output=text for +# comparison. Also translate newlines to spaces to fail with more +# noise if they are present as delimiters instead of null +# characters. This assumes there are no newlines in the data. test_begin_subtest Compare text vs. text0 for threads -notmuch search --format=text --output=threads '*' | notmuch_search_sanitize EXPECTED.$test_count -notmuch search --format=text0 --output=threads '*' | xargs -0 -L1 | notmuch_search_sanitize OUTPUT.$test_count -test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count +notmuch search --format=text --output=threads '*' | notmuch_search_sanitize EXPECTED +notmuch search --format=text0 --output=threads '*' | tr \n\0 \n | notmuch_search_sanitize OUTPUT +test_expect_equal_file EXPECTED OUTPUT test_begin_subtest Compare text vs. text0 for messages -notmuch search --format=text --output=messages '*' | notmuch_search_sanitize EXPECTED.$test_count -notmuch search --format=text0 --output=messages '*' | xargs -0 -L1 | notmuch_search_sanitize OUTPUT.$test_count -test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count +notmuch search --format=text --output=messages '*' | notmuch_search_sanitize EXPECTED +notmuch search --format=text0 --output=messages '*' | tr \n\0 \n | notmuch_search_sanitize OUTPUT +test_expect_equal_file EXPECTED OUTPUT test_begin_subtest Compare text vs. text0 for files -notmuch search --format=text --output=files '*' | notmuch_search_sanitize EXPECTED.$test_count -notmuch search --format=text0 --output=files '*' | xargs -0 -L1 | notmuch_search_sanitize OUTPUT.$test_count -test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count +notmuch search --format=text --output=files '*' | notmuch_search_sanitize EXPECTED +notmuch search --format=text0 --output=files '*' | tr \n\0 \n | notmuch_search_sanitize OUTPUT +test_expect_equal_file EXPECTED OUTPUT test_begin_subtest Compare text vs. text0 for tags -notmuch search --format=text --output=tags '*' | notmuch_search_sanitize EXPECTED.$test_count -notmuch search --format=text0 --output=tags '*' | xargs -0 -L1 | notmuch_search_sanitize OUTPUT.$test_count -test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count +notmuch search --format=text --output=tags '*' | notmuch_search_sanitize EXPECTED +notmuch search --format=text0 --output=tags '*' | tr \n\0 \n | notmuch_search_sanitize OUTPUT +test_expect_equal_file EXPECTED OUTPUT test_done ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v3 1/5] sprinter: clarify separator documentation
For text printers, the separator is a syntactic element. --- sprinter.h |9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/sprinter.h b/sprinter.h index 59776a9..f43a844 100644 --- a/sprinter.h +++ b/sprinter.h @@ -42,10 +42,11 @@ typedef struct sprinter { */ void (*map_key) (struct sprinter *, const char *); -/* Insert a separator (usually extra whitespace) for improved - * readability without affecting the abstract syntax of the - * structure being printed. - * For JSON, this could simply be a line break. +/* Insert a separator (usually extra whitespace). For the text + * printers, this is a syntactic separator. For the structured + * printers, this is for improved readability without affecting + * the abstract syntax of the structure being printed. For JSON, + * this could simply be a line break. */ void (*separator) (struct sprinter *); -- 1.7.10.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v3 2/5] sprinter: add text0 formatter for null character separated text
Same as the text formatter, but with each field separated by a null character rather than a newline character. --- sprinter-text.c | 22 ++ sprinter.h |6 ++ 2 files changed, 28 insertions(+) diff --git a/sprinter-text.c b/sprinter-text.c index 10343be..7779488 100644 --- a/sprinter-text.c +++ b/sprinter-text.c @@ -68,6 +68,14 @@ text_separator (struct sprinter *sp) } static void +text0_separator (struct sprinter *sp) +{ +struct sprinter_text *sptxt = (struct sprinter_text *) sp; + +fputc ('\0', sptxt-stream); +} + +static void text_set_prefix (struct sprinter *sp, const char *prefix) { struct sprinter_text *sptxt = (struct sprinter_text *) sp; @@ -133,3 +141,17 @@ sprinter_text_create (const void *ctx, FILE *stream) res-stream = stream; return res-vtable; } + +struct sprinter * +sprinter_text0_create (const void *ctx, FILE *stream) +{ +struct sprinter *sp; + +sp = sprinter_text_create (ctx, stream); +if (! sp) + return NULL; + +sp-separator = text0_separator; + +return sp; +} diff --git a/sprinter.h b/sprinter.h index f43a844..f859672 100644 --- a/sprinter.h +++ b/sprinter.h @@ -67,6 +67,12 @@ typedef struct sprinter { struct sprinter * sprinter_text_create (const void *ctx, FILE *stream); +/* Create a new unstructured printer that emits the text format for + * notmuch search, with each field separated by a null character + * instead of the newline character. */ +struct sprinter * +sprinter_text0_create (const void *ctx, FILE *stream); + /* Create a new structure printer that emits JSON. */ struct sprinter * sprinter_json_create (const void *ctx, FILE *stream); -- 1.7.10.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v3 3/5] cli: add --format=text0 to notmuch search
Add new format text0, which is otherwise the same as text, but use the null character as separator instead of the newline character. This is similar to find(1) -print0 option, and works together with the xargs(1) -0 option. --- notmuch-search.c | 16 ++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/notmuch-search.c b/notmuch-search.c index 6218622..627962b 100644 --- a/notmuch-search.c +++ b/notmuch-search.c @@ -305,8 +305,12 @@ notmuch_search_command (void *ctx, int argc, char *argv[]) int exclude = EXCLUDE_TRUE; unsigned int i; -enum { NOTMUCH_FORMAT_JSON, NOTMUCH_FORMAT_TEXT, NOTMUCH_FORMAT_SEXP } - format_sel = NOTMUCH_FORMAT_TEXT; +enum { + NOTMUCH_FORMAT_JSON, + NOTMUCH_FORMAT_TEXT, + NOTMUCH_FORMAT_TEXT0, + NOTMUCH_FORMAT_SEXP +} format_sel = NOTMUCH_FORMAT_TEXT; notmuch_opt_desc_t options[] = { { NOTMUCH_OPT_KEYWORD, sort, sort, 's', @@ -317,6 +321,7 @@ notmuch_search_command (void *ctx, int argc, char *argv[]) (notmuch_keyword_t []){ { json, NOTMUCH_FORMAT_JSON }, { sexp, NOTMUCH_FORMAT_SEXP }, { text, NOTMUCH_FORMAT_TEXT }, + { text0, NOTMUCH_FORMAT_TEXT0 }, { 0, 0 } } }, { NOTMUCH_OPT_KEYWORD, output, output, 'o', (notmuch_keyword_t []){ { summary, OUTPUT_SUMMARY }, @@ -345,6 +350,13 @@ notmuch_search_command (void *ctx, int argc, char *argv[]) case NOTMUCH_FORMAT_TEXT: format = sprinter_text_create (ctx, stdout); break; +case NOTMUCH_FORMAT_TEXT0: + if (output == OUTPUT_SUMMARY) { + fprintf (stderr, Error: --format=text0 is not compatible with --output=summary.\n); + return 1; + } + format = sprinter_text0_create (ctx, stdout); + break; case NOTMUCH_FORMAT_JSON: format = sprinter_json_create (ctx, stdout); break; -- 1.7.10.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v3 4/5] test: notmuch search --format=text0
--- test/text | 33 + 1 file changed, 33 insertions(+) diff --git a/test/text b/test/text index 428c89b..b5ccefc 100755 --- a/test/text +++ b/test/text @@ -52,4 +52,37 @@ output=$(notmuch search --format=text tëxt-search-méssage | notmuch_search_s test_expect_equal $output \ thread:XXX 2000-01-01 [1/1] Notmuch Test Suite; text-search-utf8-body-sübjéct (inbox unread) +add_email_corpus + +test_begin_subtest Search message tags: text0 +cat EOF EXPECTED +attachment inbox signed unread +EOF +notmuch search --format=text0 --output=tags '*' | xargs -0 | notmuch_search_sanitize OUTPUT +test_expect_equal_file EXPECTED OUTPUT + +# Use tr(1) to convert --output=text0 to --output=text for +# comparison. Also translate newlines to spaces to fail with more +# noise if they are present as delimiters instead of null +# characters. This assumes there are no newlines in the data. +test_begin_subtest Compare text vs. text0 for threads +notmuch search --format=text --output=threads '*' | notmuch_search_sanitize EXPECTED +notmuch search --format=text0 --output=threads '*' | tr \n\0 \n | notmuch_search_sanitize OUTPUT +test_expect_equal_file EXPECTED OUTPUT + +test_begin_subtest Compare text vs. text0 for messages +notmuch search --format=text --output=messages '*' | notmuch_search_sanitize EXPECTED +notmuch search --format=text0 --output=messages '*' | tr \n\0 \n | notmuch_search_sanitize OUTPUT +test_expect_equal_file EXPECTED OUTPUT + +test_begin_subtest Compare text vs. text0 for files +notmuch search --format=text --output=files '*' | notmuch_search_sanitize EXPECTED +notmuch search --format=text0 --output=files '*' | tr \n\0 \n | notmuch_search_sanitize OUTPUT +test_expect_equal_file EXPECTED OUTPUT + +test_begin_subtest Compare text vs. text0 for tags +notmuch search --format=text --output=tags '*' | notmuch_search_sanitize EXPECTED +notmuch search --format=text0 --output=tags '*' | tr \n\0 \n | notmuch_search_sanitize OUTPUT +test_expect_equal_file EXPECTED OUTPUT + test_done -- 1.7.10.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v3 5/5] man: document notmuch search --format=text0
--- man/man1/notmuch-search.1 | 26 -- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/man/man1/notmuch-search.1 b/man/man1/notmuch-search.1 index 0aff348..22bcd0a 100644 --- a/man/man1/notmuch-search.1 +++ b/man/man1/notmuch-search.1 @@ -25,9 +25,11 @@ Supported options for include .RS 4 .TP 4 -.BR \-\-format= ( json | sexp | text ) +.BR \-\-format= ( json | sexp | text | text0 ) -Presents the results in either JSON, S-Expressions or plain-text (default). +Presents the results in either JSON, S-Expressions, newline character +separated plain-text (default), or null character separated plain-text +(compatible with \fBxargs\fR(1) -0 option where available). .RE .RS 4 @@ -48,32 +50,36 @@ the authors of the thread and the subject. .B threads Output the thread IDs of all threads with any message matching the -search terms, either one per line (\-\-format=text) or as a JSON array -(\-\-format=json) or an S-Expression list (\-\-format=sexp). +search terms, either one per line (\-\-format=text), separated by null +characters (\-\-format=text0), as a JSON array (\-\-format=json), or +an S-Expression list (\-\-format=sexp). .RE .RS 4 .TP 4 .B messages Output the message IDs of all messages matching the search terms, -either one per line (\-\-format=text) or as a JSON array -(\-\-format=json) or as an S-Expression list (\-\-format=sexp). +either one per line (\-\-format=text), separated by null characters +(\-\-format=text0), as a JSON array (\-\-format=json), or as an +S-Expression list (\-\-format=sexp). .RE .RS 4 .TP 4 .B files Output the filenames of all messages matching the search terms, either -one per line (\-\-format=text) or as a JSON array (\-\-format=json) or -as an S-Expression list (\-\-format=sexp). +one per line (\-\-format=text), separated by null characters +(\-\-format=text0), as a JSON array (\-\-format=json), or as an +S-Expression list (\-\-format=sexp). .RE .RS 4 .TP 4 .B tags Output all tags that appear on any message matching the search terms, -either one per line (\-\-format=text) or as a JSON array (\-\-format=json) -or as an S-Expression list (\-\-format=sexp). +either one per line (\-\-format=text), separated by null characters +(\-\-format=text0), as a JSON array (\-\-format=json), or as an +S-Expression list (\-\-format=sexp). .RE .RE -- 1.7.10.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v3 0/5] add --format=text0 to notmuch search
On Sun, 16 Dec 2012, Jani Nikula j...@nikula.org wrote: Hi all, v3 of id:cover.1355064714.git.j...@nikula.org Changes since v2: * add new patch 1/5 to clarify sprinter documentation * fix the test patch 4/5 according to id:8738z6wguj@qmul.ac.uk and id:87y5gyvvv7@awakening.csail.mit.edu Diff to v2 at the end of the cover letter. BR, Jani. LGTM. ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v3 0/5] add --format=text0 to notmuch search
On Sun, 16 Dec 2012, Jani Nikula j...@nikula.org wrote: Hi all, v3 of id:cover.1355064714.git.j...@nikula.org Changes since v2: * add new patch 1/5 to clarify sprinter documentation * fix the test patch 4/5 according to id:8738z6wguj@qmul.ac.uk and id:87y5gyvvv7@awakening.csail.mit.edu Diff to v2 at the end of the cover letter. LGTM +1 Best wishes Mark BR, Jani. Jani Nikula (5): sprinter: clarify separator documentation sprinter: add text0 formatter for null character separated text cli: add --format=text0 to notmuch search test: notmuch search --format=text0 man: document notmuch search --format=text0 man/man1/notmuch-search.1 | 26 -- notmuch-search.c | 16 ++-- sprinter-text.c | 22 ++ sprinter.h| 15 +++ test/text | 33 + 5 files changed, 96 insertions(+), 16 deletions(-) -- 1.7.10.4 diff --git a/sprinter.h b/sprinter.h index f36b999..f859672 100644 --- a/sprinter.h +++ b/sprinter.h @@ -42,10 +42,11 @@ typedef struct sprinter { */ void (*map_key) (struct sprinter *, const char *); -/* Insert a separator (usually extra whitespace) for improved - * readability without affecting the abstract syntax of the - * structure being printed. - * For JSON, this could simply be a line break. +/* Insert a separator (usually extra whitespace). For the text + * printers, this is a syntactic separator. For the structured + * printers, this is for improved readability without affecting + * the abstract syntax of the structure being printed. For JSON, + * this could simply be a line break. */ void (*separator) (struct sprinter *); diff --git a/test/text b/test/text index e003a66..b5ccefc 100755 --- a/test/text +++ b/test/text @@ -55,30 +55,34 @@ test_expect_equal $output \ add_email_corpus test_begin_subtest Search message tags: text0 -cat EOF EXPECTED.$test_count +cat EOF EXPECTED attachment inbox signed unread EOF -notmuch search --format=text0 --output=tags '*' | xargs -0 | notmuch_search_sanitize OUTPUT.$test_count -test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count +notmuch search --format=text0 --output=tags '*' | xargs -0 | notmuch_search_sanitize OUTPUT +test_expect_equal_file EXPECTED OUTPUT +# Use tr(1) to convert --output=text0 to --output=text for +# comparison. Also translate newlines to spaces to fail with more +# noise if they are present as delimiters instead of null +# characters. This assumes there are no newlines in the data. test_begin_subtest Compare text vs. text0 for threads -notmuch search --format=text --output=threads '*' | notmuch_search_sanitize EXPECTED.$test_count -notmuch search --format=text0 --output=threads '*' | xargs -0 -L1 | notmuch_search_sanitize OUTPUT.$test_count -test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count +notmuch search --format=text --output=threads '*' | notmuch_search_sanitize EXPECTED +notmuch search --format=text0 --output=threads '*' | tr \n\0 \n | notmuch_search_sanitize OUTPUT +test_expect_equal_file EXPECTED OUTPUT test_begin_subtest Compare text vs. text0 for messages -notmuch search --format=text --output=messages '*' | notmuch_search_sanitize EXPECTED.$test_count -notmuch search --format=text0 --output=messages '*' | xargs -0 -L1 | notmuch_search_sanitize OUTPUT.$test_count -test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count +notmuch search --format=text --output=messages '*' | notmuch_search_sanitize EXPECTED +notmuch search --format=text0 --output=messages '*' | tr \n\0 \n | notmuch_search_sanitize OUTPUT +test_expect_equal_file EXPECTED OUTPUT test_begin_subtest Compare text vs. text0 for files -notmuch search --format=text --output=files '*' | notmuch_search_sanitize EXPECTED.$test_count -notmuch search --format=text0 --output=files '*' | xargs -0 -L1 | notmuch_search_sanitize OUTPUT.$test_count -test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count +notmuch search --format=text --output=files '*' | notmuch_search_sanitize EXPECTED +notmuch search --format=text0 --output=files '*' | tr \n\0 \n | notmuch_search_sanitize OUTPUT +test_expect_equal_file EXPECTED OUTPUT test_begin_subtest Compare text vs. text0 for tags -notmuch search --format=text --output=tags '*' | notmuch_search_sanitize EXPECTED.$test_count -notmuch search --format=text0 --output=tags '*' | xargs -0 -L1 | notmuch_search_sanitize OUTPUT.$test_count -test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count +notmuch search --format=text --output=tags '*' | notmuch_search_sanitize EXPECTED +notmuch search --format=text0 --output=tags '*' | tr \n\0 \n | notmuch_search_sanitize OUTPUT +test_expect_equal_file
Re: [PATCH v2 1/7] emacs: Centralize notmuch command error handling
Austin Clements amdra...@mit.edu writes: This provides library functions for unified handling of errors from the notmuch CLI. Follow-up patches will convert some scattered error handling to use this and add error handling where we currently ignore errors. pushed, d ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [RFC PATCH] cli: add --remove-all option to notmuch tag
On Sun, 16 Dec 2012, Mark Walters markwalters1...@gmail.com wrote: On Mon, 03 Dec 2012, Jani Nikula j...@nikula.org wrote: Add --remove-all option to notmuch tag to remove all tags matching query before applying the tag changes. This allows removal and unconditional setting of the tags of a message: $ notmuch tag --remove-all id:f...@example.com $ notmuch tag --remove-all +foo +bar id:f...@example.com without having to resort to the complicated (and still quoting broken): $ notmuch tag $(notmuch search --output=tags '*' | sed 's/^/-/') id:f...@example.com $ notmuch tag $(notmuch search --output=tags '*' | sed 's/^/-/') +foo +bar id:f...@example.com --- It's completely untested... This is on top of David's batch tagging branch new-tagging at git://pivot.cs.unb.ca/notmuch.git If there's interest, I'll spin a new version with tests and man page changes after David's stuff has been merged. I like this: the possibility of setting the tags to something seems nice. Thanks for the feedback; I'll rebase once we're done with the batch tagging stuff. I am not very keen on the option name but don't have anything much better: maybe --remove-all-first or --set-to? I still like --remove-all, and, like you say, those aren't much better. But we can bikeshed this later. ;) Incidentally, does this (or should this) give an error if the user calls --remove-all with -some_tag (as opposed to +some_tag)? I think not. There are no errors if you do -foo -foo, or remove a tag that isn't in any of the messages matching query. BR, Jani. Best wishes Mark --- notmuch-tag.c | 32 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/notmuch-tag.c b/notmuch-tag.c index e4fca67..67624cc 100644 --- a/notmuch-tag.c +++ b/notmuch-tag.c @@ -119,12 +119,15 @@ tag_query (void *ctx, notmuch_database_t *notmuch, const char *query_string, notmuch_messages_t *messages; notmuch_message_t *message; -/* Optimize the query so it excludes messages that already have - * the specified set of tags. */ -query_string = _optimize_tag_query (ctx, query_string, tag_ops); -if (query_string == NULL) { -fprintf (stderr, Out of memory.\n); -return 1; +if (! (flags TAG_FLAG_REMOVE_ALL)) { +/* Optimize the query so it excludes messages that already + * have the specified set of tags. */ +query_string = _optimize_tag_query (ctx, query_string, tag_ops); +if (query_string == NULL) { +fprintf (stderr, Out of memory.\n); +return 1; +} +flags |= TAG_FLAG_PRE_OPTIMIZED; } query = notmuch_query_create (notmuch, query_string); @@ -140,7 +143,7 @@ tag_query (void *ctx, notmuch_database_t *notmuch, const char *query_string, notmuch_messages_valid (messages) ! interrupted; notmuch_messages_move_to_next (messages)) { message = notmuch_messages_get (messages); -tag_op_list_apply (message, tag_ops, flags | TAG_FLAG_PRE_OPTIMIZED); +tag_op_list_apply (message, tag_ops, flags); notmuch_message_destroy (message); } @@ -157,8 +160,9 @@ notmuch_tag_command (void *ctx, int argc, char *argv[]) notmuch_config_t *config; notmuch_database_t *notmuch; struct sigaction action; -tag_op_flag_t synchronize_flags = TAG_FLAG_NONE; +tag_op_flag_t flags = TAG_FLAG_NONE; notmuch_bool_t batch = FALSE; +notmuch_bool_t remove_all = FALSE; FILE *input = stdin; char *input_file_name = NULL; int i, opt_index; @@ -174,6 +178,7 @@ notmuch_tag_command (void *ctx, int argc, char *argv[]) notmuch_opt_desc_t options[] = { { NOTMUCH_OPT_BOOLEAN, batch, batch, 0, 0 }, { NOTMUCH_OPT_STRING, input_file_name, input, 'i', 0 }, +{ NOTMUCH_OPT_BOOLEAN, remove_all, remove-all, 0, 0 }, { 0, 0, 0, 0, 0 } }; @@ -230,7 +235,7 @@ notmuch_tag_command (void *ctx, int argc, char *argv[]) } } -if (tag_op_list_size (tag_ops) == 0) { +if (tag_op_list_size (tag_ops) == 0 !remove_all) { fprintf (stderr, Error: 'notmuch tag' requires at least one tag to add or remove.\n); return 1; } @@ -252,7 +257,10 @@ notmuch_tag_command (void *ctx, int argc, char *argv[]) return 1; if (notmuch_config_get_maildir_synchronize_flags (config)) -synchronize_flags = TAG_FLAG_MAILDIR_SYNC; +flags |= TAG_FLAG_MAILDIR_SYNC; + +if (remove_all) +flags |= TAG_FLAG_REMOVE_ALL; if (batch) { @@ -280,14 +288,14 @@ notmuch_tag_command (void *ctx, int argc, char *argv[]) continue; if (ret 0 || tag_query (ctx, notmuch, query_string, - tag_ops, synchronize_flags)) + tag_ops, flags)) break; } if (line) free (line); } else -ret = tag_query (ctx, notmuch,
[PATCH v4 0/5] add --format=text0 to notmuch search
Hi all, a quick rebase of id:cover.1355691124.git.j...@nikula.org to fix a tiny conflict in patch context of 3/5. BR, Jani. Jani Nikula (5): sprinter: clarify separator documentation sprinter: add text0 formatter for null character separated text cli: add --format=text0 to notmuch search test: notmuch search --format=text0 man: document notmuch search --format=text0 man/man1/notmuch-search.1 | 26 -- notmuch-search.c | 16 ++-- sprinter-text.c | 22 ++ sprinter.h| 15 +++ test/text | 33 + 5 files changed, 96 insertions(+), 16 deletions(-) -- 1.7.10.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v4 1/5] sprinter: clarify separator documentation
For text printers, the separator is a syntactic element. --- sprinter.h |9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/sprinter.h b/sprinter.h index 59776a9..f43a844 100644 --- a/sprinter.h +++ b/sprinter.h @@ -42,10 +42,11 @@ typedef struct sprinter { */ void (*map_key) (struct sprinter *, const char *); -/* Insert a separator (usually extra whitespace) for improved - * readability without affecting the abstract syntax of the - * structure being printed. - * For JSON, this could simply be a line break. +/* Insert a separator (usually extra whitespace). For the text + * printers, this is a syntactic separator. For the structured + * printers, this is for improved readability without affecting + * the abstract syntax of the structure being printed. For JSON, + * this could simply be a line break. */ void (*separator) (struct sprinter *); -- 1.7.10.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v4 2/5] sprinter: add text0 formatter for null character separated text
Same as the text formatter, but with each field separated by a null character rather than a newline character. --- sprinter-text.c | 22 ++ sprinter.h |6 ++ 2 files changed, 28 insertions(+) diff --git a/sprinter-text.c b/sprinter-text.c index 10343be..7779488 100644 --- a/sprinter-text.c +++ b/sprinter-text.c @@ -68,6 +68,14 @@ text_separator (struct sprinter *sp) } static void +text0_separator (struct sprinter *sp) +{ +struct sprinter_text *sptxt = (struct sprinter_text *) sp; + +fputc ('\0', sptxt-stream); +} + +static void text_set_prefix (struct sprinter *sp, const char *prefix) { struct sprinter_text *sptxt = (struct sprinter_text *) sp; @@ -133,3 +141,17 @@ sprinter_text_create (const void *ctx, FILE *stream) res-stream = stream; return res-vtable; } + +struct sprinter * +sprinter_text0_create (const void *ctx, FILE *stream) +{ +struct sprinter *sp; + +sp = sprinter_text_create (ctx, stream); +if (! sp) + return NULL; + +sp-separator = text0_separator; + +return sp; +} diff --git a/sprinter.h b/sprinter.h index f43a844..f859672 100644 --- a/sprinter.h +++ b/sprinter.h @@ -67,6 +67,12 @@ typedef struct sprinter { struct sprinter * sprinter_text_create (const void *ctx, FILE *stream); +/* Create a new unstructured printer that emits the text format for + * notmuch search, with each field separated by a null character + * instead of the newline character. */ +struct sprinter * +sprinter_text0_create (const void *ctx, FILE *stream); + /* Create a new structure printer that emits JSON. */ struct sprinter * sprinter_json_create (const void *ctx, FILE *stream); -- 1.7.10.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v4 3/5] cli: add --format=text0 to notmuch search
Add new format text0, which is otherwise the same as text, but use the null character as separator instead of the newline character. This is similar to find(1) -print0 option, and works together with the xargs(1) -0 option. --- notmuch-search.c | 16 ++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/notmuch-search.c b/notmuch-search.c index 7704915..0b0a879 100644 --- a/notmuch-search.c +++ b/notmuch-search.c @@ -305,8 +305,12 @@ notmuch_search_command (void *ctx, int argc, char *argv[]) int exclude = EXCLUDE_TRUE; unsigned int i; -enum { NOTMUCH_FORMAT_JSON, NOTMUCH_FORMAT_TEXT, NOTMUCH_FORMAT_SEXP } - format_sel = NOTMUCH_FORMAT_TEXT; +enum { + NOTMUCH_FORMAT_JSON, + NOTMUCH_FORMAT_TEXT, + NOTMUCH_FORMAT_TEXT0, + NOTMUCH_FORMAT_SEXP +} format_sel = NOTMUCH_FORMAT_TEXT; notmuch_opt_desc_t options[] = { { NOTMUCH_OPT_KEYWORD, sort, sort, 's', @@ -317,6 +321,7 @@ notmuch_search_command (void *ctx, int argc, char *argv[]) (notmuch_keyword_t []){ { json, NOTMUCH_FORMAT_JSON }, { sexp, NOTMUCH_FORMAT_SEXP }, { text, NOTMUCH_FORMAT_TEXT }, + { text0, NOTMUCH_FORMAT_TEXT0 }, { 0, 0 } } }, { NOTMUCH_OPT_INT, notmuch_format_version, format-version, 0, 0 }, { NOTMUCH_OPT_KEYWORD, output, output, 'o', @@ -346,6 +351,13 @@ notmuch_search_command (void *ctx, int argc, char *argv[]) case NOTMUCH_FORMAT_TEXT: format = sprinter_text_create (ctx, stdout); break; +case NOTMUCH_FORMAT_TEXT0: + if (output == OUTPUT_SUMMARY) { + fprintf (stderr, Error: --format=text0 is not compatible with --output=summary.\n); + return 1; + } + format = sprinter_text0_create (ctx, stdout); + break; case NOTMUCH_FORMAT_JSON: format = sprinter_json_create (ctx, stdout); break; -- 1.7.10.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v4 4/5] test: notmuch search --format=text0
--- test/text | 33 + 1 file changed, 33 insertions(+) diff --git a/test/text b/test/text index 428c89b..b5ccefc 100755 --- a/test/text +++ b/test/text @@ -52,4 +52,37 @@ output=$(notmuch search --format=text tëxt-search-méssage | notmuch_search_s test_expect_equal $output \ thread:XXX 2000-01-01 [1/1] Notmuch Test Suite; text-search-utf8-body-sübjéct (inbox unread) +add_email_corpus + +test_begin_subtest Search message tags: text0 +cat EOF EXPECTED +attachment inbox signed unread +EOF +notmuch search --format=text0 --output=tags '*' | xargs -0 | notmuch_search_sanitize OUTPUT +test_expect_equal_file EXPECTED OUTPUT + +# Use tr(1) to convert --output=text0 to --output=text for +# comparison. Also translate newlines to spaces to fail with more +# noise if they are present as delimiters instead of null +# characters. This assumes there are no newlines in the data. +test_begin_subtest Compare text vs. text0 for threads +notmuch search --format=text --output=threads '*' | notmuch_search_sanitize EXPECTED +notmuch search --format=text0 --output=threads '*' | tr \n\0 \n | notmuch_search_sanitize OUTPUT +test_expect_equal_file EXPECTED OUTPUT + +test_begin_subtest Compare text vs. text0 for messages +notmuch search --format=text --output=messages '*' | notmuch_search_sanitize EXPECTED +notmuch search --format=text0 --output=messages '*' | tr \n\0 \n | notmuch_search_sanitize OUTPUT +test_expect_equal_file EXPECTED OUTPUT + +test_begin_subtest Compare text vs. text0 for files +notmuch search --format=text --output=files '*' | notmuch_search_sanitize EXPECTED +notmuch search --format=text0 --output=files '*' | tr \n\0 \n | notmuch_search_sanitize OUTPUT +test_expect_equal_file EXPECTED OUTPUT + +test_begin_subtest Compare text vs. text0 for tags +notmuch search --format=text --output=tags '*' | notmuch_search_sanitize EXPECTED +notmuch search --format=text0 --output=tags '*' | tr \n\0 \n | notmuch_search_sanitize OUTPUT +test_expect_equal_file EXPECTED OUTPUT + test_done -- 1.7.10.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH v4 5/5] man: document notmuch search --format=text0
--- man/man1/notmuch-search.1 | 26 -- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/man/man1/notmuch-search.1 b/man/man1/notmuch-search.1 index 5c771fa..12f6719 100644 --- a/man/man1/notmuch-search.1 +++ b/man/man1/notmuch-search.1 @@ -25,9 +25,11 @@ Supported options for include .RS 4 .TP 4 -.BR \-\-format= ( json | sexp | text ) +.BR \-\-format= ( json | sexp | text | text0 ) -Presents the results in either JSON, S-Expressions or plain-text (default). +Presents the results in either JSON, S-Expressions, newline character +separated plain-text (default), or null character separated plain-text +(compatible with \fBxargs\fR(1) -0 option where available). .RE .RS 4 @@ -57,32 +59,36 @@ the authors of the thread and the subject. .B threads Output the thread IDs of all threads with any message matching the -search terms, either one per line (\-\-format=text) or as a JSON array -(\-\-format=json) or an S-Expression list (\-\-format=sexp). +search terms, either one per line (\-\-format=text), separated by null +characters (\-\-format=text0), as a JSON array (\-\-format=json), or +an S-Expression list (\-\-format=sexp). .RE .RS 4 .TP 4 .B messages Output the message IDs of all messages matching the search terms, -either one per line (\-\-format=text) or as a JSON array -(\-\-format=json) or as an S-Expression list (\-\-format=sexp). +either one per line (\-\-format=text), separated by null characters +(\-\-format=text0), as a JSON array (\-\-format=json), or as an +S-Expression list (\-\-format=sexp). .RE .RS 4 .TP 4 .B files Output the filenames of all messages matching the search terms, either -one per line (\-\-format=text) or as a JSON array (\-\-format=json) or -as an S-Expression list (\-\-format=sexp). +one per line (\-\-format=text), separated by null characters +(\-\-format=text0), as a JSON array (\-\-format=json), or as an +S-Expression list (\-\-format=sexp). .RE .RS 4 .TP 4 .B tags Output all tags that appear on any message matching the search terms, -either one per line (\-\-format=text) or as a JSON array (\-\-format=json) -or as an S-Expression list (\-\-format=sexp). +either one per line (\-\-format=text), separated by null characters +(\-\-format=text0), as a JSON array (\-\-format=json), or as an +S-Expression list (\-\-format=sexp). .RE .RE -- 1.7.10.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 4/4] perf-test: add memory leak test for dump restore
da...@tethera.net writes: + +memory_run 'load nmbug tags' 'notmuch restore --accumulate --input=corpus.tags/nmbug.sup-dump' +memory_run 'dump *' 'notmuch dump --output=tags.sup' +memory_run 'restore *' 'notmuch restore --input=tags.sup' +memory_run 'dump --format=batch-tag *' 'notmuch dump --format=batch-tag --output=tags.bt' +memory_run 'restore --format=batch-tag *' 'notmuch restore --format=batch-tag --input=tags.bt' + We were talking on IRC about how/if valgrind would cope with talloc, and the possibility that chunks of memory are still reachable by talloc, but not by user code. Currently the talloc context local in main() is (slightly perversely) only freed in the case of return 1, so all the memory allocated by talloc on that contex is shown as leaked: 3,005,500 (93 direct, 3,005,407 indirect) bytes in 1 blocks are definitely lost in loss record 553 of 553 at 0x4C2A26B: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) by 0x55F14C7: talloc_strndup (in /usr/lib/x86_64-linux-gnu/libtalloc.so.2.0.7) by 0x4115E8: parse_sup_line (notmuch-restore.c:90) by 0x411AD4: notmuch_restore_command (notmuch-restore.c:209) by 0x40B2A4: main (notmuch.c:294) Although this is probably a bug in main(), it does point valgrind to the right culprit. As our memory allocation is (alas) a mix of talloc, malloc, and g_malloc, we probably need both valgrind tests, and some way to toggle talloc memory debugging. ( http://talloc.samba.org/talloc/doc/html/group__talloc__debug.html ) d ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: gmail importer script
Quoting Jason A. Donenfeld (2012-12-16 20:44:04) On Sat, Dec 15, 2012 at 11:41 AM, Patrick Totzke patricktot...@gmail.com wrote: Well, thats not the point.. the script shouldn't die like this. I think it's be better if the script caught that exception, deleted the file and continued.. Probably, but I suspect it's related to whatever mystery error you ran into before with the corruption. Does deleting that file and trying again fix it? I don't know. How would I know which file to remove? I just removed the newest files in new/cur/tmp but this doesnt solve the issue. What would really help here is some more informative error message! I tried with the `-d` option but did not see any local path in the output. Anyway, this is extremely stable for me and a few others at this point. I'm going to wait for other users to report errors. Alternatively, send me patches if you want things to happen. Personally, I don't really care if 'things are happening', as I'm kind of OK with my current offlineimap configuration and would seriously consider switching from OI only if it meant an improvement in speed or stability. I test this code because I believe that the notmuch ecosystem would benefit from a working solution and that reviews and bug-reports are important. Of course, I don't expect any miracles in terms of speed at this early stage. But the bug I reported above should be addressed at some point in order not to scare off potential users. Have you considered including your code in offlineimap? I'm asking because OI already solved some of the problems you might face later on if you want to continue working on this. * multi-threaded downloads * the ability to read passwords not as plaintext parameter.. best, /p signature.asc Description: signature ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH 3/3] notmuch-restore: allocate a temporary talloc context for each line parsed.
On Sun, 16 Dec 2012, da...@tethera.net wrote: From: David Bremner brem...@debian.org This lets the high level code in notmuch restore be ignorant about what the lower level code is doing as far as allocating memory. --- notmuch-restore.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/notmuch-restore.c b/notmuch-restore.c index 4b76d83..52e7ecb 100644 --- a/notmuch-restore.c +++ b/notmuch-restore.c @@ -122,6 +122,7 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) char *input_file_name = NULL; FILE *input = stdin; char *line = NULL; +void *line_ctx = NULL; size_t line_size; ssize_t line_len; @@ -205,10 +206,11 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) do { char *query_string; This patch only works on top of the batch tagging series, because there's still one continue statement in the id: prefix check. But you could make it work for both, *and* keep the slightly more intuitive ret checking order if you did (yes, the slightly counter-intuitive): if (line_ctxt != NULL) talloc_free (line_ctx); right here, and... + line_ctx = talloc_new (ctx); if (input_format == DUMP_FORMAT_SUP) { - ret = parse_sup_line (ctx, line, query_string, tag_ops); + ret = parse_sup_line (line_ctx, line, query_string, tag_ops); } else { - ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS, + ret = parse_tag_line (line_ctx, line, TAG_FLAG_BE_GENEROUS, query_string, tag_ops); if (ret == 0) { @@ -238,8 +240,14 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) break; } + talloc_free (line_ctx); + /* setting to NULL is important to avoid a double free */ + line_ctx = NULL; ...removed the above lines here. Otherwise I think you'll need a temporary goto until the batch tagging series. (I'm fine with that too.) Overall the series LGTM, and I like how this dodges the query_string alloc/free problem altogether. BR, Jani. } while ((line_len = getline (line, line_size, input)) != -1); +if (line_ctx != NULL) + talloc_free (line_ctx); + if (input_format == DUMP_FORMAT_SUP) regfree (regex); -- 1.7.10.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: handling mail sent to a subscribed list
If understand correctly, your concern is with the second copy of a message with the same message-id not showing up in your inbox? If so, this is more or less a feature (although in the case where the duplicated message id's are because of malice or stupidity on the sender's part, and not duplicated messages, it is also a known bug). On the command line you can try notmuch search --output=files id:foo where id:foo is copied via c i in the emacs interface. Or maybe I misunderstand your problem completely. Thanks for the reply. Yes I think you summed it up; I figured that it was behaving properly. In short it's that I have the email that I sent sitting wherever it was Fcc'ed and then shortly thereafter I receive from the email list a message with the same ID. Since the Fcc'ed one was already added to the database, the one that I received from the list just sits there. But I guess others using mailing lists have encountered this too, so I was asking how they handle it since I haven't figured it out yet. I'll give that command a try to see if I can put it to good use somehow. Regards, -brandon ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v4 0/5] add --format=text0 to notmuch search
On Mon, Dec 17 2012, Jani Nikula j...@nikula.org wrote: Hi all, a quick rebase of id:cover.1355691124.git.j...@nikula.org to fix a tiny conflict in patch context of 3/5. BR, Jani. As Mark Austin (also) gave +1 to v3 added notmuch::patch (only). Tomi Jani Nikula (5): sprinter: clarify separator documentation sprinter: add text0 formatter for null character separated text cli: add --format=text0 to notmuch search test: notmuch search --format=text0 man: document notmuch search --format=text0 man/man1/notmuch-search.1 | 26 -- notmuch-search.c | 16 ++-- sprinter-text.c | 22 ++ sprinter.h| 15 +++ test/text | 33 + 5 files changed, 96 insertions(+), 16 deletions(-) ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH v4 0/5] add --format=text0 to notmuch search
On Mon, Dec 17 2012, Tomi Ollila tomi.oll...@iki.fi wrote: On Mon, Dec 17 2012, Jani Nikula j...@nikula.org wrote: Hi all, a quick rebase of id:cover.1355691124.git.j...@nikula.org to fix a tiny conflict in patch context of 3/5. BR, Jani. As Mark Austin (also) gave +1 to v3 added notmuch::patch (only). Ok, it was like that already, my nmbug tree was out-of-sync (had to fix it with nmbug checkout; nmbug pull)... Tomi Tomi Jani Nikula (5): sprinter: clarify separator documentation sprinter: add text0 formatter for null character separated text cli: add --format=text0 to notmuch search test: notmuch search --format=text0 man: document notmuch search --format=text0 man/man1/notmuch-search.1 | 26 -- notmuch-search.c | 16 ++-- sprinter-text.c | 22 ++ sprinter.h| 15 +++ test/text | 33 + 5 files changed, 96 insertions(+), 16 deletions(-) ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 1/3] CLI: add talloc leak report, controlled by an environment variable.
From: David Bremner brem...@debian.org The argument handling in notmuch.c seems due for an overhaul, but until then use an environment variable to specify a location to write the talloc leak report to. This is only enabled for the (interesting) case where some notmuch subcommand is invoked. --- notmuch.c | 16 ++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/notmuch.c b/notmuch.c index 9516dfb..fb49c5a 100644 --- a/notmuch.c +++ b/notmuch.c @@ -322,8 +322,20 @@ main (int argc, char *argv[]) for (i = 0; i ARRAY_SIZE (commands); i++) { command = commands[i]; - if (strcmp (argv[1], command-name) == 0) - return (command-function) (local, argc - 1, argv[1]); + if (strcmp (argv[1], command-name) == 0) { + int ret; + char *talloc_report; + + ret = (command-function) (local, argc - 1, argv[1]); + + talloc_report=getenv (NOTMUCH_TALLOC_REPORT); + if (talloc_report strcmp(talloc_report, ) != 0) { + FILE *report = fopen (talloc_report, w); + talloc_report_full (NULL, report); + } + + return ret; + } } fprintf (stderr, Error: Unknown command '%s' (see \notmuch help\)\n, -- 1.7.10.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 3/3] notmuch-restore: use xtalloc version of strndup
From: David Bremner brem...@debian.org This gives line numbers for better debugging. --- notmuch-restore.c |7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/notmuch-restore.c b/notmuch-restore.c index 40596a8..0cc9c9f 100644 --- a/notmuch-restore.c +++ b/notmuch-restore.c @@ -85,9 +85,10 @@ parse_sup_line (void *ctx, char *line, return 1; } -*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, +*query_str = xtalloc_strndup (ctx, line + match[1].rm_so, + match[1].rm_eo - match[1].rm_so); + +file_tags = xtalloc_strndup (ctx, line + match[2].rm_so, match[2].rm_eo - match[2].rm_so); char *tok = file_tags; -- 1.7.10.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 2/3] util: add xtalloc.[ch]
From: David Bremner brem...@debian.org These are intended to be simple wrappers to provide slightly better debugging information than what talloc currently provides natively. --- notmuch-client.h|2 +- util/Makefile.local |2 +- util/xtalloc.c | 15 +++ util/xtalloc.h | 18 ++ 4 files changed, 35 insertions(+), 2 deletions(-) create mode 100644 util/xtalloc.c create mode 100644 util/xtalloc.h diff --git a/notmuch-client.h b/notmuch-client.h index d7b352e..60be030 100644 --- a/notmuch-client.h +++ b/notmuch-client.h @@ -58,7 +58,7 @@ typedef GMimeCipherContext notmuch_crypto_context_t; #include errno.h #include signal.h -#include talloc.h +#include xtalloc.h #define unused(x) x __attribute__ ((unused)) diff --git a/util/Makefile.local b/util/Makefile.local index a11e35b..8a62c00 100644 --- a/util/Makefile.local +++ b/util/Makefile.local @@ -4,7 +4,7 @@ dir := util extra_cflags += -I$(srcdir)/$(dir) libutil_c_srcs := $(dir)/xutil.c $(dir)/error_util.c $(dir)/hex-escape.c \ - $(dir)/string-util.c + $(dir)/string-util.c $(dir)/xtalloc.c libutil_modules := $(libutil_c_srcs:.c=.o) diff --git a/util/xtalloc.c b/util/xtalloc.c new file mode 100644 index 000..22834bd --- /dev/null +++ b/util/xtalloc.c @@ -0,0 +1,15 @@ +#include string.h +#include xtalloc.h + +char * +xtalloc_strndup_named_const (void *ctx, const char *str, +size_t len, const char *name) +{ +char *ptr = talloc_named_const (ctx, len + 1, name); + +if (ptr) { + memcpy (ptr, str, len); + *(ptr + len) = '\0'; +} +return ptr; +} diff --git a/util/xtalloc.h b/util/xtalloc.h new file mode 100644 index 000..3cc1179 --- /dev/null +++ b/util/xtalloc.h @@ -0,0 +1,18 @@ +#ifndef _XTALLOC_H +#define _XTALLOC_H + +#include talloc.h + +/* Like talloc_strndup, but take an extra parameter for the internal talloc + * name (for debugging) */ + +char * +xtalloc_strndup_named_const (void *ctx, const char *str, +size_t len, const char *name); + +/* use the __location__ macro from talloc.h to name a string according to its + * source location */ + +#define xtalloc_strndup(ctx, str, len) xtalloc_strndup_named_const (ctx, str, len, __location__) + +#endif -- 1.7.10.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
v2 restore leak fix
This obsoletes id:1355688997-19164-1-git-send-email-da...@tethera.net Actually the first patch in the series is now an unrelated bug fix, since it isn't needed anymore for 2 and 3. ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 1/3] notmuch-restore: fix return value propagation
From: David Bremner brem...@debian.org Previously notmuch_restore_command returned 0 if tag_message returned a non-zero (failure) value. This is wrong, since non-zero status indicates something mysterious went wrong with retrieving the message, or applying it. There was also a failure to check or propagate the return value from tag_op_list_apply in tag_message. --- notmuch-restore.c | 13 ++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/notmuch-restore.c b/notmuch-restore.c index 40596a8..5a02328 100644 --- a/notmuch-restore.c +++ b/notmuch-restore.c @@ -25,6 +25,9 @@ static regex_t regex; +/* Non-zero return indicates an error in retrieving the message, + * or in applying the tags. + */ static int tag_message (unused (void *ctx), notmuch_database_t *notmuch, @@ -48,7 +51,7 @@ 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)) - tag_op_list_apply (message, tag_ops, flags); + ret = tag_op_list_apply (message, tag_ops, flags); notmuch_message_destroy (message); @@ -231,8 +234,12 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) if (ret 0) continue; - if (ret 0 || tag_message (ctx, notmuch, query_string, - tag_ops, flags)) + if (ret 0) + break; + + ret = tag_message (line_ctx, notmuch, query_string, + tag_ops, flags); + if (ret) break; } while ((line_len = getline (line, line_size, input)) != -1); -- 1.7.10.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 3/3] notmuch-restore: allocate a temporary talloc context for each line parsed.
From: David Bremner brem...@debian.org This lets the high level code in notmuch restore be ignorant about what the lower level code is doing as far as allocating memory. --- notmuch-restore.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/notmuch-restore.c b/notmuch-restore.c index 665373f..9ed9b51 100644 --- a/notmuch-restore.c +++ b/notmuch-restore.c @@ -125,6 +125,7 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) char *input_file_name = NULL; FILE *input = stdin; char *line = NULL; +void *line_ctx = NULL; size_t line_size; ssize_t line_len; @@ -208,10 +209,14 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) do { char *query_string; + if (line_ctx != NULL) + talloc_free (line_ctx); + + line_ctx = talloc_new (ctx); if (input_format == DUMP_FORMAT_SUP) { - ret = parse_sup_line (ctx, line, query_string, tag_ops); + ret = parse_sup_line (line_ctx, line, query_string, tag_ops); } else { - ret = parse_tag_line (ctx, line, TAG_FLAG_BE_GENEROUS, + ret = parse_tag_line (line_ctx, line, TAG_FLAG_BE_GENEROUS, query_string, tag_ops); if (ret == 0) { @@ -244,6 +249,9 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[]) } while ((line_len = getline (line, line_size, input)) != -1); +if (line_ctx != NULL) + talloc_free (line_ctx); + if (input_format == DUMP_FORMAT_SUP) regfree (regex); -- 1.7.10.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
[PATCH 2/3] tag-utils: use the tag_opt_list_t as talloc context, if possible.
From: David Bremner brem...@debian.org This code is no less correct than the previous version, since it does not make sense for the array to live longer than the wrapping struct. By not relying on the context passed into tag_parse_line, we can allow tag_op_list_t structures to live longer than that context. --- notmuch-restore.c |2 +- tag-util.c|9 - tag-util.h|3 +-- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/notmuch-restore.c b/notmuch-restore.c index 5a02328..665373f 100644 --- a/notmuch-restore.c +++ b/notmuch-restore.c @@ -105,7 +105,7 @@ parse_sup_line (void *ctx, char *line, tok_len++; } - if (tag_op_list_append (ctx, tag_ops, tok, FALSE)) + if (tag_op_list_append (tag_ops, tok, FALSE)) return -1; } diff --git a/tag-util.c b/tag-util.c index eab482f..705b7ba 100644 --- a/tag-util.c +++ b/tag-util.c @@ -109,7 +109,7 @@ parse_tag_line (void *ctx, char *line, goto DONE; } - if (tag_op_list_append (ctx, tag_ops, tag, remove)) { + if (tag_op_list_append (tag_ops, tag, remove)) { ret = line_error (TAG_PARSE_OUT_OF_MEMORY, line_for_error, aborting); goto DONE; @@ -294,7 +294,7 @@ tag_op_list_create (void *ctx) list-size = TAG_OP_LIST_INITIAL_SIZE; list-count = 0; -list-ops = talloc_array (ctx, tag_operation_t, list-size); +list-ops = talloc_array (list, tag_operation_t, list-size); if (list-ops == NULL) return NULL; @@ -303,8 +303,7 @@ tag_op_list_create (void *ctx) int -tag_op_list_append (void *ctx, - tag_op_list_t *list, +tag_op_list_append (tag_op_list_t *list, const char *tag, notmuch_bool_t remove) { @@ -314,7 +313,7 @@ tag_op_list_append (void *ctx, if (list-count == list-size) { list-size *= 2; - list-ops = talloc_realloc (ctx, list-ops, tag_operation_t, + list-ops = talloc_realloc (list, list-ops, tag_operation_t, list-size); if (list-ops == NULL) { fprintf (stderr, Out of memory.\n); diff --git a/tag-util.h b/tag-util.h index 99b0fa0..c07bfde 100644 --- a/tag-util.h +++ b/tag-util.h @@ -87,8 +87,7 @@ tag_op_list_create (void *ctx); */ int -tag_op_list_append (void *ctx, - tag_op_list_t *list, +tag_op_list_append (tag_op_list_t *list, const char *tag, notmuch_bool_t remove); -- 1.7.10.4 ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch