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"