v4 of Performance tests

[Tomi Ollila]

On Sun, Nov 25 2012, david at tethera.net wrote:

> This version adds an optional makefile target to download the corpus.
>
> Tomi reviewed it on IRC and didn't complain.

+1

Tomi

[Patch v2 11/17] test: add test for notmuch tag --batch option

[Mark Walters]

Hi

On Sat, 24 Nov 2012, david at tethera.net wrote:
> From: Jani Nikula 
>
> Basic test of functionality, along with all combinations of options.
> ---
>  test/tagging |   46 ++
>  1 file changed, 46 insertions(+)
>
> diff --git a/test/tagging b/test/tagging
> index 980ff92..e5b8315 100755
> --- a/test/tagging
> +++ b/test/tagging
> @@ -46,6 +46,52 @@ test_expect_equal "$output" "\
>  thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; One (:\"  inbox tag1 
> unread)
>  thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Two (inbox tag1 tag4 
> unread)"
>  
> +test_begin_subtest "--batch"
> +notmuch tag --batch < +# %20 is a space in tag
> +-:"%20 -tag1 +tag5 +tag6 -- One
> ++tag1 -tag1 -tag4 +tag4 -- Two
> +-tag6 One
> ++tag5 Two
> +EOF
> +output=$(notmuch search \* | notmuch_search_sanitize)
> +test_expect_equal "$output" "\
> +thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; One (inbox tag5 unread)
> +thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Two (inbox tag4 tag5 
> unread)"
> +
> +> batch.in  < +# %20 is a space in tag
> +-:"%20 -tag1 +tag5 +tag6 -- One
> ++tag1 -tag1 -tag4 +tag4 -- Two
> +-tag6 One
> ++tag5 Two
> +EOF
> +
> +test_begin_subtest "--input"
> +notmuch tag --input=batch.in
> +output=$(notmuch search \* | notmuch_search_sanitize)
> +test_expect_equal "$output" "\
> +thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; One (inbox tag5 unread)
> +thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Two (inbox tag4 tag5 
> unread)"

Wouldn't a different set of tag changes be a better test as presumably
this test can pass if the command just does nothing? 

Mark

> +
> +test_begin_subtest "--batch --input"
> +notmuch tag --batch --input=batch.in
> +output=$(notmuch search \* | notmuch_search_sanitize)
> +test_expect_equal "$output" "\
> +thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; One (inbox tag5 unread)
> +thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Two (inbox tag4 tag5 
> unread)"
> +
> +test_begin_subtest "--batch, blank lines and comments"
> +notmuch dump | sort > EXPECTED.$test_count
> +notmuch tag --batch < +# this line is a comment; the next has only white space
> +  
> +
> +# the previous line is empty
> +EOF
> +notmuch dump | sort > OUTPUT.$test_count
> +test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
> +
>  test_expect_code 1 "Empty tag names" 'notmuch tag + One'
>  
>  test_expect_code 1 "Tag name beginning with -" 'notmuch tag +- One'
> -- 
> 1.7.10.4
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

[Patch v2 12/17] man: document notmuch tag --batch, --input options

[Mark Walters]

On Sat, 24 Nov 2012, david at tethera.net wrote:
> From: Jani Nikula 
>
> ---
>  man/man1/notmuch-tag.1 |   52 
> +++-
>  1 file changed, 51 insertions(+), 1 deletion(-)
>
> diff --git a/man/man1/notmuch-tag.1 b/man/man1/notmuch-tag.1
> index 0f86582..751db7b 100644
> --- a/man/man1/notmuch-tag.1
> +++ b/man/man1/notmuch-tag.1
> @@ -4,7 +4,12 @@ notmuch-tag \- add/remove tags for all messages matching the 
> search terms
>  
>  .SH SYNOPSIS
>  .B notmuch tag
> -.RI  "+<" tag> "|\-<" tag "> [...] [\-\-] <" search-term ">..."
> +.RI "+<" tag ">|\-<" tag "> [...] [\-\-] <" search-terms ">"
> +
> +.B notmuch tag
> +.RI "--batch"
> +.RI "[ --input=<" filename "> ]"
> +
>  
>  .SH DESCRIPTION
>  
> @@ -29,6 +34,51 @@ updates the maildir flags according to tag changes if the
>  configuration option is enabled. See \fBnotmuch-config\fR(1) for
>  details.
>  
> +Supported options for
> +.B tag
> +include
> +.RS 4
> +.TP 4
> +.BR \-\-batch
> +
> +Read batch tagging operations from standard input. This is more
> +efficient than repeated
> +.B notmuch tag
> +invocations. See
> +.B TAG FILE FORMAT
> +below for the input format. This option is not compatible with
> +specifying tagging on the command line.
> +.RE
> +
> +.RS 4
> +.TP 4
> +.BR "\-\-input=" 
> +
> +Read input from given file, instead of from stdin. Implies
> +.BR --batch .
> +
> +.SH TAG FILE FORMAT
> +
> +The input must consist of lines of the format:
> +
> +.RI "T +<" tag ">|\-<" tag "> [...] [\-\-] <" search-terms ">"

Is the T at the start of the line a remnant from a previous version?

Mark


> +
> +Each line is interpreted similarly to
> +.B notmuch tag
> +command line arguments. The delimiter is one or more spaces ' '. Any
> +characters in  and 
> +.B may
> +be hex encoded with %NN where NN is the hexadecimal value of the
> +character. Any ' ' and '%' characters in  and 
> +.B must
> +be hex encoded (using %20 and %25, respectively). Any characters that
> +are not part of  or 
> +.B must not
> +be hex encoded.
> +
> +Leading and trailing space ' ' is ignored. Empty lines and lines
> +beginning with '#' are ignored.
> +
>  .SH SEE ALSO
>  
>  \fBnotmuch\fR(1), \fBnotmuch-config\fR(1), \fBnotmuch-count\fR(1),
> -- 
> 1.7.10.4
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

[Patch v4 2/2] test: initial performance testing infrastructure

[Austin Clements]

Quoth David Bremner on Nov 25 at  8:05 pm:
> Austin Clements  writes:
> >> +add_email_corpus takes arguments "--small" and "--medium" for when you
> >> +want smaller corpuses to check.
> >
> > "corpora"?
> 
> reworded to say 
> 
> ,
> | add_email_corpus takes arguments "--small" and "--medium" for when you
> | want smaller subsets of the corpus to check.
> `

That's clearer.

> >
> > I'm a bit confused by this.  What happens if you don't specify --small
> > or --medium?  Is the "large"/default corpus just the combined small
> > and medium corpora?  Would be worth a comment, at least.
> 
> Hopefully the README makes this clear(er) now?

The README definitely helps.  Might still be worth a comment in the
code since it took me some thinking to realize it would do something
reasonable when given no argument.  Perhaps above the initial
assignment of arg,

# With no argument, use the entire (combined) corpus

to acknowledge that this is a legitimate and intentional code path?

> > This probably doesn't matter now, but I wonder if we want to unpack on
> > first use to somewhere not test-specific and then cp -rl the corpus
> > into the test directory.  I haven't tried unpacking the corpus yet,
> > but if you're running tests repeatedly to compare results, or running
> > more than one performance test, it seems like a full decompress and
> > unpack could get onerous.
> 
> Hmm. On my machine it is 10s for the copy versus 45s for a full
> unpack. For some reason I tested with "cp -a" which is incredibly slow, 
> so I thought there was no loss. For comparison the basic test takes
> about 10 minutes on the same machine.
> 
> In any case this can wait until we have a second test file and a second
> call to add_mail_corpus, adding caching now would not help.

It would help (a little) if you run basic multiple times.  I think
it's completely reasonable to leave it as is for now and see if
caching would help down the road.

[Patch v2 09/17] tag-util.[ch]: New files for common tagging routines

[Mark Walters]

This looks good. A couple of typos and a small queries (and I
agree with Tomi but I think you have already included that).

On Sat, 24 Nov 2012, david at tethera.net wrote:
> From: David Bremner 
>
> These are meant to be shared between notmuch-tag and notmuch-restore.
>
> The bulk of the routines implement a "tag operation list" abstract
> data type act as a structured representation of a set of tag
> operations (typically coming from a single tag command or line of
> input).
> ---
>  Makefile.local |1 +
>  tag-util.c |  264 
> 
>  tag-util.h |  120 ++
>  3 files changed, 385 insertions(+)
>  create mode 100644 tag-util.c
>  create mode 100644 tag-util.h
>
> diff --git a/Makefile.local b/Makefile.local
> index 2b91946..854867d 100644
> --- a/Makefile.local
> +++ b/Makefile.local
> @@ -274,6 +274,7 @@ notmuch_client_srcs = \
>   query-string.c  \
>   mime-node.c \
>   crypto.c\
> + tag-util.c
>  
>  notmuch_client_modules = $(notmuch_client_srcs:.c=.o)
>  
> diff --git a/tag-util.c b/tag-util.c
> new file mode 100644
> index 000..287cc67
> --- /dev/null
> +++ b/tag-util.c
> @@ -0,0 +1,264 @@
> +#include "string-util.h"
> +#include "tag-util.h"
> +#include "hex-escape.h"
> +
> +struct _tag_operation_t {
> +const char *tag;
> +notmuch_bool_t remove;
> +};
> +
> +struct _tag_op_list_t {
> +tag_operation_t *ops;
> +int count;
> +int size;
> +};
> +
> +int
> +parse_tag_line (void *ctx, char *line,
> + tag_op_flag_t flags,
> + char **query_string,
> + tag_op_list_t *tag_ops)
> +{
> +char *tok = line;
> +size_t tok_len = 0;
> +
> +chomp_newline (line);
> +
> +/* remove leading space */
> +while (*tok == ' ' || *tok == '\t')
> + tok++;
> +
> +/* Skip empty and comment lines. */
> +if (*tok == '\0' || *tok == '#')
> + return 1;
> +
> +tag_op_list_reset (tag_ops);
> +
> +/* Parse tags. */
> +while ((tok = strtok_len (tok + tok_len, " ", _len)) != NULL) {
> + notmuch_bool_t remove;
> + char *tag;
> +
> + /* Optional explicit end of tags marker. */
> + if (strncmp (tok, "--", tok_len) == 0) {
> + tok = strtok_len (tok + tok_len, " ", _len);
> + break;
> + }
> +
> + /* Implicit end of tags. */
> + if (*tok != '-' && *tok != '+')
> + break;
> +
> + /* If tag is terminated by NUL, there's no query string. */
> + if (*(tok + tok_len) == '\0') {
> + tok = NULL;
> + break;
> + }
> +
> + /* Terminate, and start next token after terminator. */
> + *(tok + tok_len++) = '\0';
> +
> + remove = (*tok == '-');
> + tag = tok + 1;
> +
> + /* Maybe refuse empty tags. */
> + if (!(flags & TAG_FLAG_BE_GENEROUS) && *tag == '\0') {
> + tok = NULL;
> + break;
> + }
> +
> + /* Decode tag. */
> + if (hex_decode_inplace (tag) != HEX_SUCCESS) {
> + tok = NULL;
> + break;
> + }
> +
> + if (tag_op_list_append (ctx, tag_ops, tag, remove))
> + return -1;
> +}
> +
> +if (tok == NULL || tag_ops->count == 0) {
> + /* FIXME: line has been modified! */
> + fprintf (stderr, "Warning: Ignoring invalid input line: %s\n",
> +  line);
> + return 1;
> +}
> +
> +/* tok now points to the query string */
> +if (hex_decode_inplace (tok) != HEX_SUCCESS) {
> + /* FIXME: line has been modified! */
> + fprintf (stderr, "Warning: Ignoring invalid input line: %s\n",
> +  line);
> + return 1;
> +}
> +
> +*query_string = tok;
> +
> +return 0;
> +}
> +
> +static inline void
> +message_error (notmuch_message_t *message,
> +notmuch_status_t status,
> +const char *format, ...)
> +{
> +va_list va_args;
> +
> +va_start (va_args, format);
> +
> +vfprintf (stderr, format, va_args);
> +fprintf (stderr, "Message-ID: %s\n", notmuch_message_get_message_id 
> (message));
> +fprintf (stderr, "Status: %s\n", notmuch_status_to_string (status));
> +}
> +
> +notmuch_status_t
> +tag_op_list_apply (notmuch_message_t *message,
> +tag_op_list_t *list,
> +tag_op_flag_t flags)
> +{
> +int i;
> +
> +notmuch_status_t status = 0;
> +tag_operation_t *tag_ops = list->ops;
> +
> +status = notmuch_message_freeze (message);
> +if (status) {
> + message_error (message, status, "freezing message");
> + return status;
> +}
> +
> +if (flags & TAG_FLAG_REMOVE_ALL) {
> + status = notmuch_message_remove_all_tags (message);
> + if (status) {
> + message_error (message, status, "removing all tags" );
> + return status;
> + }
> +}
> +
> +for (i = 0; i < list->count; i++) {
> + if (tag_ops[i].remove) {
> + status = notmuch_message_remove_tag 

[PATCH 3/3] lib: Reject multi-message mboxes and deprecate single-message mbox

[Tomi Ollila]

On Sun, Nov 25 2012, Austin Clements  wrote:

> Quoth Tomi Ollila on Nov 25 at  3:26 pm:
>> On Sun, Nov 25 2012, Austin Clements  wrote:
>> 
>> > Previously, we would treat multi-message mboxes as one giant email,
>> > which, besides the obvious incorrect indexing, often led to
>> > out-of-memory errors for archival mboxes.  Now we explicitly reject
>> > multi-message mboxes.  For historical reasons, we retain support for
>> > single-message mboxes, but official deprecate this behavior.
>> 
>> 
>> The series looks good to me -- but I don't know about deprecating
>> single-message mboxes:
>> 
>> * If we someday support (read-only?) mbox format, then single-message
>>   mboxes are "normal" again.
>
> If notmuch does gain mbox support, then its handling of single-message
> mboxes will *definitely* change because it will stop doing
> maildir-like things to them (flag sync, moving from new to cur, etc),
> which people may currently be depending on.  This was one of the
> motivations for deprecating the current handling of single-message
> mboxes.
>
>> * Some na?ve mb2md scripts could leave the 'From ' -line intact: for
>>   example `formail -bz -s head -3 < $MAIL`(*) can be used to demonstrate this
>
> I would call that "buggy", rather than "na?ve".  ]:--8)
>
>> * Some people may have large collection of single-file messages starting
>>   with 'From ' currently indexed. If those are to be re-indexed later
>>   without "single-message mbox" support that is somewhat of a burden to
>>   the users (**)
>
> That's why this only deprecates them (with a warning) and doesn't drop
> support for them.  The idea is to keep the historical handling for a
> few releases and then we'll have the flexibility to do what we want
> with single-message mboxes (including supporting them as real mbox).
>
> It's probably a good idea to include a script or a wiki pointer for
> fixing single-message mboxes in the NEWS.  As long as the file name is
> kept the same, notmuch won't reindex it.

Ok, I'm convinced. +1

Tomi

>
>> (*) my "mb2md" wannabe does gnus-like "$formail" -bz -R 'From ' X-From-Line: 
>> ...
>> 
>> (**) Something like the following could be used to mangle "single-file 
>> mboxes"...
>>  find . -type f | xargs perl -e 'foreach (@ARGV) { open IO, "+<", $_ or
>>  next; sysread IO, $buf, 5; if ($buf eq "From ") { sysseek IO, 0, 0;
>>  syswrite IO, "Fro:"; }}' 
>>  This breaks the multi-message mbox nicely... >;)
>> 
>> 
>> Tomi
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

[Patch v4 2/2] test: initial performance testing infrastructure

[David Bremner]

Austin Clements  writes:

>> +subdirs := compat completion emacs lib man parse-time-string
>> +subdirs := $(subdirs) performance-test util test

> += ?
>

Sure.

>> +CORPUS_NAME := notmuch-email-corpus-$(PERFTEST_VERSION).tar.xz
>
> Would it make sense to split out the different size corpora so a user
> could, say, only download the small one?

Currently the choice of test is local to given test file; one doing
something particularly intense (or just lots of repetitions) might want
to only use a subset. So I'm not sure if separate downloading of smaller
corpora makes sense. This is all hypothetical at the moment, since the
one test file uses the full corpus.

> "\nPlease download ${TXZFILE} using\n\n"?

OK

>> +add_email_corpus takes arguments "--small" and "--medium" for when you
>> +want smaller corpuses to check.
>
> "corpora"?

reworded to say 

,
| add_email_corpus takes arguments "--small" and "--medium" for when you
| want smaller subsets of the corpus to check.
`

>
> I'm a bit confused by this.  What happens if you don't specify --small
> or --medium?  Is the "large"/default corpus just the combined small
> and medium corpora?  Would be worth a comment, at least.

Hopefully the README makes this clear(er) now?

> This probably doesn't matter now, but I wonder if we want to unpack on
> first use to somewhere not test-specific and then cp -rl the corpus
> into the test directory.  I haven't tried unpacking the corpus yet,
> but if you're running tests repeatedly to compare results, or running
> more than one performance test, it seems like a full decompress and
> unpack could get onerous.

Hmm. On my machine it is 10s for the copy versus 45s for a full
unpack. For some reason I tested with "cp -a" which is incredibly slow, 
so I thought there was no loss. For comparison the basic test takes
about 10 minutes on the same machine.

In any case this can wait until we have a second test file and a second
call to add_mail_corpus, adding caching now would not help.

Notmuch scripts (again), now with more usenet

[David Bremner]

Ethan Glasser-Camp  writes:
>
> If Bremner is willing to put this package in contrib, I think he should
> do so.

I guess that's fine, but I'd be more likely to get around to it if
presented with a patch against git master 

d

[Patch v4 2/2] test: initial performance testing infrastructure

[Austin Clements]

Quoth david at tethera.net on Nov 25 at 11:02 am:
> From: David Bremner 
> 
> This is not near as fancy as as the unit tests, on the theory that
> the code should typically be crashing when performance tuning.
> Nonetheless, there is plenty of room for improvement.  Several more of
> the pieces of the test infrastructure (e.g. the option parsing) could
> be factored out into test/test-lib-common.sh
> ---
>  Makefile   |3 +-
>  performance-test/.gitignore|1 +
>  performance-test/Makefile  |7 +++
>  performance-test/Makefile.local|   32 ++
>  performance-test/README|   50 +++
>  performance-test/basic |   15 +
>  performance-test/download/.gitignore   |2 +
>  .../download/notmuch-email-corpus-0.2.tar.xz.asc   |9 +++
>  performance-test/notmuch-perf-test |   25 
>  performance-test/perf-test-lib.sh  |   65 
> 
>  performance-test/version.sh|3 +
>  11 files changed, 211 insertions(+), 1 deletion(-)
>  create mode 100644 performance-test/.gitignore
>  create mode 100644 performance-test/Makefile
>  create mode 100644 performance-test/Makefile.local
>  create mode 100644 performance-test/README
>  create mode 100755 performance-test/basic
>  create mode 100644 performance-test/download/.gitignore
>  create mode 100644 
> performance-test/download/notmuch-email-corpus-0.2.tar.xz.asc
>  create mode 100755 performance-test/notmuch-perf-test
>  create mode 100644 performance-test/perf-test-lib.sh
>  create mode 100644 performance-test/version.sh
> 
> diff --git a/Makefile b/Makefile
> index bb9c316..5decbea 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -3,7 +3,8 @@
>  all:
>  
>  # List all subdirectories here. Each contains its own Makefile.local
> -subdirs = compat completion emacs lib man parse-time-string util test
> +subdirs := compat completion emacs lib man parse-time-string
> +subdirs := $(subdirs) performance-test util test

+= ?

>  
>  # We make all targets depend on the Makefiles themselves.
>  global_deps = Makefile Makefile.config Makefile.local \
> diff --git a/performance-test/.gitignore b/performance-test/.gitignore
> new file mode 100644
> index 000..53f2697
> --- /dev/null
> +++ b/performance-test/.gitignore
> @@ -0,0 +1 @@
> +tmp.*/
> diff --git a/performance-test/Makefile b/performance-test/Makefile
> new file mode 100644
> index 000..de492a7
> --- /dev/null
> +++ b/performance-test/Makefile
> @@ -0,0 +1,7 @@
> +# See Makefile.local for the list of files to be compiled in this
> +# directory.
> +all:
> + $(MAKE) -C .. all
> +
> +.DEFAULT:
> + $(MAKE) -C .. $@
> diff --git a/performance-test/Makefile.local b/performance-test/Makefile.local
> new file mode 100644
> index 000..1114ec1
> --- /dev/null
> +++ b/performance-test/Makefile.local
> @@ -0,0 +1,32 @@
> +# -*- makefile -*-
> +
> +dir := performance-test
> +
> +include $(dir)/version.sh
> +
> +CORPUS_NAME := notmuch-email-corpus-$(PERFTEST_VERSION).tar.xz

Would it make sense to split out the different size corpora so a user
could, say, only download the small one?

> +TXZFILE := ${dir}/download/${CORPUS_NAME}
> +SIGFILE := ${TXZFILE}.asc
> +TEST_SCRIPT := ${dir}/notmuch-perf-test
> +DEFAULT_URL :=  http://notmuchmail.org/releases/${CORPUS_NAME}
> +
> +perf-test: setup-perf-test all
> + $(TEST_SCRIPT) $(OPTIONS)
> +
> +.PHONY: download-corpus setup-perf-test
> +
> +# Note that this intentionally does not depend on download-corpus.
> +setup-perf-test: $(TXZFILE)
> + gpg --verify $(SIGFILE)
> +
> +$(TXZFILE):
> + @printf "\nPlease download ${TXZFILE}.\n\n"

"\nPlease download ${TXZFILE} using\n\n"?

> + @printf "\t%% make download-corpus\n\n"
> + @echo or see http://notmuchmail.org/corpus for download locations
> + @echo
> + @false
> +
> +download-corpus:
> + wget -O ${TXZFILE} ${DEFAULT_URL}
> +
> +CLEAN := $(CLEAN) $(dir)/tmp.*
> diff --git a/performance-test/README b/performance-test/README
> new file mode 100644
> index 000..239d2fb
> --- /dev/null
> +++ b/performance-test/README
> @@ -0,0 +1,50 @@
> +Pre-requisites
> +--
> +
> +In addition to having notmuch, you need:
> +
> +- gpg
> +- gnu tar
> +- gnu time
> +- xz. Some speedup can be gotten by installing "pixz", but this is
> +  probably only worthwhile if you are debugging the tests.
> +
> +Getting set up to run tests:
> +
> +
> +First, you need to get the corpus.
> +
> +It should work to run
> +
> +   % make download-corpus
> +
> +In case that fails or is too slow, check
> +
> +   http://notmuchmail.org/corpus
> +
> +for a list of mirrors.
> +
> +Running tests
> +-
> +
> +The easiest way to run performance tests is to say "make perf-test", (or
> +simply run the 

[PATCH v2 18/20] test: add tests for insert

[Mark Walters]

On Sun, 25 Nov 2012, Peter Wang  wrote:
> Add tests for new 'insert' command.
> ---
>  test/insert   | 93 
> +++
>  test/notmuch-test |  1 +
>  2 files changed, 94 insertions(+)
>  create mode 100755 test/insert
>
> diff --git a/test/insert b/test/insert
> new file mode 100755
> index 000..d821a41
> --- /dev/null
> +++ b/test/insert
> @@ -0,0 +1,93 @@
> +#!/usr/bin/env bash
> +test_description='"notmuch insert"'
> +. ./test-lib.sh
> +
> +# Create directories and database before inserting.
> +mkdir -p "$MAIL_DIR"/{cur,new,tmp}
> +mkdir -p "$MAIL_DIR"/Drafts/{cur,new,tmp}
> +notmuch new > /dev/null
> +
> +# We use generate_message to create the temporary message file.
> +# It happens to be in the mail directory already but that is okay.

Perhaps add "since we do not call notmuch new".

> +
> +test_begin_subtest "Insert message, default"
> +generate_message \
> +"[subject]=\"insert-subject\"" \
> +"[date]=\"Sat, 01 Jan 2000 12:00:00 -\"" \
> +"[body]=\"insert-message\""
> +notmuch insert < "$gen_msg_filename"
> +test_expect_equal "`notmuch count subject:insert-subject tag:unread`" "1"

I would much prefer to test that we have the proper full message here
with something like notmuch show  or something. I would like
something that tests that the full message has actually been written to
disk.

> +test_begin_subtest "Insert message, add tag"
> +generate_message \
> +"[subject]=\"insert-subject-addtag\"" \
> +"[date]=\"Sat, 01 Jan 2000 12:00:00 -\"" \
> +"[body]=\"insert-message-addtag\""
> +notmuch insert +custom < "$gen_msg_filename"
> +test_expect_equal "`notmuch count tag:custom`" "1"
> +
> +test_begin_subtest "Insert message, add/remove tag"
> +generate_message \
> +"[subject]=\"insert-subject-addrmtag\"" \
> +"[date]=\"Sat, 01 Jan 2000 12:00:00 -\"" \
> +"[body]=\"insert-message-addrmtag\""
> +notmuch insert +custom -unread < "$gen_msg_filename"
> +test_expect_equal "`notmuch count tag:custom NOT tag:unread`" "1"
> +
> +test_begin_subtest "Insert message, folder"
> +generate_message \
> +"[subject]=\"insert-subject-draft\"" \
> +"[date]=\"Sat, 01 Jan 2000 12:00:00 -\"" \
> +"[body]=\"insert-message-draft\""
> +notmuch insert --folder=Drafts < "$gen_msg_filename"
> +test_expect_equal "`notmuch count folder:Drafts`" "1"
> +
> +test_begin_subtest "Insert message, folder and tags"
> +generate_message \
> +"[subject]=\"insert-subject-draft\"" \
> +"[date]=\"Sat, 01 Jan 2000 12:00:00 -\"" \
> +"[body]=\"insert-message-draft\""
> +notmuch insert --folder=Drafts +draft -unread < "$gen_msg_filename"
> +test_expect_equal "`notmuch count folder:Drafts tag:draft NOT tag:unread`" 
> "1"
> +
> +test_begin_subtest "Insert message, non-existent folder"
> +generate_message \
> +"[subject]=\"insert-subject-nonexistfolder\"" \
> +"[date]=\"Sat, 01 Jan 2000 12:00:00 -\"" \
> +"[body]=\"insert-message-nonexistfolder\""
> +notmuch insert --folder=nonesuch < "$gen_msg_filename"
> +test_expect_equal "$?" "1"

Here and for other error cases you could check that the correct error
message is being returned.

Best wishes

Mark
> +
> +test_begin_subtest "Insert message, create folder"
> +generate_message \
> +"[subject]=\"insert-subject-createfolder\"" \
> +"[date]=\"Sat, 01 Jan 2000 12:00:00 -\"" \
> +"[body]=\"insert-message-createfolder\""
> +notmuch insert --folder=F --create-folder +folder < "$gen_msg_filename"
> +test_expect_equal "`notmuch count folder:F tag:folder`" "1"
> +
> +test_begin_subtest "Insert message, create subfolder"
> +generate_message \
> +"[subject]=\"insert-subject-createfolder\"" \
> +"[date]=\"Sat, 01 Jan 2000 12:00:00 -\"" \
> +"[body]=\"insert-message-createfolder\""
> +notmuch insert --folder=F/G/H/I/J --create-folder +folder < 
> "$gen_msg_filename"
> +test_expect_equal "`notmuch count folder:F/G/H/I/J tag:folder`" "1"
> +
> +test_begin_subtest "Insert message, create existing subfolder"
> +generate_message \
> +"[subject]=\"insert-subject-createfolder\"" \
> +"[date]=\"Sat, 01 Jan 2000 12:00:00 -\"" \
> +"[body]=\"insert-message-createfolder\""
> +notmuch insert --folder=F/G/H/I/J --create-folder +folder < 
> "$gen_msg_filename"
> +test_expect_equal "`notmuch count folder:F/G/H/I/J tag:folder`" "2"
> +
> +test_begin_subtest "Insert message, create invalid subfolder"
> +generate_message \
> +"[subject]=\"insert-subject-createinvalidfolder\"" \
> +"[date]=\"Sat, 01 Jan 2000 12:00:00 -\"" \
> +"[body]=\"insert-message-createinvalidfolder\""
> +notmuch insert --folder=../G --create-folder < "$gen_msg_filename"
> +test_expect_equal "$?" "1"
> +
> +test_done
> diff --git a/test/notmuch-test b/test/notmuch-test
> index 9a1b375..09be44c 100755
> --- a/test/notmuch-test
> +++ b/test/notmuch-test
> @@ -22,6 +22,7 @@ TESTS="
>config
>new
>count
> +  insert
>search
>search-output

[PATCH 0/6] API for iterating over all messages in a thread

[Austin Clements]

Quoth Mark Walters on Nov 25 at  2:31 pm:
> 
> Hi
> 
> This series looks good to me (I have not reviewed the two bindings
> patches). Patch 2 looks like it makes things much easier to follow than
> the current code (if I understood the current pointer stuff it
> constructs the top-level list by doing pointer stuff to remove all
> messages which are replies from the complete message list). Indeed, the
> diff is more complicated than the new code!
> 
> On Sun, 25 Nov 2012, Austin Clements  wrote:
> > This series adds a library API for iterating over all messages in a
> > thread in sorted order.  This is easy for the library to provide and
> > difficult to obtain from the current API.  Plus, if you don't count
> > the code added to the bindings, this series is actually a net
> > decrease of 4 lines of code because of simplifications it enables.
> >
> > Do we want the API to do more?  Currently it's very minimal, but I can
> > imagine two ways it could be generalized.  It could take an argument
> > to indicate which message list to return, which could be all messages,
> > matched messages, top-level messages, or maybe even unmatched messages
> > (possibly all in terms of message flags).  It could also take an
> > argument indicating the desired sort order.  Currently, the caller can
> > use existing message flag APIs to distinguish matched and unmatched
> > messages and there's a separate function for the top-level messages.
> > However, if the API could do all of these things, it would subsume
> > various other API functions, such as notmuch_thread_get_*_date.
> 
> I don't know if this is the right API. For the matched message etc I
> think using the existing message flag APIs is simple enough. I am not
> sure about sort orders though: that looks like it would be much easier
> for the caller to have the correct sort by I am not sure what users
> would need it.

For sort order, I would be inclined to simply construct the reverse
list the first time a caller asks for it.  Theoretically the caller
could do this just as easily as the library, except that we don't
expose the list routines.

If I do add sort order, I would also want to add some control over
which list is returned, since it would be asymmetric to be able to
request all messages in either order, but top-level messages only in
oldest-first.  I think this would be pretty simple, and would give us
a reasonably general-purpose and extensible API.  (It would also solve
the naming conundrum I mentioned below in my original email.)

> Best wishes
> 
> Mark
> 
> 
> 
> 
> >
> > Also, is this the right name for the new API?  In particular, if we do
> > later want to add a function that returns, say, the list of matched
> > messages, we'll have a convention collision with
> > notmuch_thread_get_matched_messages, which returns only a count.

[PATCH v2 19/20] man: document 'insert' command

[Mark Walters]

On Sun, 25 Nov 2012, Peter Wang  wrote:
> Add initial documentation for notmuch insert command.
> ---
>  man/Makefile.local|  1 +
>  man/man1/notmuch-insert.1 | 60 
> +++
>  2 files changed, 61 insertions(+)
>  create mode 100644 man/man1/notmuch-insert.1
>
> diff --git a/man/Makefile.local b/man/Makefile.local
> index 72e2a18..216aaa0 100644
> --- a/man/Makefile.local
> +++ b/man/Makefile.local
> @@ -12,6 +12,7 @@ MAN1 := \
>   $(dir)/man1/notmuch-count.1 \
>   $(dir)/man1/notmuch-dump.1 \
>   $(dir)/man1/notmuch-restore.1 \
> + $(dir)/man1/notmuch-insert.1 \
>   $(dir)/man1/notmuch-new.1 \
>   $(dir)/man1/notmuch-reply.1 \
>   $(dir)/man1/notmuch-search.1 \
> diff --git a/man/man1/notmuch-insert.1 b/man/man1/notmuch-insert.1
> new file mode 100644
> index 000..8b1c4e9
> --- /dev/null
> +++ b/man/man1/notmuch-insert.1
> @@ -0,0 +1,60 @@
> +.TH NOTMUCH-INSERT 1 2012-xx-xx "Notmuch 0.xx"
> +.SH NAME
> +notmuch-insert \- add a message to the maildir and notmuch database
> +.SH SYNOPSIS
> +
> +.B notmuch insert
> +.RI "[" options "]"
> +.RI "[ +<" tag> "|\-<" tag "> ... ]"
> +
> +.SH DESCRIPTION
> +
> +.B notmuch insert
> +reads a message from standard input
> +and delivers it to the specified maildir folder,
> +then incorporates the message into the notmuch database.
> +It is an alternative to using a separate tool to deliver
> +the message then running
> +.B notmuch new
> +afterwards.
> +
> +The new message will be tagged with the tags specified by the
> +.B new.tags
> +configuration option.
> +Additional tagging operations may be specified on the command-line:
> +tags prefixed by '+' are added while
> +those prefixed by '\-' are removed.

Is it worth emphasising that the command line tags are applied after the
new tags so can be used to remove the new tags?

Best wishes

Mark

> +Option arguments must appear before any tag operation arguments.
> +Supported options for
> +.B insert
> +include
> +.RS 4
> +.TP 4
> +.BI "--folder=<" folder ">"
> +
> +Deliver the message to the specified folder,
> +relative to the top-level directory given by the value of
> +\fBdatabase.path\fR.
> +The default is to deliver to the top-level directory.
> +
> +.RE
> +
> +.RS 4
> +.TP 4
> +.B "--create-folder"
> +
> +Try to create the folder named by the
> +.B "--folder"
> +option, if it does not exist.
> +Otherwise the folder must already exist for mail
> +delivery to succeed.
> +
> +.RE
> +.SH SEE ALSO
> +
> +\fBnotmuch\fR(1), \fBnotmuch-config\fR(1), \fBnotmuch-count\fR(1),
> +\fBnotmuch-dump\fR(1), \fBnotmuch-hooks\fR(5), \fBnotmuch-reply\fR(1),
> +\fBnotmuch-restore\fR(1), \fBnotmuch-search\fR(1),
> +\fBnotmuch-search-terms\fR(7), \fBnotmuch-show\fR(1),
> +\fBnotmuch-tag\fR(1)
> -- 
> 1.7.12.1
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH v2 15/20] insert: fsync new directory after rename

[Mark Walters]

On Sun, 25 Nov 2012, Peter Wang  wrote:
> After moving the file from the 'tmp' to the 'new' directory,
> fsync on the 'new' directory for durability.
> ---
>  notmuch-insert.c | 39 ---
>  1 file changed, 32 insertions(+), 7 deletions(-)
>
> diff --git a/notmuch-insert.c b/notmuch-insert.c
> index f09c579..831b322 100644
> --- a/notmuch-insert.c
> +++ b/notmuch-insert.c
> @@ -143,10 +143,10 @@ maildir_create_folder (void *ctx, const char *dir)
>  /* Open a unique file in the Maildir 'tmp' directory.
>   * Returns the file descriptor on success, or -1 on failure.
>   * On success, file paths into the 'tmp' and 'new' directories are returned
> - * via tmppath and newpath. */
> + * via tmppath and newpath, and the path of the 'new' directory in newdir. */

I found this comment very difficult to understand since it looks like
the 'new' directory gets returned twice. Perhaps something like

"On success, file paths for the message in the 'tmp' and 'new'
directories are returned via tmppath and newpath, and the path of the
'new' directory itself in newdir."


Best wishes

Mark

>  static int
>  maildir_open_tmp_file (void *ctx, const char *dir,
> -char **tmppath, char **newpath)
> +char **tmppath, char **newpath, char **newdir)
>  {
>  pid_t pid;
>  char hostname[256];
> @@ -183,8 +183,9 @@ maildir_open_tmp_file (void *ctx, const char *dir,
>   return -1;
>  }
>  
> +*newdir = talloc_asprintf (ctx, "%s/new", dir);
>  *newpath = talloc_asprintf (ctx, "%s/new/%s", dir, filename);
> -if (! *newpath) {
> +if (! *newdir || ! *newpath) {
>   fprintf (stderr, "Out of memory\n");
>   close (fd);
>   unlink (*tmppath);
> @@ -204,14 +205,31 @@ maildir_open_tmp_file (void *ctx, const char *dir,
>   * http://wiki.dovecot.org/MailboxFormat/Maildir#Mail_delivery
>   */
>  static notmuch_bool_t
> -maildir_move_tmp_to_new (const char *tmppath, const char *newpath)
> +maildir_move_tmp_to_new (const char *tmppath, const char *newpath,
> +  const char *newdir)
>  {
> +notmuch_bool_t ret;
> +int fd;
> +
>  if (rename (tmppath, newpath) != 0) {
>   fprintf (stderr, "Error: rename() failed: %s\n", strerror (errno));
>   return FALSE;
>  }
>  
> -return TRUE;
> +/* Sync the 'new' directory after rename for durability. */
> +ret = TRUE;
> +fd = open (newdir, O_RDONLY);
> +if (fd == -1) {
> + fprintf (stderr, "Error: open() dir failed: %s\n", strerror (errno));
> + ret = FALSE;
> +}
> +if (ret && fsync (fd) != 0) {
> + fprintf (stderr, "Error: fsync() dir failed: %s\n", strerror (errno));
> + ret = FALSE;
> +}
> +if (fd != -1)
> + close (fd);
> +return ret;
>  }
>  
>  /* Copy the contents of fdin into fdout. */
> @@ -307,6 +325,12 @@ add_file_to_database (notmuch_database_t *notmuch, const 
> char *path,
>  
>  notmuch_message_thaw (message);
>  
> +/* notmuch_message_tags_to_maildir_flags may rename the message file
> + * once more, and does so without fsyncing the directory afterwards.
> + * rename() is atomic so after a crash the file should appear under
> + * the old or new name. notmuch new should be able to rename the file
> + * again if required, so another fsync is not required, I think.
> + */
>  notmuch_message_tags_to_maildir_flags (message);
>  
>  notmuch_message_destroy (message);
> @@ -321,10 +345,11 @@ insert_message (void *ctx, notmuch_database_t *notmuch, 
> int fdin,
>  {
>  char *tmppath;
>  char *newpath;
> +char *newdir;
>  int fdout;
>  notmuch_bool_t ret;
>  
> -fdout = maildir_open_tmp_file (ctx, dir, , );
> +fdout = maildir_open_tmp_file (ctx, dir, , , );
>  if (fdout < 0) {
>   return FALSE;
>  }
> @@ -335,7 +360,7 @@ insert_message (void *ctx, notmuch_database_t *notmuch, 
> int fdin,
>  }
>  close (fdout);
>  if (ret) {
> - ret = maildir_move_tmp_to_new (tmppath, newpath);
> + ret = maildir_move_tmp_to_new (tmppath, newpath, newdir);
>  }
>  if (!ret) {
>   unlink (tmppath);
> -- 
> 1.7.12.1
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH v2 01/20] tag: factor out tag operation parsing

[Mark Walters]

On Sun, 25 Nov 2012, Peter Wang  wrote:
> Factor out parsing of +tag, -tag operations from argv
> into a separate function.
> ---
>  notmuch-tag.c | 66 
> +--
>  1 file changed, 41 insertions(+), 25 deletions(-)
>
> diff --git a/notmuch-tag.c b/notmuch-tag.c
> index 88d559b..35a76db 100644
> --- a/notmuch-tag.c
> +++ b/notmuch-tag.c
> @@ -167,11 +167,48 @@ tag_query (void *ctx, notmuch_database_t *notmuch, 
> const char *query_string,
>  return interrupted;
>  }
>  
> +/* Parse +tag and -tag operations between argv[i] and argv[argc-1].
> + * The array tag_ops must be at least argc - i elements long.
> + * Returns the index into argv where parsing stopped, or -1 on error. */

Perhaps mention tag_ops_count (as the number of tags parsed or
something)?

> +static int
> +parse_tag_operations (int i, int argc, char *argv[],
> +   tag_operation_t *tag_ops, int *tag_ops_count)
> +{
> +*tag_ops_count = 0;
> +
> +for (; i < argc; i++) {
> + if (strcmp (argv[i], "--") == 0) {
> + i++;
> + break;
> + }
> + if (argv[i][0] == '+' || argv[i][0] == '-') {
> + if (argv[i][0] == '+' && argv[i][1] == '\0') {
> + fprintf (stderr, "Error: tag names cannot be empty.\n");
> + return -1;
> + }
> + if (argv[i][0] == '+' && argv[i][1] == '-') {
> + /* This disallows adding the non-removable tag "-" and
> +  * enables notmuch tag to take long options in the
> +  * future. */
> + fprintf (stderr, "Error: tag names must not start with '-'.\n");
> + return -1;
> + }
> + tag_ops[*tag_ops_count].tag = argv[i] + 1;
> + tag_ops[*tag_ops_count].remove = (argv[i][0] == '-');
> + (*tag_ops_count)++;
> + } else {
> + break;
> + }
> +}
> +
> +return i;
> +}
> +
>  int
>  notmuch_tag_command (void *ctx, int argc, char *argv[])
>  {
>  tag_operation_t *tag_ops;
> -int tag_ops_count = 0;
> +int tag_ops_count;
>  char *query_string;
>  notmuch_config_t *config;
>  notmuch_database_t *notmuch;
> @@ -197,30 +234,9 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
>   return 1;
>  }
>  
> -for (i = 0; i < argc; i++) {
> - if (strcmp (argv[i], "--") == 0) {
> - i++;
> - break;
> - }
> - if (argv[i][0] == '+' || argv[i][0] == '-') {
> - if (argv[i][0] == '+' && argv[i][1] == '\0') {
> - fprintf (stderr, "Error: tag names cannot be empty.\n");
> - return 1;
> - }
> - if (argv[i][0] == '+' && argv[i][1] == '-') {
> - /* This disallows adding the non-removable tag "-" and
> -  * enables notmuch tag to take long options in the
> -  * future. */
> - fprintf (stderr, "Error: tag names must not start with '-'.\n");
> - return 1;
> - }
> - tag_ops[tag_ops_count].tag = argv[i] + 1;
> - tag_ops[tag_ops_count].remove = (argv[i][0] == '-');
> - tag_ops_count++;
> - } else {
> - break;
> - }
> -}
> +i = parse_tag_operations (0, argc, argv, tag_ops, _ops_count);
> +if (i < 0)
> + return 1;
>  
>  tag_ops[tag_ops_count].tag = NULL;
>  
> -- 
> 1.7.12.1
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH v2 00/20] insert command

[Mark Walters]

This is series is looking good. I have a few minor issues but no more.

There are two parts I don't really know enough about to be certain
about: the fsyncing and the sigtrap bit. 

For fsync: I have some recollection that you have to fsync all subpaths
to the root to guarantee that it makes it to the disk. 

One in particular that may need more fsyncing is if you create a folder
A/B/C/D/E/ for the messages then I think you only fsync the message and
A/B/C/D/E/new.

My other comments are all very minor and are made in individual replies.

Best wishes

Mark


On Sun, 25 Nov 2012, Peter Wang  wrote:
> This series mainly addresses the issues raised by Mark:
>
> - check talloc failures
> - deadlock in maildir_open_tmp
> - stricter file modes (0600 and 0700)
> - shared tag operation parser with notmuch-tag.c
> - simplified mkdir_parents
> - trap SIGINT
> - fsync after writing and rename
> - added a couple of tests
> - man page wording
> - comments
>
> Due to new restriction on tags beginning with '-', an argument beginning with
> "--" is no longer ambiguous so I have removed the optional "--" separator
> between options and tag operations.
>
> Peter Wang (20):
>   tag: factor out tag operation parsing
>   tag: make tag operation parser public
>   cli: add stub for insert command
>   insert: open Maildir tmp file
>   insert: copy stdin to Maildir tmp file
>   insert: move file from Maildir tmp to new
>   insert: add new message to database
>   insert: support --folder option
>   insert: prevent writes outside Maildir hierarchy
>   insert: apply default tags to new message
>   insert: parse command-line tag operations
>   insert: apply command-line tag operations
>   insert: add --create-folder option
>   insert: fsync after writing tmp file
>   insert: fsync new directory after rename
>   insert: trap SIGINT and clean up
>   insert: add copyright line from notmuch-deliver
>   test: add tests for insert
>   man: document 'insert' command
>   man: reference notmuch-insert.1
>
>  Makefile.local  |   1 +
>  man/Makefile.local  |   1 +
>  man/man1/notmuch-config.1   |   4 +-
>  man/man1/notmuch-count.1|   4 +-
>  man/man1/notmuch-dump.1 |   4 +-
>  man/man1/notmuch-insert.1   |  60 +
>  man/man1/notmuch-new.1  |   4 +-
>  man/man1/notmuch-reply.1|   3 +-
>  man/man1/notmuch-restore.1  |   3 +-
>  man/man1/notmuch-search.1   |   3 +-
>  man/man1/notmuch-show.1 |   3 +-
>  man/man1/notmuch-tag.1  |   3 +-
>  man/man1/notmuch.1  |   3 +-
>  man/man5/notmuch-hooks.5|   4 +-
>  man/man7/notmuch-search-terms.7 |   3 +-
>  notmuch-client.h|  12 +
>  notmuch-insert.c| 493 
> 
>  notmuch-tag.c   |  79 ---
>  notmuch.c   |   3 +
>  test/insert |  93 
>  test/notmuch-test   |   1 +
>  21 files changed, 733 insertions(+), 51 deletions(-)
>  create mode 100644 man/man1/notmuch-insert.1
>  create mode 100644 notmuch-insert.c
>  create mode 100755 test/insert
>
> -- 
> 1.7.12.1
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

BUG: notmuch stop on death symlink even..

[Austin Clements]

Quoth calmar c. on Nov 24 at  8:07 pm:
> hi all, 
> 
> notmuch stops/exits on death symlink even when they are on the
> ignore-list at the same time.
> 
> on: notmuch new ... arount 360maybe in notmuch-new.c

The series in id:1353821145-29182-1-git-send-email-amdragon at mit.edu
should fix this bug.

> cheers
> marco

[PATCH 3/3] lib: Reject multi-message mboxes and deprecate single-message mbox

[Tomi Ollila]

On Sun, Nov 25 2012, Austin Clements  wrote:

> Previously, we would treat multi-message mboxes as one giant email,
> which, besides the obvious incorrect indexing, often led to
> out-of-memory errors for archival mboxes.  Now we explicitly reject
> multi-message mboxes.  For historical reasons, we retain support for
> single-message mboxes, but official deprecate this behavior.


The series looks good to me -- but I don't know about deprecating
single-message mboxes:

* If we someday support (read-only?) mbox format, then single-message
  mboxes are "normal" again.

* Some na?ve mb2md scripts could leave the 'From ' -line intact: for
  example `formail -bz -s head -3 < $MAIL`(*) can be used to demonstrate this

* Some people may have large collection of single-file messages starting
  with 'From ' currently indexed. If those are to be re-indexed later
  without "single-message mbox" support that is somewhat of a burden to
  the users (**)

(*) my "mb2md" wannabe does gnus-like "$formail" -bz -R 'From ' X-From-Line: ...

(**) Something like the following could be used to mangle "single-file 
mboxes"...
 find . -type f | xargs perl -e 'foreach (@ARGV) { open IO, "+<", $_ or
 next; sysread IO, $buf, 5; if ($buf eq "From ") { sysseek IO, 0, 0;
 syswrite IO, "Fro:"; }}' 
 This breaks the multi-message mbox nicely... >;)


Tomi

[PATCH v2 00/20] insert command

[David Bremner]

Peter Wang  writes:

> - shared tag operation parser with notmuch-tag.

Sharing tag operations with notmuch-tag is definitely a good idea, but
it will also conflict with the changes of the series at 

   id:1353792017-31459-1-git-send-email-david at tethera.net

Although I might be biased, my current plan is to merge that series
first, since it fixes a long standing bug in the dump/restore commands.
If anything about series seems particularly problematic for
notmuch-insert, let me know.  Of course we can always make changes
afterwards, there are no public API changes (i.e. stuff ./lib) in play
here.

d

[PATCH 0/6] API for iterating over all messages in a thread

[Mark Walters]

Hi

This series looks good to me (I have not reviewed the two bindings
patches). Patch 2 looks like it makes things much easier to follow than
the current code (if I understood the current pointer stuff it
constructs the top-level list by doing pointer stuff to remove all
messages which are replies from the complete message list). Indeed, the
diff is more complicated than the new code!

On Sun, 25 Nov 2012, Austin Clements  wrote:
> This series adds a library API for iterating over all messages in a
> thread in sorted order.  This is easy for the library to provide and
> difficult to obtain from the current API.  Plus, if you don't count
> the code added to the bindings, this series is actually a net
> decrease of 4 lines of code because of simplifications it enables.
>
> Do we want the API to do more?  Currently it's very minimal, but I can
> imagine two ways it could be generalized.  It could take an argument
> to indicate which message list to return, which could be all messages,
> matched messages, top-level messages, or maybe even unmatched messages
> (possibly all in terms of message flags).  It could also take an
> argument indicating the desired sort order.  Currently, the caller can
> use existing message flag APIs to distinguish matched and unmatched
> messages and there's a separate function for the top-level messages.
> However, if the API could do all of these things, it would subsume
> various other API functions, such as notmuch_thread_get_*_date.

I don't know if this is the right API. For the matched message etc I
think using the existing message flag APIs is simple enough. I am not
sure about sort orders though: that looks like it would be much easier
for the caller to have the correct sort by I am not sure what users
would need it.

Best wishes

Mark




>
> Also, is this the right name for the new API?  In particular, if we do
> later want to add a function that returns, say, the list of matched
> messages, we'll have a convention collision with
> notmuch_thread_get_matched_messages, which returns only a count.

[PATCH 3/3] lib: Reject multi-message mboxes and deprecate single-message mbox

[Austin Clements]

Quoth Tomi Ollila on Nov 25 at  3:26 pm:
> On Sun, Nov 25 2012, Austin Clements  wrote:
> 
> > Previously, we would treat multi-message mboxes as one giant email,
> > which, besides the obvious incorrect indexing, often led to
> > out-of-memory errors for archival mboxes.  Now we explicitly reject
> > multi-message mboxes.  For historical reasons, we retain support for
> > single-message mboxes, but official deprecate this behavior.
> 
> 
> The series looks good to me -- but I don't know about deprecating
> single-message mboxes:
> 
> * If we someday support (read-only?) mbox format, then single-message
>   mboxes are "normal" again.

If notmuch does gain mbox support, then its handling of single-message
mboxes will *definitely* change because it will stop doing
maildir-like things to them (flag sync, moving from new to cur, etc),
which people may currently be depending on.  This was one of the
motivations for deprecating the current handling of single-message
mboxes.

> * Some na?ve mb2md scripts could leave the 'From ' -line intact: for
>   example `formail -bz -s head -3 < $MAIL`(*) can be used to demonstrate this

I would call that "buggy", rather than "na?ve".  ]:--8)

> * Some people may have large collection of single-file messages starting
>   with 'From ' currently indexed. If those are to be re-indexed later
>   without "single-message mbox" support that is somewhat of a burden to
>   the users (**)

That's why this only deprecates them (with a warning) and doesn't drop
support for them.  The idea is to keep the historical handling for a
few releases and then we'll have the flexibility to do what we want
with single-message mboxes (including supporting them as real mbox).

It's probably a good idea to include a script or a wiki pointer for
fixing single-message mboxes in the NEWS.  As long as the file name is
kept the same, notmuch won't reindex it.

> (*) my "mb2md" wannabe does gnus-like "$formail" -bz -R 'From ' X-From-Line: 
> ...
> 
> (**) Something like the following could be used to mangle "single-file 
> mboxes"...
>  find . -type f | xargs perl -e 'foreach (@ARGV) { open IO, "+<", $_ or
>  next; sysread IO, $buf, 5; if ($buf eq "From ") { sysseek IO, 0, 0;
>  syswrite IO, "Fro:"; }}' 
>  This breaks the multi-message mbox nicely... >;)
> 
> 
> Tomi

[PATCH 0/2] Ignore ignored broken symlinks

[Tomi Ollila]

On Sun, Nov 25 2012, Austin Clements  wrote:

> calmar on IRC reported a bug today where notmuch will abort when it
> encounters a broken symlink, even if that symlink is in the
> user-specified ignore list.  These two patches test for this bug and
> fix it.

+1

Tomi

[PATCH v2 20/20] man: reference notmuch-insert.1

[Peter Wang]

Add references to notmuch-insert.1 from other man pages.
---
 man/man1/notmuch-config.1   | 4 ++--
 man/man1/notmuch-count.1| 4 ++--
 man/man1/notmuch-dump.1 | 4 ++--
 man/man1/notmuch-new.1  | 4 ++--
 man/man1/notmuch-reply.1| 3 ++-
 man/man1/notmuch-restore.1  | 3 ++-
 man/man1/notmuch-search.1   | 3 ++-
 man/man1/notmuch-show.1 | 3 ++-
 man/man1/notmuch-tag.1  | 3 ++-
 man/man1/notmuch.1  | 3 ++-
 man/man5/notmuch-hooks.5| 4 ++--
 man/man7/notmuch-search-terms.7 | 3 ++-
 12 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/man/man1/notmuch-config.1 b/man/man1/notmuch-config.1
index 10c9fd6..d526627 100644
--- a/man/man1/notmuch-config.1
+++ b/man/man1/notmuch-config.1
@@ -152,7 +152,7 @@ use ${HOME}/.notmuch\-config if this variable is not set.
 .SH SEE ALSO

 \fBnotmuch\fR(1), \fBnotmuch-count\fR(1), \fBnotmuch-dump\fR(1),
-\fBnotmuch-hooks\fR(5), \fBnotmuch-new\fR(1), \fBnotmuch-reply\fR(1),
-\fBnotmuch-restore\fR(1), \fBnotmuch-search\fR(1),
+\fBnotmuch-hooks\fR(5), \fBnotmuch-insert\fR(1), \fBnotmuch-new\fR(1),
+\fBnotmuch-reply\fR(1), \fBnotmuch-restore\fR(1), \fBnotmuch-search\fR(1),
 \fBnotmuch-search-terms\fR(7), \fBnotmuch-show\fR(1),
 \fBnotmuch-tag\fR(1)
diff --git a/man/man1/notmuch-count.1 b/man/man1/notmuch-count.1
index 9b55e7c..4161324 100644
--- a/man/man1/notmuch-count.1
+++ b/man/man1/notmuch-count.1
@@ -52,7 +52,7 @@ count (the default) or not.
 .SH SEE ALSO

 \fBnotmuch\fR(1), \fBnotmuch-config\fR(1), \fBnotmuch-dump\fR(1),
-\fBnotmuch-hooks\fR(5), \fBnotmuch-new\fR(1), \fBnotmuch-reply\fR(1),
-\fBnotmuch-restore\fR(1), \fBnotmuch-search\fR(1),
+\fBnotmuch-hooks\fR(5), \fBnotmuch-insert\fR(1), \fBnotmuch-new\fR(1),
+\fBnotmuch-reply\fR(1), \fBnotmuch-restore\fR(1), \fBnotmuch-search\fR(1),
 \fBnotmuch-search-terms\fR(7), \fBnotmuch-show\fR(1),
 \fBnotmuch-tag\fR(1)
diff --git a/man/man1/notmuch-dump.1 b/man/man1/notmuch-dump.1
index 230deec..109fd26 100644
--- a/man/man1/notmuch-dump.1
+++ b/man/man1/notmuch-dump.1
@@ -30,7 +30,7 @@ for details of the supported syntax for .
 .SH SEE ALSO

 \fBnotmuch\fR(1), \fBnotmuch-config\fR(1), \fBnotmuch-count\fR(1),
-\fBnotmuch-hooks\fR(5), \fBnotmuch-new\fR(1), \fBnotmuch-reply\fR(1),
-\fBnotmuch-restore\fR(1), \fBnotmuch-search\fR(1),
+\fBnotmuch-hooks\fR(5), \fBnotmuch-insert\fR(1), \fBnotmuch-new\fR(1),
+\fBnotmuch-reply\fR(1), \fBnotmuch-restore\fR(1), \fBnotmuch-search\fR(1),
 \fBnotmuch-search-terms\fR(7), \fBnotmuch-show\fR(1),
 \fBnotmuch-tag\fR(1)
diff --git a/man/man1/notmuch-new.1 b/man/man1/notmuch-new.1
index 0acfeac..af2daff 100644
--- a/man/man1/notmuch-new.1
+++ b/man/man1/notmuch-new.1
@@ -64,7 +64,7 @@ Prevents hooks from being run.
 .SH SEE ALSO

 \fBnotmuch\fR(1), \fBnotmuch-config\fR(1), \fBnotmuch-count\fR(1),
-\fBnotmuch-dump\fR(1), \fBnotmuch-hooks\fR(5), \fBnotmuch-reply\fR(1),
-\fBnotmuch-restore\fR(1), \fBnotmuch-search\fR(1),
+\fBnotmuch-dump\fR(1), \fBnotmuch-hooks\fR(5), \fBnotmuch-insert\fR(1),
+\fBnotmuch-reply\fR(1), \fBnotmuch-restore\fR(1), \fBnotmuch-search\fR(1),
 \fBnotmuch-search-terms\fR(7), \fBnotmuch-show\fR(1),
 \fBnotmuch-tag\fR(1)
diff --git a/man/man1/notmuch-reply.1 b/man/man1/notmuch-reply.1
index d264060..020e252 100644
--- a/man/man1/notmuch-reply.1
+++ b/man/man1/notmuch-reply.1
@@ -96,7 +96,8 @@ replying to multiple messages at once, but the JSON format 
does not.
 .SH SEE ALSO

 \fBnotmuch\fR(1), \fBnotmuch-config\fR(1), \fBnotmuch-count\fR(1),
-\fBnotmuch-dump\fR(1), \fBnotmuch-hooks\fR(5), \fBnotmuch-new\fR(1),
+\fBnotmuch-dump\fR(1), \fBnotmuch-hooks\fR(5),
+\fBnotmuch-insert\fR(1), \fBnotmuch-new\fR(1),
 \fBnotmuch-restore\fR(1), \fBnotmuch-search\fR(1),
 \fBnotmuch-search-terms\fR(7), \fBnotmuch-show\fR(1),
 \fBnotmuch-tag\fR(1)
diff --git a/man/man1/notmuch-restore.1 b/man/man1/notmuch-restore.1
index 2fa8733..c23ea2c 100644
--- a/man/man1/notmuch-restore.1
+++ b/man/man1/notmuch-restore.1
@@ -39,7 +39,8 @@ details.
 .SH SEE ALSO

 \fBnotmuch\fR(1), \fBnotmuch-config\fR(1), \fBnotmuch-count\fR(1),
-\fBnotmuch-dump\fR(1), \fBnotmuch-hooks\fR(5), \fBnotmuch-new\fR(1),
+\fBnotmuch-dump\fR(1), \fBnotmuch-hooks\fR(5),
+\fBnotmuch-insert\fR(1), \fBnotmuch-new\fR(1),
 \fBnotmuch-reply\fR(1), \fBnotmuch-search\fR(1),
 \fBnotmuch-search-terms\fR(7), \fBnotmuch-show\fR(1),
 \fBnotmuch-tag\fR(1)
diff --git a/man/man1/notmuch-search.1 b/man/man1/notmuch-search.1
index 6ccd3b8..8af2a71 100644
--- a/man/man1/notmuch-search.1
+++ b/man/man1/notmuch-search.1
@@ -128,7 +128,8 @@ is the number of matching non-excluded messages in the 
thread.
 .SH SEE ALSO

 \fBnotmuch\fR(1), \fBnotmuch-config\fR(1), \fBnotmuch-count\fR(1),
-\fBnotmuch-dump\fR(1), \fBnotmuch-hooks\fR(5), \fBnotmuch-new\fR(1),
+\fBnotmuch-dump\fR(1), \fBnotmuch-hooks\fR(5),
+\fBnotmuch-insert\fR(1), \fBnotmuch-new\fR(1),
 \fBnotmuch-reply\fR(1), \fBnotmuch-restore\fR(1),
 \fBnotmuch-search-terms\fR(7), \fBnotmuch-show\fR(1),
 

[PATCH v2 19/20] man: document 'insert' command

[Peter Wang]

Add initial documentation for notmuch insert command.
---
 man/Makefile.local|  1 +
 man/man1/notmuch-insert.1 | 60 +++
 2 files changed, 61 insertions(+)
 create mode 100644 man/man1/notmuch-insert.1

diff --git a/man/Makefile.local b/man/Makefile.local
index 72e2a18..216aaa0 100644
--- a/man/Makefile.local
+++ b/man/Makefile.local
@@ -12,6 +12,7 @@ MAN1 := \
$(dir)/man1/notmuch-count.1 \
$(dir)/man1/notmuch-dump.1 \
$(dir)/man1/notmuch-restore.1 \
+   $(dir)/man1/notmuch-insert.1 \
$(dir)/man1/notmuch-new.1 \
$(dir)/man1/notmuch-reply.1 \
$(dir)/man1/notmuch-search.1 \
diff --git a/man/man1/notmuch-insert.1 b/man/man1/notmuch-insert.1
new file mode 100644
index 000..8b1c4e9
--- /dev/null
+++ b/man/man1/notmuch-insert.1
@@ -0,0 +1,60 @@
+.TH NOTMUCH-INSERT 1 2012-xx-xx "Notmuch 0.xx"
+.SH NAME
+notmuch-insert \- add a message to the maildir and notmuch database
+.SH SYNOPSIS
+
+.B notmuch insert
+.RI "[" options "]"
+.RI "[ +<" tag> "|\-<" tag "> ... ]"
+
+.SH DESCRIPTION
+
+.B notmuch insert
+reads a message from standard input
+and delivers it to the specified maildir folder,
+then incorporates the message into the notmuch database.
+It is an alternative to using a separate tool to deliver
+the message then running
+.B notmuch new
+afterwards.
+
+The new message will be tagged with the tags specified by the
+.B new.tags
+configuration option.
+Additional tagging operations may be specified on the command-line:
+tags prefixed by '+' are added while
+those prefixed by '\-' are removed.
+
+Option arguments must appear before any tag operation arguments.
+Supported options for
+.B insert
+include
+.RS 4
+.TP 4
+.BI "--folder=<" folder ">"
+
+Deliver the message to the specified folder,
+relative to the top-level directory given by the value of
+\fBdatabase.path\fR.
+The default is to deliver to the top-level directory.
+
+.RE
+
+.RS 4
+.TP 4
+.B "--create-folder"
+
+Try to create the folder named by the
+.B "--folder"
+option, if it does not exist.
+Otherwise the folder must already exist for mail
+delivery to succeed.
+
+.RE
+.SH SEE ALSO
+
+\fBnotmuch\fR(1), \fBnotmuch-config\fR(1), \fBnotmuch-count\fR(1),
+\fBnotmuch-dump\fR(1), \fBnotmuch-hooks\fR(5), \fBnotmuch-reply\fR(1),
+\fBnotmuch-restore\fR(1), \fBnotmuch-search\fR(1),
+\fBnotmuch-search-terms\fR(7), \fBnotmuch-show\fR(1),
+\fBnotmuch-tag\fR(1)
-- 
1.7.12.1

[PATCH v2 18/20] test: add tests for insert

[Peter Wang]

Add tests for new 'insert' command.
---
 test/insert   | 93 +++
 test/notmuch-test |  1 +
 2 files changed, 94 insertions(+)
 create mode 100755 test/insert

diff --git a/test/insert b/test/insert
new file mode 100755
index 000..d821a41
--- /dev/null
+++ b/test/insert
@@ -0,0 +1,93 @@
+#!/usr/bin/env bash
+test_description='"notmuch insert"'
+. ./test-lib.sh
+
+# Create directories and database before inserting.
+mkdir -p "$MAIL_DIR"/{cur,new,tmp}
+mkdir -p "$MAIL_DIR"/Drafts/{cur,new,tmp}
+notmuch new > /dev/null
+
+# We use generate_message to create the temporary message file.
+# It happens to be in the mail directory already but that is okay.
+
+test_begin_subtest "Insert message, default"
+generate_message \
+"[subject]=\"insert-subject\"" \
+"[date]=\"Sat, 01 Jan 2000 12:00:00 -\"" \
+"[body]=\"insert-message\""
+notmuch insert < "$gen_msg_filename"
+test_expect_equal "`notmuch count subject:insert-subject tag:unread`" "1"
+
+test_begin_subtest "Insert message, add tag"
+generate_message \
+"[subject]=\"insert-subject-addtag\"" \
+"[date]=\"Sat, 01 Jan 2000 12:00:00 -\"" \
+"[body]=\"insert-message-addtag\""
+notmuch insert +custom < "$gen_msg_filename"
+test_expect_equal "`notmuch count tag:custom`" "1"
+
+test_begin_subtest "Insert message, add/remove tag"
+generate_message \
+"[subject]=\"insert-subject-addrmtag\"" \
+"[date]=\"Sat, 01 Jan 2000 12:00:00 -\"" \
+"[body]=\"insert-message-addrmtag\""
+notmuch insert +custom -unread < "$gen_msg_filename"
+test_expect_equal "`notmuch count tag:custom NOT tag:unread`" "1"
+
+test_begin_subtest "Insert message, folder"
+generate_message \
+"[subject]=\"insert-subject-draft\"" \
+"[date]=\"Sat, 01 Jan 2000 12:00:00 -\"" \
+"[body]=\"insert-message-draft\""
+notmuch insert --folder=Drafts < "$gen_msg_filename"
+test_expect_equal "`notmuch count folder:Drafts`" "1"
+
+test_begin_subtest "Insert message, folder and tags"
+generate_message \
+"[subject]=\"insert-subject-draft\"" \
+"[date]=\"Sat, 01 Jan 2000 12:00:00 -\"" \
+"[body]=\"insert-message-draft\""
+notmuch insert --folder=Drafts +draft -unread < "$gen_msg_filename"
+test_expect_equal "`notmuch count folder:Drafts tag:draft NOT tag:unread`" "1"
+
+test_begin_subtest "Insert message, non-existent folder"
+generate_message \
+"[subject]=\"insert-subject-nonexistfolder\"" \
+"[date]=\"Sat, 01 Jan 2000 12:00:00 -\"" \
+"[body]=\"insert-message-nonexistfolder\""
+notmuch insert --folder=nonesuch < "$gen_msg_filename"
+test_expect_equal "$?" "1"
+
+test_begin_subtest "Insert message, create folder"
+generate_message \
+"[subject]=\"insert-subject-createfolder\"" \
+"[date]=\"Sat, 01 Jan 2000 12:00:00 -\"" \
+"[body]=\"insert-message-createfolder\""
+notmuch insert --folder=F --create-folder +folder < "$gen_msg_filename"
+test_expect_equal "`notmuch count folder:F tag:folder`" "1"
+
+test_begin_subtest "Insert message, create subfolder"
+generate_message \
+"[subject]=\"insert-subject-createfolder\"" \
+"[date]=\"Sat, 01 Jan 2000 12:00:00 -\"" \
+"[body]=\"insert-message-createfolder\""
+notmuch insert --folder=F/G/H/I/J --create-folder +folder < "$gen_msg_filename"
+test_expect_equal "`notmuch count folder:F/G/H/I/J tag:folder`" "1"
+
+test_begin_subtest "Insert message, create existing subfolder"
+generate_message \
+"[subject]=\"insert-subject-createfolder\"" \
+"[date]=\"Sat, 01 Jan 2000 12:00:00 -\"" \
+"[body]=\"insert-message-createfolder\""
+notmuch insert --folder=F/G/H/I/J --create-folder +folder < "$gen_msg_filename"
+test_expect_equal "`notmuch count folder:F/G/H/I/J tag:folder`" "2"
+
+test_begin_subtest "Insert message, create invalid subfolder"
+generate_message \
+"[subject]=\"insert-subject-createinvalidfolder\"" \
+"[date]=\"Sat, 01 Jan 2000 12:00:00 -\"" \
+"[body]=\"insert-message-createinvalidfolder\""
+notmuch insert --folder=../G --create-folder < "$gen_msg_filename"
+test_expect_equal "$?" "1"
+
+test_done
diff --git a/test/notmuch-test b/test/notmuch-test
index 9a1b375..09be44c 100755
--- a/test/notmuch-test
+++ b/test/notmuch-test
@@ -22,6 +22,7 @@ TESTS="
   config
   new
   count
+  insert
   search
   search-output
   search-by-folder
-- 
1.7.12.1

[PATCH v2 17/20] insert: add copyright line from notmuch-deliver

[Peter Wang]

The 'insert' implementation was based partly on notmuch-deliver.
---
 notmuch-insert.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/notmuch-insert.c b/notmuch-insert.c
index 28653ee..5c65582 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -2,6 +2,9 @@
  *
  * Copyright ? 2012 Peter Wang
  *
+ * Based in part on notmuch-deliver
+ * Copyright ? 2010 Ali Polatel
+ *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
  * the Free Software Foundation, either version 3 of the License, or
-- 
1.7.12.1

[PATCH v2 16/20] insert: trap SIGINT and clean up

[Peter Wang]

The only potentially long-running part of the 'insert' command should be
copying stdin to the 'tmp' file.  If SIGINT is received during the
copying process, abort and clean up the file in 'tmp'.  At all other
points, just ignore the signal and continue.
---
 notmuch-insert.c | 27 +--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/notmuch-insert.c b/notmuch-insert.c
index 831b322..28653ee 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -24,6 +24,21 @@
 #include 
 #include 

+static volatile sig_atomic_t interrupted;
+
+static void
+handle_sigint (unused (int sig))
+{
+static char msg[] = "Stopping... \n";
+
+/* This write is "opportunistic", so it's okay to ignore the
+ * result.  It is not required for correctness, and if it does
+ * fail or produce a short write, we want to get out of the signal
+ * handler as quickly as possible, not retry it. */
+IGNORE_RESULT (write (2, msg, sizeof (msg) - 1));
+interrupted = 1;
+}
+
 /* Like gethostname but guarantees that a null-terminated hostname is
  * returned, even if it has to make one up.
  * Returns true unless hostname contains a slash. */
@@ -241,7 +256,7 @@ copy_fd_data (int fdin, int fdout)
 ssize_t remain;
 ssize_t written;

-for (;;) {
+while (! interrupted) {
remain = read (fdin, buf, sizeof(buf));
if (remain == 0)
break;
@@ -270,7 +285,7 @@ copy_fd_data (int fdin, int fdout)
} while (remain > 0);
 }

-return TRUE;
+return ! interrupted;
 }

 /* Add the specified message file to the notmuch database, applying tags.
@@ -382,6 +397,7 @@ notmuch_insert_command (void *ctx, int argc, char *argv[])
 {
 notmuch_config_t *config;
 notmuch_database_t *notmuch;
+struct sigaction action;
 const char *db_path;
 const char **new_tags;
 size_t new_tags_length;
@@ -428,6 +444,13 @@ notmuch_insert_command (void *ctx, int argc, char *argv[])
return 1;
 }

+/* Setup our handler for SIGINT */
+memset (, 0, sizeof (struct sigaction));
+action.sa_handler = handle_sigint;
+sigemptyset (_mask);
+action.sa_flags = SA_RESTART;
+sigaction (SIGINT, , NULL);
+
 config = notmuch_config_open (ctx, NULL, NULL);
 if (config == NULL)
return 1;
-- 
1.7.12.1

[PATCH v2 15/20] insert: fsync new directory after rename

[Peter Wang]

After moving the file from the 'tmp' to the 'new' directory,
fsync on the 'new' directory for durability.
---
 notmuch-insert.c | 39 ---
 1 file changed, 32 insertions(+), 7 deletions(-)

diff --git a/notmuch-insert.c b/notmuch-insert.c
index f09c579..831b322 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -143,10 +143,10 @@ maildir_create_folder (void *ctx, const char *dir)
 /* Open a unique file in the Maildir 'tmp' directory.
  * Returns the file descriptor on success, or -1 on failure.
  * On success, file paths into the 'tmp' and 'new' directories are returned
- * via tmppath and newpath. */
+ * via tmppath and newpath, and the path of the 'new' directory in newdir. */
 static int
 maildir_open_tmp_file (void *ctx, const char *dir,
-  char **tmppath, char **newpath)
+  char **tmppath, char **newpath, char **newdir)
 {
 pid_t pid;
 char hostname[256];
@@ -183,8 +183,9 @@ maildir_open_tmp_file (void *ctx, const char *dir,
return -1;
 }

+*newdir = talloc_asprintf (ctx, "%s/new", dir);
 *newpath = talloc_asprintf (ctx, "%s/new/%s", dir, filename);
-if (! *newpath) {
+if (! *newdir || ! *newpath) {
fprintf (stderr, "Out of memory\n");
close (fd);
unlink (*tmppath);
@@ -204,14 +205,31 @@ maildir_open_tmp_file (void *ctx, const char *dir,
  * http://wiki.dovecot.org/MailboxFormat/Maildir#Mail_delivery
  */
 static notmuch_bool_t
-maildir_move_tmp_to_new (const char *tmppath, const char *newpath)
+maildir_move_tmp_to_new (const char *tmppath, const char *newpath,
+const char *newdir)
 {
+notmuch_bool_t ret;
+int fd;
+
 if (rename (tmppath, newpath) != 0) {
fprintf (stderr, "Error: rename() failed: %s\n", strerror (errno));
return FALSE;
 }

-return TRUE;
+/* Sync the 'new' directory after rename for durability. */
+ret = TRUE;
+fd = open (newdir, O_RDONLY);
+if (fd == -1) {
+   fprintf (stderr, "Error: open() dir failed: %s\n", strerror (errno));
+   ret = FALSE;
+}
+if (ret && fsync (fd) != 0) {
+   fprintf (stderr, "Error: fsync() dir failed: %s\n", strerror (errno));
+   ret = FALSE;
+}
+if (fd != -1)
+   close (fd);
+return ret;
 }

 /* Copy the contents of fdin into fdout. */
@@ -307,6 +325,12 @@ add_file_to_database (notmuch_database_t *notmuch, const 
char *path,

 notmuch_message_thaw (message);

+/* notmuch_message_tags_to_maildir_flags may rename the message file
+ * once more, and does so without fsyncing the directory afterwards.
+ * rename() is atomic so after a crash the file should appear under
+ * the old or new name. notmuch new should be able to rename the file
+ * again if required, so another fsync is not required, I think.
+ */
 notmuch_message_tags_to_maildir_flags (message);

 notmuch_message_destroy (message);
@@ -321,10 +345,11 @@ insert_message (void *ctx, notmuch_database_t *notmuch, 
int fdin,
 {
 char *tmppath;
 char *newpath;
+char *newdir;
 int fdout;
 notmuch_bool_t ret;

-fdout = maildir_open_tmp_file (ctx, dir, , );
+fdout = maildir_open_tmp_file (ctx, dir, , , );
 if (fdout < 0) {
return FALSE;
 }
@@ -335,7 +360,7 @@ insert_message (void *ctx, notmuch_database_t *notmuch, int 
fdin,
 }
 close (fdout);
 if (ret) {
-   ret = maildir_move_tmp_to_new (tmppath, newpath);
+   ret = maildir_move_tmp_to_new (tmppath, newpath, newdir);
 }
 if (!ret) {
unlink (tmppath);
-- 
1.7.12.1

[PATCH v2 14/20] insert: fsync after writing tmp file

[Peter Wang]

Flush the tmp file to disk after writing for durability.
---
 notmuch-insert.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/notmuch-insert.c b/notmuch-insert.c
index b7aef95..f09c579 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -329,6 +329,10 @@ insert_message (void *ctx, notmuch_database_t *notmuch, 
int fdin,
return FALSE;
 }
 ret = copy_fd_data (fdin, fdout);
+if (ret && fsync (fdout) != 0) {
+   fprintf (stderr, "Error: fsync failed: %s\n", strerror (errno));
+   ret = FALSE;
+}
 close (fdout);
 if (ret) {
ret = maildir_move_tmp_to_new (tmppath, newpath);
-- 
1.7.12.1

[PATCH v2 13/20] insert: add --create-folder option

[Peter Wang]

Support an option to create a new folder in the maildir.
---
 notmuch-insert.c | 92 
 1 file changed, 92 insertions(+)

diff --git a/notmuch-insert.c b/notmuch-insert.c
index 81a528c..b7aef95 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -55,6 +55,91 @@ check_folder_name (const char *folder)
 }
 }

+/* Make the given directory but succeed if it already exists. */
+static int
+mkdir_exists_ok (const char *path, int mode)
+{
+int ret;
+
+ret = mkdir (path, mode);
+if (ret == 0 || errno == EEXIST)
+   return 0;
+else
+   return ret;
+}
+
+/* Make the given directory including its parent directory as necessary.
+ * Return 0 on success, -1 on error. */
+static int
+mkdir_parents (char *path, int mode)
+{
+struct stat st;
+char *start;
+char *end;
+int ret;
+
+/* First check the common case: directory already exists. */
+if (stat (path, ) == 0)
+   return (S_ISDIR (st.st_mode)) ? 0 : -1;
+
+for (start = path; *start != '\0'; start = end + 1) {
+   /* start points to the first unprocessed character.
+* Find the next slash from start onwards. */
+   end = strchr (start, '/');
+
+   /* If there are no more slashes then all the parent directories
+* have been made.  Now attempt to make the whole path. */
+   if (end == NULL)
+   return mkdir_exists_ok (path, mode);
+
+   /* Make the path up to the next slash, unless the current
+* directory component is actually empty. */
+   if (end > start) {
+   *end = '\0';
+   ret = mkdir_exists_ok (path, mode);
+   *end = '/';
+   if (ret != 0)
+   return ret;
+   }
+}
+
+return 0;
+}
+
+/* Create the given maildir folder, i.e. dir and its subdirectories
+ * 'cur', 'new', 'tmp'. */
+static notmuch_bool_t
+maildir_create_folder (void *ctx, const char *dir)
+{
+const int mode = 0700;
+char *subdir;
+char *tail;
+
+/* Create 'cur' directory, including parent directories. */
+subdir = talloc_asprintf (ctx, "%s/cur", dir);
+if (! subdir) {
+   fprintf (stderr, "Out of memory.\n");
+   return FALSE;
+}
+if (mkdir_parents (subdir, mode) != 0)
+   return FALSE;
+
+tail = subdir + strlen (subdir) - 3;
+
+/* Create 'new' directory. */
+strcpy (tail, "new");
+if (mkdir_exists_ok (subdir, mode) != 0)
+   return FALSE;
+
+/* Create 'tmp' directory. */
+strcpy (tail, "tmp");
+if (mkdir_exists_ok (subdir, mode) != 0)
+   return FALSE;
+
+talloc_free (subdir);
+return TRUE;
+}
+
 /* Open a unique file in the Maildir 'tmp' directory.
  * Returns the file descriptor on success, or -1 on failure.
  * On success, file paths into the 'tmp' and 'new' directories are returned
@@ -272,6 +357,7 @@ notmuch_insert_command (void *ctx, int argc, char *argv[])
 const char **new_tags;
 size_t new_tags_length;
 const char *folder = NULL;
+notmuch_bool_t create_folder = FALSE;
 notmuch_tag_operation_t *tag_ops;
 int tag_ops_count;
 char *maildir;
@@ -280,6 +366,7 @@ notmuch_insert_command (void *ctx, int argc, char *argv[])

 notmuch_opt_desc_t options[] = {
{ NOTMUCH_OPT_STRING, , "folder", 0, 0 },
+   { NOTMUCH_OPT_BOOLEAN, _folder, "create-folder", 0, 0 },
{ NOTMUCH_OPT_END, 0, 0, 0, 0 }
 };

@@ -332,6 +419,11 @@ notmuch_insert_command (void *ctx, int argc, char *argv[])
fprintf (stderr, "Out of memory\n");
return 1;
 }
+if (create_folder && ! maildir_create_folder (ctx, maildir)) {
+   fprintf (stderr, "Error: creating maildir %s: %s\n",
+maildir, strerror (errno));
+   return 1;
+}

 if (notmuch_database_open (notmuch_config_get_database_path (config),
   NOTMUCH_DATABASE_MODE_READ_WRITE, ))
-- 
1.7.12.1

[PATCH v2 12/20] insert: apply command-line tag operations

[Peter Wang]

Apply the +tag and -tag operations specified on the command-line.
---
 notmuch-insert.c | 19 +++
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/notmuch-insert.c b/notmuch-insert.c
index e4631a2..81a528c 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -174,7 +174,8 @@ copy_fd_data (int fdin, int fdout)
  * The file is renamed to encode notmuch tags as maildir flags. */
 static notmuch_bool_t
 add_file_to_database (notmuch_database_t *notmuch, const char *path,
- const char **new_tags)
+ const char **new_tags,
+ const notmuch_tag_operation_t *tag_ops)
 {
 notmuch_message_t *message;
 notmuch_status_t status;
@@ -211,6 +212,14 @@ add_file_to_database (notmuch_database_t *notmuch, const 
char *path,
notmuch_message_add_tag (message, new_tags[i]);
 }

+/* Apply the tags specified on the command-line. */
+for (i = 0; tag_ops[i].tag; i++) {
+   if (tag_ops[i].remove)
+   notmuch_message_remove_tag (message, tag_ops[i].tag);
+   else
+   notmuch_message_add_tag (message, tag_ops[i].tag);
+}
+
 notmuch_message_thaw (message);

 notmuch_message_tags_to_maildir_flags (message);
@@ -222,7 +231,8 @@ add_file_to_database (notmuch_database_t *notmuch, const 
char *path,

 static notmuch_bool_t
 insert_message (void *ctx, notmuch_database_t *notmuch, int fdin,
-   const char *dir, const char **new_tags)
+   const char *dir, const char **new_tags,
+   const notmuch_tag_operation_t *tag_ops)
 {
 char *tmppath;
 char *newpath;
@@ -243,7 +253,7 @@ insert_message (void *ctx, notmuch_database_t *notmuch, int 
fdin,
return FALSE;
 }

-ret = add_file_to_database (notmuch, newpath, new_tags);
+ret = add_file_to_database (notmuch, newpath, new_tags, tag_ops);
 if (!ret) {
/* XXX maybe there should be an option to keep the file in maildir? */
unlink (newpath);
@@ -327,7 +337,8 @@ notmuch_insert_command (void *ctx, int argc, char *argv[])
   NOTMUCH_DATABASE_MODE_READ_WRITE, ))
return 1;

-ret = insert_message (ctx, notmuch, STDIN_FILENO, maildir, new_tags);
+ret = insert_message (ctx, notmuch, STDIN_FILENO, maildir, new_tags,
+ tag_ops);

 notmuch_database_destroy (notmuch);

-- 
1.7.12.1

[PATCH v2 11/20] insert: parse command-line tag operations

[Peter Wang]

Reuse the parser in notmuch-tag.c to parse +tag and -tag
operations on the 'insert' command-line.
---
 notmuch-insert.c | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/notmuch-insert.c b/notmuch-insert.c
index 362da66..e4631a2 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -262,6 +262,8 @@ notmuch_insert_command (void *ctx, int argc, char *argv[])
 const char **new_tags;
 size_t new_tags_length;
 const char *folder = NULL;
+notmuch_tag_operation_t *tag_ops;
+int tag_ops_count;
 char *maildir;
 int opt_index;
 notmuch_bool_t ret;
@@ -279,6 +281,27 @@ notmuch_insert_command (void *ctx, int argc, char *argv[])
return 1;
 }

+/* Array of tagging operations (add or remove), terminated with an
+ * empty element. */
+tag_ops = talloc_array (ctx, notmuch_tag_operation_t, argc - opt_index + 
1);
+if (tag_ops == NULL) {
+   fprintf (stderr, "Out of memory.\n");
+   return 1;
+}
+
+opt_index = parse_tag_operations (opt_index, argc, argv,
+ tag_ops, _ops_count);
+if (opt_index < 0)
+   return 1;
+
+tag_ops[tag_ops_count].tag = NULL;
+
+if (opt_index != argc) {
+   fprintf (stderr, "Error: bad argument to notmuch insert: %s\n",
+argv[opt_index]);
+   return 1;
+}
+
 config = notmuch_config_open (ctx, NULL, NULL);
 if (config == NULL)
return 1;
-- 
1.7.12.1

[PATCH v2 10/20] insert: apply default tags to new message

[Peter Wang]

Apply the new.tags to messages added by 'insert'.  This mirrors the
behaviour if the message were delivered by a separate tool followed by
'notmuch new'.
---
 notmuch-insert.c | 25 -
 1 file changed, 20 insertions(+), 5 deletions(-)

diff --git a/notmuch-insert.c b/notmuch-insert.c
index 022f7cd..362da66 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -170,13 +170,15 @@ copy_fd_data (int fdin, int fdout)
 return TRUE;
 }

-/* Add the specified message file to the notmuch database.
+/* Add the specified message file to the notmuch database, applying tags.
  * The file is renamed to encode notmuch tags as maildir flags. */
 static notmuch_bool_t
-add_file_to_database (notmuch_database_t *notmuch, const char *path)
+add_file_to_database (notmuch_database_t *notmuch, const char *path,
+ const char **new_tags)
 {
 notmuch_message_t *message;
 notmuch_status_t status;
+int i;

 status = notmuch_database_add_message (notmuch, path, );
 switch (status) {
@@ -201,6 +203,16 @@ add_file_to_database (notmuch_database_t *notmuch, const 
char *path)
return FALSE;
 }

+notmuch_message_freeze (message);
+
+/* Apply the new.tags, as would happen were the message added by
+ * 'notmuch new'. */
+for (i = 0; new_tags[i]; i++) {
+   notmuch_message_add_tag (message, new_tags[i]);
+}
+
+notmuch_message_thaw (message);
+
 notmuch_message_tags_to_maildir_flags (message);

 notmuch_message_destroy (message);
@@ -210,7 +222,7 @@ add_file_to_database (notmuch_database_t *notmuch, const 
char *path)

 static notmuch_bool_t
 insert_message (void *ctx, notmuch_database_t *notmuch, int fdin,
-   const char *dir)
+   const char *dir, const char **new_tags)
 {
 char *tmppath;
 char *newpath;
@@ -231,7 +243,7 @@ insert_message (void *ctx, notmuch_database_t *notmuch, int 
fdin,
return FALSE;
 }

-ret = add_file_to_database (notmuch, newpath);
+ret = add_file_to_database (notmuch, newpath, new_tags);
 if (!ret) {
/* XXX maybe there should be an option to keep the file in maildir? */
unlink (newpath);
@@ -247,6 +259,8 @@ notmuch_insert_command (void *ctx, int argc, char *argv[])
 notmuch_config_t *config;
 notmuch_database_t *notmuch;
 const char *db_path;
+const char **new_tags;
+size_t new_tags_length;
 const char *folder = NULL;
 char *maildir;
 int opt_index;
@@ -270,6 +284,7 @@ notmuch_insert_command (void *ctx, int argc, char *argv[])
return 1;

 db_path = notmuch_config_get_database_path (config);
+new_tags = notmuch_config_get_new_tags (config, _tags_length);

 if (folder != NULL) {
if (! check_folder_name (folder)) {
@@ -289,7 +304,7 @@ notmuch_insert_command (void *ctx, int argc, char *argv[])
   NOTMUCH_DATABASE_MODE_READ_WRITE, ))
return 1;

-ret = insert_message (ctx, notmuch, STDIN_FILENO, maildir);
+ret = insert_message (ctx, notmuch, STDIN_FILENO, maildir, new_tags);

 notmuch_database_destroy (notmuch);

-- 
1.7.12.1

[PATCH v2 09/20] insert: prevent writes outside Maildir hierarchy

[Peter Wang]

Don't accept a --folder name that contains a ".." component,
in order to prevent writing outside of the Maildir hierarchy.
---
 notmuch-insert.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/notmuch-insert.c b/notmuch-insert.c
index a50eacc..022f7cd 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -38,6 +38,23 @@ safe_gethostname (char *hostname, size_t len)
 return (strchr (hostname, '/') == NULL);
 }

+/* Check the specified folder name does not contain a directory
+ * component ".." to prevent writes outside of the Maildir hierarchy. */
+static notmuch_bool_t
+check_folder_name (const char *folder)
+{
+const char *p = folder;
+
+for (;;) {
+   if ((p[0] == '.') && (p[1] == '.') && (p[2] == '\0' || p[2] == '/'))
+   return FALSE;
+   p = strchr (p, '/');
+   if (!p)
+   return TRUE;
+   p++;
+}
+}
+
 /* Open a unique file in the Maildir 'tmp' directory.
  * Returns the file descriptor on success, or -1 on failure.
  * On success, file paths into the 'tmp' and 'new' directories are returned
@@ -255,6 +272,10 @@ notmuch_insert_command (void *ctx, int argc, char *argv[])
 db_path = notmuch_config_get_database_path (config);

 if (folder != NULL) {
+   if (! check_folder_name (folder)) {
+   fprintf (stderr, "Error: bad folder name: %s\n", folder);
+   return 1;
+   }
maildir = talloc_asprintf (ctx, "%s/%s", db_path, folder);
 } else {
maildir = talloc_asprintf (ctx, "%s", db_path);
-- 
1.7.12.1

[PATCH v2 08/20] insert: support --folder option

[Peter Wang]

Allow the new message to be inserted into a folder within the Maildir
hierarchy instead of the top-level folder.
---
 notmuch-insert.c | 21 -
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/notmuch-insert.c b/notmuch-insert.c
index 020dc29..a50eacc 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -230,16 +230,35 @@ notmuch_insert_command (void *ctx, int argc, char *argv[])
 notmuch_config_t *config;
 notmuch_database_t *notmuch;
 const char *db_path;
+const char *folder = NULL;
 char *maildir;
+int opt_index;
 notmuch_bool_t ret;

+notmuch_opt_desc_t options[] = {
+   { NOTMUCH_OPT_STRING, , "folder", 0, 0 },
+   { NOTMUCH_OPT_END, 0, 0, 0, 0 }
+};
+
+opt_index = parse_arguments (argc, argv, options, 1);
+
+if (opt_index < 0) {
+   fprintf (stderr, "Error: bad argument to notmuch insert: %s\n",
+argv[-opt_index]);
+   return 1;
+}
+
 config = notmuch_config_open (ctx, NULL, NULL);
 if (config == NULL)
return 1;

 db_path = notmuch_config_get_database_path (config);

-maildir = talloc_asprintf (ctx, "%s", db_path);
+if (folder != NULL) {
+   maildir = talloc_asprintf (ctx, "%s/%s", db_path, folder);
+} else {
+   maildir = talloc_asprintf (ctx, "%s", db_path);
+}
 if (! maildir) {
fprintf (stderr, "Out of memory\n");
return 1;
-- 
1.7.12.1

[PATCH v2 07/20] insert: add new message to database

[Peter Wang]

Add the new message to the notmuch database, renaming the file to encode
notmuch tags as maildir flags.
---
 notmuch-insert.c | 49 -
 1 file changed, 48 insertions(+), 1 deletion(-)

diff --git a/notmuch-insert.c b/notmuch-insert.c
index 63a04dc..020dc29 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -153,6 +153,44 @@ copy_fd_data (int fdin, int fdout)
 return TRUE;
 }

+/* Add the specified message file to the notmuch database.
+ * The file is renamed to encode notmuch tags as maildir flags. */
+static notmuch_bool_t
+add_file_to_database (notmuch_database_t *notmuch, const char *path)
+{
+notmuch_message_t *message;
+notmuch_status_t status;
+
+status = notmuch_database_add_message (notmuch, path, );
+switch (status) {
+case NOTMUCH_STATUS_SUCCESS:
+   break;
+case NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID:
+   fprintf (stderr, "Warning: duplicate message.\n");
+   break;
+default:
+case NOTMUCH_STATUS_FILE_NOT_EMAIL:
+case NOTMUCH_STATUS_READ_ONLY_DATABASE:
+case NOTMUCH_STATUS_XAPIAN_EXCEPTION:
+case NOTMUCH_STATUS_OUT_OF_MEMORY:
+case NOTMUCH_STATUS_FILE_ERROR:
+case NOTMUCH_STATUS_NULL_POINTER:
+case NOTMUCH_STATUS_TAG_TOO_LONG:
+case NOTMUCH_STATUS_UNBALANCED_FREEZE_THAW:
+case NOTMUCH_STATUS_UNBALANCED_ATOMIC:
+case NOTMUCH_STATUS_LAST_STATUS:
+   fprintf (stderr, "Error: failed to add `%s' to notmuch database: %s\n",
+path, notmuch_status_to_string (status));
+   return FALSE;
+}
+
+notmuch_message_tags_to_maildir_flags (message);
+
+notmuch_message_destroy (message);
+
+return TRUE;
+}
+
 static notmuch_bool_t
 insert_message (void *ctx, notmuch_database_t *notmuch, int fdin,
const char *dir)
@@ -173,8 +211,17 @@ insert_message (void *ctx, notmuch_database_t *notmuch, 
int fdin,
 }
 if (!ret) {
unlink (tmppath);
+   return FALSE;
 }
-return ret;
+
+ret = add_file_to_database (notmuch, newpath);
+if (!ret) {
+   /* XXX maybe there should be an option to keep the file in maildir? */
+   unlink (newpath);
+   return FALSE;
+}
+
+return TRUE;
 }

 int
-- 
1.7.12.1

[PATCH v2 06/20] insert: move file from Maildir tmp to new

[Peter Wang]

Atomically move the new message file from the Maildir 'tmp' directory
to 'new'.
---
 notmuch-insert.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/notmuch-insert.c b/notmuch-insert.c
index 88e8533..63a04dc 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -94,6 +94,24 @@ maildir_open_tmp_file (void *ctx, const char *dir,
 return fd;
 }

+/* Atomically move the new message file from the Maildir 'tmp' directory
+ * to the 'new' directory.
+ *
+ * We follow the Dovecot recommendation to simply use rename()
+ * instead of link() and unlink().  See also:
+ * http://wiki.dovecot.org/MailboxFormat/Maildir#Mail_delivery
+ */
+static notmuch_bool_t
+maildir_move_tmp_to_new (const char *tmppath, const char *newpath)
+{
+if (rename (tmppath, newpath) != 0) {
+   fprintf (stderr, "Error: rename() failed: %s\n", strerror (errno));
+   return FALSE;
+}
+
+return TRUE;
+}
+
 /* Copy the contents of fdin into fdout. */
 static notmuch_bool_t
 copy_fd_data (int fdin, int fdout)
@@ -150,6 +168,9 @@ insert_message (void *ctx, notmuch_database_t *notmuch, int 
fdin,
 }
 ret = copy_fd_data (fdin, fdout);
 close (fdout);
+if (ret) {
+   ret = maildir_move_tmp_to_new (tmppath, newpath);
+}
 if (!ret) {
unlink (tmppath);
 }
-- 
1.7.12.1

[PATCH v2 05/20] insert: copy stdin to Maildir tmp file

[Peter Wang]

Read the new message from standard input into the Maildir tmp file.
---
 notmuch-insert.c | 51 +++
 1 file changed, 47 insertions(+), 4 deletions(-)

diff --git a/notmuch-insert.c b/notmuch-insert.c
index 371fb47..88e8533 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -94,6 +94,47 @@ maildir_open_tmp_file (void *ctx, const char *dir,
 return fd;
 }

+/* Copy the contents of fdin into fdout. */
+static notmuch_bool_t
+copy_fd_data (int fdin, int fdout)
+{
+char buf[4096];
+char *p;
+ssize_t remain;
+ssize_t written;
+
+for (;;) {
+   remain = read (fdin, buf, sizeof(buf));
+   if (remain == 0)
+   break;
+   if (remain < 0) {
+   if (errno == EINTR)
+   continue;
+   fprintf (stderr, "Error: reading from standard input: %s\n",
+strerror (errno));
+   return FALSE;
+   }
+
+   p = buf;
+   do {
+   written = write (fdout, p, remain);
+   if (written == 0)
+   return FALSE;
+   if (written < 0) {
+   if (errno == EINTR)
+   continue;
+   fprintf (stderr, "Error: writing to temporary file: %s",
+strerror (errno));
+   return FALSE;
+   }
+   p += written;
+   remain -= written;
+   } while (remain > 0);
+}
+
+return TRUE;
+}
+
 static notmuch_bool_t
 insert_message (void *ctx, notmuch_database_t *notmuch, int fdin,
const char *dir)
@@ -101,16 +142,18 @@ insert_message (void *ctx, notmuch_database_t *notmuch, 
int fdin,
 char *tmppath;
 char *newpath;
 int fdout;
+notmuch_bool_t ret;

 fdout = maildir_open_tmp_file (ctx, dir, , );
 if (fdout < 0) {
return FALSE;
 }
-
-/* For now we just delete the tmp file immediately. */
+ret = copy_fd_data (fdin, fdout);
 close (fdout);
-unlink (tmppath);
-return FALSE;
+if (!ret) {
+   unlink (tmppath);
+}
+return ret;
 }

 int
-- 
1.7.12.1

[PATCH v2 04/20] insert: open Maildir tmp file

[Peter Wang]

Open a unique file in the Maildir tmp directory.
The new message is not yet copied into the file.
---
 notmuch-insert.c | 105 ++-
 1 file changed, 104 insertions(+), 1 deletion(-)

diff --git a/notmuch-insert.c b/notmuch-insert.c
index 21424cf..371fb47 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -20,12 +20,107 @@

 #include "notmuch-client.h"

+#include 
+#include 
+#include 
+
+/* Like gethostname but guarantees that a null-terminated hostname is
+ * returned, even if it has to make one up.
+ * Returns true unless hostname contains a slash. */
+static notmuch_bool_t
+safe_gethostname (char *hostname, size_t len)
+{
+if (gethostname (hostname, len) == -1) {
+   strncpy (hostname, "unknown", len);
+}
+hostname[len - 1] = '\0';
+
+return (strchr (hostname, '/') == NULL);
+}
+
+/* Open a unique file in the Maildir 'tmp' directory.
+ * Returns the file descriptor on success, or -1 on failure.
+ * On success, file paths into the 'tmp' and 'new' directories are returned
+ * via tmppath and newpath. */
+static int
+maildir_open_tmp_file (void *ctx, const char *dir,
+  char **tmppath, char **newpath)
+{
+pid_t pid;
+char hostname[256];
+struct timeval tv;
+char *filename;
+int fd = -1;
+
+/* We follow the Dovecot file name generation algorithm. */
+pid = getpid ();
+if (! safe_gethostname (hostname, sizeof (hostname))) {
+   fprintf (stderr, "Error: invalid host name.\n");
+   return -1;
+}
+do {
+   gettimeofday (, NULL);
+   filename = talloc_asprintf (ctx, "%ld.M%ldP%d.%s",
+   tv.tv_sec, tv.tv_usec, pid, hostname);
+   if (! filename) {
+   fprintf (stderr, "Out of memory\n");
+   return -1;
+   }
+
+   *tmppath = talloc_asprintf (ctx, "%s/tmp/%s", dir, filename);
+   if (! *tmppath) {
+   fprintf (stderr, "Out of memory\n");
+   return -1;
+   }
+
+   fd = open (*tmppath, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL, 0600);
+} while (fd == -1 && errno == EEXIST);
+
+if (fd == -1) {
+   fprintf (stderr, "Error: opening %s: %s\n", *tmppath, strerror (errno));
+   return -1;
+}
+
+*newpath = talloc_asprintf (ctx, "%s/new/%s", dir, filename);
+if (! *newpath) {
+   fprintf (stderr, "Out of memory\n");
+   close (fd);
+   unlink (*tmppath);
+   return -1;
+}
+
+talloc_free (filename);
+
+return fd;
+}
+
+static notmuch_bool_t
+insert_message (void *ctx, notmuch_database_t *notmuch, int fdin,
+   const char *dir)
+{
+char *tmppath;
+char *newpath;
+int fdout;
+
+fdout = maildir_open_tmp_file (ctx, dir, , );
+if (fdout < 0) {
+   return FALSE;
+}
+
+/* For now we just delete the tmp file immediately. */
+close (fdout);
+unlink (tmppath);
+return FALSE;
+}
+
 int
 notmuch_insert_command (void *ctx, int argc, char *argv[])
 {
 notmuch_config_t *config;
 notmuch_database_t *notmuch;
 const char *db_path;
+char *maildir;
+notmuch_bool_t ret;

 config = notmuch_config_open (ctx, NULL, NULL);
 if (config == NULL)
@@ -33,11 +128,19 @@ notmuch_insert_command (void *ctx, int argc, char *argv[])

 db_path = notmuch_config_get_database_path (config);

+maildir = talloc_asprintf (ctx, "%s", db_path);
+if (! maildir) {
+   fprintf (stderr, "Out of memory\n");
+   return 1;
+}
+
 if (notmuch_database_open (notmuch_config_get_database_path (config),
   NOTMUCH_DATABASE_MODE_READ_WRITE, ))
return 1;

+ret = insert_message (ctx, notmuch, STDIN_FILENO, maildir);
+
 notmuch_database_destroy (notmuch);

-return 1;
+return (ret) ? 0 : 1;
 }
-- 
1.7.12.1

[PATCH v2 03/20] cli: add stub for insert command

[Peter Wang]

The notmuch insert command should read a message from standard input
and deliver it to a Maildir folder, and then incorporate the message
into the notmuch database.  Essentially it moves the functionality of
notmuch-deliver into notmuch.

Though it could be used as an alternative to notmuch new, the reason
I want this is to allow my notmuch frontend to add postponed or sent
messages to the mail store and notmuch database, without resorting to
another tool (e.g. notmuch-deliver) nor directly modifying the maildir.
---
 Makefile.local   |  1 +
 notmuch-client.h |  3 +++
 notmuch-insert.c | 43 +++
 notmuch.c|  3 +++
 4 files changed, 50 insertions(+)
 create mode 100644 notmuch-insert.c

diff --git a/Makefile.local b/Makefile.local
index 2b91946..6bbd588 100644
--- a/Makefile.local
+++ b/Makefile.local
@@ -261,6 +261,7 @@ notmuch_client_srcs =   \
notmuch-config.c\
notmuch-count.c \
notmuch-dump.c  \
+   notmuch-insert.c\
notmuch-new.c   \
notmuch-reply.c \
notmuch-restore.c   \
diff --git a/notmuch-client.h b/notmuch-client.h
index a7c3df2..c0d0e93 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -135,6 +135,9 @@ int
 notmuch_dump_command (void *ctx, int argc, char *argv[]);

 int
+notmuch_insert_command (void *ctx, int argc, char *argv[]);
+
+int
 notmuch_new_command (void *ctx, int argc, char *argv[]);

 int
diff --git a/notmuch-insert.c b/notmuch-insert.c
new file mode 100644
index 000..21424cf
--- /dev/null
+++ b/notmuch-insert.c
@@ -0,0 +1,43 @@
+/* notmuch - Not much of an email program, (just index and search)
+ *
+ * Copyright ? 2012 Peter Wang
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see http://www.gnu.org/licenses/ .
+ *
+ * Author: Peter Wang 
+ */
+
+#include "notmuch-client.h"
+
+int
+notmuch_insert_command (void *ctx, int argc, char *argv[])
+{
+notmuch_config_t *config;
+notmuch_database_t *notmuch;
+const char *db_path;
+
+config = notmuch_config_open (ctx, NULL, NULL);
+if (config == NULL)
+   return 1;
+
+db_path = notmuch_config_get_database_path (config);
+
+if (notmuch_database_open (notmuch_config_get_database_path (config),
+  NOTMUCH_DATABASE_MODE_READ_WRITE, ))
+   return 1;
+
+notmuch_database_destroy (notmuch);
+
+return 1;
+}
diff --git a/notmuch.c b/notmuch.c
index 477a09c..86239fd 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -53,6 +53,9 @@ static command_t commands[] = {
 { "new", notmuch_new_command,
   "[options...]",
   "Find and import new messages to the notmuch database." },
+{ "insert", notmuch_insert_command,
+  "[options...] [--] [+|- ...] < message",
+  "Add a new message into the maildir and notmuch database." },
 { "search", notmuch_search_command,
   "[options...]  [...]",
   "Search for messages matching the given search terms." },
-- 
1.7.12.1

[PATCH v2 02/20] tag: make tag operation parser public

[Peter Wang]

Make the tag operation parser accessible outside notmuch-tag.c
so it can be reused by the upcoming insert command.
---
 notmuch-client.h |  9 +
 notmuch-tag.c| 17 ++---
 2 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index ae9344b..a7c3df2 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -65,6 +65,11 @@ typedef GMimeCipherContext notmuch_crypto_context_t;
 #define STRINGIFY(s) STRINGIFY_(s)
 #define STRINGIFY_(s) #s

+typedef struct {
+const char *tag;
+notmuch_bool_t remove;
+} notmuch_tag_operation_t;
+
 typedef struct mime_node mime_node_t;
 struct sprinter;
 struct notmuch_show_params;
@@ -159,6 +164,10 @@ notmuch_cat_command (void *ctx, int argc, char *argv[]);
 int
 notmuch_config_command (void *ctx, int argc, char *argv[]);

+int
+parse_tag_operations (int i, int argc, char *argv[],
+ notmuch_tag_operation_t *tag_ops, int *tag_ops_count);
+
 const char *
 notmuch_time_relative_date (const void *ctx, time_t then);

diff --git a/notmuch-tag.c b/notmuch-tag.c
index 35a76db..831a0e4 100644
--- a/notmuch-tag.c
+++ b/notmuch-tag.c
@@ -54,14 +54,9 @@ _escape_tag (char *buf, const char *tag)
 return buf;
 }

-typedef struct {
-const char *tag;
-notmuch_bool_t remove;
-} tag_operation_t;
-
 static char *
 _optimize_tag_query (void *ctx, const char *orig_query_string,
-const tag_operation_t *tag_ops)
+const notmuch_tag_operation_t *tag_ops)
 {
 /* This is subtler than it looks.  Xapian ignores the '-' operator
  * at the beginning both queries and parenthesized groups and,
@@ -116,7 +111,7 @@ _optimize_tag_query (void *ctx, const char 
*orig_query_string,
  * element. */
 static int
 tag_query (void *ctx, notmuch_database_t *notmuch, const char *query_string,
-  tag_operation_t *tag_ops, notmuch_bool_t synchronize_flags)
+  notmuch_tag_operation_t *tag_ops, notmuch_bool_t synchronize_flags)
 {
 notmuch_query_t *query;
 notmuch_messages_t *messages;
@@ -170,9 +165,9 @@ tag_query (void *ctx, notmuch_database_t *notmuch, const 
char *query_string,
 /* Parse +tag and -tag operations between argv[i] and argv[argc-1].
  * The array tag_ops must be at least argc - i elements long.
  * Returns the index into argv where parsing stopped, or -1 on error. */
-static int
+int
 parse_tag_operations (int i, int argc, char *argv[],
- tag_operation_t *tag_ops, int *tag_ops_count)
+ notmuch_tag_operation_t *tag_ops, int *tag_ops_count)
 {
 *tag_ops_count = 0;

@@ -207,7 +202,7 @@ parse_tag_operations (int i, int argc, char *argv[],
 int
 notmuch_tag_command (void *ctx, int argc, char *argv[])
 {
-tag_operation_t *tag_ops;
+notmuch_tag_operation_t *tag_ops;
 int tag_ops_count;
 char *query_string;
 notmuch_config_t *config;
@@ -228,7 +223,7 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])

 /* Array of tagging operations (add or remove), terminated with an
  * empty element. */
-tag_ops = talloc_array (ctx, tag_operation_t, argc + 1);
+tag_ops = talloc_array (ctx, notmuch_tag_operation_t, argc + 1);
 if (tag_ops == NULL) {
fprintf (stderr, "Out of memory.\n");
return 1;
-- 
1.7.12.1

[PATCH v2 01/20] tag: factor out tag operation parsing

[Peter Wang]

Factor out parsing of +tag, -tag operations from argv
into a separate function.
---
 notmuch-tag.c | 66 +--
 1 file changed, 41 insertions(+), 25 deletions(-)

diff --git a/notmuch-tag.c b/notmuch-tag.c
index 88d559b..35a76db 100644
--- a/notmuch-tag.c
+++ b/notmuch-tag.c
@@ -167,11 +167,48 @@ tag_query (void *ctx, notmuch_database_t *notmuch, const 
char *query_string,
 return interrupted;
 }

+/* Parse +tag and -tag operations between argv[i] and argv[argc-1].
+ * The array tag_ops must be at least argc - i elements long.
+ * Returns the index into argv where parsing stopped, or -1 on error. */
+static int
+parse_tag_operations (int i, int argc, char *argv[],
+ tag_operation_t *tag_ops, int *tag_ops_count)
+{
+*tag_ops_count = 0;
+
+for (; i < argc; i++) {
+   if (strcmp (argv[i], "--") == 0) {
+   i++;
+   break;
+   }
+   if (argv[i][0] == '+' || argv[i][0] == '-') {
+   if (argv[i][0] == '+' && argv[i][1] == '\0') {
+   fprintf (stderr, "Error: tag names cannot be empty.\n");
+   return -1;
+   }
+   if (argv[i][0] == '+' && argv[i][1] == '-') {
+   /* This disallows adding the non-removable tag "-" and
+* enables notmuch tag to take long options in the
+* future. */
+   fprintf (stderr, "Error: tag names must not start with '-'.\n");
+   return -1;
+   }
+   tag_ops[*tag_ops_count].tag = argv[i] + 1;
+   tag_ops[*tag_ops_count].remove = (argv[i][0] == '-');
+   (*tag_ops_count)++;
+   } else {
+   break;
+   }
+}
+
+return i;
+}
+
 int
 notmuch_tag_command (void *ctx, int argc, char *argv[])
 {
 tag_operation_t *tag_ops;
-int tag_ops_count = 0;
+int tag_ops_count;
 char *query_string;
 notmuch_config_t *config;
 notmuch_database_t *notmuch;
@@ -197,30 +234,9 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
return 1;
 }

-for (i = 0; i < argc; i++) {
-   if (strcmp (argv[i], "--") == 0) {
-   i++;
-   break;
-   }
-   if (argv[i][0] == '+' || argv[i][0] == '-') {
-   if (argv[i][0] == '+' && argv[i][1] == '\0') {
-   fprintf (stderr, "Error: tag names cannot be empty.\n");
-   return 1;
-   }
-   if (argv[i][0] == '+' && argv[i][1] == '-') {
-   /* This disallows adding the non-removable tag "-" and
-* enables notmuch tag to take long options in the
-* future. */
-   fprintf (stderr, "Error: tag names must not start with '-'.\n");
-   return 1;
-   }
-   tag_ops[tag_ops_count].tag = argv[i] + 1;
-   tag_ops[tag_ops_count].remove = (argv[i][0] == '-');
-   tag_ops_count++;
-   } else {
-   break;
-   }
-}
+i = parse_tag_operations (0, argc, argv, tag_ops, _ops_count);
+if (i < 0)
+   return 1;

 tag_ops[tag_ops_count].tag = NULL;

-- 
1.7.12.1

[PATCH v2 00/20] insert command

[Peter Wang]

This series mainly addresses the issues raised by Mark:

- check talloc failures
- deadlock in maildir_open_tmp
- stricter file modes (0600 and 0700)
- shared tag operation parser with notmuch-tag.c
- simplified mkdir_parents
- trap SIGINT
- fsync after writing and rename
- added a couple of tests
- man page wording
- comments

Due to new restriction on tags beginning with '-', an argument beginning with
"--" is no longer ambiguous so I have removed the optional "--" separator
between options and tag operations.

Peter Wang (20):
  tag: factor out tag operation parsing
  tag: make tag operation parser public
  cli: add stub for insert command
  insert: open Maildir tmp file
  insert: copy stdin to Maildir tmp file
  insert: move file from Maildir tmp to new
  insert: add new message to database
  insert: support --folder option
  insert: prevent writes outside Maildir hierarchy
  insert: apply default tags to new message
  insert: parse command-line tag operations
  insert: apply command-line tag operations
  insert: add --create-folder option
  insert: fsync after writing tmp file
  insert: fsync new directory after rename
  insert: trap SIGINT and clean up
  insert: add copyright line from notmuch-deliver
  test: add tests for insert
  man: document 'insert' command
  man: reference notmuch-insert.1

 Makefile.local  |   1 +
 man/Makefile.local  |   1 +
 man/man1/notmuch-config.1   |   4 +-
 man/man1/notmuch-count.1|   4 +-
 man/man1/notmuch-dump.1 |   4 +-
 man/man1/notmuch-insert.1   |  60 +
 man/man1/notmuch-new.1  |   4 +-
 man/man1/notmuch-reply.1|   3 +-
 man/man1/notmuch-restore.1  |   3 +-
 man/man1/notmuch-search.1   |   3 +-
 man/man1/notmuch-show.1 |   3 +-
 man/man1/notmuch-tag.1  |   3 +-
 man/man1/notmuch.1  |   3 +-
 man/man5/notmuch-hooks.5|   4 +-
 man/man7/notmuch-search-terms.7 |   3 +-
 notmuch-client.h|  12 +
 notmuch-insert.c| 493 
 notmuch-tag.c   |  79 ---
 notmuch.c   |   3 +
 test/insert |  93 
 test/notmuch-test   |   1 +
 21 files changed, 733 insertions(+), 51 deletions(-)
 create mode 100644 man/man1/notmuch-insert.1
 create mode 100644 notmuch-insert.c
 create mode 100755 test/insert

-- 
1.7.12.1

[PATCH 0/2] Ignore ignored broken symlinks

[Mark Walters]

On Sun, 25 Nov 2012, Tomi Ollila  wrote:
> On Sun, Nov 25 2012, Austin Clements  wrote:
>
>> calmar on IRC reported a bug today where notmuch will abort when it
>> encounters a broken symlink, even if that symlink is in the
>> user-specified ignore list.  These two patches test for this bug and
>> fix it.
>
> +1


+1 from me too

Mark

[PATCH 1/3] test: Test notmuch new for single-message mbox

[Mark Walters]

This series looks fine to me. +1

Mark

On Sun, 25 Nov 2012, Austin Clements  wrote:
> We support this for historical reasons.
> ---
>  test/new |   13 +
>  1 file changed, 13 insertions(+)
>
> diff --git a/test/new b/test/new
> index 587aa11..43d56e4 100755
> --- a/test/new
> +++ b/test/new
> @@ -163,6 +163,19 @@ rm -rf "${MAIL_DIR}"/two
>  output=$(NOTMUCH_NEW)
>  test_expect_equal "$output" "No new mail. Removed 3 messages."
>  
> +test_begin_subtest "Support single-message mbox"
> +cat > "${MAIL_DIR}"/mbox_file1 < +From test_suite at notmuchmail.org Fri Jan  5 15:43:57 2001
> +From: Notmuch Test Suite 
> +To: Notmuch Test Suite 
> +Subject: Test mbox message 1
> +
> +Body.
> +EOF
> +output=$(NOTMUCH_NEW 2>&1)
> +test_expect_equal "$output" \
> +"Added 1 new message to the database."
> +
>  # This test requires that notmuch new has been run at least once.
>  test_begin_subtest "Skip and report non-mail files"
>  generate_message
> -- 
> 1.7.10.4
>
> ___
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH v3 1/2] notmuch-show.el: import calendar data with public function after CR removal

[David Bremner]

Tomi Ollila  writes:

> notmuch-get-bodypart-content provides raw data to its caller so
> that it can be stored verbatim whenever needed. icalendar functions
> expect Emacs to do EOL conversion for the data given to these. Therefore
> it the CRLF -> LF conversion is now done explicitly.

pushed both

d

[Patch v4 1/2] test: factor out part of test-lib.sh into test-lib-common.sh

[da...@tethera.net]

From: David Bremner 

The idea is to use some of the simpler parts of the test suite
infrastructure to help run performance tests.
---
 test/test-lib-common.sh |  147 +++
 test/test-lib.sh|  129 +
 2 files changed, 148 insertions(+), 128 deletions(-)
 create mode 100644 test/test-lib-common.sh

diff --git a/test/test-lib-common.sh b/test/test-lib-common.sh
new file mode 100644
index 000..e1eaa5a
--- /dev/null
+++ b/test/test-lib-common.sh
@@ -0,0 +1,147 @@
+#
+# Copyright (c) 2005 Junio C Hamano
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see http://www.gnu.org/licenses/ .
+
+# This file contains common code to be used by both the regular
+# (correctness) tests and the performance tests.
+
+find_notmuch_path ()
+{
+dir="$1"
+
+while [ -n "$dir" ]; do
+   bin="$dir/notmuch"
+   if [ -x "$bin" ]; then
+   echo "$dir"
+   return
+   fi
+   dir="$(dirname "$dir")"
+   if [ "$dir" = "/" ]; then
+   break
+   fi
+done
+}
+
+# Test the binaries we have just built.  The tests are kept in
+# test/ subdirectory and are run in 'trash directory' subdirectory.
+TEST_DIRECTORY=$(pwd)
+notmuch_path=`find_notmuch_path "$TEST_DIRECTORY"`
+if test -n "$valgrind"
+then
+   make_symlink () {
+   test -h "$2" &&
+   test "$1" = "$(readlink "$2")" || {
+   # be super paranoid
+   if mkdir "$2".lock
+   then
+   rm -f "$2" &&
+   ln -s "$1" "$2" &&
+   rm -r "$2".lock
+   else
+   while test -d "$2".lock
+   do
+   say "Waiting for lock on $2."
+   sleep 1
+   done
+   fi
+   }
+   }
+
+   make_valgrind_symlink () {
+   # handle only executables
+   test -x "$1" || return
+
+   base=$(basename "$1")
+   symlink_target=$TEST_DIRECTORY/../$base
+   # do not override scripts
+   if test -x "$symlink_target" &&
+   test ! -d "$symlink_target" &&
+   test "#!" != "$(head -c 2 < "$symlink_target")"
+   then
+   symlink_target=$TEST_DIRECTORY/valgrind.sh
+   fi
+   case "$base" in
+   *.sh|*.perl)
+   symlink_target=$TEST_DIRECTORY/unprocessed-script
+   esac
+   # create the link, or replace it if it is out of date
+   make_symlink "$symlink_target" "$GIT_VALGRIND/bin/$base" || exit
+   }
+
+   # override notmuch executable in TEST_DIRECTORY/..
+   GIT_VALGRIND=$TEST_DIRECTORY/valgrind
+   mkdir -p "$GIT_VALGRIND"/bin
+   make_valgrind_symlink $TEST_DIRECTORY/../notmuch
+   OLDIFS=$IFS
+   IFS=:
+   for path in $PATH
+   do
+   ls "$path"/notmuch 2> /dev/null |
+   while read file
+   do
+   make_valgrind_symlink "$file"
+   done
+   done
+   IFS=$OLDIFS
+   PATH=$GIT_VALGRIND/bin:$PATH
+   GIT_EXEC_PATH=$GIT_VALGRIND/bin
+   export GIT_VALGRIND
+   test -n "$notmuch_path" && MANPATH="$notmuch_path/man:$MANPATH"
+else # normal case
+   if test -n "$notmuch_path"
+   then
+   PATH="$notmuch_path:$PATH"
+   MANPATH="$notmuch_path/man:$MANPATH"
+   fi
+fi
+export PATH MANPATH
+
+# Test repository
+test="tmp.$(basename "$0" .sh)"
+test -n "$root" && test="$root/$test"
+case "$test" in
+/*) TMP_DIRECTORY="$test" ;;
+ *) TMP_DIRECTORY="$TEST_DIRECTORY/$test" ;;
+esac
+test ! -z "$debug" || remove_tmp=$TMP_DIRECTORY
+rm -fr "$test" || {
+   GIT_EXIT_OK=t
+   echo >&5 "FATAL: Cannot prepare test area"
+   exit 1
+}
+
+# A temporary home directory is needed by at least:
+# - emacs/"Sending a message via (fake) SMTP"
+# - emacs/"Reply within emacs"
+# - crypto/emacs_deliver_message
+export HOME="${TMP_DIRECTORY}/home"
+mkdir -p "${HOME}"
+
+MAIL_DIR="${TMP_DIRECTORY}/mail"
+export 

v4 of Performance tests

[da...@tethera.net]

This version adds an optional makefile target to download the corpus.

Tomi reviewed it on IRC and didn't complain.

[Patch v2 09/17] tag-util.[ch]: New files for common tagging routines

[Tomi Ollila]

On Sat, Nov 24 2012, david at tethera.net wrote:

> From: David Bremner 
>
> These are meant to be shared between notmuch-tag and notmuch-restore.
>
> The bulk of the routines implement a "tag operation list" abstract
> data type act as a structured representation of a set of tag
> operations (typically coming from a single tag command or line of
> input).
> ---

dumping all except beginning of tag-util.h...

>  tag-util.h |  120 ++
> diff --git a/tag-util.h b/tag-util.h
> new file mode 100644
> index 000..508806f
> --- /dev/null
> +++ b/tag-util.h
> @@ -0,0 +1,120 @@
> +#ifndef _TAG_UTIL_H
> +#define _TAG_UTIL_H
> +
> +#include "notmuch-client.h"
> +
> +typedef struct _tag_operation_t tag_operation_t;
> +typedef struct _tag_op_list_t tag_op_list_t;
> +
> +#define TAG_OP_LIST_INITIAL_SIZE 10
> +
> +/* Use powers of 2 */
> +typedef enum  { TAG_FLAG_NONE = 0,
> + /* Operations are synced to maildir, if possible */
> +
> + TAG_FLAG_MAILDIR_SYNC = 1,
> +
> + /* Remove all tags from message before applying
> +  * list */
> +
> + TAG_FLAG_REMOVE_ALL = 2,
> +
> + /* Don't try to avoid database operations.  Useful
> +  * when we know that message passed needs these
> +  *  operations. */
> +
> + TAG_FLAG_PRE_OPTIMIZED = 4,
> +
> + /* Accept strange tags that might be user error;
> +intended for use by notmuch-restore.
> + */
> +
> + TAG_FLAG_BE_GENEROUS = 8} tag_op_flag_t;
> +

Maybe something like the following formatted and consistency-tuned version:

typedef enum { 
   TAG_FLAG_NONE = 0,

   /* Operations are synced to maildir, if possible.
*/
   TAG_FLAG_MAILDIR_SYNC = (1 << 0),

   /* Remove all tags from message before applying list.
*/
   TAG_FLAG_REMOVE_ALL = (1 << 1),

   /* Don't try to avoid database operations. Useful when we
* know that message passed needs these operations.
*/
   TAG_FLAG_PRE_OPTIMIZED = (1 << 2),

   /* Accept strange tags that might be user error;
* intended for use by notmuch-restore.
*/
   TAG_FLAG_BE_GENEROUS = (1 << 3)

} tag_op_flag_t;

Tomi

[Patch v2 14/17] test: update dump-restore roundtripping test for batch-tag format

[Tomi Ollila]

On Sat, Nov 24 2012, david at tethera.net wrote:

> From: David Bremner 
>
> Now we can actually round trip these crazy tags and and message ids.
> hex-xcode is no longer needed as it's built in.
> ---
>  test/dump-restore |   13 +++--
>  1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/test/dump-restore b/test/dump-restore
> index a2204fb..e08b656 100755
> --- a/test/dump-restore
> +++ b/test/dump-restore
> @@ -85,13 +85,14 @@ test_begin_subtest "dump --output=outfile -- from:cworth"
>  notmuch dump --output=dump-outfile-dash-inbox.actual -- from:cworth
>  test_expect_equal_file dump-cworth.expected dump-outfile-dash-inbox.actual
>  
> +
>  test_expect_success 'roundtripping random message-ids and tags' \
> -'test_subtest_known_broken &&
> - ${TEST_DIRECTORY}/random-corpus --num-messages=10 
> --config-path=${NOTMUCH_CONFIG} &&
> - notmuch dump | ${TEST_DIRECTORY}/hex-xcode --direction=encode > 
> EXPECTED.$test_count &&
> +'${TEST_DIRECTORY}/random-corpus --num-messages=100 
> --config-path=${NOTMUCH_CONFIG} &&
> + notmuch dump --format=batch-tag | sort > EXPECTED.$test_count &&
>   notmuch tag -random-corpus tag:random-corpus &&
> - ${TEST_DIRECTORY}/hex-xcode --direction=decode < EXPECTED.$test_count | 
> notmuch restore 2>/dev/null &&
> - notmuch dump | ${TEST_DIRECTORY}/hex-xcode --direction=encode > 
> OUTPUT.$test_count &&
> + notmuch restore --format=batch-tag --input=EXPECTED.$test_count &&
> + notmuch dump --format=batch-tag | sort > OUTPUT.$test_count &&
>   test_cmp EXPECTED.$test_count OUTPUT.$test_count'
> -
>  test_done

In most cases there is empty line before `test_done` -- also other 
unrelated whitespace changes disturb review (especially when one is
tired ;)

> +
> +# Note the database is "poisoned" for sup format at this point.
> -- 
> 1.7.10.4

Tomi

[PATCH 3/3] lib: Reject multi-message mboxes and deprecate single-message mbox

[Austin Clements]

Previously, we would treat multi-message mboxes as one giant email,
which, besides the obvious incorrect indexing, often led to
out-of-memory errors for archival mboxes.  Now we explicitly reject
multi-message mboxes.  For historical reasons, we retain support for
single-message mboxes, but official deprecate this behavior.
---
 lib/database.cc |4 +++-
 lib/index.cc|   28 
 test/new|8 +---
 3 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 4df3217..91d4329 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -1821,7 +1821,9 @@ notmuch_database_add_message (notmuch_database_t *notmuch,
date = notmuch_message_file_get_header (message_file, "date");
_notmuch_message_set_header_values (message, date, from, subject);

-   _notmuch_message_index_file (message, filename);
+   ret = _notmuch_message_index_file (message, filename);
+   if (ret)
+   goto DONE;
} else {
ret = NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID;
}
diff --git a/lib/index.cc b/lib/index.cc
index e377732..da0e6ce 100644
--- a/lib/index.cc
+++ b/lib/index.cc
@@ -435,6 +435,9 @@ _notmuch_message_index_file (notmuch_message_t *message,
 const char *from, *subject;
 notmuch_status_t ret = NOTMUCH_STATUS_SUCCESS;
 static int initialized = 0;
+char from_buf[5];
+bool is_mbox = false;
+static bool mbox_warning = false;

 if (! initialized) {
g_mime_init (0);
@@ -448,13 +451,38 @@ _notmuch_message_index_file (notmuch_message_t *message,
goto DONE;
 }

+/* Is this mbox? */
+if (fread (from_buf, sizeof (from_buf), 1, file) == 1 &&
+   strncmp (from_buf, "From ", 5) == 0)
+   is_mbox = true;
+rewind (file);
+
 /* Evil GMime steals my FILE* here so I won't fclose it. */
 stream = g_mime_stream_file_new (file);

 parser = g_mime_parser_new_with_stream (stream);
+g_mime_parser_set_scan_from (parser, is_mbox);

 mime_message = g_mime_parser_construct_message (parser);

+if (is_mbox) {
+   if (!g_mime_parser_eos (parser)) {
+   /* This is a multi-message mbox. */
+   ret = NOTMUCH_STATUS_FILE_NOT_EMAIL;
+   goto DONE;
+   }
+   /* For historical reasons, we support single-message mboxes,
+* but this behavior is likely to change in the future, so
+* warn. */
+   if (!mbox_warning) {
+   mbox_warning = true;
+   fprintf (stderr, "\
+Warning: %s is an mbox containing a single message,\n\
+likely caused by misconfigured mail delivery.  Support for single-message\n\
+mboxes is deprecated and may be removed in the future.\n", filename);
+   }
+}
+
 from = g_mime_message_get_sender (mime_message);
 addresses = internet_address_list_parse_string (from);

diff --git a/test/new b/test/new
index 29f9aff..f562cec 100755
--- a/test/new
+++ b/test/new
@@ -163,7 +163,7 @@ rm -rf "${MAIL_DIR}"/two
 output=$(NOTMUCH_NEW)
 test_expect_equal "$output" "No new mail. Removed 3 messages."

-test_begin_subtest "Support single-message mbox"
+test_begin_subtest "Support single-message mbox (deprecated)"
 cat > "${MAIL_DIR}"/mbox_file1 <
@@ -174,11 +174,13 @@ Body.
 EOF
 output=$(NOTMUCH_NEW 2>&1)
 test_expect_equal "$output" \
-"Added 1 new message to the database."
+"Warning: ${MAIL_DIR}/mbox_file1 is an mbox containing a single message,
+likely caused by misconfigured mail delivery.  Support for single-message
+mboxes is deprecated and may be removed in the future.
+Added 1 new message to the database."

 # This test requires that notmuch new has been run at least once.
 test_begin_subtest "Skip and report non-mail files"
-test_subtest_known_broken
 generate_message
 mkdir -p "${MAIL_DIR}"/.git && touch "${MAIL_DIR}"/.git/config
 touch "${MAIL_DIR}"/ignored_file
-- 
1.7.10.4

[PATCH 2/3] test: Test for ignoring multi-message mbox

[Austin Clements]

This test is currently broken.  Note that its brokenness cascades and
causes the next test to fail as well (because notmuch incorrectly
indexes the mbox file).
---
 test/new |   18 ++
 1 file changed, 18 insertions(+)

diff --git a/test/new b/test/new
index 43d56e4..29f9aff 100755
--- a/test/new
+++ b/test/new
@@ -178,16 +178,34 @@ test_expect_equal "$output" \

 # This test requires that notmuch new has been run at least once.
 test_begin_subtest "Skip and report non-mail files"
+test_subtest_known_broken
 generate_message
 mkdir -p "${MAIL_DIR}"/.git && touch "${MAIL_DIR}"/.git/config
 touch "${MAIL_DIR}"/ignored_file
 touch "${MAIL_DIR}"/.ignored_hidden_file
+cat > "${MAIL_DIR}"/mbox_file <
+To: Notmuch Test Suite 
+Subject: Test mbox message 1
+
+Body.
+
+From test_suite at notmuchmail.org Fri Jan  5 15:43:57 2001
+From: Notmuch Test Suite 
+To: Notmuch Test Suite 
+Subject: Test mbox message 2
+
+Body 2.
+EOF
 output=$(NOTMUCH_NEW 2>&1)
 test_expect_equal "$output" \
 "Note: Ignoring non-mail file: ${MAIL_DIR}/.git/config
 Note: Ignoring non-mail file: ${MAIL_DIR}/.ignored_hidden_file
 Note: Ignoring non-mail file: ${MAIL_DIR}/ignored_file
+Note: Ignoring non-mail file: ${MAIL_DIR}/mbox_file
 Added 1 new message to the database."
+rm "${MAIL_DIR}"/mbox_file

 test_begin_subtest "Ignore files and directories specified in new.ignore"
 generate_message
-- 
1.7.10.4

[PATCH 1/3] test: Test notmuch new for single-message mbox

[Austin Clements]

We support this for historical reasons.
---
 test/new |   13 +
 1 file changed, 13 insertions(+)

diff --git a/test/new b/test/new
index 587aa11..43d56e4 100755
--- a/test/new
+++ b/test/new
@@ -163,6 +163,19 @@ rm -rf "${MAIL_DIR}"/two
 output=$(NOTMUCH_NEW)
 test_expect_equal "$output" "No new mail. Removed 3 messages."

+test_begin_subtest "Support single-message mbox"
+cat > "${MAIL_DIR}"/mbox_file1 <
+To: Notmuch Test Suite 
+Subject: Test mbox message 1
+
+Body.
+EOF
+output=$(NOTMUCH_NEW 2>&1)
+test_expect_equal "$output" \
+"Added 1 new message to the database."
+
 # This test requires that notmuch new has been run at least once.
 test_begin_subtest "Skip and report non-mail files"
 generate_message
-- 
1.7.10.4

[PATCH v2 1/7] cli: allow query to come from stdin

[Tomi Ollila]

On Sat, Nov 24 2012, markwalters1009 wrote:

> From: Mark Walters 
>
> After this series there will be times when a caller will want to pass
> a very large query string to notmuch (eg a list of 10,000 message-ids)
> and this can exceed the size of ARG_MAX. Hence allow notmuch to take
> the query from stdin (if the query is -).


> ---
>  query-string.c |   41 +
>  1 files changed, 41 insertions(+), 0 deletions(-)
>
> diff --git a/query-string.c b/query-string.c
> index 6536512..b1fbdeb 100644
> --- a/query-string.c
> +++ b/query-string.c
> @@ -20,6 +20,44 @@
>  
>  #include "notmuch-client.h"
>  
> +/* Read a single query string from STDIN, using
> + * 'ctx' as the talloc owner for all allocations.
> + *
> + * This function returns NULL in case of insufficient memory or read
> + * errors.
> + */
> +static char *
> +query_string_from_stdin (void *ctx)

Austin provided pretty nice alternative implementation of 
query_string_from_stdin() in his reply so I decline to
comment minor formatting issue below :D

> +{
> +char *query_string;
> +char buf[4096];
> +ssize_t remain;
> +
> +query_string = talloc_strdup (ctx, "");
> +if (query_string == NULL)
> + return NULL;
> +
> +for (;;) {
> + remain = read (STDIN_FILENO, buf, sizeof(buf) - 1);
> + if (remain == 0)
> + break;
> + if (remain < 0) {
> + if (errno == EINTR)
> + continue;
> + fprintf (stderr, "Error: reading from standard input: %s\n",
> +  strerror (errno));
> + return NULL;
> + }
> +
> + buf[remain] = '\0';
> + query_string = talloc_strdup_append (query_string, buf);
> + if (query_string == NULL)
> + return NULL;
> +}
> +
> +return query_string;
> +}
> +
>  /* Construct a single query string from the passed arguments, using
>   * 'ctx' as the talloc owner for all allocations.
>   *
> @@ -35,6 +73,9 @@ query_string_from_args (void *ctx, int argc, char *argv[])
>  char *query_string;
>  int i;
>  
> +if ((argc == 1) && (strcmp ("-", argv[0]) == 0))

the argument order in strcmp() is not consistent with all the 
other uses of strcmp () in the codebase.

> + return query_string_from_stdin (ctx);
> +
>  query_string = talloc_strdup (ctx, "");
>  if (query_string == NULL)
>   return NULL;
> -- 
> 1.7.9.1

Tomi

[PATCH v2 6/7] cli: allow search mode to include msg-ids with JSON output

[Tomi Ollila]

On Sat, Nov 24 2012, markwalters1009 wrote:

> From: Mark Walters 
>
> This adds a --queries=true option which modifies the summary output of
> notmuch search by including two extra query strings with each result:
> one query string specifies all matching messages and one query string
> all non-matching messages. Currently these are just lists of message
> ids joined with " or " but that could change in future.


Please see (mostly formatting) comments inline below.


> Currently this is not implemented for text format.
> ---
>  notmuch-search.c |   95 ++---
>  1 files changed, 89 insertions(+), 6 deletions(-)
>
> diff --git a/notmuch-search.c b/notmuch-search.c
> index 830c4e4..c8fc9a6 100644
> --- a/notmuch-search.c
> +++ b/notmuch-search.c
> @@ -26,7 +26,8 @@ typedef enum {
>  OUTPUT_THREADS,
>  OUTPUT_MESSAGES,
>  OUTPUT_FILES,
> -OUTPUT_TAGS
> +OUTPUT_TAGS,
> +OUTPUT_SUMMARY_WITH_QUERIES
>  } output_t;
>  
>  static char *
> @@ -46,6 +47,57 @@ sanitize_string (const void *ctx, const char *str)
>  return out;
>  }
>  
> +/* This function takes a message id and returns an escaped string
> + * which can be used as a Xapian query. This involves prefixing with
> + * `id:', putting the id inside double quotes, and doubling any
> + * occurence of a double quote in the message id itself.*/
> +static char *
> +xapian_escape_id (const void *ctx,
> +const char *msg_id)

second line indentation, not at '(' level as elsewhere

> +{
> +const char *c;
> +char *escaped_msg_id;
> +escaped_msg_id = talloc_strdup (ctx, "id:\"");
> +for (c=msg_id; *c; c++)

spacing above

> + if (*c == '"')
> + escaped_msg_id = talloc_asprintf_append (escaped_msg_id, "\"\"");
> + else
> + escaped_msg_id = talloc_asprintf_append (escaped_msg_id, "%c", *c);
> +escaped_msg_id = talloc_asprintf_append (escaped_msg_id, "\"");
> +return escaped_msg_id;
> +}

If Austin sees fit he can comment the O(...) of the addition of msgids
to query strings -- it would take quite an overhaul to the functionality
if the escaped msgid's were directly written to query string...

> +
> +static char *
> +output_msg_query (const void *ctx,
> + sprinter_t *format,
> + notmuch_bool_t matching,
> + notmuch_bool_t first,
> + notmuch_messages_t *messages)

indentation level above

> +{
> +notmuch_message_t *message;
> +char *query, *escaped_msg_id;
> +query = talloc_strdup (ctx, "");
> +for (;
> +  notmuch_messages_valid (messages);
> +  notmuch_messages_move_to_next (messages))
> +{
> + message = notmuch_messages_get (messages);
> + if (notmuch_message_get_flag (message, NOTMUCH_MESSAGE_FLAG_MATCH) == 
> matching) {
> + escaped_msg_id = xapian_escape_id (ctx, 
> notmuch_message_get_message_id (message));
> + if (first) {
> + query = talloc_asprintf_append (query, "%s", escaped_msg_id);
> + first = FALSE;
> + }
> + else
> + query = talloc_asprintf_append (query, " or %s", 
> escaped_msg_id);
> + talloc_free (escaped_msg_id);
> + }
> + /* output_msg_query already starts with an ` or' */
> + query = talloc_asprintf_append (query, "%s", output_msg_query (ctx, 
> format, matching, first, notmuch_message_get_replies (message)));
> +}
> +return query;
> +}
> +
>  static int
>  do_search_threads (sprinter_t *format,
>  notmuch_query_t *query,
> @@ -88,7 +140,7 @@ do_search_threads (sprinter_t *format,
>   format->string (format,
>   notmuch_thread_get_thread_id (thread));
>   format->separator (format);
> - } else { /* output == OUTPUT_SUMMARY */
> + } else { /* output == OUTPUT_SUMMARY or OUTPUT_SUMMARY_WITH_QUERIES */
>   void *ctx_quote = talloc_new (thread);
>   const char *authors = notmuch_thread_get_authors (thread);
>   const char *subject = notmuch_thread_get_subject (thread);
> @@ -108,7 +160,7 @@ do_search_threads (sprinter_t *format,
>   relative_date = notmuch_time_relative_date (ctx_quote, date);
>  
>   if (format->is_text_printer) {
> -/* Special case for the text formatter */
> +   /* Special case for the text formatter */

irrelevant spacing change

>   printf ("thread:%s %12s [%d/%d] %s; %s (",
>   thread_id,
>   relative_date,
> @@ -133,8 +185,6 @@ do_search_threads (sprinter_t *format,
>   format->string (format, subject);
>   }
>  
> - talloc_free (ctx_quote);
> -
>   format->map_key (format, "tags");
>   format->begin_list (format);
>  
> @@ -145,7 +195,7 @@ do_search_threads (sprinter_t *format,
>   const char *tag = notmuch_tags_get (tags);
>  
>   if (format->is_text_printer) {
> -  

[PATCH 2/2] new: Skip ignored broken symlinks

[Austin Clements]

We now test for user ignore patterns before attempting to determine if
a directory entry is itself a directory.  As a result, we no longer
abort for broken symlinks if the user has explicitly ignored them.

This fixes the test added in the previous patch.  It also slightly
changes the debug output checked by another test of ignores.
---
 notmuch-new.c |   26 +++---
 test/new  |6 +-
 2 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/notmuch-new.c b/notmuch-new.c
index 56c4a6f..718a538 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -350,6 +350,18 @@ add_files (notmuch_database_t *notmuch,

entry = fs_entries[i];

+   /* Ignore any files/directories the user has configured to
+* ignore.  We do this before dirent_type both for performance
+* and because we don't care if dirent_type fails on entries
+* that are explicitly ignored.
+*/
+   if (_entry_in_ignore_list (entry->d_name, state)) {
+   if (state->debug)
+   printf ("(D) add_files_recursive, pass 1: explicitly ignoring 
%s/%s\n",
+   path, entry->d_name);
+   continue;
+   }
+
/* We only want to descend into directories (and symlinks to
 * directories). */
entry_type = dirent_type (path, entry);
@@ -364,22 +376,14 @@ add_files (notmuch_database_t *notmuch,
}

/* Ignore special directories to avoid infinite recursion.
-* Also ignore the .notmuch directory, any "tmp" directory
-* that appears within a maildir and files/directories
-* the user has configured to be ignored.
+* Also ignore the .notmuch directory and any "tmp" directory
+* that appears within a maildir.
 */
if (strcmp (entry->d_name, ".") == 0 ||
strcmp (entry->d_name, "..") == 0 ||
(is_maildir && strcmp (entry->d_name, "tmp") == 0) ||
-   strcmp (entry->d_name, ".notmuch") == 0 ||
-   _entry_in_ignore_list (entry->d_name, state))
-   {
-   if (_entry_in_ignore_list (entry->d_name, state) && state->debug)
-   printf ("(D) add_files_recursive, pass 1: explicitly ignoring 
%s/%s\n",
-   path,
-   entry->d_name);
+   strcmp (entry->d_name, ".notmuch") == 0)
continue;
-   }

next = talloc_asprintf (notmuch, "%s/%s", path, entry->d_name);
status = add_files (notmuch, next, state);
diff --git a/test/new b/test/new
index 59892a7..8a76e34 100755
--- a/test/new
+++ b/test/new
@@ -192,7 +192,12 @@ touch 
"${MAIL_DIR}"/{one,one/two,one/two/three}/ignored_file
 output=$(NOTMUCH_NEW --debug 2>&1 | sort)
 test_expect_equal "$output" \
 "(D) add_files_recursive, pass 1: explicitly ignoring ${MAIL_DIR}/.git
+(D) add_files_recursive, pass 1: explicitly ignoring 
${MAIL_DIR}/.ignored_hidden_file
+(D) add_files_recursive, pass 1: explicitly ignoring ${MAIL_DIR}/ignored_file
+(D) add_files_recursive, pass 1: explicitly ignoring 
${MAIL_DIR}/one/ignored_file
+(D) add_files_recursive, pass 1: explicitly ignoring 
${MAIL_DIR}/one/two/ignored_file
 (D) add_files_recursive, pass 1: explicitly ignoring 
${MAIL_DIR}/one/two/three/.git
+(D) add_files_recursive, pass 1: explicitly ignoring 
${MAIL_DIR}/one/two/three/ignored_file
 (D) add_files_recursive, pass 2: explicitly ignoring ${MAIL_DIR}/.git
 (D) add_files_recursive, pass 2: explicitly ignoring 
${MAIL_DIR}/.ignored_hidden_file
 (D) add_files_recursive, pass 2: explicitly ignoring ${MAIL_DIR}/ignored_file
@@ -204,7 +209,6 @@ No new mail."


 test_begin_subtest "Don't stop for ignored broken symlinks"
-test_subtest_known_broken
 notmuch config set new.ignore .git ignored_file .ignored_hidden_file 
broken_link
 ln -s i_do_not_exist "${MAIL_DIR}"/broken_link
 output=$(NOTMUCH_NEW 2>&1)
-- 
1.7.10.4

[PATCH 1/2] test: Add a test for skipping ignored broken symlinks

[Austin Clements]

Currently marked as broken because we abort on broken symlinks, even
if they are in the ignore list.
---
 test/new |7 +++
 1 file changed, 7 insertions(+)

diff --git a/test/new b/test/new
index 587aa11..59892a7 100755
--- a/test/new
+++ b/test/new
@@ -203,4 +203,11 @@ test_expect_equal "$output" \
 No new mail."


+test_begin_subtest "Don't stop for ignored broken symlinks"
+test_subtest_known_broken
+notmuch config set new.ignore .git ignored_file .ignored_hidden_file 
broken_link
+ln -s i_do_not_exist "${MAIL_DIR}"/broken_link
+output=$(NOTMUCH_NEW 2>&1)
+test_expect_equal "$output" "No new mail."
+
 test_done
-- 
1.7.10.4

[PATCH 0/2] Ignore ignored broken symlinks

[Austin Clements]

calmar on IRC reported a bug today where notmuch will abort when it
encounters a broken symlink, even if that symlink is in the
user-specified ignore list.  These two patches test for this bug and
fix it.

Re: [PATCH 0/2] Ignore ignored broken symlinks

[Tomi Ollila]

On Sun, Nov 25 2012, Austin Clements amdra...@mit.edu wrote:

 calmar on IRC reported a bug today where notmuch will abort when it
 encounters a broken symlink, even if that symlink is in the
 user-specified ignore list.  These two patches test for this bug and
 fix it.

+1

Tomi
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

BUG: notmuch stop on death symlink even..

[calmar c.]

hi all, 

notmuch stops/exits on death symlink even when they are on the
ignore-list at the same time.

on: notmuch new ... arount 360maybe in notmuch-new.c


cheers
marco

-- 
   (o_  It rocks: LINUX + Command-Line-Interface
   //\   GPG: 0x59D90F4D
   V_/_ http://www.calmar.ws
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 1/3] test: Test notmuch new for single-message mbox

[Mark Walters]

This series looks fine to me. +1

Mark

On Sun, 25 Nov 2012, Austin Clements amdra...@mit.edu wrote:
 We support this for historical reasons.
 ---
  test/new |   13 +
  1 file changed, 13 insertions(+)

 diff --git a/test/new b/test/new
 index 587aa11..43d56e4 100755
 --- a/test/new
 +++ b/test/new
 @@ -163,6 +163,19 @@ rm -rf ${MAIL_DIR}/two
  output=$(NOTMUCH_NEW)
  test_expect_equal $output No new mail. Removed 3 messages.
  
 +test_begin_subtest Support single-message mbox
 +cat  ${MAIL_DIR}/mbox_file1 EOF
 +From test_su...@notmuchmail.org Fri Jan  5 15:43:57 2001
 +From: Notmuch Test Suite test_su...@notmuchmail.org
 +To: Notmuch Test Suite test_su...@notmuchmail.org
 +Subject: Test mbox message 1
 +
 +Body.
 +EOF
 +output=$(NOTMUCH_NEW 21)
 +test_expect_equal $output \
 +Added 1 new message to the database.
 +
  # This test requires that notmuch new has been run at least once.
  test_begin_subtest Skip and report non-mail files
  generate_message
 -- 
 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: [PATCH 0/2] Ignore ignored broken symlinks

[Mark Walters]

On Sun, 25 Nov 2012, Tomi Ollila tomi.oll...@iki.fi wrote:
 On Sun, Nov 25 2012, Austin Clements amdra...@mit.edu wrote:

 calmar on IRC reported a bug today where notmuch will abort when it
 encounters a broken symlink, even if that symlink is in the
 user-specified ignore list.  These two patches test for this bug and
 fix it.

 +1


+1 from me too

Mark

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Many color per line in notuch emacs mode

[kedals0]

Hi,

I can set color per line using notmuch-search-line-faces. But I would
like to specify color per line field, i.e a color for date, another for
authors for exemple.

To do this, I would like to use a 3-tuple list for
notmuch-search-line-faces:
:type '(alist :key-type (string) :field (string) :value-type
(custom-face-edit))

The problem is function notmuch-search-color-line take a non structured
line in args. Then it is needed to parse it to find out field.

Do you think it would be possible to add a mecanism which permit to
color a field during line building ?

Thx,
Dal
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 3/3] lib: Reject multi-message mboxes and deprecate single-message mbox

[Tomi Ollila]

On Sun, Nov 25 2012, Austin Clements amdra...@mit.edu wrote:

 Previously, we would treat multi-message mboxes as one giant email,
 which, besides the obvious incorrect indexing, often led to
 out-of-memory errors for archival mboxes.  Now we explicitly reject
 multi-message mboxes.  For historical reasons, we retain support for
 single-message mboxes, but official deprecate this behavior.


The series looks good to me -- but I don't know about deprecating
single-message mboxes:

* If we someday support (read-only?) mbox format, then single-message
  mboxes are normal again.

* Some naïve mb2md scripts could leave the 'From ' -line intact: for
  example `formail -bz -s head -3  $MAIL`(*) can be used to demonstrate this

* Some people may have large collection of single-file messages starting
  with 'From ' currently indexed. If those are to be re-indexed later
  without single-message mbox support that is somewhat of a burden to
  the users (**)

(*) my mb2md wannabe does gnus-like $formail -bz -R 'From ' X-From-Line: ...

(**) Something like the following could be used to mangle single-file 
mboxes...
 find . -type f | xargs perl -e 'foreach (@ARGV) { open IO, +, $_ or
 next; sysread IO, $buf, 5; if ($buf eq From ) { sysseek IO, 0, 0;
 syswrite IO, Fro:; }}' 
 This breaks the multi-message mbox nicely... ;)


Tomi
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 0/6] API for iterating over all messages in a thread

[Mark Walters]

Hi

This series looks good to me (I have not reviewed the two bindings
patches). Patch 2 looks like it makes things much easier to follow than
the current code (if I understood the current pointer stuff it
constructs the top-level list by doing pointer stuff to remove all
messages which are replies from the complete message list). Indeed, the
diff is more complicated than the new code!

On Sun, 25 Nov 2012, Austin Clements amdra...@mit.edu wrote:
 This series adds a library API for iterating over all messages in a
 thread in sorted order.  This is easy for the library to provide and
 difficult to obtain from the current API.  Plus, if you don't count
 the code added to the bindings, this series is actually a net
 decrease of 4 lines of code because of simplifications it enables.

 Do we want the API to do more?  Currently it's very minimal, but I can
 imagine two ways it could be generalized.  It could take an argument
 to indicate which message list to return, which could be all messages,
 matched messages, top-level messages, or maybe even unmatched messages
 (possibly all in terms of message flags).  It could also take an
 argument indicating the desired sort order.  Currently, the caller can
 use existing message flag APIs to distinguish matched and unmatched
 messages and there's a separate function for the top-level messages.
 However, if the API could do all of these things, it would subsume
 various other API functions, such as notmuch_thread_get_*_date.

I don't know if this is the right API. For the matched message etc I
think using the existing message flag APIs is simple enough. I am not
sure about sort orders though: that looks like it would be much easier
for the caller to have the correct sort by I am not sure what users
would need it.

Best wishes

Mark





 Also, is this the right name for the new API?  In particular, if we do
 later want to add a function that returns, say, the list of matched
 messages, we'll have a convention collision with
 notmuch_thread_get_matched_messages, which returns only a count.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

v4 of Performance tests

[david]

This version adds an optional makefile target to download the corpus.

Tomi reviewed it on IRC and didn't complain.

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[Patch v4 1/2] test: factor out part of test-lib.sh into test-lib-common.sh

[david]

From: David Bremner brem...@debian.org

The idea is to use some of the simpler parts of the test suite
infrastructure to help run performance tests.
---
 test/test-lib-common.sh |  147 +++
 test/test-lib.sh|  129 +
 2 files changed, 148 insertions(+), 128 deletions(-)
 create mode 100644 test/test-lib-common.sh

diff --git a/test/test-lib-common.sh b/test/test-lib-common.sh
new file mode 100644
index 000..e1eaa5a
--- /dev/null
+++ b/test/test-lib-common.sh
@@ -0,0 +1,147 @@
+#
+# Copyright (c) 2005 Junio C Hamano
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see http://www.gnu.org/licenses/ .
+
+# This file contains common code to be used by both the regular
+# (correctness) tests and the performance tests.
+
+find_notmuch_path ()
+{
+dir=$1
+
+while [ -n $dir ]; do
+   bin=$dir/notmuch
+   if [ -x $bin ]; then
+   echo $dir
+   return
+   fi
+   dir=$(dirname $dir)
+   if [ $dir = / ]; then
+   break
+   fi
+done
+}
+
+# Test the binaries we have just built.  The tests are kept in
+# test/ subdirectory and are run in 'trash directory' subdirectory.
+TEST_DIRECTORY=$(pwd)
+notmuch_path=`find_notmuch_path $TEST_DIRECTORY`
+if test -n $valgrind
+then
+   make_symlink () {
+   test -h $2 
+   test $1 = $(readlink $2) || {
+   # be super paranoid
+   if mkdir $2.lock
+   then
+   rm -f $2 
+   ln -s $1 $2 
+   rm -r $2.lock
+   else
+   while test -d $2.lock
+   do
+   say Waiting for lock on $2.
+   sleep 1
+   done
+   fi
+   }
+   }
+
+   make_valgrind_symlink () {
+   # handle only executables
+   test -x $1 || return
+
+   base=$(basename $1)
+   symlink_target=$TEST_DIRECTORY/../$base
+   # do not override scripts
+   if test -x $symlink_target 
+   test ! -d $symlink_target 
+   test #! != $(head -c 2  $symlink_target)
+   then
+   symlink_target=$TEST_DIRECTORY/valgrind.sh
+   fi
+   case $base in
+   *.sh|*.perl)
+   symlink_target=$TEST_DIRECTORY/unprocessed-script
+   esac
+   # create the link, or replace it if it is out of date
+   make_symlink $symlink_target $GIT_VALGRIND/bin/$base || exit
+   }
+
+   # override notmuch executable in TEST_DIRECTORY/..
+   GIT_VALGRIND=$TEST_DIRECTORY/valgrind
+   mkdir -p $GIT_VALGRIND/bin
+   make_valgrind_symlink $TEST_DIRECTORY/../notmuch
+   OLDIFS=$IFS
+   IFS=:
+   for path in $PATH
+   do
+   ls $path/notmuch 2 /dev/null |
+   while read file
+   do
+   make_valgrind_symlink $file
+   done
+   done
+   IFS=$OLDIFS
+   PATH=$GIT_VALGRIND/bin:$PATH
+   GIT_EXEC_PATH=$GIT_VALGRIND/bin
+   export GIT_VALGRIND
+   test -n $notmuch_path  MANPATH=$notmuch_path/man:$MANPATH
+else # normal case
+   if test -n $notmuch_path
+   then
+   PATH=$notmuch_path:$PATH
+   MANPATH=$notmuch_path/man:$MANPATH
+   fi
+fi
+export PATH MANPATH
+
+# Test repository
+test=tmp.$(basename $0 .sh)
+test -n $root  test=$root/$test
+case $test in
+/*) TMP_DIRECTORY=$test ;;
+ *) TMP_DIRECTORY=$TEST_DIRECTORY/$test ;;
+esac
+test ! -z $debug || remove_tmp=$TMP_DIRECTORY
+rm -fr $test || {
+   GIT_EXIT_OK=t
+   echo 5 FATAL: Cannot prepare test area
+   exit 1
+}
+
+# A temporary home directory is needed by at least:
+# - emacs/Sending a message via (fake) SMTP
+# - emacs/Reply within emacs
+# - crypto/emacs_deliver_message
+export HOME=${TMP_DIRECTORY}/home
+mkdir -p ${HOME}
+
+MAIL_DIR=${TMP_DIRECTORY}/mail
+export GNUPGHOME=${TMP_DIRECTORY}/gnupg
+export NOTMUCH_CONFIG=${TMP_DIRECTORY}/notmuch-config
+
+mkdir -p ${test}
+mkdir -p ${MAIL_DIR}
+
+cat EOF 

[Patch v4 2/2] test: initial performance testing infrastructure

[david]

From: David Bremner brem...@debian.org

This is not near as fancy as as the unit tests, on the theory that
the code should typically be crashing when performance tuning.
Nonetheless, there is plenty of room for improvement.  Several more of
the pieces of the test infrastructure (e.g. the option parsing) could
be factored out into test/test-lib-common.sh
---
 Makefile   |3 +-
 performance-test/.gitignore|1 +
 performance-test/Makefile  |7 +++
 performance-test/Makefile.local|   32 ++
 performance-test/README|   50 +++
 performance-test/basic |   15 +
 performance-test/download/.gitignore   |2 +
 .../download/notmuch-email-corpus-0.2.tar.xz.asc   |9 +++
 performance-test/notmuch-perf-test |   25 
 performance-test/perf-test-lib.sh  |   65 
 performance-test/version.sh|3 +
 11 files changed, 211 insertions(+), 1 deletion(-)
 create mode 100644 performance-test/.gitignore
 create mode 100644 performance-test/Makefile
 create mode 100644 performance-test/Makefile.local
 create mode 100644 performance-test/README
 create mode 100755 performance-test/basic
 create mode 100644 performance-test/download/.gitignore
 create mode 100644 
performance-test/download/notmuch-email-corpus-0.2.tar.xz.asc
 create mode 100755 performance-test/notmuch-perf-test
 create mode 100644 performance-test/perf-test-lib.sh
 create mode 100644 performance-test/version.sh

diff --git a/Makefile b/Makefile
index bb9c316..5decbea 100644
--- a/Makefile
+++ b/Makefile
@@ -3,7 +3,8 @@
 all:
 
 # List all subdirectories here. Each contains its own Makefile.local
-subdirs = compat completion emacs lib man parse-time-string util test
+subdirs := compat completion emacs lib man parse-time-string
+subdirs := $(subdirs) performance-test util test
 
 # We make all targets depend on the Makefiles themselves.
 global_deps = Makefile Makefile.config Makefile.local \
diff --git a/performance-test/.gitignore b/performance-test/.gitignore
new file mode 100644
index 000..53f2697
--- /dev/null
+++ b/performance-test/.gitignore
@@ -0,0 +1 @@
+tmp.*/
diff --git a/performance-test/Makefile b/performance-test/Makefile
new file mode 100644
index 000..de492a7
--- /dev/null
+++ b/performance-test/Makefile
@@ -0,0 +1,7 @@
+# See Makefile.local for the list of files to be compiled in this
+# directory.
+all:
+   $(MAKE) -C .. all
+
+.DEFAULT:
+   $(MAKE) -C .. $@
diff --git a/performance-test/Makefile.local b/performance-test/Makefile.local
new file mode 100644
index 000..1114ec1
--- /dev/null
+++ b/performance-test/Makefile.local
@@ -0,0 +1,32 @@
+# -*- makefile -*-
+
+dir := performance-test
+
+include $(dir)/version.sh
+
+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: setup-perf-test all
+   $(TEST_SCRIPT) $(OPTIONS)
+
+.PHONY: download-corpus setup-perf-test
+
+# Note that this intentionally does not depend on download-corpus.
+setup-perf-test: $(TXZFILE)
+   gpg --verify $(SIGFILE)
+
+$(TXZFILE):
+   @printf \nPlease download ${TXZFILE}.\n\n
+   @printf \t%% make download-corpus\n\n
+   @echo or see http://notmuchmail.org/corpus for download locations
+   @echo
+   @false
+
+download-corpus:
+   wget -O ${TXZFILE} ${DEFAULT_URL}
+
+CLEAN := $(CLEAN) $(dir)/tmp.*
diff --git a/performance-test/README b/performance-test/README
new file mode 100644
index 000..239d2fb
--- /dev/null
+++ b/performance-test/README
@@ -0,0 +1,50 @@
+Pre-requisites
+--
+
+In addition to having notmuch, you need:
+
+- gpg
+- gnu tar
+- gnu time
+- xz. Some speedup can be gotten by installing pixz, but this is
+  probably only worthwhile if you are debugging the tests.
+
+Getting set up to run tests:
+
+
+First, you need to get the corpus.
+
+It should work to run
+
+   % make download-corpus
+
+In case that fails or is too slow, check
+
+   http://notmuchmail.org/corpus
+
+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
+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).
+
+
+Writing tests
+-
+
+Have a look at basic for an example.
+
+add_email_corpus takes arguments --small and --medium for when you
+want smaller corpuses to check.
+
+time_done does the cleanup; comment it out or define $debug to leave
+the 

Re: [PATCH v3 1/2] notmuch-show.el: import calendar data with public function after CR removal

[David Bremner]

Tomi Ollila tomi.oll...@iki.fi writes:

 notmuch-get-bodypart-content provides raw data to its caller so
 that it can be stored verbatim whenever needed. icalendar functions
 expect Emacs to do EOL conversion for the data given to these. Therefore
 it the CRLF - LF conversion is now done explicitly.

pushed both

d
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2 00/20] insert command

[Mark Walters]

This is series is looking good. I have a few minor issues but no more.

There are two parts I don't really know enough about to be certain
about: the fsyncing and the sigtrap bit. 

For fsync: I have some recollection that you have to fsync all subpaths
to the root to guarantee that it makes it to the disk. 

One in particular that may need more fsyncing is if you create a folder
A/B/C/D/E/ for the messages then I think you only fsync the message and
A/B/C/D/E/new.

My other comments are all very minor and are made in individual replies.

Best wishes

Mark


On Sun, 25 Nov 2012, Peter Wang noval...@gmail.com wrote:
 This series mainly addresses the issues raised by Mark:

 - check talloc failures
 - deadlock in maildir_open_tmp
 - stricter file modes (0600 and 0700)
 - shared tag operation parser with notmuch-tag.c
 - simplified mkdir_parents
 - trap SIGINT
 - fsync after writing and rename
 - added a couple of tests
 - man page wording
 - comments

 Due to new restriction on tags beginning with '-', an argument beginning with
 -- is no longer ambiguous so I have removed the optional -- separator
 between options and tag operations.

 Peter Wang (20):
   tag: factor out tag operation parsing
   tag: make tag operation parser public
   cli: add stub for insert command
   insert: open Maildir tmp file
   insert: copy stdin to Maildir tmp file
   insert: move file from Maildir tmp to new
   insert: add new message to database
   insert: support --folder option
   insert: prevent writes outside Maildir hierarchy
   insert: apply default tags to new message
   insert: parse command-line tag operations
   insert: apply command-line tag operations
   insert: add --create-folder option
   insert: fsync after writing tmp file
   insert: fsync new directory after rename
   insert: trap SIGINT and clean up
   insert: add copyright line from notmuch-deliver
   test: add tests for insert
   man: document 'insert' command
   man: reference notmuch-insert.1

  Makefile.local  |   1 +
  man/Makefile.local  |   1 +
  man/man1/notmuch-config.1   |   4 +-
  man/man1/notmuch-count.1|   4 +-
  man/man1/notmuch-dump.1 |   4 +-
  man/man1/notmuch-insert.1   |  60 +
  man/man1/notmuch-new.1  |   4 +-
  man/man1/notmuch-reply.1|   3 +-
  man/man1/notmuch-restore.1  |   3 +-
  man/man1/notmuch-search.1   |   3 +-
  man/man1/notmuch-show.1 |   3 +-
  man/man1/notmuch-tag.1  |   3 +-
  man/man1/notmuch.1  |   3 +-
  man/man5/notmuch-hooks.5|   4 +-
  man/man7/notmuch-search-terms.7 |   3 +-
  notmuch-client.h|  12 +
  notmuch-insert.c| 493 
 
  notmuch-tag.c   |  79 ---
  notmuch.c   |   3 +
  test/insert |  93 
  test/notmuch-test   |   1 +
  21 files changed, 733 insertions(+), 51 deletions(-)
  create mode 100644 man/man1/notmuch-insert.1
  create mode 100644 notmuch-insert.c
  create mode 100755 test/insert

 -- 
 1.7.12.1

 ___
 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: [PATCH v2 01/20] tag: factor out tag operation parsing

[Mark Walters]

On Sun, 25 Nov 2012, Peter Wang noval...@gmail.com wrote:
 Factor out parsing of +tag, -tag operations from argv
 into a separate function.
 ---
  notmuch-tag.c | 66 
 +--
  1 file changed, 41 insertions(+), 25 deletions(-)

 diff --git a/notmuch-tag.c b/notmuch-tag.c
 index 88d559b..35a76db 100644
 --- a/notmuch-tag.c
 +++ b/notmuch-tag.c
 @@ -167,11 +167,48 @@ tag_query (void *ctx, notmuch_database_t *notmuch, 
 const char *query_string,
  return interrupted;
  }
  
 +/* Parse +tag and -tag operations between argv[i] and argv[argc-1].
 + * The array tag_ops must be at least argc - i elements long.
 + * Returns the index into argv where parsing stopped, or -1 on error. */

Perhaps mention tag_ops_count (as the number of tags parsed or
something)?

 +static int
 +parse_tag_operations (int i, int argc, char *argv[],
 +   tag_operation_t *tag_ops, int *tag_ops_count)
 +{
 +*tag_ops_count = 0;
 +
 +for (; i  argc; i++) {
 + if (strcmp (argv[i], --) == 0) {
 + i++;
 + break;
 + }
 + if (argv[i][0] == '+' || argv[i][0] == '-') {
 + if (argv[i][0] == '+'  argv[i][1] == '\0') {
 + fprintf (stderr, Error: tag names cannot be empty.\n);
 + return -1;
 + }
 + if (argv[i][0] == '+'  argv[i][1] == '-') {
 + /* This disallows adding the non-removable tag - and
 +  * enables notmuch tag to take long options in the
 +  * future. */
 + fprintf (stderr, Error: tag names must not start with '-'.\n);
 + return -1;
 + }
 + tag_ops[*tag_ops_count].tag = argv[i] + 1;
 + tag_ops[*tag_ops_count].remove = (argv[i][0] == '-');
 + (*tag_ops_count)++;
 + } else {
 + break;
 + }
 +}
 +
 +return i;
 +}
 +
  int
  notmuch_tag_command (void *ctx, int argc, char *argv[])
  {
  tag_operation_t *tag_ops;
 -int tag_ops_count = 0;
 +int tag_ops_count;
  char *query_string;
  notmuch_config_t *config;
  notmuch_database_t *notmuch;
 @@ -197,30 +234,9 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
   return 1;
  }
  
 -for (i = 0; i  argc; i++) {
 - if (strcmp (argv[i], --) == 0) {
 - i++;
 - break;
 - }
 - if (argv[i][0] == '+' || argv[i][0] == '-') {
 - if (argv[i][0] == '+'  argv[i][1] == '\0') {
 - fprintf (stderr, Error: tag names cannot be empty.\n);
 - return 1;
 - }
 - if (argv[i][0] == '+'  argv[i][1] == '-') {
 - /* This disallows adding the non-removable tag - and
 -  * enables notmuch tag to take long options in the
 -  * future. */
 - fprintf (stderr, Error: tag names must not start with '-'.\n);
 - return 1;
 - }
 - tag_ops[tag_ops_count].tag = argv[i] + 1;
 - tag_ops[tag_ops_count].remove = (argv[i][0] == '-');
 - tag_ops_count++;
 - } else {
 - break;
 - }
 -}
 +i = parse_tag_operations (0, argc, argv, tag_ops, tag_ops_count);
 +if (i  0)
 + return 1;
  
  tag_ops[tag_ops_count].tag = NULL;
  
 -- 
 1.7.12.1

 ___
 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: [PATCH v2 15/20] insert: fsync new directory after rename

[Mark Walters]

On Sun, 25 Nov 2012, Peter Wang noval...@gmail.com wrote:
 After moving the file from the 'tmp' to the 'new' directory,
 fsync on the 'new' directory for durability.
 ---
  notmuch-insert.c | 39 ---
  1 file changed, 32 insertions(+), 7 deletions(-)

 diff --git a/notmuch-insert.c b/notmuch-insert.c
 index f09c579..831b322 100644
 --- a/notmuch-insert.c
 +++ b/notmuch-insert.c
 @@ -143,10 +143,10 @@ maildir_create_folder (void *ctx, const char *dir)
  /* Open a unique file in the Maildir 'tmp' directory.
   * Returns the file descriptor on success, or -1 on failure.
   * On success, file paths into the 'tmp' and 'new' directories are returned
 - * via tmppath and newpath. */
 + * via tmppath and newpath, and the path of the 'new' directory in newdir. */

I found this comment very difficult to understand since it looks like
the 'new' directory gets returned twice. Perhaps something like

On success, file paths for the message in the 'tmp' and 'new'
directories are returned via tmppath and newpath, and the path of the
'new' directory itself in newdir.


Best wishes

Mark

  static int
  maildir_open_tmp_file (void *ctx, const char *dir,
 -char **tmppath, char **newpath)
 +char **tmppath, char **newpath, char **newdir)
  {
  pid_t pid;
  char hostname[256];
 @@ -183,8 +183,9 @@ maildir_open_tmp_file (void *ctx, const char *dir,
   return -1;
  }
  
 +*newdir = talloc_asprintf (ctx, %s/new, dir);
  *newpath = talloc_asprintf (ctx, %s/new/%s, dir, filename);
 -if (! *newpath) {
 +if (! *newdir || ! *newpath) {
   fprintf (stderr, Out of memory\n);
   close (fd);
   unlink (*tmppath);
 @@ -204,14 +205,31 @@ maildir_open_tmp_file (void *ctx, const char *dir,
   * http://wiki.dovecot.org/MailboxFormat/Maildir#Mail_delivery
   */
  static notmuch_bool_t
 -maildir_move_tmp_to_new (const char *tmppath, const char *newpath)
 +maildir_move_tmp_to_new (const char *tmppath, const char *newpath,
 +  const char *newdir)
  {
 +notmuch_bool_t ret;
 +int fd;
 +
  if (rename (tmppath, newpath) != 0) {
   fprintf (stderr, Error: rename() failed: %s\n, strerror (errno));
   return FALSE;
  }
  
 -return TRUE;
 +/* Sync the 'new' directory after rename for durability. */
 +ret = TRUE;
 +fd = open (newdir, O_RDONLY);
 +if (fd == -1) {
 + fprintf (stderr, Error: open() dir failed: %s\n, strerror (errno));
 + ret = FALSE;
 +}
 +if (ret  fsync (fd) != 0) {
 + fprintf (stderr, Error: fsync() dir failed: %s\n, strerror (errno));
 + ret = FALSE;
 +}
 +if (fd != -1)
 + close (fd);
 +return ret;
  }
  
  /* Copy the contents of fdin into fdout. */
 @@ -307,6 +325,12 @@ add_file_to_database (notmuch_database_t *notmuch, const 
 char *path,
  
  notmuch_message_thaw (message);
  
 +/* notmuch_message_tags_to_maildir_flags may rename the message file
 + * once more, and does so without fsyncing the directory afterwards.
 + * rename() is atomic so after a crash the file should appear under
 + * the old or new name. notmuch new should be able to rename the file
 + * again if required, so another fsync is not required, I think.
 + */
  notmuch_message_tags_to_maildir_flags (message);
  
  notmuch_message_destroy (message);
 @@ -321,10 +345,11 @@ insert_message (void *ctx, notmuch_database_t *notmuch, 
 int fdin,
  {
  char *tmppath;
  char *newpath;
 +char *newdir;
  int fdout;
  notmuch_bool_t ret;
  
 -fdout = maildir_open_tmp_file (ctx, dir, tmppath, newpath);
 +fdout = maildir_open_tmp_file (ctx, dir, tmppath, newpath, newdir);
  if (fdout  0) {
   return FALSE;
  }
 @@ -335,7 +360,7 @@ insert_message (void *ctx, notmuch_database_t *notmuch, 
 int fdin,
  }
  close (fdout);
  if (ret) {
 - ret = maildir_move_tmp_to_new (tmppath, newpath);
 + ret = maildir_move_tmp_to_new (tmppath, newpath, newdir);
  }
  if (!ret) {
   unlink (tmppath);
 -- 
 1.7.12.1

 ___
 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: [PATCH v2 18/20] test: add tests for insert

[Mark Walters]

On Sun, 25 Nov 2012, Peter Wang noval...@gmail.com wrote:
 Add tests for new 'insert' command.
 ---
  test/insert   | 93 
 +++
  test/notmuch-test |  1 +
  2 files changed, 94 insertions(+)
  create mode 100755 test/insert

 diff --git a/test/insert b/test/insert
 new file mode 100755
 index 000..d821a41
 --- /dev/null
 +++ b/test/insert
 @@ -0,0 +1,93 @@
 +#!/usr/bin/env bash
 +test_description='notmuch insert'
 +. ./test-lib.sh
 +
 +# Create directories and database before inserting.
 +mkdir -p $MAIL_DIR/{cur,new,tmp}
 +mkdir -p $MAIL_DIR/Drafts/{cur,new,tmp}
 +notmuch new  /dev/null
 +
 +# We use generate_message to create the temporary message file.
 +# It happens to be in the mail directory already but that is okay.

Perhaps add since we do not call notmuch new.

 +
 +test_begin_subtest Insert message, default
 +generate_message \
 +[subject]=\insert-subject\ \
 +[date]=\Sat, 01 Jan 2000 12:00:00 -\ \
 +[body]=\insert-message\
 +notmuch insert  $gen_msg_filename
 +test_expect_equal `notmuch count subject:insert-subject tag:unread` 1

I would much prefer to test that we have the proper full message here
with something like notmuch show  or something. I would like
something that tests that the full message has actually been written to
disk.

 +test_begin_subtest Insert message, add tag
 +generate_message \
 +[subject]=\insert-subject-addtag\ \
 +[date]=\Sat, 01 Jan 2000 12:00:00 -\ \
 +[body]=\insert-message-addtag\
 +notmuch insert +custom  $gen_msg_filename
 +test_expect_equal `notmuch count tag:custom` 1
 +
 +test_begin_subtest Insert message, add/remove tag
 +generate_message \
 +[subject]=\insert-subject-addrmtag\ \
 +[date]=\Sat, 01 Jan 2000 12:00:00 -\ \
 +[body]=\insert-message-addrmtag\
 +notmuch insert +custom -unread  $gen_msg_filename
 +test_expect_equal `notmuch count tag:custom NOT tag:unread` 1
 +
 +test_begin_subtest Insert message, folder
 +generate_message \
 +[subject]=\insert-subject-draft\ \
 +[date]=\Sat, 01 Jan 2000 12:00:00 -\ \
 +[body]=\insert-message-draft\
 +notmuch insert --folder=Drafts  $gen_msg_filename
 +test_expect_equal `notmuch count folder:Drafts` 1
 +
 +test_begin_subtest Insert message, folder and tags
 +generate_message \
 +[subject]=\insert-subject-draft\ \
 +[date]=\Sat, 01 Jan 2000 12:00:00 -\ \
 +[body]=\insert-message-draft\
 +notmuch insert --folder=Drafts +draft -unread  $gen_msg_filename
 +test_expect_equal `notmuch count folder:Drafts tag:draft NOT tag:unread` 
 1
 +
 +test_begin_subtest Insert message, non-existent folder
 +generate_message \
 +[subject]=\insert-subject-nonexistfolder\ \
 +[date]=\Sat, 01 Jan 2000 12:00:00 -\ \
 +[body]=\insert-message-nonexistfolder\
 +notmuch insert --folder=nonesuch  $gen_msg_filename
 +test_expect_equal $? 1

Here and for other error cases you could check that the correct error
message is being returned.

Best wishes

Mark
 +
 +test_begin_subtest Insert message, create folder
 +generate_message \
 +[subject]=\insert-subject-createfolder\ \
 +[date]=\Sat, 01 Jan 2000 12:00:00 -\ \
 +[body]=\insert-message-createfolder\
 +notmuch insert --folder=F --create-folder +folder  $gen_msg_filename
 +test_expect_equal `notmuch count folder:F tag:folder` 1
 +
 +test_begin_subtest Insert message, create subfolder
 +generate_message \
 +[subject]=\insert-subject-createfolder\ \
 +[date]=\Sat, 01 Jan 2000 12:00:00 -\ \
 +[body]=\insert-message-createfolder\
 +notmuch insert --folder=F/G/H/I/J --create-folder +folder  
 $gen_msg_filename
 +test_expect_equal `notmuch count folder:F/G/H/I/J tag:folder` 1
 +
 +test_begin_subtest Insert message, create existing subfolder
 +generate_message \
 +[subject]=\insert-subject-createfolder\ \
 +[date]=\Sat, 01 Jan 2000 12:00:00 -\ \
 +[body]=\insert-message-createfolder\
 +notmuch insert --folder=F/G/H/I/J --create-folder +folder  
 $gen_msg_filename
 +test_expect_equal `notmuch count folder:F/G/H/I/J tag:folder` 2
 +
 +test_begin_subtest Insert message, create invalid subfolder
 +generate_message \
 +[subject]=\insert-subject-createinvalidfolder\ \
 +[date]=\Sat, 01 Jan 2000 12:00:00 -\ \
 +[body]=\insert-message-createinvalidfolder\
 +notmuch insert --folder=../G --create-folder  $gen_msg_filename
 +test_expect_equal $? 1
 +
 +test_done
 diff --git a/test/notmuch-test b/test/notmuch-test
 index 9a1b375..09be44c 100755
 --- a/test/notmuch-test
 +++ b/test/notmuch-test
 @@ -22,6 +22,7 @@ TESTS=
config
new
count
 +  insert
search
search-output
search-by-folder
 -- 
 1.7.12.1

 ___
 notmuch mailing list
 notmuch@notmuchmail.org
 http://notmuchmail.org/mailman/listinfo/notmuch
___
notmuch mailing list
notmuch@notmuchmail.org

Re: [PATCH 3/3] lib: Reject multi-message mboxes and deprecate single-message mbox

[Austin Clements]

Quoth Tomi Ollila on Nov 25 at  3:26 pm:
 On Sun, Nov 25 2012, Austin Clements amdra...@mit.edu wrote:
 
  Previously, we would treat multi-message mboxes as one giant email,
  which, besides the obvious incorrect indexing, often led to
  out-of-memory errors for archival mboxes.  Now we explicitly reject
  multi-message mboxes.  For historical reasons, we retain support for
  single-message mboxes, but official deprecate this behavior.
 
 
 The series looks good to me -- but I don't know about deprecating
 single-message mboxes:
 
 * If we someday support (read-only?) mbox format, then single-message
   mboxes are normal again.

If notmuch does gain mbox support, then its handling of single-message
mboxes will *definitely* change because it will stop doing
maildir-like things to them (flag sync, moving from new to cur, etc),
which people may currently be depending on.  This was one of the
motivations for deprecating the current handling of single-message
mboxes.

 * Some naïve mb2md scripts could leave the 'From ' -line intact: for
   example `formail -bz -s head -3  $MAIL`(*) can be used to demonstrate this

I would call that buggy, rather than naïve.  ]:--8)

 * Some people may have large collection of single-file messages starting
   with 'From ' currently indexed. If those are to be re-indexed later
   without single-message mbox support that is somewhat of a burden to
   the users (**)

That's why this only deprecates them (with a warning) and doesn't drop
support for them.  The idea is to keep the historical handling for a
few releases and then we'll have the flexibility to do what we want
with single-message mboxes (including supporting them as real mbox).

It's probably a good idea to include a script or a wiki pointer for
fixing single-message mboxes in the NEWS.  As long as the file name is
kept the same, notmuch won't reindex it.

 (*) my mb2md wannabe does gnus-like $formail -bz -R 'From ' X-From-Line: 
 ...
 
 (**) Something like the following could be used to mangle single-file 
 mboxes...
  find . -type f | xargs perl -e 'foreach (@ARGV) { open IO, +, $_ or
  next; sysread IO, $buf, 5; if ($buf eq From ) { sysseek IO, 0, 0;
  syswrite IO, Fro:; }}' 
  This breaks the multi-message mbox nicely... ;)
 
 
 Tomi
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH v2 00/20] insert command

[David Bremner]

Peter Wang noval...@gmail.com writes:

 - shared tag operation parser with notmuch-tag.

Sharing tag operations with notmuch-tag is definitely a good idea, but
it will also conflict with the changes of the series at 

   id:1353792017-31459-1-git-send-email-da...@tethera.net

Although I might be biased, my current plan is to merge that series
first, since it fixes a long standing bug in the dump/restore commands.
If anything about series seems particularly problematic for
notmuch-insert, let me know.  Of course we can always make changes
afterwards, there are no public API changes (i.e. stuff ./lib) in play
here.

d
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 3/3] lib: Reject multi-message mboxes and deprecate single-message mbox

[Tomi Ollila]

On Sun, Nov 25 2012, Austin Clements amdra...@mit.edu wrote:

 Quoth Tomi Ollila on Nov 25 at  3:26 pm:
 On Sun, Nov 25 2012, Austin Clements amdra...@mit.edu wrote:
 
  Previously, we would treat multi-message mboxes as one giant email,
  which, besides the obvious incorrect indexing, often led to
  out-of-memory errors for archival mboxes.  Now we explicitly reject
  multi-message mboxes.  For historical reasons, we retain support for
  single-message mboxes, but official deprecate this behavior.
 
 
 The series looks good to me -- but I don't know about deprecating
 single-message mboxes:
 
 * If we someday support (read-only?) mbox format, then single-message
   mboxes are normal again.

 If notmuch does gain mbox support, then its handling of single-message
 mboxes will *definitely* change because it will stop doing
 maildir-like things to them (flag sync, moving from new to cur, etc),
 which people may currently be depending on.  This was one of the
 motivations for deprecating the current handling of single-message
 mboxes.

 * Some naïve mb2md scripts could leave the 'From ' -line intact: for
   example `formail -bz -s head -3  $MAIL`(*) can be used to demonstrate this

 I would call that buggy, rather than naïve.  ]:--8)

 * Some people may have large collection of single-file messages starting
   with 'From ' currently indexed. If those are to be re-indexed later
   without single-message mbox support that is somewhat of a burden to
   the users (**)

 That's why this only deprecates them (with a warning) and doesn't drop
 support for them.  The idea is to keep the historical handling for a
 few releases and then we'll have the flexibility to do what we want
 with single-message mboxes (including supporting them as real mbox).

 It's probably a good idea to include a script or a wiki pointer for
 fixing single-message mboxes in the NEWS.  As long as the file name is
 kept the same, notmuch won't reindex it.

Ok, I'm convinced. +1

Tomi


 (*) my mb2md wannabe does gnus-like $formail -bz -R 'From ' X-From-Line: 
 ...
 
 (**) Something like the following could be used to mangle single-file 
 mboxes...
  find . -type f | xargs perl -e 'foreach (@ARGV) { open IO, +, $_ or
  next; sysread IO, $buf, 5; if ($buf eq From ) { sysseek IO, 0, 0;
  syswrite IO, Fro:; }}' 
  This breaks the multi-message mbox nicely... ;)
 
 
 Tomi
 ___
 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: [PATCH 0/6] API for iterating over all messages in a thread

[Austin Clements]

Quoth Mark Walters on Nov 25 at  2:31 pm:
 
 Hi
 
 This series looks good to me (I have not reviewed the two bindings
 patches). Patch 2 looks like it makes things much easier to follow than
 the current code (if I understood the current pointer stuff it
 constructs the top-level list by doing pointer stuff to remove all
 messages which are replies from the complete message list). Indeed, the
 diff is more complicated than the new code!
 
 On Sun, 25 Nov 2012, Austin Clements amdra...@mit.edu wrote:
  This series adds a library API for iterating over all messages in a
  thread in sorted order.  This is easy for the library to provide and
  difficult to obtain from the current API.  Plus, if you don't count
  the code added to the bindings, this series is actually a net
  decrease of 4 lines of code because of simplifications it enables.
 
  Do we want the API to do more?  Currently it's very minimal, but I can
  imagine two ways it could be generalized.  It could take an argument
  to indicate which message list to return, which could be all messages,
  matched messages, top-level messages, or maybe even unmatched messages
  (possibly all in terms of message flags).  It could also take an
  argument indicating the desired sort order.  Currently, the caller can
  use existing message flag APIs to distinguish matched and unmatched
  messages and there's a separate function for the top-level messages.
  However, if the API could do all of these things, it would subsume
  various other API functions, such as notmuch_thread_get_*_date.
 
 I don't know if this is the right API. For the matched message etc I
 think using the existing message flag APIs is simple enough. I am not
 sure about sort orders though: that looks like it would be much easier
 for the caller to have the correct sort by I am not sure what users
 would need it.

For sort order, I would be inclined to simply construct the reverse
list the first time a caller asks for it.  Theoretically the caller
could do this just as easily as the library, except that we don't
expose the list routines.

If I do add sort order, I would also want to add some control over
which list is returned, since it would be asymmetric to be able to
request all messages in either order, but top-level messages only in
oldest-first.  I think this would be pretty simple, and would give us
a reasonably general-purpose and extensible API.  (It would also solve
the naming conundrum I mentioned below in my original email.)

 Best wishes
 
 Mark
 
 
 
 
 
  Also, is this the right name for the new API?  In particular, if we do
  later want to add a function that returns, say, the list of matched
  messages, we'll have a convention collision with
  notmuch_thread_get_matched_messages, which returns only a count.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [Patch v4 2/2] test: initial performance testing infrastructure

[Austin Clements]

Quoth da...@tethera.net on Nov 25 at 11:02 am:
 From: David Bremner brem...@debian.org
 
 This is not near as fancy as as the unit tests, on the theory that
 the code should typically be crashing when performance tuning.
 Nonetheless, there is plenty of room for improvement.  Several more of
 the pieces of the test infrastructure (e.g. the option parsing) could
 be factored out into test/test-lib-common.sh
 ---
  Makefile   |3 +-
  performance-test/.gitignore|1 +
  performance-test/Makefile  |7 +++
  performance-test/Makefile.local|   32 ++
  performance-test/README|   50 +++
  performance-test/basic |   15 +
  performance-test/download/.gitignore   |2 +
  .../download/notmuch-email-corpus-0.2.tar.xz.asc   |9 +++
  performance-test/notmuch-perf-test |   25 
  performance-test/perf-test-lib.sh  |   65 
 
  performance-test/version.sh|3 +
  11 files changed, 211 insertions(+), 1 deletion(-)
  create mode 100644 performance-test/.gitignore
  create mode 100644 performance-test/Makefile
  create mode 100644 performance-test/Makefile.local
  create mode 100644 performance-test/README
  create mode 100755 performance-test/basic
  create mode 100644 performance-test/download/.gitignore
  create mode 100644 
 performance-test/download/notmuch-email-corpus-0.2.tar.xz.asc
  create mode 100755 performance-test/notmuch-perf-test
  create mode 100644 performance-test/perf-test-lib.sh
  create mode 100644 performance-test/version.sh
 
 diff --git a/Makefile b/Makefile
 index bb9c316..5decbea 100644
 --- a/Makefile
 +++ b/Makefile
 @@ -3,7 +3,8 @@
  all:
  
  # List all subdirectories here. Each contains its own Makefile.local
 -subdirs = compat completion emacs lib man parse-time-string util test
 +subdirs := compat completion emacs lib man parse-time-string
 +subdirs := $(subdirs) performance-test util test

+= ?

  
  # We make all targets depend on the Makefiles themselves.
  global_deps = Makefile Makefile.config Makefile.local \
 diff --git a/performance-test/.gitignore b/performance-test/.gitignore
 new file mode 100644
 index 000..53f2697
 --- /dev/null
 +++ b/performance-test/.gitignore
 @@ -0,0 +1 @@
 +tmp.*/
 diff --git a/performance-test/Makefile b/performance-test/Makefile
 new file mode 100644
 index 000..de492a7
 --- /dev/null
 +++ b/performance-test/Makefile
 @@ -0,0 +1,7 @@
 +# See Makefile.local for the list of files to be compiled in this
 +# directory.
 +all:
 + $(MAKE) -C .. all
 +
 +.DEFAULT:
 + $(MAKE) -C .. $@
 diff --git a/performance-test/Makefile.local b/performance-test/Makefile.local
 new file mode 100644
 index 000..1114ec1
 --- /dev/null
 +++ b/performance-test/Makefile.local
 @@ -0,0 +1,32 @@
 +# -*- makefile -*-
 +
 +dir := performance-test
 +
 +include $(dir)/version.sh
 +
 +CORPUS_NAME := notmuch-email-corpus-$(PERFTEST_VERSION).tar.xz

Would it make sense to split out the different size corpora so a user
could, say, only download the small one?

 +TXZFILE := ${dir}/download/${CORPUS_NAME}
 +SIGFILE := ${TXZFILE}.asc
 +TEST_SCRIPT := ${dir}/notmuch-perf-test
 +DEFAULT_URL :=  http://notmuchmail.org/releases/${CORPUS_NAME}
 +
 +perf-test: setup-perf-test all
 + $(TEST_SCRIPT) $(OPTIONS)
 +
 +.PHONY: download-corpus setup-perf-test
 +
 +# Note that this intentionally does not depend on download-corpus.
 +setup-perf-test: $(TXZFILE)
 + gpg --verify $(SIGFILE)
 +
 +$(TXZFILE):
 + @printf \nPlease download ${TXZFILE}.\n\n

\nPlease download ${TXZFILE} using\n\n?

 + @printf \t%% make download-corpus\n\n
 + @echo or see http://notmuchmail.org/corpus for download locations
 + @echo
 + @false
 +
 +download-corpus:
 + wget -O ${TXZFILE} ${DEFAULT_URL}
 +
 +CLEAN := $(CLEAN) $(dir)/tmp.*
 diff --git a/performance-test/README b/performance-test/README
 new file mode 100644
 index 000..239d2fb
 --- /dev/null
 +++ b/performance-test/README
 @@ -0,0 +1,50 @@
 +Pre-requisites
 +--
 +
 +In addition to having notmuch, you need:
 +
 +- gpg
 +- gnu tar
 +- gnu time
 +- xz. Some speedup can be gotten by installing pixz, but this is
 +  probably only worthwhile if you are debugging the tests.
 +
 +Getting set up to run tests:
 +
 +
 +First, you need to get the corpus.
 +
 +It should work to run
 +
 +   % make download-corpus
 +
 +In case that fails or is too slow, check
 +
 +   http://notmuchmail.org/corpus
 +
 +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
 +available performance tests.
 +
 +Alternately, you can run a specific subset of tests by simply 

Re: [Patch v2 09/17] tag-util.[ch]: New files for common tagging routines

[Mark Walters]

This looks good. A couple of typos and a small queries (and I
agree with Tomi but I think you have already included that).

On Sat, 24 Nov 2012, da...@tethera.net wrote:
 From: David Bremner brem...@debian.org

 These are meant to be shared between notmuch-tag and notmuch-restore.

 The bulk of the routines implement a tag operation list abstract
 data type act as a structured representation of a set of tag
 operations (typically coming from a single tag command or line of
 input).
 ---
  Makefile.local |1 +
  tag-util.c |  264 
 
  tag-util.h |  120 ++
  3 files changed, 385 insertions(+)
  create mode 100644 tag-util.c
  create mode 100644 tag-util.h

 diff --git a/Makefile.local b/Makefile.local
 index 2b91946..854867d 100644
 --- a/Makefile.local
 +++ b/Makefile.local
 @@ -274,6 +274,7 @@ notmuch_client_srcs = \
   query-string.c  \
   mime-node.c \
   crypto.c\
 + tag-util.c
  
  notmuch_client_modules = $(notmuch_client_srcs:.c=.o)
  
 diff --git a/tag-util.c b/tag-util.c
 new file mode 100644
 index 000..287cc67
 --- /dev/null
 +++ b/tag-util.c
 @@ -0,0 +1,264 @@
 +#include string-util.h
 +#include tag-util.h
 +#include hex-escape.h
 +
 +struct _tag_operation_t {
 +const char *tag;
 +notmuch_bool_t remove;
 +};
 +
 +struct _tag_op_list_t {
 +tag_operation_t *ops;
 +int count;
 +int size;
 +};
 +
 +int
 +parse_tag_line (void *ctx, char *line,
 + tag_op_flag_t flags,
 + char **query_string,
 + tag_op_list_t *tag_ops)
 +{
 +char *tok = line;
 +size_t tok_len = 0;
 +
 +chomp_newline (line);
 +
 +/* remove leading space */
 +while (*tok == ' ' || *tok == '\t')
 + tok++;
 +
 +/* Skip empty and comment lines. */
 +if (*tok == '\0' || *tok == '#')
 + return 1;
 +
 +tag_op_list_reset (tag_ops);
 +
 +/* Parse tags. */
 +while ((tok = strtok_len (tok + tok_len,  , tok_len)) != NULL) {
 + notmuch_bool_t remove;
 + char *tag;
 +
 + /* Optional explicit end of tags marker. */
 + if (strncmp (tok, --, tok_len) == 0) {
 + tok = strtok_len (tok + tok_len,  , tok_len);
 + break;
 + }
 +
 + /* Implicit end of tags. */
 + if (*tok != '-'  *tok != '+')
 + break;
 +
 + /* If tag is terminated by NUL, there's no query string. */
 + if (*(tok + tok_len) == '\0') {
 + tok = NULL;
 + break;
 + }
 +
 + /* Terminate, and start next token after terminator. */
 + *(tok + tok_len++) = '\0';
 +
 + remove = (*tok == '-');
 + tag = tok + 1;
 +
 + /* Maybe refuse empty tags. */
 + if (!(flags  TAG_FLAG_BE_GENEROUS)  *tag == '\0') {
 + tok = NULL;
 + break;
 + }
 +
 + /* Decode tag. */
 + if (hex_decode_inplace (tag) != HEX_SUCCESS) {
 + tok = NULL;
 + break;
 + }
 +
 + if (tag_op_list_append (ctx, tag_ops, tag, remove))
 + return -1;
 +}
 +
 +if (tok == NULL || tag_ops-count == 0) {
 + /* FIXME: line has been modified! */
 + fprintf (stderr, Warning: Ignoring invalid input line: %s\n,
 +  line);
 + return 1;
 +}
 +
 +/* tok now points to the query string */
 +if (hex_decode_inplace (tok) != HEX_SUCCESS) {
 + /* FIXME: line has been modified! */
 + fprintf (stderr, Warning: Ignoring invalid input line: %s\n,
 +  line);
 + return 1;
 +}
 +
 +*query_string = tok;
 +
 +return 0;
 +}
 +
 +static inline void
 +message_error (notmuch_message_t *message,
 +notmuch_status_t status,
 +const char *format, ...)
 +{
 +va_list va_args;
 +
 +va_start (va_args, format);
 +
 +vfprintf (stderr, format, va_args);
 +fprintf (stderr, Message-ID: %s\n, notmuch_message_get_message_id 
 (message));
 +fprintf (stderr, Status: %s\n, notmuch_status_to_string (status));
 +}
 +
 +notmuch_status_t
 +tag_op_list_apply (notmuch_message_t *message,
 +tag_op_list_t *list,
 +tag_op_flag_t flags)
 +{
 +int i;
 +
 +notmuch_status_t status = 0;
 +tag_operation_t *tag_ops = list-ops;
 +
 +status = notmuch_message_freeze (message);
 +if (status) {
 + message_error (message, status, freezing message);
 + return status;
 +}
 +
 +if (flags  TAG_FLAG_REMOVE_ALL) {
 + status = notmuch_message_remove_all_tags (message);
 + if (status) {
 + message_error (message, status, removing all tags );
 + return status;
 + }
 +}
 +
 +for (i = 0; i  list-count; i++) {
 + if (tag_ops[i].remove) {
 + status = notmuch_message_remove_tag (message, tag_ops[i].tag);
 + if (status) {
 + message_error (message, status, removing tag %s, 
 tag_ops[i].tag);
 + return status;
 + }
 + } else {
 +

Re: [Patch v2 12/17] man: document notmuch tag --batch, --input options

[Mark Walters]

On Sat, 24 Nov 2012, da...@tethera.net wrote:
 From: Jani Nikula j...@nikula.org

 ---
  man/man1/notmuch-tag.1 |   52 
 +++-
  1 file changed, 51 insertions(+), 1 deletion(-)

 diff --git a/man/man1/notmuch-tag.1 b/man/man1/notmuch-tag.1
 index 0f86582..751db7b 100644
 --- a/man/man1/notmuch-tag.1
 +++ b/man/man1/notmuch-tag.1
 @@ -4,7 +4,12 @@ notmuch-tag \- add/remove tags for all messages matching the 
 search terms
  
  .SH SYNOPSIS
  .B notmuch tag
 -.RI  + tag |\- tag  [...] [\-\-]  search-term ...
 +.RI + tag |\- tag  [...] [\-\-]  search-terms 
 +
 +.B notmuch tag
 +.RI --batch
 +.RI [ --input= filename  ]
 +
  
  .SH DESCRIPTION
  
 @@ -29,6 +34,51 @@ updates the maildir flags according to tag changes if the
  configuration option is enabled. See \fBnotmuch-config\fR(1) for
  details.
  
 +Supported options for
 +.B tag
 +include
 +.RS 4
 +.TP 4
 +.BR \-\-batch
 +
 +Read batch tagging operations from standard input. This is more
 +efficient than repeated
 +.B notmuch tag
 +invocations. See
 +.B TAG FILE FORMAT
 +below for the input format. This option is not compatible with
 +specifying tagging on the command line.
 +.RE
 +
 +.RS 4
 +.TP 4
 +.BR \-\-input= filename
 +
 +Read input from given file, instead of from stdin. Implies
 +.BR --batch .
 +
 +.SH TAG FILE FORMAT
 +
 +The input must consist of lines of the format:
 +
 +.RI T + tag |\- tag  [...] [\-\-]  search-terms 

Is the T at the start of the line a remnant from a previous version?

Mark


 +
 +Each line is interpreted similarly to
 +.B notmuch tag
 +command line arguments. The delimiter is one or more spaces ' '. Any
 +characters in tag and search-terms
 +.B may
 +be hex encoded with %NN where NN is the hexadecimal value of the
 +character. Any ' ' and '%' characters in tag and search-terms
 +.B must
 +be hex encoded (using %20 and %25, respectively). Any characters that
 +are not part of tag or search-terms
 +.B must not
 +be hex encoded.
 +
 +Leading and trailing space ' ' is ignored. Empty lines and lines
 +beginning with '#' are ignored.
 +
  .SH SEE ALSO
  
  \fBnotmuch\fR(1), \fBnotmuch-config\fR(1), \fBnotmuch-count\fR(1),
 -- 
 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: [Patch v2 11/17] test: add test for notmuch tag --batch option

[Mark Walters]

Hi

On Sat, 24 Nov 2012, da...@tethera.net wrote:
 From: Jani Nikula j...@nikula.org

 Basic test of functionality, along with all combinations of options.
 ---
  test/tagging |   46 ++
  1 file changed, 46 insertions(+)

 diff --git a/test/tagging b/test/tagging
 index 980ff92..e5b8315 100755
 --- a/test/tagging
 +++ b/test/tagging
 @@ -46,6 +46,52 @@ test_expect_equal $output \
  thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; One (:\  inbox tag1 
 unread)
  thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Two (inbox tag1 tag4 
 unread)
  
 +test_begin_subtest --batch
 +notmuch tag --batch EOF
 +# %20 is a space in tag
 +-:%20 -tag1 +tag5 +tag6 -- One
 ++tag1 -tag1 -tag4 +tag4 -- Two
 +-tag6 One
 ++tag5 Two
 +EOF
 +output=$(notmuch search \* | notmuch_search_sanitize)
 +test_expect_equal $output \
 +thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; One (inbox tag5 unread)
 +thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Two (inbox tag4 tag5 
 unread)
 +
 + batch.in  EOF
 +# %20 is a space in tag
 +-:%20 -tag1 +tag5 +tag6 -- One
 ++tag1 -tag1 -tag4 +tag4 -- Two
 +-tag6 One
 ++tag5 Two
 +EOF
 +
 +test_begin_subtest --input
 +notmuch tag --input=batch.in
 +output=$(notmuch search \* | notmuch_search_sanitize)
 +test_expect_equal $output \
 +thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; One (inbox tag5 unread)
 +thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Two (inbox tag4 tag5 
 unread)

Wouldn't a different set of tag changes be a better test as presumably
this test can pass if the command just does nothing? 

Mark

 +
 +test_begin_subtest --batch --input
 +notmuch tag --batch --input=batch.in
 +output=$(notmuch search \* | notmuch_search_sanitize)
 +test_expect_equal $output \
 +thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; One (inbox tag5 unread)
 +thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Two (inbox tag4 tag5 
 unread)
 +
 +test_begin_subtest --batch, blank lines and comments
 +notmuch dump | sort  EXPECTED.$test_count
 +notmuch tag --batch EOF
 +# this line is a comment; the next has only white space
 +  
 +
 +# the previous line is empty
 +EOF
 +notmuch dump | sort  OUTPUT.$test_count
 +test_expect_equal_file EXPECTED.$test_count OUTPUT.$test_count
 +
  test_expect_code 1 Empty tag names 'notmuch tag + One'
  
  test_expect_code 1 Tag name beginning with - 'notmuch tag +- One'
 -- 
 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: Notmuch scripts (again), now with more usenet

[David Bremner]

Ethan Glasser-Camp ethan.glasser.c...@gmail.com writes:

 If Bremner is willing to put this package in contrib, I think he should
 do so.

I guess that's fine, but I'd be more likely to get around to it if
presented with a patch against git master 

d
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [Patch v4 2/2] test: initial performance testing infrastructure

[Austin Clements]

Quoth David Bremner on Nov 25 at  8:05 pm:
 Austin Clements amdra...@mit.edu writes:
  +add_email_corpus takes arguments --small and --medium for when you
  +want smaller corpuses to check.
 
  corpora?
 
 reworded to say 
 
 ,
 | add_email_corpus takes arguments --small and --medium for when you
 | want smaller subsets of the corpus to check.
 `

That's clearer.

 
  I'm a bit confused by this.  What happens if you don't specify --small
  or --medium?  Is the large/default corpus just the combined small
  and medium corpora?  Would be worth a comment, at least.
 
 Hopefully the README makes this clear(er) now?

The README definitely helps.  Might still be worth a comment in the
code since it took me some thinking to realize it would do something
reasonable when given no argument.  Perhaps above the initial
assignment of arg,

# With no argument, use the entire (combined) corpus

to acknowledge that this is a legitimate and intentional code path?

  This probably doesn't matter now, but I wonder if we want to unpack on
  first use to somewhere not test-specific and then cp -rl the corpus
  into the test directory.  I haven't tried unpacking the corpus yet,
  but if you're running tests repeatedly to compare results, or running
  more than one performance test, it seems like a full decompress and
  unpack could get onerous.
 
 Hmm. On my machine it is 10s for the copy versus 45s for a full
 unpack. For some reason I tested with cp -a which is incredibly slow, 
 so I thought there was no loss. For comparison the basic test takes
 about 10 minutes on the same machine.
 
 In any case this can wait until we have a second test file and a second
 call to add_mail_corpus, adding caching now would not help.

It would help (a little) if you run basic multiple times.  I think
it's completely reasonable to leave it as is for now and see if
caching would help down the road.
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch