unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH] notmuch restore --accumulate
@ 2011-09-05 19:07 Thomas Schwinge
  2011-09-05 19:31 ` Jameson Graef Rollins
  2011-09-29 17:36 ` [PATCH, v2] notmuch restore --accumulate Thomas Schwinge
  0 siblings, 2 replies; 22+ messages in thread
From: Thomas Schwinge @ 2011-09-05 19:07 UTC (permalink / raw)
  To: notmuch; +Cc: Thomas Schwinge

From: Thomas Schwinge <thomas@schwinge.name>

Also enhance the dump-restore testsuite, and make it generally more
failure-proof.

Signed-off-by: Thomas Schwinge <thomas@schwinge.name>

---

Hi!

Beware that I have not yet used this new functionality in the wild.  ;-)
(But I do plan to do so, soon.)  And, I think that the testsuite
enhancements cover quite a number of real-world scenarios.


Grüße,
 Thomas

---

 NEWS              |   13 ++++++++++
 notmuch-restore.c |   42 ++++++++++++++++++++++++++-------
 notmuch.1         |   14 ++++++++---
 notmuch.c         |   10 +++++--
 test/dump-restore |   67 ++++++++++++++++++++++++++++++++++++++++------------
 test/test-lib.sh  |    1 +
 6 files changed, 115 insertions(+), 32 deletions(-)

diff --git a/NEWS b/NEWS
index f715142..d2a788f 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,16 @@
+Notmuch TODO (TODO)
+===================
+
+New command-line features
+-------------------------
+
+Add "notmuch restore --accumulate" option
+
+  The --accumulate switch causes the union of the existing and new tags to be
+  applied, instead of replacing each message's tags as they are read in from
+  the dump file.
+
+
 Notmuch 0.7 (2011-08-01)
 ========================
 
diff --git a/notmuch-restore.c b/notmuch-restore.c
index f095f64..5aad60c 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -31,7 +31,8 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[])
     size_t line_size;
     ssize_t line_len;
     regex_t regex;
-    int rerr;
+    notmuch_bool_t accumulate;
+    int i, rerr;
 
     config = notmuch_config_open (ctx, NULL, NULL);
     if (config == NULL)
@@ -44,14 +45,28 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[])
 
     synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config);
 
-    if (argc) {
-	input = fopen (argv[0], "r");
-	if (input == NULL) {
-	    fprintf (stderr, "Error opening %s for reading: %s\n",
-		     argv[0], strerror (errno));
-	    return 1;
+    accumulate = FALSE;
+    input = NULL;
+    for (i = 0; i < argc; i++) {
+	if (STRNCMP_LITERAL (argv[i], "--accumulate") == 0) {
+	    accumulate = TRUE;
+	} else {
+	    if (input == NULL) {
+		input = fopen (argv[i], "r");
+		if (input == NULL) {
+		    fprintf (stderr, "Error opening %s for reading: %s\n",
+			     argv[i], strerror (errno));
+		    return 1;
+		}
+	    } else {
+		fprintf (stderr,
+			 "Cannot read dump from more than one file: %s\n",
+			 argv[i]);
+		return 1;
+	    }
 	}
-    } else {
+    }
+    if (input == NULL) {
 	printf ("No filename given. Reading dump from stdin.\n");
 	input = stdin;
     }
@@ -94,6 +109,13 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[])
 	    goto NEXT_LINE;
 	}
 
+	/* In order to detect missing messages, this check/optimization is
+	 * intentionally done *after* first finding the message.  */
+	if (accumulate && (file_tags == NULL || *file_tags == '\0'))
+	{
+	    goto NEXT_LINE;
+	}
+
 	db_tags_str = NULL;
 	for (db_tags = notmuch_message_get_tags (message);
 	     notmuch_tags_valid (db_tags);
@@ -115,7 +137,9 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[])
 	}
 
 	notmuch_message_freeze (message);
-	notmuch_message_remove_all_tags (message);
+
+	if (!accumulate)
+	    notmuch_message_remove_all_tags (message);
 
 	next = file_tags;
 	while (next) {
diff --git a/notmuch.1 b/notmuch.1
index 5a8c83d..883371d 100644
--- a/notmuch.1
+++ b/notmuch.1
@@ -454,7 +454,7 @@ section below for details of the supported syntax for <search-terms>.
 The
 .BR dump " and " restore
 commands can be used to create a textual dump of email tags for backup
-purposes, and to restore from that dump
+purposes, and to restore from that dump.
 
 .RS 4
 .TP 4
@@ -462,17 +462,19 @@ purposes, and to restore from that dump
 
 Creates a plain-text dump of the tags of each message.
 
-The output is to the given filename, if any, or to stdout.
+The output is written to the given filename, if any, or to stdout.
 
 These tags are the only data in the notmuch database that can't be
 recreated from the messages themselves.  The output of notmuch dump is
 therefore the only critical thing to backup (and much more friendly to
 incremental backup than the native database files.)
 .TP
-.BR restore " <filename>"
+.BR restore " [--accumulate] [<filename>]"
 
 Restores the tags from the given file (see
-.BR "notmuch dump" "."
+.BR "notmuch dump" ")."
+
+The input is read from the given filename, if any, or from stdin.
 
 Note: The dump file format is specifically chosen to be
 compatible with the format of files produced by sup-dump.
@@ -480,6 +482,10 @@ So if you've previously been using sup for mail, then the
 .B "notmuch restore"
 command provides you a way to import all of your tags (or labels as
 sup calls them).
+
+The --accumulate switch causes the union of the existing and new tags to be
+applied, instead of replacing each message's tags as they are read in from the
+dump file.
 .RE
 
 The
diff --git a/notmuch.c b/notmuch.c
index 3973e35..def52b0 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -377,20 +377,24 @@ static command_t commands[] = {
     { "dump", notmuch_dump_command,
       "[<filename>]",
       "Create a plain-text dump of the tags for each message.",
-      "\tOutput is to the given filename, if any, or to stdout.\n"
+      "\tOutput is written to the given filename, if any, or to stdout.\n"
       "\tThese tags are the only data in the notmuch database\n"
       "\tthat can't be recreated from the messages themselves.\n"
       "\tThe output of notmuch dump is therefore the only\n"
       "\tcritical thing to backup (and much more friendly to\n"
       "\tincremental backup than the native database files.)" },
     { "restore", notmuch_restore_command,
-      "<filename>",
+      "[--accumulate] [<filename>]",
       "Restore the tags from the given dump file (see 'dump').",
+      "\tInput is read from the given filename, if any, or from stdin.\n"
       "\tNote: The dump file format is specifically chosen to be\n"
       "\tcompatible with the format of files produced by sup-dump.\n"
       "\tSo if you've previously been using sup for mail, then the\n"
       "\t\"notmuch restore\" command provides you a way to import\n"
-      "\tall of your tags (or labels as sup calls them)." },
+      "\tall of your tags (or labels as sup calls them).\n"
+      "\tThe --accumulate switch causes the union of the existing and new\n"
+      "\ttags to be applied, instead of replacing each message's tags as\n"
+      "\tthey are read in from the dump file."},
     { "config", notmuch_config_command,
       "[get|set] <section>.<item> [value ...]",
       "Get or set settings in the notmuch configuration file.",
diff --git a/test/dump-restore b/test/dump-restore
index a4de370..c85f10e 100755
--- a/test/dump-restore
+++ b/test/dump-restore
@@ -4,21 +4,56 @@ test_description="\"notmuch dump\" and \"notmuch restore\""
 
 add_email_corpus
 
-test_expect_success "Dumping all tags" "generate_message &&
-notmuch new &&
-notmuch dump dump.expected"
-
-test_begin_subtest "Clearing all tags"
-sed -e "s/(\([^(]*\))$/()/" < dump.expected > clear.expected
-notmuch restore clear.expected
-notmuch dump clear.actual
-test_expect_equal "$(< clear.actual)" "$(< clear.expected)"
-
-test_begin_subtest "Restoring original tags"
-notmuch restore dump.expected
-notmuch dump dump.actual
-test_expect_equal "$(< dump.actual)" "$(< dump.expected)"
-
-test_expect_success "Restore with nothing to do" "notmuch restore dump.expected"
+test_expect_success 'Dumping all tags' \
+  'generate_message &&
+  notmuch new &&
+  notmuch dump dump.expected'
+
+# This is rather arbitrary: it matches some of the email corpus' messages, but
+# not all of them.
+search_term=from:worth
+
+test_expect_success 'Dumping all tags to stdout' \
+  'notmuch tag +ABC +DEF -- $search_term &&
+  notmuch dump > dump-ABC_DEF.expected &&
+  ! cmp dump.expected dump-ABC_DEF.expected'
+
+test_expect_success 'Clearing all tags' \
+  'sed -e "s/(\([^(]*\))$/()/" < dump.expected > clear.expected &&
+  notmuch restore clear.expected &&
+  notmuch dump clear.actual &&
+  test_cmp clear.expected clear.actual'
+
+test_expect_success 'Accumulate original tags' \
+  'notmuch tag +ABC +DEF -- $search_term &&
+  notmuch restore --accumulate < dump.expected &&
+  notmuch dump dump.actual &&
+  test_cmp dump-ABC_DEF.expected dump.actual'
+
+test_expect_success 'Restoring original tags' \
+  'notmuch restore dump.expected &&
+  notmuch dump dump.actual &&
+  test_cmp dump.expected dump.actual'
+
+test_expect_success 'Restore with nothing to do' \
+  'notmuch restore < dump.expected &&
+  notmuch dump > dump.actual &&
+  test_cmp dump.expected dump.actual'
+
+test_expect_success 'Restore with nothing to do, II' \
+  'notmuch restore --accumulate dump.expected &&
+  notmuch dump dump.actual &&
+  test_cmp dump.expected dump.actual'
+
+test_expect_success 'Restore with nothing to do, III' \
+  'notmuch restore --accumulate < clear.expected &&
+  notmuch dump dump.actual &&
+  test_cmp dump.expected dump.actual'
+
+test_expect_success 'Invalid restore invocation' \
+  '! notmuch restore one two'
+
+test_expect_success 'Invalid restore invocation, II' \
+  '! notmuch restore --accumulate one two'
 
 test_done
diff --git a/test/test-lib.sh b/test/test-lib.sh
index 22e387e..56bbce4 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -461,6 +461,7 @@ test_expect_equal ()
     fi
 }
 
+# Like test_expect_equal, but takes two filenames.
 test_expect_equal_file ()
 {
 	exec 1>&6 2>&7		# Restore stdout and stderr
-- 
tg: (8e2a14b..) t/restore-accumulate (depends on: master)

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

* Re: [PATCH] notmuch restore --accumulate
  2011-09-05 19:07 [PATCH] notmuch restore --accumulate Thomas Schwinge
@ 2011-09-05 19:31 ` Jameson Graef Rollins
  2011-09-09  9:06   ` Louis Rilling
  2011-09-29 17:36 ` [PATCH, v2] notmuch restore --accumulate Thomas Schwinge
  1 sibling, 1 reply; 22+ messages in thread
From: Jameson Graef Rollins @ 2011-09-05 19:31 UTC (permalink / raw)
  To: Thomas Schwinge, notmuch

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

On Mon,  5 Sep 2011 21:07:17 +0200, Thomas Schwinge <thomas@schwinge.name> wrote:
> From: Thomas Schwinge <thomas@schwinge.name>
> 
> Also enhance the dump-restore testsuite, and make it generally more
> failure-proof.

Hi, Thomas.  Your commit log doesn't describe what this patch does.  If
you're adding a new feature you should describe in detail what this new
feature does and how it works.

Also, we generally prefer to have modifications to the test suite in
separate patches that precede the patches that add the features/fix the
bugs.

> Signed-off-by: Thomas Schwinge <thomas@schwinge.name>

Finally, you don't need to sign off on your own patches.

Thanks.

jamie.

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

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

* Re: [PATCH] notmuch restore --accumulate
  2011-09-05 19:31 ` Jameson Graef Rollins
@ 2011-09-09  9:06   ` Louis Rilling
  2011-09-09  9:45     ` Thomas Schwinge
  0 siblings, 1 reply; 22+ messages in thread
From: Louis Rilling @ 2011-09-09  9:06 UTC (permalink / raw)
  To: Jameson Graef Rollins; +Cc: notmuch, Thomas Schwinge

On 05/09/11 12:31 -0700, Jameson Graef Rollins wrote:
> Also, we generally prefer to have modifications to the test suite in
> separate patches that precede the patches that add the features/fix the
> bugs.
> 

For new features, this does not look like 'git bisect'-safe. I would say that
the testsuite patch should follow the new feature patch, don't you?

Thanks,

Louis

> > Signed-off-by: Thomas Schwinge <thomas@schwinge.name>
> 
> Finally, you don't need to sign off on your own patches.
> 
> Thanks.
> 
> jamie.



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

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

* Re: [PATCH] notmuch restore --accumulate
  2011-09-09  9:06   ` Louis Rilling
@ 2011-09-09  9:45     ` Thomas Schwinge
  2011-09-09 16:13       ` Austin Clements
  0 siblings, 1 reply; 22+ messages in thread
From: Thomas Schwinge @ 2011-09-09  9:45 UTC (permalink / raw)
  To: Louis Rilling, Jameson Graef Rollins; +Cc: notmuch

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

Hi!

On Fri, 9 Sep 2011 11:06:34 +0200, Louis Rilling <l.rilling@av7.net> wrote:
> On 05/09/11 12:31 -0700, Jameson Graef Rollins wrote:
> > Also, we generally prefer to have modifications to the test suite in
> > separate patches that precede the patches that add the features/fix the
> > bugs.
> > 
> 
> For new features, this does not look like 'git bisect'-safe.

Exactly my thoughts.  I can perhaps see the usefulness (for first
separately committing the testcase) for bugfixes, but why for new
features?

> I would say that
> the testsuite patch should follow the new feature patch, don't you?

I would keep them together; why separate them?


Grüße,
 Thomas

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

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

* Re: [PATCH] notmuch restore --accumulate
  2011-09-09  9:45     ` Thomas Schwinge
@ 2011-09-09 16:13       ` Austin Clements
  2011-09-09 17:22         ` Thomas Schwinge
  0 siblings, 1 reply; 22+ messages in thread
From: Austin Clements @ 2011-09-09 16:13 UTC (permalink / raw)
  To: Thomas Schwinge; +Cc: notmuch

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

The idea behind sending the test first is that people can see that it fails
and that the subsequent patch indeed fixes it.  What I find works well is to
submit the test case with the test marked as broken and then the main patch,
including the change to un-mark it as broken.
On Sep 9, 2011 5:45 AM, "Thomas Schwinge" <thomas@schwinge.name> wrote:
> Hi!
>
> On Fri, 9 Sep 2011 11:06:34 +0200, Louis Rilling <l.rilling@av7.net>
wrote:
>> On 05/09/11 12:31 -0700, Jameson Graef Rollins wrote:
>> > Also, we generally prefer to have modifications to the test suite in
>> > separate patches that precede the patches that add the features/fix the
>> > bugs.
>> >
>>
>> For new features, this does not look like 'git bisect'-safe.
>
> Exactly my thoughts. I can perhaps see the usefulness (for first
> separately committing the testcase) for bugfixes, but why for new
> features?
>
>> I would say that
>> the testsuite patch should follow the new feature patch, don't you?
>
> I would keep them together; why separate them?
>
>
> Grüße,
> Thomas

[-- Attachment #2: Type: text/html, Size: 1415 bytes --]

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

* Re: [PATCH] notmuch restore --accumulate
  2011-09-09 16:13       ` Austin Clements
@ 2011-09-09 17:22         ` Thomas Schwinge
  2011-09-09 17:35           ` Austin Clements
  2011-09-09 18:58           ` Jameson Graef Rollins
  0 siblings, 2 replies; 22+ messages in thread
From: Thomas Schwinge @ 2011-09-09 17:22 UTC (permalink / raw)
  To: Austin Clements; +Cc: notmuch

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

Hi!

On Fri, 9 Sep 2011 12:13:06 -0400, Austin Clements <amdragon@mit.edu> wrote:
> The idea behind sending the test first is that people can see that it fails
> and that the subsequent patch indeed fixes it.  What I find works well is to
> submit the test case with the test marked as broken and then the main patch,
> including the change to un-mark it as broken.

Ah, that's indeed a good approach for bug fixes (and it also preserves
git bisect compatibility), but still: why separate patches for new
functionality?  (I'm not trying to be a pain here, but would like to
understand your rationale behind this.)


Grüße,
 Thomas

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

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

* Re: [PATCH] notmuch restore --accumulate
  2011-09-09 17:22         ` Thomas Schwinge
@ 2011-09-09 17:35           ` Austin Clements
  2011-09-09 18:58           ` Jameson Graef Rollins
  1 sibling, 0 replies; 22+ messages in thread
From: Austin Clements @ 2011-09-09 17:35 UTC (permalink / raw)
  To: Thomas Schwinge; +Cc: notmuch

Quoth Thomas Schwinge on Sep 09 at  7:22 pm:
> Hi!
> 
> On Fri, 9 Sep 2011 12:13:06 -0400, Austin Clements <amdragon@mit.edu> wrote:
> > The idea behind sending the test first is that people can see that it fails
> > and that the subsequent patch indeed fixes it.  What I find works well is to
> > submit the test case with the test marked as broken and then the main patch,
> > including the change to un-mark it as broken.
> 
> Ah, that's indeed a good approach for bug fixes (and it also preserves
> git bisect compatibility), but still: why separate patches for new
> functionality?  (I'm not trying to be a pain here, but would like to
> understand your rationale behind this.)

*shrugs* I don't think it's too important for new functionality,
though for review it's nice to be able to focus on just the
implementation or just the tests, rather than having the patches
intermingled.

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

* Re: [PATCH] notmuch restore --accumulate
  2011-09-09 17:22         ` Thomas Schwinge
  2011-09-09 17:35           ` Austin Clements
@ 2011-09-09 18:58           ` Jameson Graef Rollins
  2011-09-29 18:24             ` [PATCH] patchformatting: Test Suite Enhancements Thomas Schwinge
  1 sibling, 1 reply; 22+ messages in thread
From: Jameson Graef Rollins @ 2011-09-09 18:58 UTC (permalink / raw)
  To: Thomas Schwinge, Austin Clements; +Cc: notmuch

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

On Fri, 09 Sep 2011 19:22:49 +0200, Thomas Schwinge <thomas@schwinge.name> wrote:
> Ah, that's indeed a good approach for bug fixes (and it also preserves
> git bisect compatibility), but still: why separate patches for new
> functionality?  (I'm not trying to be a pain here, but would like to
> understand your rationale behind this.)

I tend to think of the test as an actual spec of the behavior I'm trying
to implement.  By defining before hand exactly what I expect to happen I
can confirm that my patch achieves what I want it to.

As an example, you might look at my patch that adds better rfc822 part
handling [0].  I tried to fully flesh out what I wanted to happen in the
test first, and then modified the code to achieve that result.

jamie.

[0] id:"1307320169-29905-4-git-send-email-jrollins@finestructure.net"

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

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

* [PATCH, v2] notmuch restore --accumulate
  2011-09-05 19:07 [PATCH] notmuch restore --accumulate Thomas Schwinge
  2011-09-05 19:31 ` Jameson Graef Rollins
@ 2011-09-29 17:36 ` Thomas Schwinge
  2011-09-29 17:37   ` Thomas Schwinge
  2011-10-16 21:48   ` [PATCH, v2] " David Bremner
  1 sibling, 2 replies; 22+ messages in thread
From: Thomas Schwinge @ 2011-09-29 17:36 UTC (permalink / raw)
  To: notmuch

From: Thomas Schwinge <thomas@schwinge.name>

Flesh out what ``notmuch restore --accumulate'' is supposed to do.  Its tests
are currently XFAILed; the functionality will be added in another patch.

Also generally enhance the dump-restore testsuite, and make it more
failure-proof.

---
 test/dump-restore |   77 ++++++++++++++++++++++++++++++++++++++++++-----------
 test/test-lib.sh  |    3 +-
 2 files changed, 63 insertions(+), 17 deletions(-)

diff --git a/test/dump-restore b/test/dump-restore
index a4de370..d253756 100755
--- a/test/dump-restore
+++ b/test/dump-restore
@@ -4,21 +4,66 @@ test_description="\"notmuch dump\" and \"notmuch restore\""
 
 add_email_corpus
 
-test_expect_success "Dumping all tags" "generate_message &&
-notmuch new &&
-notmuch dump dump.expected"
-
-test_begin_subtest "Clearing all tags"
-sed -e "s/(\([^(]*\))$/()/" < dump.expected > clear.expected
-notmuch restore clear.expected
-notmuch dump clear.actual
-test_expect_equal "$(< clear.actual)" "$(< clear.expected)"
-
-test_begin_subtest "Restoring original tags"
-notmuch restore dump.expected
-notmuch dump dump.actual
-test_expect_equal "$(< dump.actual)" "$(< dump.expected)"
-
-test_expect_success "Restore with nothing to do" "notmuch restore dump.expected"
+test_expect_success 'Dumping all tags' \
+  'generate_message &&
+  notmuch new &&
+  notmuch dump dump.expected'
+
+# This is rather arbitrary: it matches some of the email corpus' messages, but
+# not all of them.
+search_term=from:worth
+
+test_expect_success 'Dumping all tags to stdout' \
+  'notmuch tag +ABC +DEF -- $search_term &&
+  notmuch dump > dump-ABC_DEF.expected &&
+  ! cmp dump.expected dump-ABC_DEF.expected'
+
+test_expect_success 'Clearing all tags' \
+  'sed -e "s/(\([^(]*\))$/()/" < dump.expected > clear.expected &&
+  notmuch restore clear.expected &&
+  notmuch dump clear.actual &&
+  test_cmp clear.expected clear.actual'
+
+# Missing notmuch restore --accumulate.
+test_subtest_known_broken
+test_expect_success 'Accumulate original tags' \
+  'notmuch tag +ABC +DEF -- $search_term &&
+  notmuch restore --accumulate < dump.expected &&
+  notmuch dump dump.actual &&
+  test_cmp dump-ABC_DEF.expected dump.actual'
+
+test_expect_success 'Restoring original tags' \
+  'notmuch restore dump.expected &&
+  notmuch dump dump.actual &&
+  test_cmp dump.expected dump.actual'
+
+test_expect_success 'Restore with nothing to do' \
+  'notmuch restore < dump.expected &&
+  notmuch dump > dump.actual &&
+  test_cmp dump.expected dump.actual'
+
+# Missing notmuch restore --accumulate.
+test_subtest_known_broken
+test_expect_success 'Restore with nothing to do, II' \
+  'notmuch restore --accumulate dump.expected &&
+  notmuch dump dump.actual &&
+  test_cmp dump.expected dump.actual'
+
+# Missing notmuch restore --accumulate.
+test_subtest_known_broken
+test_expect_success 'Restore with nothing to do, III' \
+  'notmuch restore --accumulate < clear.expected &&
+  notmuch dump dump.actual &&
+  test_cmp dump.expected dump.actual'
+
+# notmuch restore currently only considers the first argument.
+test_subtest_known_broken
+test_expect_success 'Invalid restore invocation' \
+  'test_must_fail notmuch restore dump.expected another_one'
+
+# The follwing test already succeeds due to notmuch restore currently only
+# considering the first argument.
+test_expect_success 'Invalid restore invocation, II' \
+  'test_must_fail notmuch restore --accumulate dump.expected another_one'
 
 test_done
diff --git a/test/test-lib.sh b/test/test-lib.sh
index f8df6a5..f524ebf 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -462,6 +462,7 @@ test_expect_equal ()
     fi
 }
 
+# Like test_expect_equal, but takes two filenames.
 test_expect_equal_file ()
 {
 	exec 1>&6 2>&7		# Restore stdout and stderr
@@ -724,7 +725,7 @@ test_external_without_stderr () {
 	fi
 }
 
-# This is not among top-level (test_expect_success | test_expect_failure)
+# This is not among top-level (test_expect_success)
 # but is a prefix that can be used in the test script, like:
 #
 #	test_expect_success 'complain and die' '
-- 
tg: (f63d605..) t/restore-accumulate-test (depends on: master)

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

* [PATCH, v2] notmuch restore --accumulate
  2011-09-29 17:36 ` [PATCH, v2] notmuch restore --accumulate Thomas Schwinge
@ 2011-09-29 17:37   ` Thomas Schwinge
  2011-10-21 19:47     ` Updated v3 patches for " David Bremner
  2011-10-16 21:48   ` [PATCH, v2] " David Bremner
  1 sibling, 1 reply; 22+ messages in thread
From: Thomas Schwinge @ 2011-09-29 17:37 UTC (permalink / raw)
  To: notmuch

From: Thomas Schwinge <thomas@schwinge.name>

The --accumulate switch causes the union of the existing and new tags to be
applied, instead of replacing each message's tags as they are read in from the
dump file.

---

Hi!

Beware that I have not yet used this new functionality in the wild.  ;-)
(But I do plan to do so, soon.)  And, I think that the testsuite
enhancements cover quite a number of real-world scenarios.

This is v2; it addresses Jameson's and Austin's comments.


Grüße,
 Thomas

---

 NEWS              |    9 +++++++++
 notmuch-restore.c |   42 +++++++++++++++++++++++++++++++++---------
 notmuch.1         |   14 ++++++++++----
 notmuch.c         |   10 +++++++---
 test/dump-restore |   10 ----------
 5 files changed, 59 insertions(+), 26 deletions(-)

diff --git a/NEWS b/NEWS
index ee84e9a..2a184ef 100644
--- a/NEWS
+++ b/NEWS
@@ -12,6 +12,15 @@ Correct handling of interruptions during "notmuch new"
   detect messages on resume, or leave the database in a state
   temporarily or permanently inconsistent with the mail store.
 
+New command-line features
+-------------------------
+
+Add "notmuch restore --accumulate" option
+
+  The --accumulate switch causes the union of the existing and new tags to be
+  applied, instead of replacing each message's tags as they are read in from
+  the dump file.
+
 Library changes
 ---------------
 
diff --git a/notmuch-restore.c b/notmuch-restore.c
index f095f64..5aad60c 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -31,7 +31,8 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[])
     size_t line_size;
     ssize_t line_len;
     regex_t regex;
-    int rerr;
+    notmuch_bool_t accumulate;
+    int i, rerr;
 
     config = notmuch_config_open (ctx, NULL, NULL);
     if (config == NULL)
@@ -44,14 +45,28 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[])
 
     synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config);
 
-    if (argc) {
-	input = fopen (argv[0], "r");
-	if (input == NULL) {
-	    fprintf (stderr, "Error opening %s for reading: %s\n",
-		     argv[0], strerror (errno));
-	    return 1;
+    accumulate = FALSE;
+    input = NULL;
+    for (i = 0; i < argc; i++) {
+	if (STRNCMP_LITERAL (argv[i], "--accumulate") == 0) {
+	    accumulate = TRUE;
+	} else {
+	    if (input == NULL) {
+		input = fopen (argv[i], "r");
+		if (input == NULL) {
+		    fprintf (stderr, "Error opening %s for reading: %s\n",
+			     argv[i], strerror (errno));
+		    return 1;
+		}
+	    } else {
+		fprintf (stderr,
+			 "Cannot read dump from more than one file: %s\n",
+			 argv[i]);
+		return 1;
+	    }
 	}
-    } else {
+    }
+    if (input == NULL) {
 	printf ("No filename given. Reading dump from stdin.\n");
 	input = stdin;
     }
@@ -94,6 +109,13 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[])
 	    goto NEXT_LINE;
 	}
 
+	/* In order to detect missing messages, this check/optimization is
+	 * intentionally done *after* first finding the message.  */
+	if (accumulate && (file_tags == NULL || *file_tags == '\0'))
+	{
+	    goto NEXT_LINE;
+	}
+
 	db_tags_str = NULL;
 	for (db_tags = notmuch_message_get_tags (message);
 	     notmuch_tags_valid (db_tags);
@@ -115,7 +137,9 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[])
 	}
 
 	notmuch_message_freeze (message);
-	notmuch_message_remove_all_tags (message);
+
+	if (!accumulate)
+	    notmuch_message_remove_all_tags (message);
 
 	next = file_tags;
 	while (next) {
diff --git a/notmuch.1 b/notmuch.1
index 5a8c83d..883371d 100644
--- a/notmuch.1
+++ b/notmuch.1
@@ -454,7 +454,7 @@ section below for details of the supported syntax for <search-terms>.
 The
 .BR dump " and " restore
 commands can be used to create a textual dump of email tags for backup
-purposes, and to restore from that dump
+purposes, and to restore from that dump.
 
 .RS 4
 .TP 4
@@ -462,17 +462,19 @@ purposes, and to restore from that dump
 
 Creates a plain-text dump of the tags of each message.
 
-The output is to the given filename, if any, or to stdout.
+The output is written to the given filename, if any, or to stdout.
 
 These tags are the only data in the notmuch database that can't be
 recreated from the messages themselves.  The output of notmuch dump is
 therefore the only critical thing to backup (and much more friendly to
 incremental backup than the native database files.)
 .TP
-.BR restore " <filename>"
+.BR restore " [--accumulate] [<filename>]"
 
 Restores the tags from the given file (see
-.BR "notmuch dump" "."
+.BR "notmuch dump" ")."
+
+The input is read from the given filename, if any, or from stdin.
 
 Note: The dump file format is specifically chosen to be
 compatible with the format of files produced by sup-dump.
@@ -480,6 +482,10 @@ So if you've previously been using sup for mail, then the
 .B "notmuch restore"
 command provides you a way to import all of your tags (or labels as
 sup calls them).
+
+The --accumulate switch causes the union of the existing and new tags to be
+applied, instead of replacing each message's tags as they are read in from the
+dump file.
 .RE
 
 The
diff --git a/notmuch.c b/notmuch.c
index f9d6629..9a9b0a2 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -377,20 +377,24 @@ static command_t commands[] = {
     { "dump", notmuch_dump_command,
       "[<filename>]",
       "Create a plain-text dump of the tags for each message.",
-      "\tOutput is to the given filename, if any, or to stdout.\n"
+      "\tOutput is written to the given filename, if any, or to stdout.\n"
       "\tThese tags are the only data in the notmuch database\n"
       "\tthat can't be recreated from the messages themselves.\n"
       "\tThe output of notmuch dump is therefore the only\n"
       "\tcritical thing to backup (and much more friendly to\n"
       "\tincremental backup than the native database files.)" },
     { "restore", notmuch_restore_command,
-      "<filename>",
+      "[--accumulate] [<filename>]",
       "Restore the tags from the given dump file (see 'dump').",
+      "\tInput is read from the given filename, if any, or from stdin.\n"
       "\tNote: The dump file format is specifically chosen to be\n"
       "\tcompatible with the format of files produced by sup-dump.\n"
       "\tSo if you've previously been using sup for mail, then the\n"
       "\t\"notmuch restore\" command provides you a way to import\n"
-      "\tall of your tags (or labels as sup calls them)." },
+      "\tall of your tags (or labels as sup calls them).\n"
+      "\tThe --accumulate switch causes the union of the existing and new\n"
+      "\ttags to be applied, instead of replacing each message's tags as\n"
+      "\tthey are read in from the dump file."},
     { "config", notmuch_config_command,
       "[get|set] <section>.<item> [value ...]",
       "Get or set settings in the notmuch configuration file.",
diff --git a/test/dump-restore b/test/dump-restore
index d253756..be0eb2a 100755
--- a/test/dump-restore
+++ b/test/dump-restore
@@ -24,8 +24,6 @@ test_expect_success 'Clearing all tags' \
   notmuch dump clear.actual &&
   test_cmp clear.expected clear.actual'
 
-# Missing notmuch restore --accumulate.
-test_subtest_known_broken
 test_expect_success 'Accumulate original tags' \
   'notmuch tag +ABC +DEF -- $search_term &&
   notmuch restore --accumulate < dump.expected &&
@@ -42,27 +40,19 @@ test_expect_success 'Restore with nothing to do' \
   notmuch dump > dump.actual &&
   test_cmp dump.expected dump.actual'
 
-# Missing notmuch restore --accumulate.
-test_subtest_known_broken
 test_expect_success 'Restore with nothing to do, II' \
   'notmuch restore --accumulate dump.expected &&
   notmuch dump dump.actual &&
   test_cmp dump.expected dump.actual'
 
-# Missing notmuch restore --accumulate.
-test_subtest_known_broken
 test_expect_success 'Restore with nothing to do, III' \
   'notmuch restore --accumulate < clear.expected &&
   notmuch dump dump.actual &&
   test_cmp dump.expected dump.actual'
 
-# notmuch restore currently only considers the first argument.
-test_subtest_known_broken
 test_expect_success 'Invalid restore invocation' \
   'test_must_fail notmuch restore dump.expected another_one'
 
-# The follwing test already succeeds due to notmuch restore currently only
-# considering the first argument.
 test_expect_success 'Invalid restore invocation, II' \
   'test_must_fail notmuch restore --accumulate dump.expected another_one'
 
-- 
tg: (b968722..) t/restore-accumulate (depends on: master t/restore-accumulate-test)

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

* [PATCH] patchformatting: Test Suite Enhancements
  2011-09-09 18:58           ` Jameson Graef Rollins
@ 2011-09-29 18:24             ` Thomas Schwinge
  0 siblings, 0 replies; 22+ messages in thread
From: Thomas Schwinge @ 2011-09-29 18:24 UTC (permalink / raw)
  To: notmuch; +Cc: Thomas Schwinge

Based on the emails starting at
id:"87liu2kcq6.fsf@servo.factory.finestructure.net".
---

Hi!

I applied the attached patch, based on this thread's discussion.


Grüße,
 Thomas


---
 patchformatting.mdwn |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/patchformatting.mdwn b/patchformatting.mdwn
index cf5bf81..914371d 100644
--- a/patchformatting.mdwn
+++ b/patchformatting.mdwn
@@ -41,6 +41,16 @@ Eric S. Raymond has written good
 [Software Release Practice HOWTO](http://tldp.org/HOWTO/Software-Release-Practice-HOWTO/).
 Check what he has to say about this issue. 
 
+### Test Suite Enhancements
+
+New features as well as bug fixes should typically come with test suite
+enhancements.  The test suite changes should be done first (tagged as *expected
+to fail*), and the feature implementation or bug fix should come second
+(removing the *expected to fail* tag).  This way, the test suite specifies the
+behavior you're trying to implement, be it a new feature or a bug fix.  By
+defining beforehand exactly what you expect to happen, everyone can confirm
+that your patch achieves what it is meant it to.
+
 ## Prepare patches for e-mail submission
 
 If you've made just one commit (containing just one bugfix or new feature)
-- 
1.7.6.3

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

* Re: [PATCH, v2] notmuch restore --accumulate
  2011-09-29 17:36 ` [PATCH, v2] notmuch restore --accumulate Thomas Schwinge
  2011-09-29 17:37   ` Thomas Schwinge
@ 2011-10-16 21:48   ` David Bremner
  1 sibling, 0 replies; 22+ messages in thread
From: David Bremner @ 2011-10-16 21:48 UTC (permalink / raw)
  To: Thomas Schwinge, notmuch

On Thu, 29 Sep 2011 19:36:51 +0200, Thomas Schwinge <thomas@schwinge.name> wrote:
> From: Thomas Schwinge <thomas@schwinge.name>
> 
> Flesh out what ``notmuch restore --accumulate'' is supposed to do.  Its tests
> are currently XFAILed; the functionality will be added in another patch.
> 

I was looking at these patches and it occured to me that a
generalization like

     notmuch restore --replace=ALL      current behaviour and default

     notmuch restore --replace=NONE     proposed --accumulate behaviour

     notmuch restore --replace=<regex>  delete all tags matching regex
                                        before adding tags from file.

would solve your use case (some kind of merging?) and the case of
restoring a namespace of tags like "notmuch\..*"

Let the bikeshedding begin!

d

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

* Updated v3 patches for notmuch restore --accumulate.
  2011-09-29 17:37   ` Thomas Schwinge
@ 2011-10-21 19:47     ` David Bremner
  2011-10-21 19:47       ` [PATCH 1/6] test/test-lib.sh: update comments David Bremner
                         ` (6 more replies)
  0 siblings, 7 replies; 22+ messages in thread
From: David Bremner @ 2011-10-21 19:47 UTC (permalink / raw)
  To: notmuch

These are rebased quite violently from what Thomas posted.  They are
also the first use of getopt (because I plan to add another option to
restore). They depend on id:"1319199557-16888-1-git-send-email-david@tethera.net".

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

* [PATCH 1/6] test/test-lib.sh: update comments
  2011-10-21 19:47     ` Updated v3 patches for " David Bremner
@ 2011-10-21 19:47       ` David Bremner
  2011-10-21 19:47       ` [PATCH 2/6] test/dump-restore: expand test suite for dump-restore, make more robust David Bremner
                         ` (5 subsequent siblings)
  6 siblings, 0 replies; 22+ messages in thread
From: David Bremner @ 2011-10-21 19:47 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

From: David Bremner <bremner@debian.org>

- explain test_expect_equal_file
- remove mention of test_expect_failure, since that function was removed.

Based on id:"1317317811-29540-1-git-send-email-thomas@schwinge.name"
---
 test/test-lib.sh |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/test/test-lib.sh b/test/test-lib.sh
index f8df6a5..f524ebf 100755
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -462,6 +462,7 @@ test_expect_equal ()
     fi
 }
 
+# Like test_expect_equal, but takes two filenames.
 test_expect_equal_file ()
 {
 	exec 1>&6 2>&7		# Restore stdout and stderr
@@ -724,7 +725,7 @@ test_external_without_stderr () {
 	fi
 }
 
-# This is not among top-level (test_expect_success | test_expect_failure)
+# This is not among top-level (test_expect_success)
 # but is a prefix that can be used in the test script, like:
 #
 #	test_expect_success 'complain and die' '
-- 
1.7.5.4

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

* [PATCH 2/6] test/dump-restore: expand test suite for dump-restore, make more robust
  2011-10-21 19:47     ` Updated v3 patches for " David Bremner
  2011-10-21 19:47       ` [PATCH 1/6] test/test-lib.sh: update comments David Bremner
@ 2011-10-21 19:47       ` David Bremner
  2011-10-21 19:47       ` [PATCH 3/6] test/dump-restore: add tests for restore --accumulate David Bremner
                         ` (4 subsequent siblings)
  6 siblings, 0 replies; 22+ messages in thread
From: David Bremner @ 2011-10-21 19:47 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

From: David Bremner <bremner@debian.org>

Several new tests are added, and existing use of test_begin_subtest is
replaced by test_expect_success to catch failing commands in cases where
we execute more than one command.

Based on changes in

      id:"1317317811-29540-1-git-send-email-thomas@schwinge.name"
---
 test/dump-restore |   41 +++++++++++++++++++++++++----------------
 1 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/test/dump-restore b/test/dump-restore
index de85693..502fb82 100755
--- a/test/dump-restore
+++ b/test/dump-restore
@@ -4,9 +4,31 @@ test_description="\"notmuch dump\" and \"notmuch restore\""
 
 add_email_corpus
 
-test_expect_success "Dumping all tags" "generate_message &&
-notmuch new &&
-notmuch dump > dump.expected"
+test_expect_success 'Dumping all tags' \
+  'generate_message &&
+  notmuch new &&
+  notmuch dump > dump.expected'
+
+test_expect_success 'Clearing all tags' \
+  'sed -e "s/(\([^(]*\))$/()/" < dump.expected > clear.expected &&
+  notmuch restore clear.expected &&
+  notmuch dump > clear.actual &&
+  test_cmp clear.expected clear.actual'
+
+test_expect_success 'Restoring original tags' \
+  'notmuch restore dump.expected &&
+  notmuch dump > dump.actual &&
+  test_cmp dump.expected dump.actual'
+
+test_expect_success 'Restore with nothing to do' \
+  'notmuch restore < dump.expected &&
+  notmuch dump > dump.actual &&
+  test_cmp dump.expected dump.actual'
+
+# notmuch restore currently only considers the first argument.
+test_subtest_known_broken
+test_expect_success 'Invalid restore invocation' \
+  'test_must_fail notmuch restore dump.expected another_one'
 
 test_begin_subtest "dump outfile"
 notmuch dump dump-outfile.actual
@@ -37,17 +59,4 @@ test_begin_subtest "dump outfile -- from:cworth"
 notmuch dump dump-outfile-dash-inbox.actual -- from:cworth
 test_expect_equal_file dump-cworth.expected dump-outfile-dash-inbox.actual
 
-test_begin_subtest "Clearing all tags"
-sed -e "s/(\([^(]*\))$/()/" < dump.expected > clear.expected
-notmuch restore < clear.expected
-notmuch dump > clear.actual
-test_expect_equal "$(< clear.actual)" "$(< clear.expected)"
-
-test_begin_subtest "Restoring original tags"
-notmuch restore < dump.expected
-notmuch dump > dump.actual
-test_expect_equal "$(< dump.actual)" "$(< dump.expected)"
-
-test_expect_success "Restore with nothing to do" "notmuch restore dump.expected"
-
 test_done
-- 
1.7.5.4

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

* [PATCH 3/6] test/dump-restore: add tests for restore --accumulate
  2011-10-21 19:47     ` Updated v3 patches for " David Bremner
  2011-10-21 19:47       ` [PATCH 1/6] test/test-lib.sh: update comments David Bremner
  2011-10-21 19:47       ` [PATCH 2/6] test/dump-restore: expand test suite for dump-restore, make more robust David Bremner
@ 2011-10-21 19:47       ` David Bremner
  2011-10-21 19:47       ` [PATCH 4/6] notmuch-restore: implement --accumulate option David Bremner
                         ` (3 subsequent siblings)
  6 siblings, 0 replies; 22+ messages in thread
From: David Bremner @ 2011-10-21 19:47 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

From: David Bremner <bremner@debian.org>

Flesh out what ``notmuch restore --accumulate'' is supposed to do.
Its tests are currently XFAILed; the functionality will be added in
future patch(es).

Based on a patch by Thomas Schwinge:

      id:"1317317811-29540-1-git-send-email-thomas@schwinge.name"
---
 test/dump-restore |   27 +++++++++++++++++++++++++++
 1 files changed, 27 insertions(+), 0 deletions(-)

diff --git a/test/dump-restore b/test/dump-restore
index 502fb82..276dd6f 100755
--- a/test/dump-restore
+++ b/test/dump-restore
@@ -9,12 +9,27 @@ test_expect_success 'Dumping all tags' \
   notmuch new &&
   notmuch dump > dump.expected'
 
+# The use of from:cworth is rather arbitrary: it matches some of the
+# email corpus' messages, but not all of them.
+
+test_expect_success 'Dumping all tags II' \
+  'notmuch tag +ABC +DEF -- from:cworth &&
+  notmuch dump > dump-ABC_DEF.expected &&
+  ! cmp dump.expected dump-ABC_DEF.expected'
+
 test_expect_success 'Clearing all tags' \
   'sed -e "s/(\([^(]*\))$/()/" < dump.expected > clear.expected &&
   notmuch restore clear.expected &&
   notmuch dump > clear.actual &&
   test_cmp clear.expected clear.actual'
 
+test_subtest_known_broken	# missing --accumuluate
+test_expect_success 'Accumulate original tags' \
+  'notmuch tag +ABC +DEF -- from:cworth &&
+  notmuch restore --accumulate < dump.expected &&
+  notmuch dump > dump.actual &&
+  test_cmp dump-ABC_DEF.expected dump.actual'
+
 test_expect_success 'Restoring original tags' \
   'notmuch restore dump.expected &&
   notmuch dump > dump.actual &&
@@ -25,6 +40,18 @@ test_expect_success 'Restore with nothing to do' \
   notmuch dump > dump.actual &&
   test_cmp dump.expected dump.actual'
 
+test_subtest_known_broken	# missing --accumuluate
+test_expect_success 'Restore with nothing to do, II' \
+  'notmuch restore --accumulate dump.expected &&
+  notmuch dump > dump.actual &&
+  test_cmp dump.expected dump.actual'
+
+test_subtest_known_broken	# missing --accumuluate
+test_expect_success 'Restore with nothing to do, III' \
+  'notmuch restore --accumulate < clear.expected &&
+  notmuch dump > dump.actual &&
+  test_cmp dump.expected dump.actual'
+
 # notmuch restore currently only considers the first argument.
 test_subtest_known_broken
 test_expect_success 'Invalid restore invocation' \
-- 
1.7.5.4

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

* [PATCH 4/6] notmuch-restore: implement --accumulate option
  2011-10-21 19:47     ` Updated v3 patches for " David Bremner
                         ` (2 preceding siblings ...)
  2011-10-21 19:47       ` [PATCH 3/6] test/dump-restore: add tests for restore --accumulate David Bremner
@ 2011-10-21 19:47       ` David Bremner
  2011-10-21 19:47       ` [PATCH 5/6] test/dump-restore: Fix quoting on grep David Bremner
                         ` (2 subsequent siblings)
  6 siblings, 0 replies; 22+ messages in thread
From: David Bremner @ 2011-10-21 19:47 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

From: David Bremner <bremner@debian.org>

Modify command line argument handling to take a --accumulate flag.
Test for extra arguments beyond the input file.

The --accumulate switch causes the union of the existing and new tags to be
applied, instead of replacing each message's tags as they are read in from the
dump file.

Based on a patch by Thomas Schwinge:

      id:"1317317857-29636-1-git-send-email-thomas@schwinge.name"
---
 notmuch-restore.c |   45 ++++++++++++++++++++++++++++++++++++---------
 test/dump-restore |    3 ---
 2 files changed, 36 insertions(+), 12 deletions(-)

diff --git a/notmuch-restore.c b/notmuch-restore.c
index f530bb4..097ca4b 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -18,6 +18,8 @@
  * Author: Carl Worth <cworth@cworth.org>
  */
 
+#include <getopt.h>
+
 #include "notmuch-client.h"
 
 int
@@ -26,7 +28,8 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[])
     notmuch_config_t *config;
     notmuch_database_t *notmuch;
     notmuch_bool_t synchronize_flags;
-    FILE *input;
+    notmuch_bool_t accumulate = FALSE;
+    FILE *input = stdin;
     char *line = NULL;
     size_t line_size;
     ssize_t line_len;
@@ -44,18 +47,33 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[])
 
     synchronize_flags = notmuch_config_get_maildir_synchronize_flags (config);
 
-    argc--; argv++; /* skip subcommand argument */
+    struct option options[] = {
+	{ "accumulate",   no_argument,       0, 'a' },
+	{ 0, 0, 0, 0}
+    };
+
+    int opt;
+    do {
+	opt = getopt_long (argc, argv, "", options, NULL);
+
+	switch (opt) {
+	case 'a':
+	    accumulate = 1;
+	    break;
+	case '?':
+	    break;
+	}
+
+    } while (opt != -1);
 
-    if (argc) {
-	input = fopen (argv[0], "r");
+    if (optind < argc) {
+	input = fopen (argv[optind], "r");
 	if (input == NULL) {
 	    fprintf (stderr, "Error opening %s for reading: %s\n",
-		     argv[0], strerror (errno));
+		     argv[optind], strerror (errno));
 	    return 1;
 	}
-    } else {
-	printf ("No filename given. Reading dump from stdin.\n");
-	input = stdin;
+	optind++;
     }
 
     /* Dump output is one line per message. We match a sequence of
@@ -99,6 +117,13 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[])
 	    goto NEXT_LINE;
 	}
 
+	/* In order to detect missing messages, this check/optimization is
+	 * intentionally done *after* first finding the message.  */
+	if (accumulate && (file_tags == NULL || *file_tags == '\0'))
+	{
+	    goto NEXT_LINE;
+	}
+
 	db_tags_str = NULL;
 	for (db_tags = notmuch_message_get_tags (message);
 	     notmuch_tags_valid (db_tags);
@@ -120,7 +145,9 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[])
 	}
 
 	notmuch_message_freeze (message);
-	notmuch_message_remove_all_tags (message);
+
+	if (!accumulate)
+	    notmuch_message_remove_all_tags (message);
 
 	next = file_tags;
 	while (next) {
diff --git a/test/dump-restore b/test/dump-restore
index 276dd6f..2f8dcc7 100755
--- a/test/dump-restore
+++ b/test/dump-restore
@@ -23,7 +23,6 @@ test_expect_success 'Clearing all tags' \
   notmuch dump > clear.actual &&
   test_cmp clear.expected clear.actual'
 
-test_subtest_known_broken	# missing --accumuluate
 test_expect_success 'Accumulate original tags' \
   'notmuch tag +ABC +DEF -- from:cworth &&
   notmuch restore --accumulate < dump.expected &&
@@ -40,13 +39,11 @@ test_expect_success 'Restore with nothing to do' \
   notmuch dump > dump.actual &&
   test_cmp dump.expected dump.actual'
 
-test_subtest_known_broken	# missing --accumuluate
 test_expect_success 'Restore with nothing to do, II' \
   'notmuch restore --accumulate dump.expected &&
   notmuch dump > dump.actual &&
   test_cmp dump.expected dump.actual'
 
-test_subtest_known_broken	# missing --accumuluate
 test_expect_success 'Restore with nothing to do, III' \
   'notmuch restore --accumulate < clear.expected &&
   notmuch dump > dump.actual &&
-- 
1.7.5.4

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

* [PATCH 5/6] test/dump-restore: Fix quoting on grep
  2011-10-21 19:47     ` Updated v3 patches for " David Bremner
                         ` (3 preceding siblings ...)
  2011-10-21 19:47       ` [PATCH 4/6] notmuch-restore: implement --accumulate option David Bremner
@ 2011-10-21 19:47       ` David Bremner
  2011-10-21 19:47       ` [PATCH 6/6] notmuch-restore: check for extra arguments David Bremner
  2011-10-23 19:50       ` Updated v3 patches for notmuch " David Bremner
  6 siblings, 0 replies; 22+ messages in thread
From: David Bremner @ 2011-10-21 19:47 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

From: David Bremner <bremner@debian.org>

Thanks to Thomas Schwinge for noticing yet another place where quoting
matters. Since the shell translates \. to ., the regex passed to grep
is too generous without the quotes.
---
 test/dump-restore |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/test/dump-restore b/test/dump-restore
index 2f8dcc7..a6d6d65 100755
--- a/test/dump-restore
+++ b/test/dump-restore
@@ -69,7 +69,7 @@ test_expect_equal_file dump.expected dump-1-arg-dash.actual
 # Note, we assume all messages from cworth have a message-id
 # containing cworth.org
 
-grep cworth\.org dump.expected > dump-cworth.expected
+grep 'cworth\.org' dump.expected > dump-cworth.expected
 
 test_begin_subtest "dump -- from:cworth"
 notmuch dump -- from:cworth > dump-dash-cworth.actual
-- 
1.7.5.4

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

* [PATCH 6/6] notmuch-restore: check for extra arguments.
  2011-10-21 19:47     ` Updated v3 patches for " David Bremner
                         ` (4 preceding siblings ...)
  2011-10-21 19:47       ` [PATCH 5/6] test/dump-restore: Fix quoting on grep David Bremner
@ 2011-10-21 19:47       ` David Bremner
  2011-10-21 22:21         ` [PATCH 1/2] notmuch.1: typo fixes new wording for dump/restore David Bremner
  2011-10-23 19:50       ` Updated v3 patches for notmuch " David Bremner
  6 siblings, 1 reply; 22+ messages in thread
From: David Bremner @ 2011-10-21 19:47 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

From: David Bremner <bremner@debian.org>

We consider it an error to pass more than one file to restore, since
extra ones are ignored.
---
 notmuch-restore.c |    7 +++++++
 test/dump-restore |    1 -
 2 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/notmuch-restore.c b/notmuch-restore.c
index 097ca4b..c388c23 100644
--- a/notmuch-restore.c
+++ b/notmuch-restore.c
@@ -76,6 +76,13 @@ notmuch_restore_command (unused (void *ctx), int argc, char *argv[])
 	optind++;
     }
 
+    if (optind < argc) {
+	fprintf (stderr,
+	 "Cannot read dump from more than one file: %s\n",
+		 argv[optind]);
+	return 1;
+    }
+
     /* Dump output is one line per message. We match a sequence of
      * non-space characters for the message-id, then one or more
      * spaces, then a list of space-separated tags as a sequence of
diff --git a/test/dump-restore b/test/dump-restore
index a6d6d65..d4b424e 100755
--- a/test/dump-restore
+++ b/test/dump-restore
@@ -50,7 +50,6 @@ test_expect_success 'Restore with nothing to do, III' \
   test_cmp dump.expected dump.actual'
 
 # notmuch restore currently only considers the first argument.
-test_subtest_known_broken
 test_expect_success 'Invalid restore invocation' \
   'test_must_fail notmuch restore dump.expected another_one'
 
-- 
1.7.5.4

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

* [PATCH 1/2] notmuch.1: typo fixes new wording for dump/restore
  2011-10-21 19:47       ` [PATCH 6/6] notmuch-restore: check for extra arguments David Bremner
@ 2011-10-21 22:21         ` David Bremner
  2011-10-21 22:21           ` [PATCH 2/2] docs: Update news, man page, and online help for restore --accumulate David Bremner
  0 siblings, 1 reply; 22+ messages in thread
From: David Bremner @ 2011-10-21 22:21 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

From: David Bremner <bremner@debian.org>

These changes were included in Thomas's restore --accumulate patch,
but are actually more generally applicable.
---
 notmuch.1 |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/notmuch.1 b/notmuch.1
index 10ed32e..4f864a8 100644
--- a/notmuch.1
+++ b/notmuch.1
@@ -454,7 +454,7 @@ section below for details of the supported syntax for <search-terms>.
 The
 .BR dump " and " restore
 commands can be used to create a textual dump of email tags for backup
-purposes, and to restore from that dump
+purposes, and to restore from that dump.
 
 .RS 4
 .TP 4
@@ -483,7 +483,9 @@ section below for details of the supported syntax for <search-terms>.
 .BR restore " <filename>"
 
 Restores the tags from the given file (see
-.BR "notmuch dump" "."
+.BR "notmuch dump" ")."
+
+The input is read from the given filename, if any, or from stdin.
 
 Note: The dump file format is specifically chosen to be
 compatible with the format of files produced by sup-dump.
-- 
1.7.6.3

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

* [PATCH 2/2] docs: Update news, man page, and online help for restore --accumulate
  2011-10-21 22:21         ` [PATCH 1/2] notmuch.1: typo fixes new wording for dump/restore David Bremner
@ 2011-10-21 22:21           ` David Bremner
  0 siblings, 0 replies; 22+ messages in thread
From: David Bremner @ 2011-10-21 22:21 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

From: David Bremner <bremner@debian.org>

As a side effect, reformat the NEWs entry for notmuch dump for
consistency with the notmuch restore NEWS submitted by Thomas
Schwinge.
---
 NEWS      |   19 ++++++++++++++-----
 notmuch.1 |    6 +++++-
 notmuch.c |    8 ++++++--
 3 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/NEWS b/NEWS
index d4f02d7..e00452a 100644
--- a/NEWS
+++ b/NEWS
@@ -1,11 +1,20 @@
 Notmuch 0.10 (2011-xx-xx)
+=========================
 
-notmuch dump changes 
---------------------
+New command-line features
+-------------------------
+
+Add "notmuch restore --accumulate" option
+
+  The --accumulate switch causes the union of the existing and new tags to be
+  applied, instead of replacing each message's tags as they are read in from
+  the dump file.
+
+Add search terms to  "notmuch dump"
 
-The dump command now takes an optional search term much like notmuch
-search/show/tag. The output file argument of dump is deprecated in
-favour of using stdout.
+  The dump command now takes an optional search term much like notmuch
+  search/show/tag. The output file argument of dump is deprecated in
+  favour of using stdout.
 
 Notmuch 0.9 (2011-10-01)
 ========================
diff --git a/notmuch.1 b/notmuch.1
index 4f864a8..bba479e 100644
--- a/notmuch.1
+++ b/notmuch.1
@@ -480,7 +480,7 @@ section below for details of the supported syntax for <search-terms>.
 .RE
 
 .TP
-.BR restore " <filename>"
+.BR restore " [--accumulate] [<filename>]"
 
 Restores the tags from the given file (see
 .BR "notmuch dump" ")."
@@ -493,6 +493,10 @@ So if you've previously been using sup for mail, then the
 .B "notmuch restore"
 command provides you a way to import all of your tags (or labels as
 sup calls them).
+
+The --accumulate switch causes the union of the existing and new tags to be
+applied, instead of replacing each message's tags as they are read in from the
+dump file.
 .RE
 
 The
diff --git a/notmuch.c b/notmuch.c
index 9c77349..309d750 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -393,13 +393,17 @@ static command_t commands[] = {
       "\tSee \"notmuch help search-terms\" for the search-term syntax.\n"      
  },
     { "restore", notmuch_restore_command,
-      "<filename>",
+      "[--accumulate] [<filename>]",
       "Restore the tags from the given dump file (see 'dump').",
+      "\tInput is read from the given filename, if any, or from stdin.\n"
       "\tNote: The dump file format is specifically chosen to be\n"
       "\tcompatible with the format of files produced by sup-dump.\n"
       "\tSo if you've previously been using sup for mail, then the\n"
       "\t\"notmuch restore\" command provides you a way to import\n"
-      "\tall of your tags (or labels as sup calls them)." },
+      "\tall of your tags (or labels as sup calls them).\n"
+      "\tThe --accumulate switch causes the union of the existing and new\n"
+      "\ttags to be applied, instead of replacing each message's tags as\n"
+      "\tthey are read in from the dump file."},
     { "config", notmuch_config_command,
       "[get|set] <section>.<item> [value ...]",
       "Get or set settings in the notmuch configuration file.",
-- 
1.7.6.3

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

* Re: Updated v3 patches for notmuch restore --accumulate.
  2011-10-21 19:47     ` Updated v3 patches for " David Bremner
                         ` (5 preceding siblings ...)
  2011-10-21 19:47       ` [PATCH 6/6] notmuch-restore: check for extra arguments David Bremner
@ 2011-10-23 19:50       ` David Bremner
  6 siblings, 0 replies; 22+ messages in thread
From: David Bremner @ 2011-10-23 19:50 UTC (permalink / raw)
  To: notmuch

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

On Fri, 21 Oct 2011 16:47:51 -0300, David Bremner <david@tethera.net> wrote:
> These are rebased quite violently from what Thomas posted.  They are
> also the first use of getopt (because I plan to add another option to
> restore). They depend on id:"1319199557-16888-1-git-send-email-david@tethera.net".

I pushed a (reordered) version of this series. 

Note that notmuch will currently not build on any system lacking
getopt_long.  If/when this is a problem for someone, we can ship either
the GLIBC or the BSD version.

d

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

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

end of thread, other threads:[~2011-10-23 19:51 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-05 19:07 [PATCH] notmuch restore --accumulate Thomas Schwinge
2011-09-05 19:31 ` Jameson Graef Rollins
2011-09-09  9:06   ` Louis Rilling
2011-09-09  9:45     ` Thomas Schwinge
2011-09-09 16:13       ` Austin Clements
2011-09-09 17:22         ` Thomas Schwinge
2011-09-09 17:35           ` Austin Clements
2011-09-09 18:58           ` Jameson Graef Rollins
2011-09-29 18:24             ` [PATCH] patchformatting: Test Suite Enhancements Thomas Schwinge
2011-09-29 17:36 ` [PATCH, v2] notmuch restore --accumulate Thomas Schwinge
2011-09-29 17:37   ` Thomas Schwinge
2011-10-21 19:47     ` Updated v3 patches for " David Bremner
2011-10-21 19:47       ` [PATCH 1/6] test/test-lib.sh: update comments David Bremner
2011-10-21 19:47       ` [PATCH 2/6] test/dump-restore: expand test suite for dump-restore, make more robust David Bremner
2011-10-21 19:47       ` [PATCH 3/6] test/dump-restore: add tests for restore --accumulate David Bremner
2011-10-21 19:47       ` [PATCH 4/6] notmuch-restore: implement --accumulate option David Bremner
2011-10-21 19:47       ` [PATCH 5/6] test/dump-restore: Fix quoting on grep David Bremner
2011-10-21 19:47       ` [PATCH 6/6] notmuch-restore: check for extra arguments David Bremner
2011-10-21 22:21         ` [PATCH 1/2] notmuch.1: typo fixes new wording for dump/restore David Bremner
2011-10-21 22:21           ` [PATCH 2/2] docs: Update news, man page, and online help for restore --accumulate David Bremner
2011-10-23 19:50       ` Updated v3 patches for notmuch " David Bremner
2011-10-16 21:48   ` [PATCH, v2] " David Bremner

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).