[PATCH v2 00/20] insert command

[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 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, &tag_ops_count);
+    if (i < 0)
+	return 1;
 
     tag_ops[tag_ops_count].tag = NULL;
 
-- 
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 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 0000000..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 <novalazy@gmail.com>
+ */
+
+#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, &notmuch))
+	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...] [--] [+<tag>|-<tag> ...] < message",
+      "Add a new message into the maildir and notmuch database." },
     { "search", notmuch_search_command,
       "[options...] <search-terms> [...]",
       "Search for messages matching the given search terms." },
-- 
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 <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+
+/* 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 (&tv, 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, &tmppath, &newpath);
+    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, &notmuch))
 	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 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, &tmppath, &newpath);
     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 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 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, &message);
+    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 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, "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 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 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, &message);
     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, &new_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, &notmuch))
 	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 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, &tag_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 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, &notmuch))
 	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 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, &st) == 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, "folder", 0, 0 },
+	{ NOTMUCH_OPT_BOOLEAN, &create_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, &notmuch))
-- 
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 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, &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

[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 <sys/stat.h>
 #include <fcntl.h>
 
+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 (&action, 0, sizeof (struct sigaction));
+    action.sa_handler = handle_sigint;
+    sigemptyset (&action.sa_mask);
+    action.sa_flags = SA_RESTART;
+    sigaction (SIGINT, &action, NULL);
+
     config = notmuch_config_open (ctx, NULL, NULL);
     if (config == NULL)
 	return 1;
-- 
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 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 0000000..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 -0000\"" \
+    "[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 -0000\"" \
+    "[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 -0000\"" \
+    "[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 -0000\"" \
+    "[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 -0000\"" \
+    "[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 -0000\"" \
+    "[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 -0000\"" \
+    "[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 -0000\"" \
+    "[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 -0000\"" \
+    "[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 -0000\"" \
+    "[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 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 0000000..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 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 <search-terms>.
 .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),
 \fBnotmuch-tag\fR(1)
diff --git a/man/man1/notmuch-show.1 b/man/man1/notmuch-show.1
index 4481f21..08d64c9 100644
--- a/man/man1/notmuch-show.1
+++ b/man/man1/notmuch-show.1
@@ -184,7 +184,8 @@ command.
 .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\fR(1), \fBnotmuch-search-terms\fR(7),
 \fBnotmuch-tag\fR(1)
diff --git a/man/man1/notmuch-tag.1 b/man/man1/notmuch-tag.1
index 0f86582..83c3ab8 100644
--- a/man/man1/notmuch-tag.1
+++ b/man/man1/notmuch-tag.1
@@ -32,7 +32,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-restore\fR(1),
 \fBnotmuch-search\fR(1), \fBnotmuch-search-terms\fR(7),
 \fBnotmuch-show\fR(1),
diff --git a/man/man1/notmuch.1 b/man/man1/notmuch.1
index f06b224..ab41887 100644
--- a/man/man1/notmuch.1
+++ b/man/man1/notmuch.1
@@ -130,7 +130,8 @@ use ${HOME}/.notmuch\-config if this variable is not set.
 .SH SEE ALSO
 
 \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\fR(1), \fBnotmuch-search-terms\fR(7),
 \fBnotmuch-show\fR(1), \fBnotmuch-tag\fR(1)
diff --git a/man/man5/notmuch-hooks.5 b/man/man5/notmuch-hooks.5
index 07b64f5..829df8f 100644
--- a/man/man5/notmuch-hooks.5
+++ b/man/man5/notmuch-hooks.5
@@ -42,7 +42,7 @@ imported messages.
 .SH SEE ALSO
 
 \fBnotmuch\fR(1), \fBnotmuch-config\fR(1), \fBnotmuch-count\fR(1),
-\fBnotmuch-dump\fR(1), \fBnotmuch-new\fR(1), \fBnotmuch-reply\fR(1),
-\fBnotmuch-restore\fR(1), \fBnotmuch-search\fR(1),
+\fBnotmuch-dump\fR(1), \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/man7/notmuch-search-terms.7 b/man/man7/notmuch-search-terms.7
index e39b944..2766729 100644
--- a/man/man7/notmuch-search-terms.7
+++ b/man/man7/notmuch-search-terms.7
@@ -261,6 +261,7 @@ Some time zone codes, e.g. UTC, EET.
 .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\fR(1), \fBnotmuch-show\fR(1), \fBnotmuch-tag\fR(1)
-- 
1.7.12.1

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 <novalazy@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

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

[Mark Walters]

On Sun, 25 Nov 2012, Peter Wang <novalazy@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

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

[Mark Walters]

On Sun, 25 Nov 2012, Peter Wang <novalazy@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

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

[Mark Walters]

On Sun, 25 Nov 2012, Peter Wang <novalazy@gmail.com> 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 0000000..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@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 <novalazy@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 0000000..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 -0000\"" \
> +    "[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 -0000\"" \
> +    "[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 -0000\"" \
> +    "[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 -0000\"" \
> +    "[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 -0000\"" \
> +    "[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 -0000\"" \
> +    "[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 -0000\"" \
> +    "[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 -0000\"" \
> +    "[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 -0000\"" \
> +    "[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 -0000\"" \
> +    "[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

Re: [PATCH v2 00/20] insert command

[David Bremner]

Peter Wang <novalazy@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-david@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

Re: [PATCH v2 00/20] insert command

[Peter Wang]

On Sun, 25 Nov 2012 15:21:12 -0400, David Bremner <david@tethera.net> wrote:
> Peter Wang <novalazy@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-david@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.

That would be fine.

Peter

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

[Tomi Ollila]

On Sun, Nov 25 2012, Peter Wang <novalazy@gmail.com> wrote:

> Read the new message from standard input into the Maildir tmp file.
> ---

There are a few issues that gort my attention in this particular function:


>  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];

Copying in 4k blocks is slow when at least when doing file to file copy.
Also socket buffers can often hold much more data. When reading from
network and saving to file (in low-load machine) this is OK, but otherwise
something like 64k buffer works better(*).

(*) Now that I said it I have to measure this yet another time ;)

> +    char *p;
> +    ssize_t remain;
> +    ssize_t written;
> +
> +    for (;;) {
> +	remain = read (fdin, buf, sizeof(buf));

space between sizeof and (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;
> +	}

You're claiming in function name & and its description that this is more
"generic" copy function -- yet error message speaks about 'standard input'.

> +
> +	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;
> +	    }

Ditto.

> +	    p += written;
> +	    remain -= written;
> +	} while (remain > 0);
> +    }
> +
> +    return TRUE;
> +}
> +

Tomi

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

[Peter Wang]

On Mon, 26 Nov 2012 11:39:41 +0200, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> On Sun, Nov 25 2012, Peter Wang <novalazy@gmail.com> wrote:
> 
> > Read the new message from standard input into the Maildir tmp file.
> > ---
> 
> There are a few issues that gort my attention in this particular function:
> 
> 
> >  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];
> 
> Copying in 4k blocks is slow when at least when doing file to file copy.
> Also socket buffers can often hold much more data. When reading from
> network and saving to file (in low-load machine) this is OK, but otherwise
> something like 64k buffer works better(*).
> 
> (*) Now that I said it I have to measure this yet another time ;)

I get up to a whopping 10 ms faster copy of a 100 MiB file by increasing
the buffer size to 8192 bytes or more (standalone program, stdin to
disk, no fsync, no xapian).  Feel free to fiddle with it after it's
pushed, if you think it's worthwhile.

> You're claiming in function name & and its description that this is more
> "generic" copy function -- yet error message speaks about 'standard input'.

I'll rename it.

Peter