From 64648198afb5f3fec1f9c0f5110e998e5d7f48b2 Mon Sep 17 00:00:00 2001 From: Peter Wang Date: Thu, 24 Jan 2013 23:07:56 +1100 Subject: [PATCH] [PATCH v4 00/12] insert command --- d2/86c5b2e4a8da72519c085f75ed10f2cb41c70e | 424 ++++++++++++++++++++++ 1 file changed, 424 insertions(+) create mode 100644 d2/86c5b2e4a8da72519c085f75ed10f2cb41c70e diff --git a/d2/86c5b2e4a8da72519c085f75ed10f2cb41c70e b/d2/86c5b2e4a8da72519c085f75ed10f2cb41c70e new file mode 100644 index 000000000..0e5e79137 --- /dev/null +++ b/d2/86c5b2e4a8da72519c085f75ed10f2cb41c70e @@ -0,0 +1,424 @@ +Return-Path: +X-Original-To: notmuch@notmuchmail.org +Delivered-To: notmuch@notmuchmail.org +Received: from localhost (localhost [127.0.0.1]) + by olra.theworths.org (Postfix) with ESMTP id 25FF0431FAF + for ; Thu, 24 Jan 2013 04:09:18 -0800 (PST) +X-Virus-Scanned: Debian amavisd-new at olra.theworths.org +X-Spam-Flag: NO +X-Spam-Score: -0.799 +X-Spam-Level: +X-Spam-Status: No, score=-0.799 tagged_above=-999 required=5 + tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, + FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled +Received: from olra.theworths.org ([127.0.0.1]) + by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) + with ESMTP id TsCRlnDgBiCk for ; + Thu, 24 Jan 2013 04:09:16 -0800 (PST) +Received: from mail-pb0-f43.google.com (mail-pb0-f43.google.com + [209.85.160.43]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) + (No client certificate requested) + by olra.theworths.org (Postfix) with ESMTPS id 06190431FAE + for ; Thu, 24 Jan 2013 04:09:15 -0800 (PST) +Received: by mail-pb0-f43.google.com with SMTP id jt11so3997106pbb.2 + for ; Thu, 24 Jan 2013 04:09:15 -0800 (PST) +DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; + h=x-received:from:to:cc:subject:date:message-id:x-mailer; + bh=pq+bVJLMrvoa1umMAPToGStcuLLinxwPlewyyNvRcZg=; + b=J9QAYybUCsclcIpETQaEK/WLjlVFqVnIkwRd8263Wsu2gNNJ/FpMpAC6os4fCiUwoo + 5mzxgrqvFD06ExhIQ8+BPQFTdPhPJY5Zm/n4BGIXuvsdAQEwDHci0N46octc3iUq1qi9 + FEL1LT/3htG7XNRsdIzcuzaXZ1Z8lEKGR/SNY5BWm+SM1ROkRi5jELa1zIYjrSeHZmCh + soU2/kKhGEpw8lWsGgtwhyVUGbg1X6Mseb9/dBswlF5sVcHSK56rEiQTEQ782qKnKIxC + LKuF49SqmLaXdFde7IyCHvFhYIz1sHf7NU5cdzBNxhrpI8dImr8RvzGoBfg5+eGXZPtg + MDzg== +X-Received: by 10.68.223.35 with SMTP id qr3mr4330939pbc.27.1359029354173; + Thu, 24 Jan 2013 04:09:14 -0800 (PST) +Received: from localhost (215.42.233.220.static.exetel.com.au. + [220.233.42.215]) + by mx.google.com with ESMTPS id bj9sm2746834pab.22.2013.01.24.04.09.11 + (version=TLSv1.2 cipher=RC4-SHA bits=128/128); + Thu, 24 Jan 2013 04:09:13 -0800 (PST) +From: Peter Wang +To: notmuch@notmuchmail.org +Subject: [PATCH v4 00/12] insert command +Date: Thu, 24 Jan 2013 23:07:56 +1100 +Message-Id: <1359029288-12132-1-git-send-email-novalazy@gmail.com> +X-Mailer: git-send-email 1.7.12.1 +X-BeenThere: notmuch@notmuchmail.org +X-Mailman-Version: 2.1.13 +Precedence: list +List-Id: "Use and development of the notmuch mail system." + +List-Unsubscribe: , + +List-Archive: +List-Post: +List-Help: +List-Subscribe: , + +X-List-Received-Date: Thu, 24 Jan 2013 12:09:18 -0000 + +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" -- 2.26.2