unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH v4 00/12] insert command
@ 2013-01-24 12:07 Peter Wang
  2013-01-24 12:07 ` [PATCH v4 01/12] tag-util: move out 'tag' command-line checks Peter Wang
                   ` (11 more replies)
  0 siblings, 12 replies; 19+ messages in thread
From: Peter Wang @ 2013-01-24 12:07 UTC (permalink / raw)
  To: notmuch

Differences from v3:

 - squashed patches; take it up with Jani
 - address some review comments (interdiff follows)
 - some stylistic things I left for someone who cares
   (either I tried it and didn't like it, or disagree with the premise)
 - split doc and test patches so series can be partially applied
   without --folder or --create-folder options

Peter Wang (12):
  tag-util: move out 'tag' command-line checks
  tag-util: do not reset list in parse_tag_command_line
  cli: add insert command
  man: document 'insert' command
  man: reference notmuch-insert.1
  test: add tests for insert
  insert: add --folder option
  man: document insert --folder option
  test: test insert --folder option
  insert: add --create-folder option
  man: document insert --create-folder
  test: test insert --create-folder option

 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       |  63 ++++++
 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                |   3 +
 notmuch-insert.c                | 484 ++++++++++++++++++++++++++++++++++++++++
 notmuch-tag.c                   |  10 +
 notmuch.c                       |   3 +
 tag-util.c                      |  13 +-
 tag-util.h                      |   2 +
 test/insert                     | 110 +++++++++
 test/notmuch-test               |   1 +
 23 files changed, 705 insertions(+), 27 deletions(-)
 create mode 100644 man/man1/notmuch-insert.1
 create mode 100644 notmuch-insert.c
 create mode 100755 test/insert

-- 
1.7.12.1


diff --git a/man/man1/notmuch-insert.1 b/man/man1/notmuch-insert.1
index 4a7cbeb..8ce634e 100644
--- a/man/man1/notmuch-insert.1
+++ b/man/man1/notmuch-insert.1
@@ -24,6 +24,10 @@ configuration option, then by operations specified on the command-line:
 tags prefixed by '+' are added while
 those prefixed by '\-' are removed.
 
+If the new message is a duplicate of an existing message in the database
+(it has same Message-ID), it will be added to the maildir folder and
+notmuch database, but the tags will not be changed.
+
 Option arguments must appear before any tag operation arguments.
 Supported options for
 .B insert
diff --git a/notmuch-insert.c b/notmuch-insert.c
index 6b3e380..69329ad 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -44,33 +44,22 @@ handle_sigint (unused (int sig))
 }
 
 /* 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
+ * returned, even if it has to make one up. Invalid characters are
+ * substituted such that the hostname can be used within a filename.
+ */
+static void
 safe_gethostname (char *hostname, size_t len)
 {
+    char *p;
+
     if (gethostname (hostname, len) == -1) {
 	strncpy (hostname, "unknown", len);
     }
     hostname[len - 1] = '\0';
 
-    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++;
+    for (p = hostname; *p != '\0'; p++) {
+	if (*p == '/' || *p == ':')
+	    *p = '_';
     }
 }
 
@@ -96,6 +85,23 @@ sync_dir (const char *dir)
     return ret;
 }
 
+/* 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++;
+    }
+}
+
 /* Make the given directory, succeeding if it already exists. */
 static notmuch_bool_t
 make_directory (char *path, int mode)
@@ -206,10 +212,7 @@ maildir_open_tmp_file (void *ctx, const char *dir,
 
     /* 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;
-    }
+    safe_gethostname (hostname, sizeof (hostname));
     do {
 	gettimeofday (&tv, NULL);
 	filename = talloc_asprintf (ctx, "%ld.M%ldP%d.%s",
@@ -247,26 +250,6 @@ 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,
-		         const char *newdir)
-{
-    if (rename (tmppath, newpath) != 0) {
-	fprintf (stderr, "Error: rename() failed: %s\n", strerror (errno));
-	return FALSE;
-    }
-
-    /* Sync the 'new' directory after rename for durability. */
-    return sync_dir (newdir);
-}
-
 /* Copy the contents of standard input (fdin) into fdout. */
 static notmuch_bool_t
 copy_stdin (int fdin, int fdout)
@@ -291,11 +274,9 @@ copy_stdin (int fdin, int fdout)
 	p = buf;
 	do {
 	    written = write (fdout, p, remain);
-	    if (written == 0)
-		return FALSE;
-	    if (written < 0) {
-		if (errno == EINTR)
-		    continue;
+	    if (written < 0 && errno == EINTR)
+		continue;
+	    if (written <= 0) {
 		fprintf (stderr, "Error: writing to temporary file: %s",
 			 strerror (errno));
 		return FALSE;
@@ -320,9 +301,7 @@ add_file_to_database (notmuch_database_t *notmuch, const char *path,
     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:
@@ -340,11 +319,18 @@ add_file_to_database (notmuch_database_t *notmuch, const char *path,
 	return FALSE;
     }
 
-    tag_op_list_apply (message, tag_ops, TAG_FLAG_MAILDIR_SYNC);
+    if (status == NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID) {
+	/* Don't change tags of an existing message. */
+	status = notmuch_message_tags_to_maildir_flags (message);
+	if (status != NOTMUCH_STATUS_SUCCESS)
+	    fprintf (stderr, "Error: failed to sync tags to maildir flags\n");
+    } else {
+	status = tag_op_list_apply (message, tag_ops, TAG_FLAG_MAILDIR_SYNC);
+    }
 
     notmuch_message_destroy (message);
 
-    return TRUE;
+    return (status == NOTMUCH_STATUS_SUCCESS) ? TRUE : FALSE;
 }
 
 static notmuch_bool_t
@@ -355,29 +341,45 @@ insert_message (void *ctx, notmuch_database_t *notmuch, int fdin,
     char *newpath;
     char *newdir;
     int fdout;
-    notmuch_bool_t ret;
 
     fdout = maildir_open_tmp_file (ctx, dir, &tmppath, &newpath, &newdir);
     if (fdout < 0) {
 	return FALSE;
     }
-    ret = copy_stdin (fdin, fdout);
-    if (ret && fsync (fdout) != 0) {
+
+    if (! copy_stdin (fdin, fdout)) {
+	close (fdout);
+	unlink (tmppath);
+	return FALSE;
+    }
+
+    if (fsync (fdout) != 0) {
 	fprintf (stderr, "Error: fsync failed: %s\n", strerror (errno));
-	ret = FALSE;
+	close (fdout);
+	unlink (tmppath);
+	return FALSE;
     }
+
     close (fdout);
-    if (ret) {
-	ret = maildir_move_tmp_to_new (tmppath, newpath, newdir);
-    }
-    if (!ret) {
+
+    /* 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
+     */
+    if (rename (tmppath, newpath) != 0) {
+	fprintf (stderr, "Error: rename() failed: %s\n", strerror (errno));
 	unlink (tmppath);
 	return FALSE;
     }
 
-    ret = add_file_to_database (notmuch, newpath, tag_ops);
-    if (!ret) {
-	/* XXX maybe there should be an option to keep the file in maildir? */
+    if (! add_file_to_database (notmuch, newpath, tag_ops)) {
+	/* XXX add an option to keep the file in maildir? */
+	unlink (newpath);
+	return FALSE;
+    }
+
+    if (! sync_dir (newdir)) {
 	unlink (newpath);
 	return FALSE;
     }
@@ -398,7 +400,7 @@ notmuch_insert_command (void *ctx, int argc, char *argv[])
     char *query_string = NULL;
     const char *folder = NULL;
     notmuch_bool_t create_folder = FALSE;
-    char *maildir;
+    const char *maildir;
     int opt_index;
     unsigned int i;
     notmuch_bool_t ret;
@@ -443,23 +445,23 @@ notmuch_insert_command (void *ctx, int argc, char *argv[])
 	return 1;
     }
 
-    if (folder != NULL) {
+    if (folder == NULL) {
+	maildir = db_path;
+    } else {
 	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);
-    }
-    if (! maildir) {
-	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 (! maildir) {
+	    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;
+	}
     }
 
     /* Setup our handler for SIGINT. We do not set SA_RESTART so that copying
diff --git a/tag-util.c b/tag-util.c
index 41f2c09..b57ee32 100644
--- a/tag-util.c
+++ b/tag-util.c
@@ -188,6 +188,11 @@ parse_tag_command_line (void *ctx, int argc, char **argv,
 
     *query_str = query_string_from_args (ctx, argc - i, &argv[i]);
 
+    if (*query_str == NULL) {
+       fprintf (stderr, "Out of memory.\n");
+       return TAG_PARSE_OUT_OF_MEMORY;
+    }
+
     return TAG_PARSE_SUCCESS;
 }
 
diff --git a/test/insert b/test/insert
index a3b6283..24a61e1 100755
--- a/test/insert
+++ b/test/insert
@@ -46,10 +46,14 @@ expected='[[[{
 test_expect_equal_json "$output" "$expected"
 
 test_begin_subtest "Insert message, duplicate message"
-notmuch insert < "$gen_msg_filename"
+notmuch insert +duptag -unread < "$gen_msg_filename"
 output=$(notmuch search --output=files "subject:insert-subject" | wc -l)
 test_expect_equal "$output" 2
 
+test_begin_subtest "Insert message, duplicate message does not change tags"
+output=$(notmuch search --format=json --output=tags "subject:insert-subject")
+test_expect_equal_json "$output" '["inbox", "unread"]'
+
 test_begin_subtest "Insert message, add tag"
 gen_insert_msg
 notmuch insert +custom < "$gen_msg_filename"

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v4 01/12] tag-util: move out 'tag' command-line checks
  2013-01-24 12:07 [PATCH v4 00/12] insert command Peter Wang
@ 2013-01-24 12:07 ` Peter Wang
  2013-03-31 10:00   ` Jani Nikula
  2013-01-24 12:07 ` [PATCH v4 02/12] tag-util: do not reset list in parse_tag_command_line Peter Wang
                   ` (10 subsequent siblings)
  11 siblings, 1 reply; 19+ messages in thread
From: Peter Wang @ 2013-01-24 12:07 UTC (permalink / raw)
  To: notmuch

parse_tag_command_line checked for two error conditions which are
specific to the 'tag' command.  It can be reused for the forthcoming
notmuch 'insert' command if we move the checks out, into notmuch-tag.c.
---
 notmuch-tag.c | 10 ++++++++++
 tag-util.c    | 11 +++--------
 2 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/notmuch-tag.c b/notmuch-tag.c
index d9daf8f..a901dad 100644
--- a/notmuch-tag.c
+++ b/notmuch-tag.c
@@ -234,6 +234,16 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
 	if (parse_tag_command_line (ctx, argc - opt_index, argv + opt_index,
 				    &query_string, tag_ops))
 	    return 1;
+
+	if (tag_op_list_size (tag_ops) == 0) {
+	    fprintf (stderr, "Error: 'notmuch tag' requires at least one tag to add or remove.\n");
+	    return 1;
+	}
+
+	if (*query_string == '\0') {
+	    fprintf (stderr, "Error: notmuch tag requires at least one search term.\n");
+	    return 1;
+	}
     }
 
     config = notmuch_config_open (ctx, NULL, NULL);
diff --git a/tag-util.c b/tag-util.c
index 701d329..743d591 100644
--- a/tag-util.c
+++ b/tag-util.c
@@ -188,16 +188,11 @@ parse_tag_command_line (void *ctx, int argc, char **argv,
 	tag_op_list_append (tag_ops, argv[i] + 1, is_remove);
     }
 
-    if (tag_op_list_size (tag_ops) == 0) {
-	fprintf (stderr, "Error: 'notmuch tag' requires at least one tag to add or remove.\n");
-	return TAG_PARSE_INVALID;
-    }
-
     *query_str = query_string_from_args (ctx, argc - i, &argv[i]);
 
-    if (*query_str == NULL || **query_str == '\0') {
-	fprintf (stderr, "Error: notmuch tag requires at least one search term.\n");
-	return TAG_PARSE_INVALID;
+    if (*query_str == NULL) {
+       fprintf (stderr, "Out of memory.\n");
+       return TAG_PARSE_OUT_OF_MEMORY;
     }
 
     return TAG_PARSE_SUCCESS;
-- 
1.7.12.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v4 02/12] tag-util: do not reset list in parse_tag_command_line
  2013-01-24 12:07 [PATCH v4 00/12] insert command Peter Wang
  2013-01-24 12:07 ` [PATCH v4 01/12] tag-util: move out 'tag' command-line checks Peter Wang
@ 2013-01-24 12:07 ` Peter Wang
  2013-01-24 12:07 ` [PATCH v4 03/12] cli: add insert command Peter Wang
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: Peter Wang @ 2013-01-24 12:07 UTC (permalink / raw)
  To: notmuch

No current callers of parse_tag_command_line require that it clear its
tag list argument.  The notmuch 'insert' command will be better served
if the function modifies a pre-populated list (of new.tags) instead of
clobbering it outright.
---
 tag-util.c | 2 --
 tag-util.h | 2 ++
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tag-util.c b/tag-util.c
index 743d591..b57ee32 100644
--- a/tag-util.c
+++ b/tag-util.c
@@ -165,8 +165,6 @@ parse_tag_command_line (void *ctx, int argc, char **argv,
 
     int i;
 
-    tag_op_list_reset (tag_ops);
-
     for (i = 0; i < argc; i++) {
 	if (strcmp (argv[i], "--") == 0) {
 	    i++;
diff --git a/tag-util.h b/tag-util.h
index 246de85..4628f16 100644
--- a/tag-util.h
+++ b/tag-util.h
@@ -81,6 +81,8 @@ parse_tag_line (void *ctx, char *line,
  * Output Parameters:
  *	ops	contains a list of tag operations
  *	query_str the search terms.
+ *
+ * The ops argument is not cleared.
  */
 
 tag_parse_status_t
-- 
1.7.12.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v4 03/12] cli: add insert command
  2013-01-24 12:07 [PATCH v4 00/12] insert command Peter Wang
  2013-01-24 12:07 ` [PATCH v4 01/12] tag-util: move out 'tag' command-line checks Peter Wang
  2013-01-24 12:07 ` [PATCH v4 02/12] tag-util: do not reset list in parse_tag_command_line Peter Wang
@ 2013-01-24 12:07 ` Peter Wang
  2013-03-06  1:13   ` David Bremner
  2013-01-24 12:08 ` [PATCH v4 04/12] man: document 'insert' command Peter Wang
                   ` (8 subsequent siblings)
  11 siblings, 1 reply; 19+ messages in thread
From: Peter Wang @ 2013-01-24 12:07 UTC (permalink / raw)
  To: notmuch

The notmuch insert command reads a message from standard input,
writes it to a Maildir folder, and then incorporates 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 | 341 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 notmuch.c        |   3 +
 4 files changed, 348 insertions(+)
 create mode 100644 notmuch-insert.c

diff --git a/Makefile.local b/Makefile.local
index c274f07..bb2381d 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 5f28836..af7d094 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -175,6 +175,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..31c1152
--- /dev/null
+++ b/notmuch-insert.c
@@ -0,0 +1,341 @@
+/* notmuch - Not much of an email program, (just index and search)
+ *
+ * Copyright © 2013 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
+ * (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"
+#include "tag-util.h"
+
+#include <sys/types.h>
+#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. Invalid characters are
+ * substituted such that the hostname can be used within a filename.
+ */
+static void
+safe_gethostname (char *hostname, size_t len)
+{
+    char *p;
+
+    if (gethostname (hostname, len) == -1) {
+	strncpy (hostname, "unknown", len);
+    }
+    hostname[len - 1] = '\0';
+
+    for (p = hostname; *p != '\0'; p++) {
+	if (*p == '/' || *p == ':')
+	    *p = '_';
+    }
+}
+
+/* Call fsync() on a directory path. */
+static notmuch_bool_t
+sync_dir (const char *dir)
+{
+    notmuch_bool_t ret;
+    int fd;
+
+    ret = TRUE;
+    fd = open (dir, 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;
+}
+
+/* Open a unique file in the Maildir 'tmp' directory.
+ * Returns the file descriptor on success, or -1 on failure.
+ * 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. */
+static int
+maildir_open_tmp_file (void *ctx, const char *dir,
+		       char **tmppath, char **newpath, char **newdir)
+{
+    pid_t pid;
+    char hostname[256];
+    struct timeval tv;
+    char *filename;
+    int fd = -1;
+
+    /* We follow the Dovecot file name generation algorithm. */
+    pid = getpid ();
+    safe_gethostname (hostname, sizeof (hostname));
+    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;
+    }
+
+    *newdir = talloc_asprintf (ctx, "%s/new", dir);
+    *newpath = talloc_asprintf (ctx, "%s/new/%s", dir, filename);
+    if (! *newdir || ! *newpath) {
+	fprintf (stderr, "Out of memory\n");
+	close (fd);
+	unlink (*tmppath);
+	return -1;
+    }
+
+    talloc_free (filename);
+
+    return fd;
+}
+
+/* Copy the contents of standard input (fdin) into fdout. */
+static notmuch_bool_t
+copy_stdin (int fdin, int fdout)
+{
+    char buf[4096];
+    char *p;
+    ssize_t remain;
+    ssize_t written;
+
+    while (! interrupted) {
+	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 && errno == EINTR)
+		continue;
+	    if (written <= 0) {
+		fprintf (stderr, "Error: writing to temporary file: %s",
+			 strerror (errno));
+		return FALSE;
+	    }
+	    p += written;
+	    remain -= written;
+	} while (remain > 0);
+    }
+
+    return ! interrupted;
+}
+
+/* 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,
+		      tag_op_list_t *tag_ops)
+{
+    notmuch_message_t *message;
+    notmuch_status_t status;
+
+    status = notmuch_database_add_message (notmuch, path, &message);
+    switch (status) {
+    case NOTMUCH_STATUS_SUCCESS:
+    case NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID:
+	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;
+    }
+
+    if (status == NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID) {
+	/* Don't change tags of an existing message. */
+	status = notmuch_message_tags_to_maildir_flags (message);
+	if (status != NOTMUCH_STATUS_SUCCESS)
+	    fprintf (stderr, "Error: failed to sync tags to maildir flags\n");
+    } else {
+	status = tag_op_list_apply (message, tag_ops, TAG_FLAG_MAILDIR_SYNC);
+    }
+
+    notmuch_message_destroy (message);
+
+    return (status == NOTMUCH_STATUS_SUCCESS) ? TRUE : FALSE;
+}
+
+static notmuch_bool_t
+insert_message (void *ctx, notmuch_database_t *notmuch, int fdin,
+		const char *dir, tag_op_list_t *tag_ops)
+{
+    char *tmppath;
+    char *newpath;
+    char *newdir;
+    int fdout;
+
+    fdout = maildir_open_tmp_file (ctx, dir, &tmppath, &newpath, &newdir);
+    if (fdout < 0) {
+	return FALSE;
+    }
+
+    if (! copy_stdin (fdin, fdout)) {
+	close (fdout);
+	unlink (tmppath);
+	return FALSE;
+    }
+
+    if (fsync (fdout) != 0) {
+	fprintf (stderr, "Error: fsync failed: %s\n", strerror (errno));
+	close (fdout);
+	unlink (tmppath);
+	return FALSE;
+    }
+
+    close (fdout);
+
+    /* 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
+     */
+    if (rename (tmppath, newpath) != 0) {
+	fprintf (stderr, "Error: rename() failed: %s\n", strerror (errno));
+	unlink (tmppath);
+	return FALSE;
+    }
+
+    if (! add_file_to_database (notmuch, newpath, tag_ops)) {
+	/* XXX add an option to keep the file in maildir? */
+	unlink (newpath);
+	return FALSE;
+    }
+
+    if (! sync_dir (newdir)) {
+	unlink (newpath);
+	return FALSE;
+    }
+
+    return TRUE;
+}
+
+int
+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;
+    tag_op_list_t *tag_ops;
+    char *query_string = NULL;
+    const char *maildir;
+    int opt_index = 1;
+    unsigned int i;
+    notmuch_bool_t ret;
+
+    config = notmuch_config_open (ctx, NULL, NULL);
+    if (config == NULL)
+	return 1;
+
+    db_path = notmuch_config_get_database_path (config);
+    new_tags = notmuch_config_get_new_tags (config, &new_tags_length);
+
+    tag_ops = tag_op_list_create (ctx);
+    if (tag_ops == NULL) {
+	fprintf (stderr, "Out of memory.\n");
+	return 1;
+    }
+    for (i = 0; i < new_tags_length; i++) {
+	if (tag_op_list_append (tag_ops, new_tags[i], FALSE))
+	    return 1;
+    }
+
+    if (parse_tag_command_line (ctx, argc - opt_index, argv + opt_index,
+				&query_string, tag_ops))
+	return 1;
+
+    if (*query_string != '\0') {
+	fprintf (stderr, "Error: unexpected query string: %s\n", query_string);
+	return 1;
+    }
+
+    maildir = db_path;
+
+    /* Setup our handler for SIGINT. We do not set SA_RESTART so that copying
+     * from standard input may be interrupted. */
+    memset (&action, 0, sizeof (struct sigaction));
+    action.sa_handler = handle_sigint;
+    sigemptyset (&action.sa_mask);
+    action.sa_flags = 0;
+    sigaction (SIGINT, &action, NULL);
+
+    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, tag_ops);
+
+    notmuch_database_destroy (notmuch);
+
+    return (ret) ? 0 : 1;
+}
diff --git a/notmuch.c b/notmuch.c
index 4fc0973..1c3b893 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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v4 04/12] man: document 'insert' command
  2013-01-24 12:07 [PATCH v4 00/12] insert command Peter Wang
                   ` (2 preceding siblings ...)
  2013-01-24 12:07 ` [PATCH v4 03/12] cli: add insert command Peter Wang
@ 2013-01-24 12:08 ` Peter Wang
  2013-03-29 23:25   ` David Bremner
  2013-01-24 12:08 ` [PATCH v4 05/12] man: reference notmuch-insert.1 Peter Wang
                   ` (7 subsequent siblings)
  11 siblings, 1 reply; 19+ messages in thread
From: Peter Wang @ 2013-01-24 12:08 UTC (permalink / raw)
  To: notmuch

Add initial documentation for notmuch insert command.
---
 man/Makefile.local        |  1 +
 man/man1/notmuch-insert.1 | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 38 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..387e274
--- /dev/null
+++ b/man/man1/notmuch-insert.1
@@ -0,0 +1,37 @@
+.TH NOTMUCH-INSERT 1 2013-xx-xx "Notmuch 0.xx"
+.SH NAME
+notmuch-insert \- add a message to the maildir and notmuch database
+.SH SYNOPSIS
+
+.B notmuch insert
+.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, then by operations specified on the command-line:
+tags prefixed by '+' are added while
+those prefixed by '\-' are removed.
+
+If the new message is a duplicate of an existing message in the database
+(it has same Message-ID), it will be added to the maildir folder and
+notmuch database, but the tags will not be changed.
+
+.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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v4 05/12] man: reference notmuch-insert.1
  2013-01-24 12:07 [PATCH v4 00/12] insert command Peter Wang
                   ` (3 preceding siblings ...)
  2013-01-24 12:08 ` [PATCH v4 04/12] man: document 'insert' command Peter Wang
@ 2013-01-24 12:08 ` Peter Wang
  2013-01-24 12:08 ` [PATCH v4 06/12] test: add tests for insert Peter Wang
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: Peter Wang @ 2013-01-24 12:08 UTC (permalink / raw)
  To: notmuch

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 557eae5..e6d960d 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 d63be99..2651412 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 613fd69..5e58cf9 100644
--- a/man/man1/notmuch-dump.1
+++ b/man/man1/notmuch-dump.1
@@ -92,7 +92,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 06c4dfa..e0ede8b 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 13e50ad..82437a4 100644
--- a/man/man1/notmuch-reply.1
+++ b/man/man1/notmuch-reply.1
@@ -123,7 +123,8 @@ The requested format version is too new.
 .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 895c6d2..65ecfc8 100644
--- a/man/man1/notmuch-restore.1
+++ b/man/man1/notmuch-restore.1
@@ -84,7 +84,8 @@ should be accurate.
 .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 321d779..67fd34b 100644
--- a/man/man1/notmuch-search.1
+++ b/man/man1/notmuch-search.1
@@ -155,7 +155,8 @@ The requested format version is too new.
 .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 3f9584b..90c2556 100644
--- a/man/man1/notmuch-show.1
+++ b/man/man1/notmuch-show.1
@@ -224,7 +224,8 @@ The requested format version is too new.
 .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 081bd0e..14a9856 100644
--- a/man/man1/notmuch-tag.1
+++ b/man/man1/notmuch-tag.1
@@ -125,7 +125,8 @@ of the tag
 .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 69805cb..b5829fc 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 4ed61c1..97c45e6 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 19e5fd5..45fb525 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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v4 06/12] test: add tests for insert
  2013-01-24 12:07 [PATCH v4 00/12] insert command Peter Wang
                   ` (4 preceding siblings ...)
  2013-01-24 12:08 ` [PATCH v4 05/12] man: reference notmuch-insert.1 Peter Wang
@ 2013-01-24 12:08 ` Peter Wang
  2013-03-29 23:59   ` David Bremner
  2013-01-24 12:08 ` [PATCH v4 07/12] insert: add --folder option Peter Wang
                   ` (5 subsequent siblings)
  11 siblings, 1 reply; 19+ messages in thread
From: Peter Wang @ 2013-01-24 12:08 UTC (permalink / raw)
  To: notmuch

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

diff --git a/test/insert b/test/insert
new file mode 100755
index 0000000..d880af9
--- /dev/null
+++ b/test/insert
@@ -0,0 +1,69 @@
+#!/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 files.
+# They happen to be in the mail directory already but that is okay
+# since we do not call notmuch new hereafter.
+
+gen_insert_msg() {
+    generate_message \
+	"[subject]=\"insert-subject\"" \
+	"[date]=\"Sat, 01 Jan 2000 12:00:00 -0000\"" \
+	"[body]=\"insert-message\""
+}
+
+test_begin_subtest "Insert message, copied exactly"
+gen_insert_msg
+notmuch insert < "$gen_msg_filename"
+cur_msg_filename=$(notmuch search --output=files "subject:insert-subject")
+test_expect_equal_file "$cur_msg_filename" "$gen_msg_filename"
+
+test_begin_subtest "Insert message, default tags"
+output=$(notmuch show --format=json "subject:insert-subject")
+expected='[[[{
+ "id": "'"${gen_msg_id}"'",
+ "match": true,
+ "excluded": false,
+ "filename": "'"${cur_msg_filename}"'",
+ "timestamp": 946728000,
+ "date_relative": "2000-01-01",
+ "tags": ["inbox","unread"],
+ "headers": {
+  "Subject": "insert-subject",
+  "From": "Notmuch Test Suite <test_suite@notmuchmail.org>",
+  "To": "Notmuch Test Suite <test_suite@notmuchmail.org>",
+  "Date": "Sat, 01 Jan 2000 12:00:00 +0000"},
+ "body": [{"id": 1,
+  "content-type": "text/plain",
+  "content": "insert-message\n"}]},
+ []]]]'
+test_expect_equal_json "$output" "$expected"
+
+test_begin_subtest "Insert message, duplicate message"
+notmuch insert +duptag -unread < "$gen_msg_filename"
+output=$(notmuch search --output=files "subject:insert-subject" | wc -l)
+test_expect_equal "$output" 2
+
+test_begin_subtest "Insert message, duplicate message does not change tags"
+output=$(notmuch search --format=json --output=tags "subject:insert-subject")
+test_expect_equal_json "$output" '["inbox", "unread"]'
+
+test_begin_subtest "Insert message, add tag"
+gen_insert_msg
+notmuch insert +custom < "$gen_msg_filename"
+output=$(notmuch count tag:custom)
+test_expect_equal "$output" "1"
+
+test_begin_subtest "Insert message, add/remove tag"
+gen_insert_msg
+notmuch insert +custom -unread < "$gen_msg_filename"
+output=$(notmuch count tag:custom NOT tag:unread)
+test_expect_equal "$output" "1"
+
+test_done
diff --git a/test/notmuch-test b/test/notmuch-test
index ca9c3dc..6952f0a 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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v4 07/12] insert: add --folder option
  2013-01-24 12:07 [PATCH v4 00/12] insert command Peter Wang
                   ` (5 preceding siblings ...)
  2013-01-24 12:08 ` [PATCH v4 06/12] test: add tests for insert Peter Wang
@ 2013-01-24 12:08 ` Peter Wang
  2013-01-24 12:08 ` [PATCH v4 08/12] man: document insert " Peter Wang
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: Peter Wang @ 2013-01-24 12:08 UTC (permalink / raw)
  To: notmuch

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

diff --git a/notmuch-insert.c b/notmuch-insert.c
index 31c1152..bc848ff 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -85,6 +85,23 @@ sync_dir (const char *dir)
     return ret;
 }
 
+/* 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 for the message in the 'tmp' and 'new'
@@ -288,11 +305,25 @@ notmuch_insert_command (void *ctx, int argc, char *argv[])
     size_t new_tags_length;
     tag_op_list_t *tag_ops;
     char *query_string = NULL;
+    const char *folder = NULL;
     const char *maildir;
-    int opt_index = 1;
+    int opt_index;
     unsigned int i;
     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;
@@ -319,7 +350,19 @@ notmuch_insert_command (void *ctx, int argc, char *argv[])
 	return 1;
     }
 
-    maildir = db_path;
+    if (folder == NULL) {
+	maildir = db_path;
+    } else {
+	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);
+	if (! maildir) {
+	    fprintf (stderr, "Out of memory\n");
+	    return 1;
+	}
+    }
 
     /* Setup our handler for SIGINT. We do not set SA_RESTART so that copying
      * from standard input may be interrupted. */
-- 
1.7.12.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v4 08/12] man: document insert --folder option
  2013-01-24 12:07 [PATCH v4 00/12] insert command Peter Wang
                   ` (6 preceding siblings ...)
  2013-01-24 12:08 ` [PATCH v4 07/12] insert: add --folder option Peter Wang
@ 2013-01-24 12:08 ` Peter Wang
  2013-01-24 12:08 ` [PATCH v4 09/12] test: test " Peter Wang
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: Peter Wang @ 2013-01-24 12:08 UTC (permalink / raw)
  To: notmuch

Add documentation for notmuch insert --folder option.
---
 man/man1/notmuch-insert.1 | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/man/man1/notmuch-insert.1 b/man/man1/notmuch-insert.1
index 387e274..27c2658 100644
--- a/man/man1/notmuch-insert.1
+++ b/man/man1/notmuch-insert.1
@@ -4,6 +4,7 @@ notmuch-insert \- add a message to the maildir and notmuch database
 .SH SYNOPSIS
 
 .B notmuch insert
+.RI "[" options "]"
 .RI "[ +<" tag> "|\-<" tag "> ... ]"
 
 .SH DESCRIPTION
@@ -27,6 +28,19 @@ If the new message is a duplicate of an existing message in the database
 (it has same Message-ID), it will be added to the maildir folder and
 notmuch database, but the tags will not be changed.
 
+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
 .SH SEE ALSO
 
-- 
1.7.12.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v4 09/12] test: test insert --folder option
  2013-01-24 12:07 [PATCH v4 00/12] insert command Peter Wang
                   ` (7 preceding siblings ...)
  2013-01-24 12:08 ` [PATCH v4 08/12] man: document insert " Peter Wang
@ 2013-01-24 12:08 ` Peter Wang
  2013-01-24 12:08 ` [PATCH v4 10/12] insert: add --create-folder option Peter Wang
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 19+ messages in thread
From: Peter Wang @ 2013-01-24 12:08 UTC (permalink / raw)
  To: notmuch

Add tests for notmuch insert --folder option.
---
 test/insert | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/test/insert b/test/insert
index d880af9..44e071c 100755
--- a/test/insert
+++ b/test/insert
@@ -66,4 +66,21 @@ notmuch insert +custom -unread < "$gen_msg_filename"
 output=$(notmuch count tag:custom NOT tag:unread)
 test_expect_equal "$output" "1"
 
+test_begin_subtest "Insert message, folder"
+gen_insert_msg
+notmuch insert --folder=Drafts < "$gen_msg_filename"
+output=$(notmuch search --output=files folder:Drafts)
+dirname=$(dirname "$output")
+test_expect_equal "$dirname" "$MAIL_DIR/Drafts/cur"
+
+test_begin_subtest "Insert message, folder and tags"
+gen_insert_msg
+notmuch insert --folder=Drafts +draft -unread < "$gen_msg_filename"
+output=$(notmuch count folder:Drafts tag:draft NOT tag:unread)
+test_expect_equal "$output" "1"
+
+gen_insert_msg
+test_expect_code 1 "Insert message, non-existent folder" \
+    "notmuch insert --folder=nonesuch < $gen_msg_filename"
+
 test_done
-- 
1.7.12.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v4 10/12] insert: add --create-folder option
  2013-01-24 12:07 [PATCH v4 00/12] insert command Peter Wang
                   ` (8 preceding siblings ...)
  2013-01-24 12:08 ` [PATCH v4 09/12] test: test " Peter Wang
@ 2013-01-24 12:08 ` Peter Wang
  2013-01-24 12:08 ` [PATCH v4 11/12] man: document insert --create-folder Peter Wang
  2013-01-24 12:08 ` [PATCH v4 12/12] test: test insert --create-folder option Peter Wang
  11 siblings, 0 replies; 19+ messages in thread
From: Peter Wang @ 2013-01-24 12:08 UTC (permalink / raw)
  To: notmuch

Allow the insert command to create the maildir folder
into which the new message should be delivered.
---
 notmuch-insert.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 100 insertions(+)

diff --git a/notmuch-insert.c b/notmuch-insert.c
index bc848ff..69329ad 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -102,6 +102,99 @@ check_folder_name (const char *folder)
     }
 }
 
+/* Make the given directory, succeeding if it already exists. */
+static notmuch_bool_t
+make_directory (char *path, int mode)
+{
+    notmuch_bool_t ret;
+    char *slash;
+
+    if (mkdir (path, mode) != 0)
+	return (errno == EEXIST);
+
+    /* Sync the parent directory for durability. */
+    ret = TRUE;
+    slash = strrchr (path, '/');
+    if (slash) {
+	*slash = '\0';
+	ret = sync_dir (path);
+	*slash = '/';
+    }
+    return ret;
+}
+
+/* Make the given directory including its parent directories as necessary.
+ * Return TRUE on success, FALSE on error. */
+static notmuch_bool_t
+make_directory_and_parents (char *path, int mode)
+{
+    struct stat st;
+    char *start;
+    char *end;
+    notmuch_bool_t ret;
+
+    /* First check the common case: directory already exists. */
+    if (stat (path, &st) == 0)
+	return S_ISDIR (st.st_mode) ? TRUE : FALSE;
+
+    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 make_directory (path, mode);
+
+	/* Make the path up to the next slash, unless the current
+	 * directory component is actually empty. */
+	if (end > start) {
+	    *end = '\0';
+	    ret = make_directory (path, mode);
+	    *end = '/';
+	    if (! ret)
+		return FALSE;
+	}
+    }
+
+    return TRUE;
+}
+
+/* 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 (! make_directory_and_parents (subdir, mode))
+	return FALSE;
+
+    tail = subdir + strlen (subdir) - 3;
+
+    /* Create 'new' directory. */
+    strcpy (tail, "new");
+    if (! make_directory (subdir, mode))
+	return FALSE;
+
+    /* Create 'tmp' directory. */
+    strcpy (tail, "tmp");
+    if (! make_directory (subdir, mode))
+	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 for the message in the 'tmp' and 'new'
@@ -306,6 +399,7 @@ notmuch_insert_command (void *ctx, int argc, char *argv[])
     tag_op_list_t *tag_ops;
     char *query_string = NULL;
     const char *folder = NULL;
+    notmuch_bool_t create_folder = FALSE;
     const char *maildir;
     int opt_index;
     unsigned int i;
@@ -313,6 +407,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 }
     };
 
@@ -362,6 +457,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;
+	}
     }
 
     /* Setup our handler for SIGINT. We do not set SA_RESTART so that copying
-- 
1.7.12.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v4 11/12] man: document insert --create-folder
  2013-01-24 12:07 [PATCH v4 00/12] insert command Peter Wang
                   ` (9 preceding siblings ...)
  2013-01-24 12:08 ` [PATCH v4 10/12] insert: add --create-folder option Peter Wang
@ 2013-01-24 12:08 ` Peter Wang
  2013-01-24 12:08 ` [PATCH v4 12/12] test: test insert --create-folder option Peter Wang
  11 siblings, 0 replies; 19+ messages in thread
From: Peter Wang @ 2013-01-24 12:08 UTC (permalink / raw)
  To: notmuch

Add documentation for notmuch insert --create-folder option.
---
 man/man1/notmuch-insert.1 | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/man/man1/notmuch-insert.1 b/man/man1/notmuch-insert.1
index 27c2658..8ce634e 100644
--- a/man/man1/notmuch-insert.1
+++ b/man/man1/notmuch-insert.1
@@ -42,6 +42,18 @@ relative to the top-level directory given by the value of
 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),
-- 
1.7.12.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v4 12/12] test: test insert --create-folder option
  2013-01-24 12:07 [PATCH v4 00/12] insert command Peter Wang
                   ` (10 preceding siblings ...)
  2013-01-24 12:08 ` [PATCH v4 11/12] man: document insert --create-folder Peter Wang
@ 2013-01-24 12:08 ` Peter Wang
  11 siblings, 0 replies; 19+ messages in thread
From: Peter Wang @ 2013-01-24 12:08 UTC (permalink / raw)
  To: notmuch

Add tests for notmuch insert --create-folder option.
---
 test/insert | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/test/insert b/test/insert
index 44e071c..24a61e1 100755
--- a/test/insert
+++ b/test/insert
@@ -83,4 +83,28 @@ gen_insert_msg
 test_expect_code 1 "Insert message, non-existent folder" \
     "notmuch insert --folder=nonesuch < $gen_msg_filename"
 
+test_begin_subtest "Insert message, create folder"
+gen_insert_msg
+notmuch insert --folder=F --create-folder +folder < "$gen_msg_filename"
+output=$(notmuch search --output=files folder:F tag:folder)
+basename=$(basename "$output")
+test_expect_equal_file "$gen_msg_filename" "$MAIL_DIR/F/cur/${basename}"
+
+test_begin_subtest "Insert message, create subfolder"
+gen_insert_msg
+notmuch insert --folder=F/G/H/I/J --create-folder +folder < "$gen_msg_filename"
+output=$(notmuch search --output=files folder:F/G/H/I/J tag:folder)
+basename=$(basename "$output")
+test_expect_equal_file "$gen_msg_filename" "${MAIL_DIR}/F/G/H/I/J/cur/${basename}"
+
+test_begin_subtest "Insert message, create existing subfolder"
+gen_insert_msg
+notmuch insert --folder=F/G/H/I/J --create-folder +folder < "$gen_msg_filename"
+output=$(notmuch count folder:F/G/H/I/J tag:folder)
+test_expect_equal "$output" "2"
+
+gen_insert_msg
+test_expect_code 1 "Insert message, create invalid subfolder" \
+    "notmuch insert --folder=../G --create-folder $gen_msg_filename"
+
 test_done
-- 
1.7.12.1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v4 03/12] cli: add insert command
  2013-01-24 12:07 ` [PATCH v4 03/12] cli: add insert command Peter Wang
@ 2013-03-06  1:13   ` David Bremner
  0 siblings, 0 replies; 19+ messages in thread
From: David Bremner @ 2013-03-06  1:13 UTC (permalink / raw)
  To: Peter Wang, notmuch

Peter Wang <novalazy@gmail.com> writes:

> The notmuch insert command reads a message from standard input,
> writes it to a Maildir folder, and then incorporates the message into
> the notmuch database.  Essentially it moves the functionality of
> notmuch-deliver into notmuch.

The first three patches in this series look OK to me so far.

There is some conflict with Jani's --config command line argument patches,
but that should be easy/generic to fix.

I'll try to review the next 3 patches in this series this week; this
message should serve as a heads up for people interested in this series
that its next on my agenda after Jani's config file patches.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v4 04/12] man: document 'insert' command
  2013-01-24 12:08 ` [PATCH v4 04/12] man: document 'insert' command Peter Wang
@ 2013-03-29 23:25   ` David Bremner
  0 siblings, 0 replies; 19+ messages in thread
From: David Bremner @ 2013-03-29 23:25 UTC (permalink / raw)
  To: Peter Wang, notmuch

Peter Wang <novalazy@gmail.com> writes:
> +
> +.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.
> +

I think this should document how the maildir is selected. I agree it
isn't that surprising, but the language "specified maildir folder" makes
me think it is somehow specified as part of the command invocation.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v4 06/12] test: add tests for insert
  2013-01-24 12:08 ` [PATCH v4 06/12] test: add tests for insert Peter Wang
@ 2013-03-29 23:59   ` David Bremner
  2013-03-30  4:05     ` Peter Wang
  0 siblings, 1 reply; 19+ messages in thread
From: David Bremner @ 2013-03-29 23:59 UTC (permalink / raw)
  To: Peter Wang, notmuch


It took longer than I thought (of course) but I finally finished looking
at the first 6 patches. 

I already mentioned a minor man page issue in a seperate message.

I took a second pass through 03/12, and I think I would prefer thethe
control flow of insert_message be closer to the standard style in
notmuch of using a return value variable and a single cleanup block at
the end.  Reasonable people can disagree about issues of style, but in
the end consistency of the code base is also important.

d

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v4 06/12] test: add tests for insert
  2013-03-29 23:59   ` David Bremner
@ 2013-03-30  4:05     ` Peter Wang
  2013-04-01 19:38       ` Jani Nikula
  0 siblings, 1 reply; 19+ messages in thread
From: Peter Wang @ 2013-03-30  4:05 UTC (permalink / raw)
  To: David Bremner; +Cc: notmuch

On Fri, 29 Mar 2013 19:59:56 -0400, David Bremner <david@tethera.net> wrote:
> 
> It took longer than I thought (of course) but I finally finished looking
> at the first 6 patches. 
> 
> I already mentioned a minor man page issue in a seperate message.
> 
> I took a second pass through 03/12, and I think I would prefer thethe
> control flow of insert_message be closer to the standard style in
> notmuch of using a return value variable and a single cleanup block at
> the end.  Reasonable people can disagree about issues of style, but in
> the end consistency of the code base is also important.
> 
> d

static notmuch_bool_t
insert_message (void *ctx, notmuch_database_t *notmuch, int fdin,
		const char *dir, tag_op_list_t *tag_ops)
{
    char *tmppath;
    char *newpath;
    char *newdir;
    int fdout;
    char *cleanup_path;

    fdout = maildir_open_tmp_file (ctx, dir, &tmppath, &newpath, &newdir);
    if (fdout < 0)
	return FALSE;

    cleanup_path = tmppath;

    if (! copy_stdin (fdin, fdout))
	goto DONE;

    if (fsync (fdout) != 0) {
	fprintf (stderr, "Error: fsync failed: %s\n", strerror (errno));
	goto DONE;
    }

    close (fdout);
    fdout = -1;

    /* 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
     */
    if (rename (tmppath, newpath) != 0) {
	fprintf (stderr, "Error: rename() failed: %s\n", strerror (errno));
	goto DONE;
    }

    cleanup_path = newpath;

    if (! add_file_to_database (notmuch, newpath, tag_ops)) {
	/* XXX add an option to keep the file in maildir? */
	goto DONE;
    }

    if (! sync_dir (newdir))
	goto DONE;

    cleanup_path = NULL; /* success */

  DONE:
    if (fdout >= 0)
	close (fdout);

    if (cleanup_path) {
	unlink (cleanup_path);
	return FALSE;
    }

    return TRUE;
}

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v4 01/12] tag-util: move out 'tag' command-line checks
  2013-01-24 12:07 ` [PATCH v4 01/12] tag-util: move out 'tag' command-line checks Peter Wang
@ 2013-03-31 10:00   ` Jani Nikula
  0 siblings, 0 replies; 19+ messages in thread
From: Jani Nikula @ 2013-03-31 10:00 UTC (permalink / raw)
  To: Peter Wang, notmuch

On Thu, 24 Jan 2013, Peter Wang <novalazy@gmail.com> wrote:
> parse_tag_command_line checked for two error conditions which are
> specific to the 'tag' command.  It can be reused for the forthcoming
> notmuch 'insert' command if we move the checks out, into notmuch-tag.c.

FYI, this patch no longer applies to master.

Jani.

> ---
>  notmuch-tag.c | 10 ++++++++++
>  tag-util.c    | 11 +++--------
>  2 files changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/notmuch-tag.c b/notmuch-tag.c
> index d9daf8f..a901dad 100644
> --- a/notmuch-tag.c
> +++ b/notmuch-tag.c
> @@ -234,6 +234,16 @@ notmuch_tag_command (void *ctx, int argc, char *argv[])
>  	if (parse_tag_command_line (ctx, argc - opt_index, argv + opt_index,
>  				    &query_string, tag_ops))
>  	    return 1;
> +
> +	if (tag_op_list_size (tag_ops) == 0) {
> +	    fprintf (stderr, "Error: 'notmuch tag' requires at least one tag to add or remove.\n");
> +	    return 1;
> +	}
> +
> +	if (*query_string == '\0') {
> +	    fprintf (stderr, "Error: notmuch tag requires at least one search term.\n");
> +	    return 1;
> +	}
>      }
>  
>      config = notmuch_config_open (ctx, NULL, NULL);
> diff --git a/tag-util.c b/tag-util.c
> index 701d329..743d591 100644
> --- a/tag-util.c
> +++ b/tag-util.c
> @@ -188,16 +188,11 @@ parse_tag_command_line (void *ctx, int argc, char **argv,
>  	tag_op_list_append (tag_ops, argv[i] + 1, is_remove);
>      }
>  
> -    if (tag_op_list_size (tag_ops) == 0) {
> -	fprintf (stderr, "Error: 'notmuch tag' requires at least one tag to add or remove.\n");
> -	return TAG_PARSE_INVALID;
> -    }
> -
>      *query_str = query_string_from_args (ctx, argc - i, &argv[i]);
>  
> -    if (*query_str == NULL || **query_str == '\0') {
> -	fprintf (stderr, "Error: notmuch tag requires at least one search term.\n");
> -	return TAG_PARSE_INVALID;
> +    if (*query_str == NULL) {
> +       fprintf (stderr, "Out of memory.\n");
> +       return TAG_PARSE_OUT_OF_MEMORY;
>      }
>  
>      return TAG_PARSE_SUCCESS;
> -- 
> 1.7.12.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v4 06/12] test: add tests for insert
  2013-03-30  4:05     ` Peter Wang
@ 2013-04-01 19:38       ` Jani Nikula
  0 siblings, 0 replies; 19+ messages in thread
From: Jani Nikula @ 2013-04-01 19:38 UTC (permalink / raw)
  To: Peter Wang, David Bremner; +Cc: notmuch

On Sat, 30 Mar 2013, Peter Wang <novalazy@gmail.com> wrote:
> On Fri, 29 Mar 2013 19:59:56 -0400, David Bremner <david@tethera.net> wrote:
>> 
>> It took longer than I thought (of course) but I finally finished looking
>> at the first 6 patches. 
>> 
>> I already mentioned a minor man page issue in a seperate message.
>> 
>> I took a second pass through 03/12, and I think I would prefer thethe
>> control flow of insert_message be closer to the standard style in
>> notmuch of using a return value variable and a single cleanup block at
>> the end.  Reasonable people can disagree about issues of style, but in
>> the end consistency of the code base is also important.

I think sync_dir() was worse than insert_message() in this regard.

>> 
>> d
>       
> static notmuch_bool_t
> insert_message (void *ctx, notmuch_database_t *notmuch, int fdin,
> 		const char *dir, tag_op_list_t *tag_ops)
> {
>     char *tmppath;
>     char *newpath;
>     char *newdir;
>     int fdout;
>     char *cleanup_path;
>
>     fdout = maildir_open_tmp_file (ctx, dir, &tmppath, &newpath, &newdir);
>     if (fdout < 0)
> 	return FALSE;
>
>     cleanup_path = tmppath;
>
>     if (! copy_stdin (fdin, fdout))
> 	goto DONE;
>
>     if (fsync (fdout) != 0) {
> 	fprintf (stderr, "Error: fsync failed: %s\n", strerror (errno));
> 	goto DONE;
>     }
>
>     close (fdout);
>     fdout = -1;
>
>     /* 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
>      */
>     if (rename (tmppath, newpath) != 0) {
> 	fprintf (stderr, "Error: rename() failed: %s\n", strerror (errno));
> 	goto DONE;
>     }
>
>     cleanup_path = newpath;
>
>     if (! add_file_to_database (notmuch, newpath, tag_ops)) {
> 	/* XXX add an option to keep the file in maildir? */
> 	goto DONE;
>     }
>
>     if (! sync_dir (newdir))
> 	goto DONE;
>
>     cleanup_path = NULL; /* success */

Put the happy day return TRUE here, and don't bother with the above
statement.

>
>   DONE:
>     if (fdout >= 0)
> 	close (fdout);
>
>     if (cleanup_path) {
> 	unlink (cleanup_path);
> 	return FALSE;
>     }
>
>     return TRUE;

Only have the return FALSE path here. You can unconditionally unlink
(cleanup_path) too AFAICS.

> }
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2013-04-01 19:38 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-01-24 12:07 [PATCH v4 00/12] insert command Peter Wang
2013-01-24 12:07 ` [PATCH v4 01/12] tag-util: move out 'tag' command-line checks Peter Wang
2013-03-31 10:00   ` Jani Nikula
2013-01-24 12:07 ` [PATCH v4 02/12] tag-util: do not reset list in parse_tag_command_line Peter Wang
2013-01-24 12:07 ` [PATCH v4 03/12] cli: add insert command Peter Wang
2013-03-06  1:13   ` David Bremner
2013-01-24 12:08 ` [PATCH v4 04/12] man: document 'insert' command Peter Wang
2013-03-29 23:25   ` David Bremner
2013-01-24 12:08 ` [PATCH v4 05/12] man: reference notmuch-insert.1 Peter Wang
2013-01-24 12:08 ` [PATCH v4 06/12] test: add tests for insert Peter Wang
2013-03-29 23:59   ` David Bremner
2013-03-30  4:05     ` Peter Wang
2013-04-01 19:38       ` Jani Nikula
2013-01-24 12:08 ` [PATCH v4 07/12] insert: add --folder option Peter Wang
2013-01-24 12:08 ` [PATCH v4 08/12] man: document insert " Peter Wang
2013-01-24 12:08 ` [PATCH v4 09/12] test: test " Peter Wang
2013-01-24 12:08 ` [PATCH v4 10/12] insert: add --create-folder option Peter Wang
2013-01-24 12:08 ` [PATCH v4 11/12] man: document insert --create-folder Peter Wang
2013-01-24 12:08 ` [PATCH v4 12/12] test: test insert --create-folder option Peter Wang

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).