[PATCH 0/3] Introduce the add command

[PATCH 0/3] Introduce the add command

[Adam Wolfe Gordon]

Hi everyone,

The recent introduction of insert inspired me to finally add a feature I've
been wanting: a command to index a specific file in the maildir. My usecase
for this is that I have an inotify-based script that watches for new mail
and calls notmuch new when new mail shows up. Running notmuch new in this
situation is overkill, since I know exactly what's changed. A faster command
that just adds a single file reduces contention on the database lock.

This series introduces a new command, "notmuch add", which indexes a file
that already exists in the maildir. It is implemented in notmuch-insert.c
because it uses the basic infrastructure introduced for the insert command.

Missing man page for now - wanted to get the code out first for review.

-- Adam

Adam Wolfe Gordon (3):
  cli: Return an error code from add_message_to_database
  cli: Introduce the add command
  test: Add simple tests for the add command

 notmuch-client.h |    3 +++
 notmuch-insert.c |   57 +++++++++++++++++++++++++++++++++++++++++++++++++++---
 notmuch.c        |    2 ++
 test/insert      |   35 +++++++++++++++++++++++++++++++++
 4 files changed, 94 insertions(+), 3 deletions(-)

-- 
1.7.9.5

[PATCH 1/3] cli: Return an error code from add_message_to_database

[Adam Wolfe Gordon]

The error code isn't necessary when we're inserting a new message from
standard input, but it will be useful for an upcoming command that
allows an existing file to be indexed.
---
 notmuch-insert.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/notmuch-insert.c b/notmuch-insert.c
index 2207b1e..d32a535 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -293,7 +293,7 @@ copy_stdin (int fdin, int fdout)
 
 /* Add the specified message file to the notmuch database, applying tags.
  * The file is renamed to encode notmuch tags as maildir flags. */
-static void
+static int
 add_file_to_database (notmuch_database_t *notmuch, const char *path,
 		      tag_op_list_t *tag_ops)
 {
@@ -318,7 +318,7 @@ add_file_to_database (notmuch_database_t *notmuch, const char *path,
     case NOTMUCH_STATUS_LAST_STATUS:
 	fprintf (stderr, "Error: failed to add `%s' to notmuch database: %s\n",
 		 path, notmuch_status_to_string (status));
-	return;
+	return status;
     }
 
     if (status == NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID) {
@@ -331,6 +331,8 @@ add_file_to_database (notmuch_database_t *notmuch, const char *path,
     }
 
     notmuch_message_destroy (message);
+
+    return status;
 }
 
 static notmuch_bool_t
@@ -377,7 +379,7 @@ insert_message (void *ctx, notmuch_database_t *notmuch, int fdin,
 
     /* Even if adding the message to the notmuch database fails,
      * the message is on disk and we consider the delivery completed. */
-    add_file_to_database (notmuch, newpath, tag_ops);
+    (void) add_file_to_database (notmuch, newpath, tag_ops);
 
     return TRUE;
 
-- 
1.7.9.5

[PATCH 2/3] cli: Introduce the add command

[Adam Wolfe Gordon]

Introduce a new add command, which works the same as insert, except
that it is for adding files that already exist in the maildir. This
provides an alternative to notmuch new for situations where the user
knows what has changed. For example, if the user uses inotify to check
for new files in the maildir, the new files can be indexed using
notmuch add, avoiding the need to scan the whole maildir.
---
 notmuch-client.h |    3 +++
 notmuch-insert.c |   49 +++++++++++++++++++++++++++++++++++++++++++++++++
 notmuch.c        |    2 ++
 3 files changed, 54 insertions(+)

diff --git a/notmuch-client.h b/notmuch-client.h
index da332f3..09d7e5c 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -183,6 +183,9 @@ int
 notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[]);
 
 int
+notmuch_add_command (notmuch_config_t *config, int argc, char *argv[]);
+
+int
 notmuch_reply_command (notmuch_config_t *config, int argc, char *argv[]);
 
 int
diff --git a/notmuch-insert.c b/notmuch-insert.c
index d32a535..c141450 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -479,3 +479,52 @@ notmuch_insert_command (notmuch_config_t *config, int argc, char *argv[])
 
     return (ret) ? 0 : 1;
 }
+
+int
+notmuch_add_command (notmuch_config_t *config, int argc, char *argv[])
+{
+    notmuch_database_t *notmuch;
+    const char *db_path;
+    const char **new_tags;
+    size_t new_tags_length;
+    tag_op_list_t *tag_ops;
+    const char *filename = NULL;
+    unsigned int i;
+    notmuch_bool_t ret;
+
+    if (argc != 2) {
+	fprintf (stderr, "Error: no filename given.\n");
+	return 1;
+    }
+    filename = argv[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 (config);
+    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;
+    }
+
+    /* Check that the message given on the commandline is in the maildir. */
+    if (strncmp (filename, db_path, strlen(db_path)) != 0) {
+	fprintf(stderr, "Error: file %s is not in maildir %s.\n", filename,
+		db_path);
+	return 1;
+    }
+
+    if (notmuch_database_open (notmuch_config_get_database_path (config),
+			       NOTMUCH_DATABASE_MODE_READ_WRITE, &notmuch))
+	return 1;
+
+    ret = add_file_to_database (notmuch, filename, tag_ops);
+
+    notmuch_database_destroy (notmuch);
+
+    return (ret) ? 0 : 1;
+}
diff --git a/notmuch.c b/notmuch.c
index 78d29a8..b61149d 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -46,6 +46,8 @@ static command_t commands[] = {
       "Find and import new messages to the notmuch database." },
     { "insert", notmuch_insert_command, FALSE,
       "Add a new message into the maildir and notmuch database." },
+    { "add", notmuch_add_command, FALSE,
+      "Add a message from the maildir into the notmuch database." },
     { "search", notmuch_search_command, FALSE,
       "Search for messages matching the given search terms." },
     { "show", notmuch_show_command, FALSE,
-- 
1.7.9.5

[PATCH 3/3] test: Add simple tests for the add command

[Adam Wolfe Gordon]

---
 test/insert |   35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/test/insert b/test/insert
index 021edb6..eb23e02 100755
--- a/test/insert
+++ b/test/insert
@@ -18,6 +18,14 @@ gen_insert_msg() {
 	"[body]=\"insert-message\""
 }
 
+gen_add_msg() {
+    generate_message \
+        "[dir]=add-messages/cur" \
+        "[subject]=\"add-subject\"" \
+        "[body]=\"add-message\"" \
+        "[id]=\"add-message\""
+}
+
 test_expect_code 1 "Insert zero-length file" \
     "notmuch insert < /dev/null"
 
@@ -118,4 +126,31 @@ gen_insert_msg
 test_expect_code 1 "Insert message, create invalid subfolder" \
     "notmuch insert --folder=../G --create-folder $gen_msg_filename"
 
+gen_add_msg
+tmp_filename=/tmp/$(basename $gen_msg_filename)
+mv $gen_msg_filename $tmp_filename
+notmuch add $tmp_filename
+test_expect_code 1 "Add message from outside maildir returns 1" \
+    "notmuch add $tmp_filename"
+
+test_begin_subtest "Add message from outside maildir does nothing"
+output=$(notmuch count id:$gen_msg_id)
+test_expect_equal "$output" "0"
+
+test_begin_subtest "Add message from inside maildir"
+gen_add_msg
+notmuch add $gen_msg_filename
+output=$(notmuch count id:$gen_msg_id)
+test_expect_equal "$output" "1"
+
+test_begin_subtest "Add duplicate message"
+gen_add_msg
+notmuch add "$gen_msg_filename"
+output=$(notmuch search --output=files "id:$gen_msg_id" | wc -l)
+test_expect_equal "$output" 2
+
+test_begin_subtest "Adding duplicate message does not change tags"
+output=$(notmuch search --format=json --output=tags "id:$gen_msg_id")
+test_expect_equal_json "$output" '["inbox", "unread"]'
+
 test_done
-- 
1.7.9.5

Re: [PATCH 0/3] Introduce the add command

[Tomi Ollila]

On Sun, Jul 14 2013, Adam Wolfe Gordon <awg+notmuch@xvx.ca> wrote:

> Hi everyone,
>
> The recent introduction of insert inspired me to finally add a feature I've
> been wanting: a command to index a specific file in the maildir. My usecase
> for this is that I have an inotify-based script that watches for new mail
> and calls notmuch new when new mail shows up. Running notmuch new in this
> situation is overkill, since I know exactly what's changed. A faster command
> that just adds a single file reduces contention on the database lock.
>
> This series introduces a new command, "notmuch add", which indexes a file
> that already exists in the maildir. It is implemented in notmuch-insert.c
> because it uses the basic infrastructure introduced for the insert command.
>
> Missing man page for now - wanted to get the code out first for review.

The code looks good to me. 

The only issue I noticed when comparing with new is that check for notmuch
database upgrade (and actual upgrade) is not done...

... nor is this done in notmuch insert either.

Maybe in these cases it is better than the upgrade is not attempted but
it might be good to address this in the namual page (here and in insert
page) ?

Anyway I like the feature and would take it into use sometime in the
(distant?) future (i.e. when I'd tune my mail delivery again).

>
> -- Adam

Tomi

>
> Adam Wolfe Gordon (3):
>   cli: Return an error code from add_message_to_database
>   cli: Introduce the add command
>   test: Add simple tests for the add command
>
>  notmuch-client.h |    3 +++
>  notmuch-insert.c |   57 +++++++++++++++++++++++++++++++++++++++++++++++++++---
>  notmuch.c        |    2 ++
>  test/insert      |   35 +++++++++++++++++++++++++++++++++
>  4 files changed, 94 insertions(+), 3 deletions(-)
>
> -- 
> 1.7.9.5
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 0/3] Introduce the add command

[Jameson Graef Rollins]

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

On Sat, Jul 13 2013, Adam Wolfe Gordon <awg+notmuch@xvx.ca> wrote:
> The recent introduction of insert inspired me to finally add a feature I've
> been wanting: a command to index a specific file in the maildir. My usecase
> for this is that I have an inotify-based script that watches for new mail
> and calls notmuch new when new mail shows up. Running notmuch new in this
> situation is overkill, since I know exactly what's changed. A faster command
> that just adds a single file reduces contention on the database lock.
>
> This series introduces a new command, "notmuch add", which indexes a file
> that already exists in the maildir. It is implemented in notmuch-insert.c
> because it uses the basic infrastructure introduced for the insert command.
>
> Missing man page for now - wanted to get the code out first for review.

Hey, Adam.  This feature seems fine, but it seems similar enought to
insert that I wonder if they can just be unified.  What if insert just
took an optional path argument as well, e.g.:

  notmuch insert [options] [ +<tag>|-<tag> ... ] [-- /path/to/file]

If the path is not in the db, it would insert it the same as if it had
come in via stdin.  If the path *is* in the db, it could just do the add
part that you're looking for.  That seems like it might be a more
intuitive UI experience to me.

jamie.

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

Re: [PATCH 0/3] Introduce the add command

[Mark Walters]

> On Sat, Jul 13 2013, Adam Wolfe Gordon <awg+notmuch@xvx.ca> wrote:
>> The recent introduction of insert inspired me to finally add a feature I've
>> been wanting: a command to index a specific file in the maildir. My usecase
>> for this is that I have an inotify-based script that watches for new mail
>> and calls notmuch new when new mail shows up. Running notmuch new in this
>> situation is overkill, since I know exactly what's changed. A faster command
>> that just adds a single file reduces contention on the database lock.
>>
>> This series introduces a new command, "notmuch add", which indexes a file
>> that already exists in the maildir. It is implemented in notmuch-insert.c
>> because it uses the basic infrastructure introduced for the insert command.
>>
>> Missing man page for now - wanted to get the code out first for review.
>
> Hey, Adam.  This feature seems fine, but it seems similar enought to
> insert that I wonder if they can just be unified.  What if insert just
> took an optional path argument as well, e.g.:
>
>   notmuch insert [options] [ +<tag>|-<tag> ... ] [-- /path/to/file]
>
> If the path is not in the db, it would insert it the same as if it had
> come in via stdin.  If the path *is* in the db, it could just do the add
> part that you're looking for.  That seems like it might be a more
> intuitive UI experience to me.

I sort of agree but wonder if it would be more natural under "new" than
"insert". so notmuch new /path/to/file just adds that file (provided it
is in the database; perhaps a relative path?) This would also be
extensible to do a whole mail sub-directory which seems like it might
also be a useful feature. (I am definitely not saying that this needs to
be implemented now!)

Best wishes

Mark





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

Re: [PATCH 0/3] Introduce the add command

[Jameson Graef Rollins]

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

On Sun, Jul 21 2013, Mark Walters <markwalters1009@gmail.com> wrote:
> I sort of agree but wonder if it would be more natural under "new" than
> "insert". so notmuch new /path/to/file just adds that file (provided it
> is in the database; perhaps a relative path?) This would also be
> extensible to do a whole mail sub-directory which seems like it might
> also be a useful feature. (I am definitely not saying that this needs to
> be implemented now!)

Oh, yeah, good call.  That definitely seems more natural to me as well.

jamie.

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

Re: Re: [PATCH 0/3] Introduce the add command

[Vladimir Marek]

> > On Sat, Jul 13 2013, Adam Wolfe Gordon <awg+notmuch@xvx.ca> wrote:
> >> The recent introduction of insert inspired me to finally add a feature I've
> >> been wanting: a command to index a specific file in the maildir. My usecase
> >> for this is that I have an inotify-based script that watches for new mail
> >> and calls notmuch new when new mail shows up. Running notmuch new in this
> >> situation is overkill, since I know exactly what's changed. A faster command
> >> that just adds a single file reduces contention on the database lock.
> >>
> >> This series introduces a new command, "notmuch add", which indexes a file
> >> that already exists in the maildir. It is implemented in notmuch-insert.c
> >> because it uses the basic infrastructure introduced for the insert command.
> >>
> >> Missing man page for now - wanted to get the code out first for review.
> >
> > Hey, Adam.  This feature seems fine, but it seems similar enought to
> > insert that I wonder if they can just be unified.  What if insert just
> > took an optional path argument as well, e.g.:
> >
> >   notmuch insert [options] [ +<tag>|-<tag> ... ] [-- /path/to/file]
> >
> > If the path is not in the db, it would insert it the same as if it had
> > come in via stdin.  If the path *is* in the db, it could just do the add
> > part that you're looking for.  That seems like it might be a more
> > intuitive UI experience to me.
> 
> I sort of agree but wonder if it would be more natural under "new" than
> "insert". so notmuch new /path/to/file just adds that file (provided it
> is in the database; perhaps a relative path?) This would also be
> extensible to do a whole mail sub-directory which seems like it might
> also be a useful feature. (I am definitely not saying that this needs to
> be implemented now!)

That would be mostly welcome addition indeed ...

Cheers
-- 
	Vlad