[PATCH 01/18] cli: add stub for insert command

[PATCH 01/18] cli: add stub for insert command

[Peter Wang]

This does nothing yet.
---
 Makefile.local   |    1 +
 notmuch-client.h |    3 +++
 notmuch-insert.c |   27 +++++++++++++++++++++++++++
 notmuch.c        |    3 +++
 4 files changed, 34 insertions(+), 0 deletions(-)
 create mode 100644 notmuch-insert.c

diff --git a/Makefile.local b/Makefile.local
index 296995d..950f046 100644
--- a/Makefile.local
+++ b/Makefile.local
@@ -282,6 +282,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 f930798..edbd3ee 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -132,6 +132,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..0e061a0
--- /dev/null
+++ b/notmuch-insert.c
@@ -0,0 +1,27 @@
+/* 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[])
+{
+    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.4.4

[PATCH 02/18] insert: open database

[Peter Wang]

Open the notmuch configuration file and database.
---
 notmuch-insert.c |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/notmuch-insert.c b/notmuch-insert.c
index 0e061a0..21424cf 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -23,5 +23,21 @@
 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;
 }
-- 
1.7.4.4

[PATCH 03/18] 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 |   80 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 79 insertions(+), 1 deletions(-)

diff --git a/notmuch-insert.c b/notmuch-insert.c
index 21424cf..f01a6f2 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -20,12 +20,86 @@
 
 #include "notmuch-client.h"
 
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+
+static notmuch_bool_t
+safe_gethostname (char *hostname, size_t hostname_size)
+{
+    if (gethostname (hostname, hostname_size) == -1) {
+	strncpy (hostname, "unknown", hostname_size);
+    }
+    hostname[hostname_size - 1] = '\0';
+
+    return (strchr (hostname, '/') == NULL);
+}
+
+static int
+maildir_open_tmp (void *ctx, const char *dir, char **tmppath, char **newpath)
+{
+    pid_t pid;
+    char hostname[256];
+    struct timeval tv;
+    char *filename;
+    int fd = -1;
+
+    /* This is the file name format used by Dovecot. */
+    pid = getpid ();
+    if (! safe_gethostname (hostname, sizeof (hostname))) {
+	fprintf (stderr, "Error: invalid host name.\n");
+	return -1;
+    }
+    gettimeofday (&tv, NULL);
+    filename = talloc_asprintf (ctx, "%ld.M%ldP%d.%s",
+				tv.tv_sec, tv.tv_usec, pid, hostname);
+
+    *tmppath = talloc_asprintf (ctx, "%s/tmp/%s", dir, filename);
+
+    do {
+	fd = open (*tmppath, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL, 0666);
+    } while (fd == -1 && errno == EEXIST);
+
+    if (fd != -1) {
+	*newpath = talloc_asprintf (ctx, "%s/new/%s", dir, filename);
+    }
+    else {
+	fprintf (stderr, "Error: opening %s: %s\n",
+		 *tmppath, strerror (errno));
+	talloc_free (*tmppath);
+    }
+
+    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 (ctx, dir, &tmppath, &newpath);
+    if (fdout < 0) {
+	return FALSE;
+    }
+
+    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 +107,15 @@ 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 (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.4.4

[PATCH 04/18] 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 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/notmuch-insert.c b/notmuch-insert.c
index f01a6f2..340f7e4 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -75,21 +75,68 @@ maildir_open_tmp (void *ctx, const char *dir, char **tmppath, char **newpath)
 }
 
 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)
 {
     char *tmppath;
     char *newpath;
     int fdout;
+    notmuch_bool_t ret;
 
     fdout = maildir_open_tmp (ctx, dir, &tmppath, &newpath);
     if (fdout < 0) {
 	return FALSE;
     }
 
+    ret = copy_fd_data (fdin, fdout);
+
     close (fdout);
-    unlink (tmppath);
-    return FALSE;
+
+    if (!ret) {
+	unlink (tmppath);
+    }
+
+    return ret;
 }
 
 int
-- 
1.7.4.4

[PATCH 05/18] 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 |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/notmuch-insert.c b/notmuch-insert.c
index 340f7e4..bab1fed 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -75,6 +75,20 @@ maildir_open_tmp (void *ctx, const char *dir, char **tmppath, char **newpath)
 }
 
 static notmuch_bool_t
+maildir_move_to_new (const char *tmppath, const char *newpath)
+{
+    /* We follow the Dovecot recommendation to simply use rename()
+     * instead of link() and unlink().
+     */
+    if (rename (tmppath, newpath) == 0) {
+	return TRUE;
+    }
+
+    fprintf (stderr, "Error: rename() failed: %s\n", strerror (errno));
+    return FALSE;
+}
+
+static notmuch_bool_t
 copy_fd_data (int fdin, int fdout)
 {
     char buf[4096];
@@ -132,6 +146,10 @@ insert_message (void *ctx, notmuch_database_t *notmuch, int fdin,
 
     close (fdout);
 
+    if (ret) {
+	ret = maildir_move_to_new (tmppath, newpath);
+    }
+
     if (!ret) {
 	unlink (tmppath);
     }
-- 
1.7.4.4

[PATCH 06/18] 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 |   44 ++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 44 insertions(+), 0 deletions(-)

diff --git a/notmuch-insert.c b/notmuch-insert.c
index bab1fed..dd449bc 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -129,6 +129,42 @@ copy_fd_data (int fdin, int fdout)
 }
 
 static notmuch_bool_t
+save_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)
 {
@@ -152,6 +188,14 @@ insert_message (void *ctx, notmuch_database_t *notmuch, int fdin,
 
     if (!ret) {
 	unlink (tmppath);
+	return FALSE;
+    }
+
+    ret = save_database (notmuch, newpath);
+
+    if (!ret) {
+	/* XXX maybe there should be an option to keep the file in maildir? */
+	unlink (newpath);
     }
 
     return ret;
-- 
1.7.4.4

[PATCH 07/18] insert: add --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 files changed, 20 insertions(+), 1 deletions(-)

diff --git a/notmuch-insert.c b/notmuch-insert.c
index dd449bc..6398618 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -207,16 +207,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 (notmuch_database_open (notmuch_config_get_database_path (config),
 			       NOTMUCH_DATABASE_MODE_READ_WRITE, &notmuch))
-- 
1.7.4.4

[PATCH 08/18] insert: check folder name

[Peter Wang]

Don't accept folder names containing a ".." component,
to prevent writing outside of the maildir.
---
 notmuch-insert.c |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/notmuch-insert.c b/notmuch-insert.c
index 6398618..ee51a87 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -25,6 +25,22 @@
 #include <fcntl.h>
 
 static notmuch_bool_t
+check_folder_name (const char *folder)
+{
+    const char *p = folder;
+
+    /* Check ".." appears nowhere in the folder name. */
+    for (;;) {
+	if ((p[0] == '.') && (p[1] == '.') && (p[2] == '\0' || p[2] == '/'))
+	    return FALSE;
+	p = strchr (p, '/');
+	if (!p)
+	    return TRUE;
+	p++;
+    }
+}
+
+static notmuch_bool_t
 safe_gethostname (char *hostname, size_t hostname_size)
 {
     if (gethostname (hostname, hostname_size) == -1) {
@@ -232,6 +248,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.4.4

[PATCH 09/18] 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 |   21 +++++++++++++++++----
 1 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/notmuch-insert.c b/notmuch-insert.c
index ee51a87..4fb3ea3 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -145,10 +145,12 @@ copy_fd_data (int fdin, int fdout)
 }
 
 static notmuch_bool_t
-save_database (notmuch_database_t *notmuch, const char *path)
+save_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) {
@@ -173,6 +175,14 @@ save_database (notmuch_database_t *notmuch, const char *path)
 	return FALSE;
     }
 
+    notmuch_message_freeze (message);
+
+    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);
@@ -182,7 +192,7 @@ save_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;
@@ -207,7 +217,7 @@ insert_message (void *ctx, notmuch_database_t *notmuch, int fdin,
 	return FALSE;
     }
 
-    ret = save_database (notmuch, newpath);
+    ret = save_database (notmuch, newpath, new_tags);
 
     if (!ret) {
 	/* XXX maybe there should be an option to keep the file in maildir? */
@@ -223,6 +233,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;
@@ -246,6 +258,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)) {
@@ -261,7 +274,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.4.4

[PATCH 10/18] insert: parse command-line tag operations

[Peter Wang]

Parse +tag and -tag on the 'insert' command-line.
Issue a warning about ambiguous -tag arguments which don't follow
+tag nor an explicit option list terminator.
---
 notmuch-insert.c |   50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 50 insertions(+), 0 deletions(-)

diff --git a/notmuch-insert.c b/notmuch-insert.c
index 4fb3ea3..6db03e3 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -24,6 +24,11 @@
 #include <sys/stat.h>
 #include <fcntl.h>
 
+typedef struct {
+    const char *tag;
+    notmuch_bool_t remove;
+} tag_operation_t;
+
 static notmuch_bool_t
 check_folder_name (const char *folder)
 {
@@ -236,8 +241,11 @@ notmuch_insert_command (void *ctx, int argc, char *argv[])
     const char **new_tags;
     size_t new_tags_length;
     const char *folder = NULL;
+    tag_operation_t *tag_ops;
+    int tag_ops_count = 0;
     char *maildir;
     int opt_index;
+    notmuch_bool_t warn_tag_rem;
     notmuch_bool_t ret;
 
     notmuch_opt_desc_t options[] = {
@@ -253,6 +261,48 @@ notmuch_insert_command (void *ctx, int argc, char *argv[])
 	return 1;
     }
 
+    if (opt_index > 0 && strcmp (argv[opt_index - 1], "--") == 0) {
+	warn_tag_rem = FALSE;
+    } else {
+	warn_tag_rem = TRUE;
+    }
+
+    /* Array of tagging operations (add or remove), terminated with an
+     * empty element. */
+    tag_ops = talloc_array (ctx, tag_operation_t, argc - opt_index + 1);
+    if (tag_ops == NULL) {
+	fprintf (stderr, "Out of memory.\n");
+	return 1;
+    }
+
+    for (; opt_index < argc; opt_index++) {
+	if (argv[opt_index][0] == '+') {
+	    tag_ops[tag_ops_count].tag = argv[opt_index] + 1;
+	    tag_ops[tag_ops_count].remove = FALSE;
+	    tag_ops_count++;
+	    warn_tag_rem = FALSE;
+	} else if (argv[opt_index][0] == '-') {
+	    if (warn_tag_rem) {
+		fprintf (stderr,
+			 "Warning: ambiguous argument treated as tag removal: %s\n",
+			 argv[opt_index]);
+	    }
+	    tag_ops[tag_ops_count].tag = argv[opt_index] + 1;
+	    tag_ops[tag_ops_count].remove = TRUE;
+	    tag_ops_count++;
+	} else {
+	    break;
+	}
+    }
+
+    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.4.4

[PATCH 11/18] insert: apply command-line tag operations

[Peter Wang]

Apply the +tag and -tag operations which were specified on the
command-line.
---
 notmuch-insert.c |   18 ++++++++++++++----
 1 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/notmuch-insert.c b/notmuch-insert.c
index 6db03e3..5fd79be 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -151,7 +151,7 @@ copy_fd_data (int fdin, int fdout)
 
 static notmuch_bool_t
 save_database (notmuch_database_t *notmuch, const char *path,
-	       const char **new_tags)
+	       const char **new_tags, const tag_operation_t *tag_ops)
 {
     notmuch_message_t *message;
     notmuch_status_t status;
@@ -186,6 +186,14 @@ save_database (notmuch_database_t *notmuch, const char *path,
 	notmuch_message_add_tag (message, new_tags[i]);
     }
 
+    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);
@@ -197,7 +205,8 @@ save_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 tag_operation_t *tag_ops)
 {
     char *tmppath;
     char *newpath;
@@ -222,7 +231,7 @@ insert_message (void *ctx, notmuch_database_t *notmuch, int fdin,
 	return FALSE;
     }
 
-    ret = save_database (notmuch, newpath, new_tags);
+    ret = save_database (notmuch, newpath, new_tags, tag_ops);
 
     if (!ret) {
 	/* XXX maybe there should be an option to keep the file in maildir? */
@@ -324,7 +333,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.4.4

[PATCH 12/18] insert: add copyright line from notmuch-deliver

[Peter Wang]

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

diff --git a/notmuch-insert.c b/notmuch-insert.c
index 5fd79be..a69dfe6 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.4.4

[PATCH 13/18] test: add tests for insert

[Peter Wang]

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

diff --git a/test/insert b/test/insert
new file mode 100755
index 0000000..3514920
--- /dev/null
+++ b/test/insert
@@ -0,0 +1,53 @@
+#!/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_done
diff --git a/test/notmuch-test b/test/notmuch-test
index ea39dfc..b40ba0b 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.4.4

[PATCH 14/18] man: document 'insert' command

[Peter Wang]

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

diff --git a/man/Makefile.local b/man/Makefile.local
index d43a949..f91bfd4 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..7d281b5
--- /dev/null
+++ b/man/man1/notmuch-insert.1
@@ -0,0 +1,50 @@
+.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 "--"
+.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.  notmuch will warn about ambiguous tag removal arguments
+which may be confused with option arguments.  It is recommended to use
+the option list terminator "--" to avoid ambiguity.
+
+Supported options for
+.B insert
+include
+.RS 4
+.TP 4
+.BI "--folder=<" folder ">"
+
+Deliver the message to the
+.RI Maildir/ folder
+directory, which must already exist.
+The default is to deliver to the top-level directory.
+
+.RE
+.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.4.4

[PATCH 15/18] 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 2ee555d..484ccdb 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 8551ab2..f9ba926 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 64abf01..4f219e7 100644
--- a/man/man1/notmuch-dump.1
+++ b/man/man1/notmuch-dump.1
@@ -31,7 +31,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 e01f2eb..b5c14f3 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 5aa86c0..416c796 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 18281c7..55a902d 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 b42eb2c..1794a5e 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 506583a..a7b1928 100644
--- a/man/man1/notmuch-show.1
+++ b/man/man1/notmuch-show.1
@@ -179,7 +179,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 d810e1b..87d9a0c 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 ebea4aa..0681f86 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 b914a29..ad42f39 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 b8ab52d..4baf3d1 100644
--- a/man/man7/notmuch-search-terms.7
+++ b/man/man7/notmuch-search-terms.7
@@ -135,6 +135,7 @@ current time:
 .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.4.4

[PATCH 16/18] insert: add --create-folder option

[Peter Wang]

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

diff --git a/notmuch-insert.c b/notmuch-insert.c
index a69dfe6..380c520 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -48,6 +48,70 @@ check_folder_name (const char *folder)
     }
 }
 
+static int
+mkdir_parents (void *ctx, const char *path, int mode)
+{
+    struct stat st;
+    char *pathcopy;
+    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;
+    }
+
+    pathcopy = talloc_strdup (ctx, path);
+    ret = 0;
+
+    for (start = pathcopy; *start != '\0'; start = end + 1) {
+	end = strchr (start + 1, '/');
+	if (!end) {
+	    ret = mkdir (path, mode);
+	    break;
+	}
+	*end = '\0';
+	ret = mkdir (pathcopy, mode);
+	if (ret != 0 && errno != EEXIST) {
+	    break;
+	}
+	*end = '/';
+    }
+
+    talloc_free (pathcopy);
+
+    return ret;
+}
+
+static notmuch_bool_t
+maildir_create (void *ctx, const char *dir)
+{
+    const int mode = 0755;
+    char *subdir;
+    char *end;
+
+    /* Create 'cur' directory, including parent directories. */
+    subdir = talloc_asprintf (ctx, "%s/cur", dir);
+    if (mkdir_parents (ctx, subdir, mode) != 0)
+	return FALSE;
+
+    end = subdir + strlen (subdir);
+
+    /* Create 'new' directory. */
+    strcpy (end - 3, "new");
+    if (mkdir (subdir, mode) != 0 && errno != EEXIST)
+	return FALSE;
+
+    /* Create 'tmp' directory. */
+    strcpy (end - 3, "tmp");
+    if (mkdir (subdir, mode) != 0 && errno != EEXIST)
+	return FALSE;
+
+    talloc_free (subdir);
+    return TRUE;
+}
+
 static notmuch_bool_t
 safe_gethostname (char *hostname, size_t hostname_size)
 {
@@ -253,6 +317,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;
     tag_operation_t *tag_ops;
     int tag_ops_count = 0;
     char *maildir;
@@ -262,6 +327,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 }
     };
 
@@ -328,6 +394,11 @@ notmuch_insert_command (void *ctx, int argc, char *argv[])
 	    return 1;
 	}
 	maildir = talloc_asprintf (ctx, "%s/%s", db_path, folder);
+	if (create_folder && ! maildir_create (ctx, maildir)) {
+	    fprintf (stderr, "Error: creating maildir %s: %s\n",
+		     maildir, strerror (errno));
+	    return 1;
+	}
     } else {
 	maildir = talloc_asprintf (ctx, "%s", db_path);
     }
-- 
1.7.4.4

[PATCH 17/18] man: document insert --create-folder option

[Peter Wang]

Document the new option.
---
 man/man1/notmuch-insert.1 |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/man/man1/notmuch-insert.1 b/man/man1/notmuch-insert.1
index 7d281b5..4bd8a35 100644
--- a/man/man1/notmuch-insert.1
+++ b/man/man1/notmuch-insert.1
@@ -36,10 +36,17 @@ include
 
 Deliver the message to the
 .RI Maildir/ folder
-directory, which must already exist.
+directory.
 The default is to deliver to the top-level directory.
 
 .RE
+
+.RS 4
+.TP 4
+.BI "--create-folder"
+
+Try to create the named folder if it does not exist.
+
 .RE
 .SH SEE ALSO
 
-- 
1.7.4.4

[PATCH 18/18] test: test insert --create-folder

[Peter Wang]

Add tests for the new option.
---
 test/insert |   24 ++++++++++++++++++++++++
 1 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/test/insert b/test/insert
index 3514920..548da58 100755
--- a/test/insert
+++ b/test/insert
@@ -50,4 +50,28 @@ generate_message \
 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, 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_done
-- 
1.7.4.4

Re: [PATCH 01/18] cli: add stub for insert command

[Jameson Graef Rollins]

[-- Attachment #1: Type: text/plain, Size: 958 bytes --]

On Wed, Jul 25 2012, Peter Wang <novalazy@gmail.com> wrote:
> This does nothing yet.

Hi, Peter.  This commit log is rather terse.  Maybe the commit message
should explain a bit what this stub is intended to do.

This is an interesting series, but let me play devil's advocate for a
moment: Why is this functionality needed?  What can this functionality
do that notmuch new can't?  Or how is it better?  I feel like everything
that this series does can be done with notmuch new, post-new hooks, and
folder searches.  Does this makes things more efficient in some way?

There reason I'm asking is that I wonder if what you're trying to
accomplish can be better addresses in a different way.  For instance,
there has been discussion about putting together an inotify hook for
notmuch that would insert messages individually when they are delivered
on the system.  That seems like it might be more useful and efficient
for individual message insertion.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH 01/18] cli: add stub for insert command

[Peter Wang]

On Wed, 25 Jul 2012 10:11:14 -0700, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> 
> This is an interesting series, but let me play devil's advocate for a
> moment: Why is this functionality needed?  What can this functionality
> do that notmuch new can't?

My mail is stored on a remote server running notmuch.
My email client runs locally, calling notmuch on the server via ssh.

After sending a message, I immediately add that message to the
Maildir/Sent folder and tag it `sent'.  When postponing a message,
I add the message to the Maildir/Drafts folder and tag it `draft'
(excluded by default).

notmuch new doesn't help me get the message into the Maildir in the
first place.  I would also need a second step to tag the messages.
Right now notmuch-deliver serves my purposes, but I would prefer it to
be part of notmuch.

Peter

Re: [PATCH 03/18] insert: open Maildir tmp file

[Mark Walters]

Hi

I know this has been around for a long time but it still applies so I
will try and review it. I have been using it somewhat and all seems to
work.

As an aside I have a postpone/resume implementation based on top
of it.

One general remark is that I think more comments could be helpful.

On Wed, 25 Jul 2012, Peter Wang <novalazy@gmail.com> wrote:
> Open a unique file in the Maildir tmp directory.
> The new message is not yet copied into the file.
> ---
>  notmuch-insert.c |   80 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 79 insertions(+), 1 deletions(-)
>
> diff --git a/notmuch-insert.c b/notmuch-insert.c
> index 21424cf..f01a6f2 100644
> --- a/notmuch-insert.c
> +++ b/notmuch-insert.c
> @@ -20,12 +20,86 @@
>  
>  #include "notmuch-client.h"
>  
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <fcntl.h>
> +
> +static notmuch_bool_t
> +safe_gethostname (char *hostname, size_t hostname_size)
> +{
> +    if (gethostname (hostname, hostname_size) == -1) {
> +	strncpy (hostname, "unknown", hostname_size);
> +    }
> +    hostname[hostname_size - 1] = '\0';
> +
> +    return (strchr (hostname, '/') == NULL);
> +}

Here is an example: a comment saying "like gethostname but guarantees
that hostname is  null terminated. Returns true unless hostname contains
a /.  

Also I found hostname_size a confusing term as it the size of the buffer
which is allowed to contain hostname rather than the size of
hostname. Maybe hostname_buffer_size or just len?

> +
> +static int
> +maildir_open_tmp (void *ctx, const char *dir, char **tmppath, char **newpath)
> +{
> +    pid_t pid;
> +    char hostname[256];
> +    struct timeval tv;
> +    char *filename;
> +    int fd = -1;
> +
> +    /* This is the file name format used by Dovecot. */
> +    pid = getpid ();
> +    if (! safe_gethostname (hostname, sizeof (hostname))) {
> +	fprintf (stderr, "Error: invalid host name.\n");
> +	return -1;
> +    }
> +    gettimeofday (&tv, NULL);
> +    filename = talloc_asprintf (ctx, "%ld.M%ldP%d.%s",
> +				tv.tv_sec, tv.tv_usec, pid, hostname);
> +
> +    *tmppath = talloc_asprintf (ctx, "%s/tmp/%s", dir, filename);

Does there need to be rather more error checking after talloc
operations? Eg what if this allocation fails?

> +
> +    do {
> +	fd = open (*tmppath, O_WRONLY | O_CREAT | O_TRUNC | O_EXCL, 0666);
> +    } while (fd == -1 && errno == EEXIST);

Am I misunderstanding or does this deadlock if the file *tmppath exists?
Do you want to recalculate tmppath inside the do while loop?

Also is 0666 standard or would 0644 or 0600 be better?

Best wishes

Mark

> +
> +    if (fd != -1) {
> +	*newpath = talloc_asprintf (ctx, "%s/new/%s", dir, filename);
> +    }
> +    else {
> +	fprintf (stderr, "Error: opening %s: %s\n",
> +		 *tmppath, strerror (errno));
> +	talloc_free (*tmppath);
> +    }
> +
> +    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 (ctx, dir, &tmppath, &newpath);
> +    if (fdout < 0) {
> +	return FALSE;
> +    }
> +
> +    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 +107,15 @@ 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 (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.4.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 06/18] insert: add new message to database

[Mark Walters]

Hi

On Wed, 25 Jul 2012, Peter Wang <novalazy@gmail.com> wrote:
> Add the new message to the notmuch database, renaming the file to encode
> notmuch tags as maildir flags.
> ---
>  notmuch-insert.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 44 insertions(+), 0 deletions(-)
>
> diff --git a/notmuch-insert.c b/notmuch-insert.c
> index bab1fed..dd449bc 100644
> --- a/notmuch-insert.c
> +++ b/notmuch-insert.c
> @@ -129,6 +129,42 @@ copy_fd_data (int fdin, int fdout)
>  }
>  
>  static notmuch_bool_t
> +save_database (notmuch_database_t *notmuch, const char *path)

I would prefer a different name here: save_database suggests to me that
it is a "whole database" operation, whereas it is actually just adding a
single message. Maybe add_message or notmuch_insert_add_message?

Best wishes

Mark

> +{
> +    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)
>  {
> @@ -152,6 +188,14 @@ insert_message (void *ctx, notmuch_database_t *notmuch, int fdin,
>  
>      if (!ret) {
>  	unlink (tmppath);
> +	return FALSE;
> +    }
> +
> +    ret = save_database (notmuch, newpath);
> +
> +    if (!ret) {
> +	/* XXX maybe there should be an option to keep the file in maildir? */
> +	unlink (newpath);
>      }
>  
>      return ret;
> -- 
> 1.7.4.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 10/18] insert: parse command-line tag operations

[Mark Walters]

Hi

On Wed, 25 Jul 2012, Peter Wang <novalazy@gmail.com> wrote:
> Parse +tag and -tag on the 'insert' command-line.
> Issue a warning about ambiguous -tag arguments which don't follow
> +tag nor an explicit option list terminator.
> ---
>  notmuch-insert.c |   50 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 50 insertions(+), 0 deletions(-)
>
> diff --git a/notmuch-insert.c b/notmuch-insert.c
> index 4fb3ea3..6db03e3 100644
> --- a/notmuch-insert.c
> +++ b/notmuch-insert.c
> @@ -24,6 +24,11 @@
>  #include <sys/stat.h>
>  #include <fcntl.h>
>  
> +typedef struct {
> +    const char *tag;
> +    notmuch_bool_t remove;
> +} tag_operation_t;
> +
>  static notmuch_bool_t
>  check_folder_name (const char *folder)
>  {
> @@ -236,8 +241,11 @@ notmuch_insert_command (void *ctx, int argc, char *argv[])
>      const char **new_tags;
>      size_t new_tags_length;
>      const char *folder = NULL;
> +    tag_operation_t *tag_ops;
> +    int tag_ops_count = 0;
>      char *maildir;
>      int opt_index;
> +    notmuch_bool_t warn_tag_rem;
>      notmuch_bool_t ret;
>  
>      notmuch_opt_desc_t options[] = {
> @@ -253,6 +261,48 @@ notmuch_insert_command (void *ctx, int argc, char *argv[])
>  	return 1;
>      }
>  
> +    if (opt_index > 0 && strcmp (argv[opt_index - 1], "--") == 0) {
> +	warn_tag_rem = FALSE;
> +    } else {
> +	warn_tag_rem = TRUE;
> +    }
> +
> +    /* Array of tagging operations (add or remove), terminated with an
> +     * empty element. */
> +    tag_ops = talloc_array (ctx, tag_operation_t, argc - opt_index + 1);
> +    if (tag_ops == NULL) {
> +	fprintf (stderr, "Out of memory.\n");
> +	return 1;
> +    }
> +
> +    for (; opt_index < argc; opt_index++) {
> +	if (argv[opt_index][0] == '+') {
> +	    tag_ops[tag_ops_count].tag = argv[opt_index] + 1;
> +	    tag_ops[tag_ops_count].remove = FALSE;
> +	    tag_ops_count++;
> +	    warn_tag_rem = FALSE;
> +	} else if (argv[opt_index][0] == '-') {
> +	    if (warn_tag_rem) {
> +		fprintf (stderr,
> +			 "Warning: ambiguous argument treated as tag removal: %s\n",
> +			 argv[opt_index]);
> +	    }
> +	    tag_ops[tag_ops_count].tag = argv[opt_index] + 1;
> +	    tag_ops[tag_ops_count].remove = TRUE;
> +	    tag_ops_count++;
> +	} else {
> +	    break;
> +	}
> +    }

I don't like the code duplication between this and notmuch-tag.c. In
particular the two (now) differ on how they deal with malformed tags
etc. Would it be possible to unify them?

Best wishes

Mark

> +
> +    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.4.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 14/18] man: document 'insert' command

[Mark Walters]

On Wed, 25 Jul 2012, Peter Wang <novalazy@gmail.com> wrote:
> Add initial documentation for notmuch insert command.
> ---
>  man/Makefile.local        |    1 +
>  man/man1/notmuch-insert.1 |   50 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 51 insertions(+), 0 deletions(-)
>  create mode 100644 man/man1/notmuch-insert.1
>
> diff --git a/man/Makefile.local b/man/Makefile.local
> index d43a949..f91bfd4 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..7d281b5
> --- /dev/null
> +++ b/man/man1/notmuch-insert.1
> @@ -0,0 +1,50 @@
> +.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 "--"
> +.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.  notmuch will warn about ambiguous tag removal arguments
> +which may be confused with option arguments.  It is recommended to use
> +the option list terminator "--" to avoid ambiguity.
> +
> +Supported options for
> +.B insert
> +include
> +.RS 4
> +.TP 4
> +.BI "--folder=<" folder ">"
> +
> +Deliver the message to the
> +.RI Maildir/ folder
> +directory, which must already exist.
> +The default is to deliver to the top-level directory.

I think this should say something about notmuch database root: so that
by default it delivers to the top-level directory of the notmuch
database, and otherwise is relative to that. Maybe just replace Maildir
by something like "<notmuch-database-root>" or something?

Best wishes

Mark



> +
> +.RE
> +.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.4.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 16/18] insert: add --create-folder option

[Mark Walters]

Hi

On Wed, 25 Jul 2012, Peter Wang <novalazy@gmail.com> wrote:
> Support an option to create a new folder in the maildir.
> ---
>  notmuch-insert.c |   71 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 71 insertions(+), 0 deletions(-)
>
> diff --git a/notmuch-insert.c b/notmuch-insert.c
> index a69dfe6..380c520 100644
> --- a/notmuch-insert.c
> +++ b/notmuch-insert.c
> @@ -48,6 +48,70 @@ check_folder_name (const char *folder)
>      }
>  }
>  
> +static int
> +mkdir_parents (void *ctx, const char *path, int mode)
> +{
> +    struct stat st;
> +    char *pathcopy;
> +    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;
> +    }
> +
> +    pathcopy = talloc_strdup (ctx, path);
> +    ret = 0;
> +
> +    for (start = pathcopy; *start != '\0'; start = end + 1) {
> +	end = strchr (start + 1, '/');
> +	if (!end) {
> +	    ret = mkdir (path, mode);
> +	    break;
> +	}
> +	*end = '\0';
> +	ret = mkdir (pathcopy, mode);
> +	if (ret != 0 && errno != EEXIST) {
> +	    break;
> +	}
> +	*end = '/';
> +    }
I am a little confused by this: why is there a +1 in the start = end + 1
line and in the end = strchr line? I believe it doesn't matter but it
makes me wonder if I am missing something.

Best wishes

Mark



> +
> +    talloc_free (pathcopy);
> +
> +    return ret;
> +}
> +
> +static notmuch_bool_t
> +maildir_create (void *ctx, const char *dir)
> +{
> +    const int mode = 0755;
> +    char *subdir;
> +    char *end;
> +
> +    /* Create 'cur' directory, including parent directories. */
> +    subdir = talloc_asprintf (ctx, "%s/cur", dir);
> +    if (mkdir_parents (ctx, subdir, mode) != 0)
> +	return FALSE;
> +
> +    end = subdir + strlen (subdir);
> +
> +    /* Create 'new' directory. */
> +    strcpy (end - 3, "new");
> +    if (mkdir (subdir, mode) != 0 && errno != EEXIST)
> +	return FALSE;
> +
> +    /* Create 'tmp' directory. */
> +    strcpy (end - 3, "tmp");
> +    if (mkdir (subdir, mode) != 0 && errno != EEXIST)
> +	return FALSE;
> +
> +    talloc_free (subdir);
> +    return TRUE;
> +}
> +
>  static notmuch_bool_t
>  safe_gethostname (char *hostname, size_t hostname_size)
>  {
> @@ -253,6 +317,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;
>      tag_operation_t *tag_ops;
>      int tag_ops_count = 0;
>      char *maildir;
> @@ -262,6 +327,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 }
>      };
>  
> @@ -328,6 +394,11 @@ notmuch_insert_command (void *ctx, int argc, char *argv[])
>  	    return 1;
>  	}
>  	maildir = talloc_asprintf (ctx, "%s/%s", db_path, folder);
> +	if (create_folder && ! maildir_create (ctx, maildir)) {
> +	    fprintf (stderr, "Error: creating maildir %s: %s\n",
> +		     maildir, strerror (errno));
> +	    return 1;
> +	}
>      } else {
>  	maildir = talloc_asprintf (ctx, "%s", db_path);
>      }
> -- 
> 1.7.4.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 05/18] insert: move file from Maildir tmp to new

[Mark Walters]

On Wed, 25 Jul 2012, Peter Wang <novalazy@gmail.com> wrote:
> Atomically move the new message file from the Maildir 'tmp' directory
> to 'new'.
> ---
>  notmuch-insert.c |   18 ++++++++++++++++++
>  1 files changed, 18 insertions(+), 0 deletions(-)
>
> diff --git a/notmuch-insert.c b/notmuch-insert.c
> index 340f7e4..bab1fed 100644
> --- a/notmuch-insert.c
> +++ b/notmuch-insert.c
> @@ -75,6 +75,20 @@ maildir_open_tmp (void *ctx, const char *dir, char **tmppath, char **newpath)
>  }
>  
>  static notmuch_bool_t
> +maildir_move_to_new (const char *tmppath, const char *newpath)
> +{
> +    /* We follow the Dovecot recommendation to simply use rename()
> +     * instead of link() and unlink().
> +     */
> +    if (rename (tmppath, newpath) == 0) {
> +	return TRUE;
> +    }

Do we want to overwrite an existing message with this name? As far as I
can see rename does overwrite and link would not: was that why rename is
better than link/unlink?

I would prefer not to overwrite but maybe there is a reason we need to. 
Would a possible alternative be to loop when finding a tmp file until
both the tmp file and the new file do not exist?

Best wishes

Mark


> +
> +    fprintf (stderr, "Error: rename() failed: %s\n", strerror (errno));
> +    return FALSE;
> +}
> +
> +static notmuch_bool_t
>  copy_fd_data (int fdin, int fdout)
>  {
>      char buf[4096];
> @@ -132,6 +146,10 @@ insert_message (void *ctx, notmuch_database_t *notmuch, int fdin,
>  
>      close (fdout);
>  
> +    if (ret) {
> +	ret = maildir_move_to_new (tmppath, newpath);
> +    }
> +
>      if (!ret) {
>  	unlink (tmppath);
>      }
> -- 
> 1.7.4.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 01/18] cli: add stub for insert command

[Mark Walters]

Hi

I have now been through essentially the whole series (except the tests)
and broadly like it. Just to summarise my concerns from the individual
replies here:

What should insert do when interrupted and is it safe? I am not
knowledgeable enough to be confident about that.

I think some of the talloc allocations need their return values
checked. I am more worried about this here then in most of the rest of
notmuch as we are writing to the database (and even to the mailstore
itself).

My other concerns (code duplication with notmuch-tag.c and possible
deadlock in the tmp file code and file overwriting) are easily fixable

Best wishes

Mark




On Wed, 25 Jul 2012, Peter Wang <novalazy@gmail.com> wrote:
> This does nothing yet.
> ---
>  Makefile.local   |    1 +
>  notmuch-client.h |    3 +++
>  notmuch-insert.c |   27 +++++++++++++++++++++++++++
>  notmuch.c        |    3 +++
>  4 files changed, 34 insertions(+), 0 deletions(-)
>  create mode 100644 notmuch-insert.c
>
> diff --git a/Makefile.local b/Makefile.local
> index 296995d..950f046 100644
> --- a/Makefile.local
> +++ b/Makefile.local
> @@ -282,6 +282,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 f930798..edbd3ee 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -132,6 +132,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..0e061a0
> --- /dev/null
> +++ b/notmuch-insert.c
> @@ -0,0 +1,27 @@
> +/* 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[])
> +{
> +    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.4.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 01/18] cli: add stub for insert command

[David Bremner]

Jameson Graef Rollins <jrollins@finestructure.net> writes:
>
> This is an interesting series, but let me play devil's advocate for a
> moment: Why is this functionality needed?  What can this functionality
> do that notmuch new can't?  Or how is it better?  I feel like everything
> that this series does can be done with notmuch new, post-new hooks, and
> folder searches.  Does this makes things more efficient in some way?
>

One thing that Austin mentioned is that (roughly speaking) notmuch-new
has no good way of dealing with duplicate message id's, so that might be
another reason to integrate this functionality.

d

Re: [PATCH 05/18] insert: move file from Maildir tmp to new

[Peter Wang]

On Sun, 18 Nov 2012 17:33:46 +0000, Mark Walters <markwalters1009@gmail.com> wrote:
> On Wed, 25 Jul 2012, Peter Wang <novalazy@gmail.com> wrote:
> > Atomically move the new message file from the Maildir 'tmp' directory
> > to 'new'.
> > ---
> >  notmuch-insert.c |   18 ++++++++++++++++++
> >  1 files changed, 18 insertions(+), 0 deletions(-)
> >
> > diff --git a/notmuch-insert.c b/notmuch-insert.c
> > index 340f7e4..bab1fed 100644
> > --- a/notmuch-insert.c
> > +++ b/notmuch-insert.c
> > @@ -75,6 +75,20 @@ maildir_open_tmp (void *ctx, const char *dir, char **tmppath, char **newpath)
> >  }
> >  
> >  static notmuch_bool_t
> > +maildir_move_to_new (const char *tmppath, const char *newpath)
> > +{
> > +    /* We follow the Dovecot recommendation to simply use rename()
> > +     * instead of link() and unlink().
> > +     */
> > +    if (rename (tmppath, newpath) == 0) {
> > +	return TRUE;
> > +    }
> 
> Do we want to overwrite an existing message with this name? As far as I
> can see rename does overwrite and link would not: was that why rename is
> better than link/unlink?
> 
> I would prefer not to overwrite but maybe there is a reason we need to. 
> Would a possible alternative be to loop when finding a tmp file until
> both the tmp file and the new file do not exist?

According to [1] it's all pointless -- just generate unique file names.

The dovecot maildir-save.c has this comment:

/* maildir spec says we should use link() + unlink() here. however
   since our filename is guaranteed to be unique, rename() works just
   as well, except faster. even if the filename wasn't unique, the
   problem could still happen if the file was already moved from
   new/ to cur/, so link() doesn't really provide any safety anyway.

   Besides the small temporary performance benefits, this rename() is
   almost required with OSX's HFS+ filesystem, since it implements
   hard links in a pretty ugly way, which makes the performance crawl
   when a lot of hard links are used. */

Well, that's one point of view.  I can't say I know any better.

Peter

[1]: http://wiki.dovecot.org/MailboxFormat/Maildir#Mail_delivery

Re: [PATCH 01/18] cli: add stub for insert command

[Peter Wang]

On Sun, 18 Nov 2012 17:35:23 +0000, Mark Walters <markwalters1009@gmail.com> wrote:
> 
> Hi
> 
> I have now been through essentially the whole series (except the tests)
> and broadly like it. Just to summarise my concerns from the individual
> replies here:

Thanks for the review.

> 
> What should insert do when interrupted and is it safe? I am not
> knowledgeable enough to be confident about that.

Do you mean cleaning up stray files?  Otherwise I think we just return
an error code, and then it's the caller's responsibility.

> 
> I think some of the talloc allocations need their return values
> checked. I am more worried about this here then in most of the rest of
> notmuch as we are writing to the database (and even to the mailstore
> itself).

Will do.

Peter

Re: [PATCH 05/18] insert: move file from Maildir tmp to new

[Mark Walters]

Hi 

On Mon, 19 Nov 2012, Peter Wang <novalazy@gmail.com> wrote:
> On Sun, 18 Nov 2012 17:33:46 +0000, Mark Walters <markwalters1009@gmail.com> wrote:
>> On Wed, 25 Jul 2012, Peter Wang <novalazy@gmail.com> wrote:
>> > Atomically move the new message file from the Maildir 'tmp' directory
>> > to 'new'.
>> > ---
>> >  notmuch-insert.c |   18 ++++++++++++++++++
>> >  1 files changed, 18 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/notmuch-insert.c b/notmuch-insert.c
>> > index 340f7e4..bab1fed 100644
>> > --- a/notmuch-insert.c
>> > +++ b/notmuch-insert.c
>> > @@ -75,6 +75,20 @@ maildir_open_tmp (void *ctx, const char *dir, char **tmppath, char **newpath)
>> >  }
>> >  
>> >  static notmuch_bool_t
>> > +maildir_move_to_new (const char *tmppath, const char *newpath)
>> > +{
>> > +    /* We follow the Dovecot recommendation to simply use rename()
>> > +     * instead of link() and unlink().
>> > +     */
>> > +    if (rename (tmppath, newpath) == 0) {
>> > +	return TRUE;
>> > +    }
>> 
>> Do we want to overwrite an existing message with this name? As far as I
>> can see rename does overwrite and link would not: was that why rename is
>> better than link/unlink?
>> 
>> I would prefer not to overwrite but maybe there is a reason we need to. 
>> Would a possible alternative be to loop when finding a tmp file until
>> both the tmp file and the new file do not exist?
>
> According to [1] it's all pointless -- just generate unique file names.
>
> The dovecot maildir-save.c has this comment:
>
> /* maildir spec says we should use link() + unlink() here. however
>    since our filename is guaranteed to be unique, rename() works just
>    as well, except faster. even if the filename wasn't unique, the
>    problem could still happen if the file was already moved from
>    new/ to cur/, so link() doesn't really provide any safety anyway.
>
>    Besides the small temporary performance benefits, this rename() is
>    almost required with OSX's HFS+ filesystem, since it implements
>    hard links in a pretty ugly way, which makes the performance crawl
>    when a lot of hard links are used. */
>
> Well, that's one point of view.  I can't say I know any better.

I think I agree with you/them. Indeed, since files in cur can have
maildir flags we could find that this rename works but then the new file
gets stomped on by notmuch_message_tags_to_maildir_flags which also uses
rename. It might be work adding the link to [1] in the comment, but in
any case I am happy with this now.

Best wishes

Mark

>
> Peter
>
> [1]: http://wiki.dovecot.org/MailboxFormat/Maildir#Mail_delivery

Re: [PATCH 01/18] cli: add stub for insert command

[Mark Walters]

Hi
On Mon, 19 Nov 2012, Peter Wang <novalazy@gmail.com> wrote:
> On Sun, 18 Nov 2012 17:35:23 +0000, Mark Walters <markwalters1009@gmail.com> wrote:
>> 
>> Hi
>> 
>> I have now been through essentially the whole series (except the tests)
>> and broadly like it. Just to summarise my concerns from the individual
>> replies here:
>
> Thanks for the review.
>
>> 
>> What should insert do when interrupted and is it safe? I am not
>> knowledgeable enough to be confident about that.
>
> Do you mean cleaning up stray files?  Otherwise I think we just return
> an error code, and then it's the caller's responsibility.

It's more that I am concerned that we don't corrupt the database (or get
into some inconsistent state). I have some recollection that there were some
changes to notmuch new to try and make it interrupt safe but I don't
know what they were or whether they apply here.

Best wishes

Mark


>
>> 
>> I think some of the talloc allocations need their return values
>> checked. I am more worried about this here then in most of the rest of
>> notmuch as we are writing to the database (and even to the mailstore
>> itself).
>
> Will do.
>
> Peter

Re: [PATCH 04/18] insert: copy stdin to Maildir tmp file

[Ali Polatel]

[-- Attachment #1: Type: text/plain, Size: 2371 bytes --]

On Wed, Jul 25, 2012 at 11:42:33PM +1000, Peter Wang wrote:
>Read the new message from standard input into the Maildir tmp file.
>---
> notmuch-insert.c |   51 +++++++++++++++++++++++++++++++++++++++++++++++++--
> 1 files changed, 49 insertions(+), 2 deletions(-)
>
>diff --git a/notmuch-insert.c b/notmuch-insert.c
>index f01a6f2..340f7e4 100644
>--- a/notmuch-insert.c
>+++ b/notmuch-insert.c
>@@ -75,21 +75,68 @@ maildir_open_tmp (void *ctx, const char *dir, char **tmppath, char **newpath)
> }
>
> 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;
>+}

LGTM. As an optimisation we can also consider using the splice(2) system
call if it is available (as notmuch-deliver does). In case splice()
fails with ENOSYS or EINVAL we can fall back to this method.

I can write the patch for it once this is accepted.
Thanks for the good work!

>+static notmuch_bool_t
> insert_message (void *ctx, notmuch_database_t *notmuch, int fdin,
> 		const char *dir)
> {
>     char *tmppath;
>     char *newpath;
>     int fdout;
>+    notmuch_bool_t ret;
>
>     fdout = maildir_open_tmp (ctx, dir, &tmppath, &newpath);
>     if (fdout < 0) {
> 	return FALSE;
>     }
>
>+    ret = copy_fd_data (fdin, fdout);
>+
>     close (fdout);
>-    unlink (tmppath);
>-    return FALSE;
>+
>+    if (!ret) {
>+	unlink (tmppath);
>+    }
>+
>+    return ret;
> }
>
> int
>-- 
>1.7.4.4
>
>_______________________________________________
>notmuch mailing list
>notmuch@notmuchmail.org
>http://notmuchmail.org/mailman/listinfo/notmuch

[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]