[PATCH, v2] notmuch restore --accumulate

2011-10-16 Thread David Bremner
On Thu, 29 Sep 2011 19:36:51 +0200, Thomas Schwinge  
wrote:
> From: Thomas Schwinge 
> 
> 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=  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


Re: [PATCH, v2] notmuch restore --accumulate

2011-10-16 Thread David Bremner
On Thu, 29 Sep 2011 19:36:51 +0200, Thomas Schwinge  
wrote:
> From: Thomas Schwinge 
> 
> 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=  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
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch


[PATCH, v2] notmuch restore --accumulate

2011-09-29 Thread Thomas Schwinge
From: Thomas Schwinge 

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 
.
 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 " "
+.BR restore " [--accumulate] []"

 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 spec

[PATCH, v2] notmuch restore --accumulate

2011-09-29 Thread Thomas Schwinge
From: Thomas Schwinge 

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)


[PATCH, v2] notmuch restore --accumulate

2011-09-29 Thread Thomas Schwinge
From: Thomas Schwinge 

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 
.
 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 " "
+.BR restore " [--accumulate] []"
 
 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 

[PATCH, v2] notmuch restore --accumulate

2011-09-29 Thread Thomas Schwinge
From: Thomas Schwinge 

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)
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch