WIP: add metadata to dump output

WIP: add metadata to dump output

[David Bremner]

It seems (at least to me) that xapian metadata is the right way store
certain configuration data, including tag aliases [1] and perhaps some
non-CLI specific configuration. On the other hand we don't want to
have things lost if we dump and restore a database. Hence this series,
which is a start at dumping and restore such config.

The main idea here is that various classes of metadata can be defined
by using prefixes, in exactly the same way as tags are defined for
documents. This will hopefully help prevent e.g. config from stomping
on tag aliases.

The first 6 patches impliment iterators for simple "queries" on
metadata. They are probably split a bit fine, but that's the way I
developed them.

The last 3 impliment the printing of metadata in dump output. In order
to be upwardly compatible, it uses the old dodge of hiding things in
comments. In fact the comment syntax (# in first column) was never
well documented; this does mean that the notmuch dump output can be
tested without breaking the current restore tests. I threw an @ in to
help autodetection of formats; obviously this is not foolproof. On the
other hand, I don't know how much people currently rely on comments in
dump files, since notmuch doesn't generate them.

There's lots of bikes to shed here. Probably the most important bits
are the library API, the dump output format, and of course the ever
tricky command line argument names.

Getting the memory ownership semantics is tricky, especially with the
mix of C++ objects and talloc. So I'd appreciate a critical eye on
those bits of metadata.cc.

[1]: id:1448504191-30974-1-git-send-email-igor.contato@gmail.com

[WIP patch 1/9] lib: initial API for prefixed metadata

[David Bremner]

Start with get and set of a single key
---
 lib/Makefile.local    |   1 +
 lib/metadata.cc       | 150 ++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/notmuch.h         |  25 +++++++++
 test/T590-metadata.sh |  58 +++++++++++++++++++
 4 files changed, 234 insertions(+)
 create mode 100644 lib/metadata.cc
 create mode 100755 test/T590-metadata.sh

diff --git a/lib/Makefile.local b/lib/Makefile.local
index 3a07090..ccc1e49 100644
--- a/lib/Makefile.local
+++ b/lib/Makefile.local
@@ -48,6 +48,7 @@ libnotmuch_cxx_srcs =		\
 	$(dir)/index.cc		\
 	$(dir)/message.cc	\
 	$(dir)/query.cc		\
+	$(dir)/metadata.cc	\
 	$(dir)/thread.cc
 
 libnotmuch_modules := $(libnotmuch_c_srcs:.c=.o) $(libnotmuch_cxx_srcs:.cc=.o)
diff --git a/lib/metadata.cc b/lib/metadata.cc
new file mode 100644
index 0000000..a068ed1
--- /dev/null
+++ b/lib/metadata.cc
@@ -0,0 +1,150 @@
+/* metadata.cc - API for database metadata
+ *
+ * Copyright © 2015 David Bremner
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see http://www.gnu.org/licenses/ .
+ *
+ * Author: David Bremner <david@tethera.net>
+ */
+
+#include "notmuch.h"
+#include "notmuch-private.h"
+#include "database-private.h"
+
+static
+const char *
+_find_metadata_prefix (notmuch_metadata_class_t mclass)
+{
+    switch (mclass) {
+    case NOTMUCH_METADATA_CONFIG:
+	return "C";
+    default:
+	return NULL;
+    }
+}
+
+notmuch_status_t _make_key(void *ctx, notmuch_metadata_class_t mclass,
+			    const char *in, const char **out)
+{
+    const char *term;
+    const char *prefix = NULL;
+
+    if (!out)
+	return NOTMUCH_STATUS_NULL_POINTER;
+
+    prefix = _find_metadata_prefix(mclass);
+    if (!prefix)
+	return NOTMUCH_STATUS_UNSUPPORTED_OPERATION;
+
+    term = talloc_asprintf (ctx, "%s%s",
+			    prefix, in);
+    if (!term)
+	return NOTMUCH_STATUS_OUT_OF_MEMORY;
+
+    *out = term;
+    return NOTMUCH_STATUS_SUCCESS;
+}
+
+notmuch_status_t
+notmuch_database_set_metadata (notmuch_database_t *notmuch,
+			       notmuch_metadata_class_t mclass,
+			       const char *key,
+			       const char *value)
+{
+    notmuch_status_t status;
+    Xapian::WritableDatabase *db;
+    const char *key_term = NULL;
+    void *local;
+
+    local = talloc_new (NULL);
+
+    status = _notmuch_database_ensure_writable (notmuch);
+    if (status)
+	goto DONE;
+
+    status = _make_key (local, mclass, key, &key_term);
+    if (status)
+	goto DONE;
+
+    try {
+	db = static_cast <Xapian::WritableDatabase *> (notmuch->xapian_db);
+	db->set_metadata (key_term, value);
+    } catch (const Xapian::Error &error) {
+	status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
+	notmuch->exception_reported = TRUE;
+	if (! notmuch->exception_reported) {
+	    _notmuch_database_log (notmuch, "Error: A Xapian exception occurred setting metadata: %s\n",
+				   error.get_msg().c_str());
+	}
+    }
+ DONE:
+    talloc_free (local);
+
+    return status;
+}
+
+static notmuch_status_t
+_metadata_value (notmuch_database_t *notmuch,
+		 notmuch_metadata_class_t mclass,
+		 const char *key,
+		 std::string &value)
+{
+    notmuch_status_t status;
+
+    const char *key_term = NULL;
+    void *local;
+
+    local = talloc_new (NULL);
+
+    status = _make_key (local, mclass, key, &key_term);
+    if (status)
+	goto DONE;
+
+    try {
+
+	value = notmuch->xapian_db->get_metadata (key_term);
+
+    } catch (const Xapian::Error &error) {
+	status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
+	notmuch->exception_reported = TRUE;
+	if (! notmuch->exception_reported) {
+	    _notmuch_database_log (notmuch, "Error: A Xapian exception occurred getting metadata: %s\n",
+				   error.get_msg().c_str());
+	}
+    }
+
+ DONE:
+    talloc_free (local);
+    return status;
+}
+
+notmuch_status_t
+notmuch_database_get_metadata (notmuch_database_t *notmuch,
+			       notmuch_metadata_class_t mclass,
+			       const char *key,
+			       char **value) {
+    std::string strval;
+    notmuch_status_t status;
+
+    if (!value)
+	return NOTMUCH_STATUS_NULL_POINTER;
+
+    status = _metadata_value (notmuch, mclass, key, strval);
+    if (status)
+	return status;
+
+    *value = strdup (strval.c_str ());
+
+    return NOTMUCH_STATUS_SUCCESS;
+}
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 310a8b8..448f405 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -1829,6 +1829,31 @@ notmuch_filenames_move_to_next (notmuch_filenames_t *filenames);
 void
 notmuch_filenames_destroy (notmuch_filenames_t *filenames);
 
+/**
+ * metadata class
+ */
+
+typedef enum _notmuch_metadata_class {
+    NOTMUCH_METADATA_FIRST_CLASS = 1,
+    NOTMUCH_METADATA_CONFIG = 1,
+    NOTMUCH_METADATA_LAST_CLASS
+} notmuch_metadata_class_t;
+
+/**
+ * set metadata
+ *
+ */
+notmuch_status_t
+notmuch_database_set_metadata (notmuch_database_t *db, notmuch_metadata_class_t mclass, const char *key, const char *value);
+
+/**
+ * retrieve one metadata value
+ *
+ * return value is allocated by malloc and should be freed by the caller.
+ */
+notmuch_status_t
+notmuch_database_get_metadata (notmuch_database_t *db, notmuch_metadata_class_t mclass, const char *key, char **value);
+
 /* @} */
 
 NOTMUCH_END_DECLS
diff --git a/test/T590-metadata.sh b/test/T590-metadata.sh
new file mode 100755
index 0000000..29aeaa2
--- /dev/null
+++ b/test/T590-metadata.sh
@@ -0,0 +1,58 @@
+#!/usr/bin/env bash
+test_description="metadata API"
+
+. ./test-lib.sh || exit 1
+
+add_email_corpus
+
+cat <<EOF > c_head
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+#include <notmuch.h>
+
+void run(int line, notmuch_status_t ret)
+{
+   if (ret) {
+	fprintf (stderr, "line %d: %s\n", line, ret);
+	exit (1);
+   }
+}
+
+#define RUN(v)  run(__LINE__, v);
+
+int main (int argc, char** argv)
+{
+   notmuch_database_t *db;
+   char *val;
+   notmuch_status_t stat;
+
+   RUN(notmuch_database_open (argv[1], NOTMUCH_DATABASE_MODE_READ_WRITE, &db));
+
+EOF
+
+cat <<EOF > c_tail
+   RUN(notmuch_database_destroy(db));
+}
+EOF
+
+test_begin_subtest "notmuch_database_{set,get}_metadata"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+{
+   RUN(notmuch_database_set_metadata (db, NOTMUCH_METADATA_CONFIG, "testkey1", "testvalue1"));
+   RUN(notmuch_database_set_metadata (db, NOTMUCH_METADATA_CONFIG, "testkey2", "testvalue2"));
+   RUN(notmuch_database_get_metadata (db, NOTMUCH_METADATA_CONFIG, "testkey1", &val));
+   printf("testkey1 = %s\n", val);
+   RUN(notmuch_database_get_metadata (db, NOTMUCH_METADATA_CONFIG, "testkey2", &val));
+   printf("testkey2 = %s\n", val);
+}
+EOF
+cat <<'EOF' >EXPECTED
+== stdout ==
+testkey1 = testvalue1
+testkey2 = testvalue2
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_done
-- 
2.6.4

[WIP patch 2/9] lib: notmuch_metadata_t: iterators for metadata

[David Bremner]

The rough idea is to give a C interface to the Xapian::TermIterator class.
---
 lib/metadata.cc       | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/notmuch.h         | 12 ++++++++++
 test/T590-metadata.sh | 15 ++++++++++++
 3 files changed, 90 insertions(+)

diff --git a/lib/metadata.cc b/lib/metadata.cc
index a068ed1..5d319c5 100644
--- a/lib/metadata.cc
+++ b/lib/metadata.cc
@@ -22,6 +22,17 @@
 #include "notmuch-private.h"
 #include "database-private.h"
 
+struct _notmuch_metadata {
+    notmuch_database_t *notmuch;
+    Xapian::TermIterator *iterator;
+    notmuch_metadata_class_t mclass;
+};
+
+static int
+_notmuch_metadata_destroy (notmuch_metadata_t *list) {
+    delete list->iterator;
+    return 0;
+}
 static
 const char *
 _find_metadata_prefix (notmuch_metadata_class_t mclass)
@@ -148,3 +159,55 @@ notmuch_database_get_metadata (notmuch_database_t *notmuch,
 
     return NOTMUCH_STATUS_SUCCESS;
 }
+
+notmuch_status_t
+notmuch_database_get_all_metadata (notmuch_database_t *notmuch,
+				   notmuch_metadata_class_t mclass,
+				   notmuch_metadata_t **out)
+{
+    notmuch_metadata_t *list = NULL;
+    notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
+
+    const char *prefix = _find_metadata_prefix (mclass);
+
+    list = talloc (notmuch, notmuch_metadata_t);
+    if (!list) {
+	status = NOTMUCH_STATUS_OUT_OF_MEMORY;
+	goto DONE;
+    }
+
+    talloc_set_destructor(list, _notmuch_metadata_destroy);
+    list->iterator = new Xapian::TermIterator;
+    list->notmuch = notmuch;
+    list->mclass = mclass;
+
+    try {
+
+	*list->iterator = notmuch->xapian_db->metadata_keys_begin();
+	list->iterator->skip_to (prefix);
+
+    } catch (const Xapian::Error &error) {
+	_notmuch_database_log (notmuch, "A Xapian exception occurred getting metadata iterator: %s.\n",
+			       error.get_msg().c_str());
+	notmuch->exception_reported = TRUE;
+	status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
+    }
+
+    *out = list;
+
+ DONE:
+    if (status && list)
+	talloc_free (list);
+
+    return status;
+}
+
+notmuch_bool_t
+notmuch_metadata_valid (notmuch_metadata_t *metadata)
+{
+    const char *prefix = _find_metadata_prefix (metadata->mclass);
+    if (*(metadata->iterator) == metadata->notmuch->xapian_db->metadata_keys_end())
+	return FALSE;
+
+    return (strncmp((**(metadata->iterator)).c_str (), prefix, strlen (prefix)) == 0);
+}
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 448f405..a2b7040 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -197,6 +197,7 @@ typedef struct _notmuch_message notmuch_message_t;
 typedef struct _notmuch_tags notmuch_tags_t;
 typedef struct _notmuch_directory notmuch_directory_t;
 typedef struct _notmuch_filenames notmuch_filenames_t;
+typedef struct _notmuch_metadata notmuch_metadata_t;
 #endif /* __DOXYGEN__ */
 
 /**
@@ -1854,6 +1855,17 @@ notmuch_database_set_metadata (notmuch_database_t *db, notmuch_metadata_class_t
 notmuch_status_t
 notmuch_database_get_metadata (notmuch_database_t *db, notmuch_metadata_class_t mclass, const char *key, char **value);
 
+/**
+ * get all metadata of a given class
+ */
+notmuch_status_t
+notmuch_database_get_all_metadata (notmuch_database_t *db, notmuch_metadata_class_t mclass, notmuch_metadata_t **out);
+
+/**
+ * Is 'metadata' iterator valid (i.e. _key, _value, _move_to_next can be called).
+ */
+notmuch_bool_t
+notmuch_metadata_valid (notmuch_metadata_t *metadata);
 /* @} */
 
 NOTMUCH_END_DECLS
diff --git a/test/T590-metadata.sh b/test/T590-metadata.sh
index 29aeaa2..c36a7d7 100755
--- a/test/T590-metadata.sh
+++ b/test/T590-metadata.sh
@@ -55,4 +55,19 @@ testkey2 = testvalue2
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "notmuch_database_get_all_metadata initially valid"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+{
+   notmuch_metadata_t *meta;
+   RUN(notmuch_database_get_all_metadata (db, NOTMUCH_METADATA_CONFIG, &meta));
+   printf("valid = %d\n", notmuch_metadata_valid (meta));
+}
+EOF
+cat <<'EOF' >EXPECTED
+== stdout ==
+valid = 1
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_done
-- 
2.6.4

[WIP patch 3/9] lib: add and test function to retrive current key

[David Bremner]

---
 lib/metadata.cc       | 17 +++++++++++++++++
 lib/notmuch.h         |  6 ++++++
 test/T590-metadata.sh |  4 +++-
 3 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/lib/metadata.cc b/lib/metadata.cc
index 5d319c5..20805e5 100644
--- a/lib/metadata.cc
+++ b/lib/metadata.cc
@@ -26,6 +26,7 @@ struct _notmuch_metadata {
     notmuch_database_t *notmuch;
     Xapian::TermIterator *iterator;
     notmuch_metadata_class_t mclass;
+    char *current_key;
 };
 
 static int
@@ -180,6 +181,7 @@ notmuch_database_get_all_metadata (notmuch_database_t *notmuch,
     list->iterator = new Xapian::TermIterator;
     list->notmuch = notmuch;
     list->mclass = mclass;
+    list->current_key = NULL;
 
     try {
 
@@ -211,3 +213,18 @@ notmuch_metadata_valid (notmuch_metadata_t *metadata)
 
     return (strncmp((**(metadata->iterator)).c_str (), prefix, strlen (prefix)) == 0);
 }
+
+const char *
+notmuch_metadata_key (notmuch_metadata_t *metadata)
+{
+    const char *prefix = _find_metadata_prefix (metadata->mclass);
+
+    if (metadata->current_key)
+	talloc_free (metadata->current_key);
+    metadata->current_key = talloc_strdup (metadata, (**(metadata->iterator)).c_str () + strlen (prefix));
+
+    return  metadata->current_key;
+}
+
+const char *
+}
diff --git a/lib/notmuch.h b/lib/notmuch.h
index a2b7040..16d0973 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -1866,6 +1866,12 @@ notmuch_database_get_all_metadata (notmuch_database_t *db, notmuch_metadata_clas
  */
 notmuch_bool_t
 notmuch_metadata_valid (notmuch_metadata_t *metadata);
+
+/**
+ * return key for current metadata pair
+ */
+const char *
+notmuch_metadata_key (notmuch_metadata_t *metadata);
 /* @} */
 
 NOTMUCH_END_DECLS
diff --git a/test/T590-metadata.sh b/test/T590-metadata.sh
index c36a7d7..2c44746 100755
--- a/test/T590-metadata.sh
+++ b/test/T590-metadata.sh
@@ -55,17 +55,19 @@ testkey2 = testvalue2
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
-test_begin_subtest "notmuch_database_get_all_metadata initially valid"
+test_begin_subtest "notmuch_database_get_all_metadata first item"
 cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
 {
    notmuch_metadata_t *meta;
    RUN(notmuch_database_get_all_metadata (db, NOTMUCH_METADATA_CONFIG, &meta));
    printf("valid = %d\n", notmuch_metadata_valid (meta));
+   printf("key = %s\n", notmuch_metadata_key (meta));
 }
 EOF
 cat <<'EOF' >EXPECTED
 == stdout ==
 valid = 1
+key = testkey1
 == stderr ==
 EOF
 test_expect_equal_file EXPECTED OUTPUT
-- 
2.6.4

[WIP patch 4/9] lib: add and test function to retrieve current metadata value

[David Bremner]

---
 lib/metadata.cc       | 11 +++++++++++
 lib/notmuch.h         |  7 +++++++
 test/T590-metadata.sh |  2 ++
 3 files changed, 20 insertions(+)

diff --git a/lib/metadata.cc b/lib/metadata.cc
index 20805e5..6236992 100644
--- a/lib/metadata.cc
+++ b/lib/metadata.cc
@@ -227,4 +227,15 @@ notmuch_metadata_key (notmuch_metadata_t *metadata)
 }
 
 const char *
+notmuch_metadata_value (notmuch_metadata_t *metadata)
+{
+    const char *key = notmuch_metadata_key (metadata);
+    char *val;
+    notmuch_status_t status;
+
+    status=notmuch_database_get_metadata (metadata->notmuch, metadata->mclass, key, &val);
+    if (status)
+	return NULL;
+
+    return val;
 }
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 16d0973..dd359c8 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -1872,6 +1872,13 @@ notmuch_metadata_valid (notmuch_metadata_t *metadata);
  */
 const char *
 notmuch_metadata_key (notmuch_metadata_t *metadata);
+
+/**
+ * return value for current metadata pair
+ */
+const char *
+notmuch_metadata_value (notmuch_metadata_t *metadata);
+
 /* @} */
 
 NOTMUCH_END_DECLS
diff --git a/test/T590-metadata.sh b/test/T590-metadata.sh
index 2c44746..d2326d4 100755
--- a/test/T590-metadata.sh
+++ b/test/T590-metadata.sh
@@ -62,12 +62,14 @@ cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
    RUN(notmuch_database_get_all_metadata (db, NOTMUCH_METADATA_CONFIG, &meta));
    printf("valid = %d\n", notmuch_metadata_valid (meta));
    printf("key = %s\n", notmuch_metadata_key (meta));
+   printf("val = %s\n", notmuch_metadata_value (meta));
 }
 EOF
 cat <<'EOF' >EXPECTED
 == stdout ==
 valid = 1
 key = testkey1
+val = testvalue1
 == stderr ==
 EOF
 test_expect_equal_file EXPECTED OUTPUT
-- 
2.6.4

[WIP patch 5/9] lib: add notmuch_metadata_move_to_next

[David Bremner]

---
 lib/metadata.cc       |  6 ++++++
 lib/notmuch.h         |  6 ++++++
 test/T590-metadata.sh | 12 +++++++-----
 3 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/lib/metadata.cc b/lib/metadata.cc
index 6236992..a355b75 100644
--- a/lib/metadata.cc
+++ b/lib/metadata.cc
@@ -239,3 +239,9 @@ notmuch_metadata_value (notmuch_metadata_t *metadata)
 
     return val;
 }
+
+void
+notmuch_metadata_move_to_next (notmuch_metadata_t *metadata)
+{
+    (*(metadata->iterator))++;
+}
diff --git a/lib/notmuch.h b/lib/notmuch.h
index dd359c8..b00126d 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -1879,6 +1879,12 @@ notmuch_metadata_key (notmuch_metadata_t *metadata);
 const char *
 notmuch_metadata_value (notmuch_metadata_t *metadata);
 
+/**
+ * move 'metadata' iterator to the next pair
+ */
+void
+notmuch_metadata_move_to_next (notmuch_metadata_t *metadata);
+
 /* @} */
 
 NOTMUCH_END_DECLS
diff --git a/test/T590-metadata.sh b/test/T590-metadata.sh
index d2326d4..4f37c09 100755
--- a/test/T590-metadata.sh
+++ b/test/T590-metadata.sh
@@ -55,21 +55,23 @@ testkey2 = testvalue2
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
-test_begin_subtest "notmuch_database_get_all_metadata first item"
+test_begin_subtest "get all metadata in one class"
 cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
 {
    notmuch_metadata_t *meta;
    RUN(notmuch_database_get_all_metadata (db, NOTMUCH_METADATA_CONFIG, &meta));
-   printf("valid = %d\n", notmuch_metadata_valid (meta));
-   printf("key = %s\n", notmuch_metadata_key (meta));
-   printf("val = %s\n", notmuch_metadata_value (meta));
+   for (; notmuch_metadata_valid (meta); notmuch_metadata_move_to_next (meta)) {
+      printf("key = %s\n", notmuch_metadata_key (meta));
+      printf("val = %s\n", notmuch_metadata_value (meta));
+   }
 }
 EOF
 cat <<'EOF' >EXPECTED
 == stdout ==
-valid = 1
 key = testkey1
 val = testvalue1
+key = testkey2
+val = testvalue2
 == stderr ==
 EOF
 test_expect_equal_file EXPECTED OUTPUT
-- 
2.6.4

[WIP patch 6/9] lib: add notmuch_metadata_destroy

[David Bremner]

---
 lib/metadata.cc       |  6 ++++++
 lib/notmuch.h         | 14 +++++++++++++-
 test/T590-metadata.sh |  1 +
 3 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/lib/metadata.cc b/lib/metadata.cc
index a355b75..79a0a9e 100644
--- a/lib/metadata.cc
+++ b/lib/metadata.cc
@@ -245,3 +245,9 @@ notmuch_metadata_move_to_next (notmuch_metadata_t *metadata)
 {
     (*(metadata->iterator))++;
 }
+
+void
+notmuch_metadata_destroy (notmuch_metadata_t *metadata)
+{
+    talloc_free (metadata);
+}
diff --git a/lib/notmuch.h b/lib/notmuch.h
index b00126d..6773253 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -1869,12 +1869,18 @@ notmuch_metadata_valid (notmuch_metadata_t *metadata);
 
 /**
  * return key for current metadata pair
+ *
+ * return value is owned by the iterator, and will be destroyed by the
+ * next call to notmuch_metadata_key or notmuch_metadata_destroy.
  */
 const char *
 notmuch_metadata_key (notmuch_metadata_t *metadata);
 
 /**
- * return value for current metadata pair
+ * return 'value' for current metadata pair
+ *
+ * return value is owned by the iterator, and will be destroyed by the
+ * next call to notmuch_metadata_value
  */
 const char *
 notmuch_metadata_value (notmuch_metadata_t *metadata);
@@ -1885,6 +1891,12 @@ notmuch_metadata_value (notmuch_metadata_t *metadata);
 void
 notmuch_metadata_move_to_next (notmuch_metadata_t *metadata);
 
+/**
+ * free any resources held by 'metadata'
+ */
+void
+notmuch_metadata_destroy (notmuch_metadata_t * metadata);
+
 /* @} */
 
 NOTMUCH_END_DECLS
diff --git a/test/T590-metadata.sh b/test/T590-metadata.sh
index 4f37c09..4d55298 100755
--- a/test/T590-metadata.sh
+++ b/test/T590-metadata.sh
@@ -64,6 +64,7 @@ cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
       printf("key = %s\n", notmuch_metadata_key (meta));
       printf("val = %s\n", notmuch_metadata_value (meta));
    }
+   notmuch_metadata_destroy (meta);
 }
 EOF
 cat <<'EOF' >EXPECTED
-- 
2.6.4

[WIP patch 7/9] CLI: add print_status_database

[David Bremner]

This could probably be used at quite a few places in the existing code,
but in the immediate future I plan to use in some new code in
notmuch-dump
---
 notmuch-client.h |  5 +++++
 status.c         | 17 +++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/notmuch-client.h b/notmuch-client.h
index 3bd2903..7c9a1ea 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -459,6 +459,11 @@ print_status_query (const char *loc,
 		    const notmuch_query_t *query,
 		    notmuch_status_t status);
 
+notmuch_status_t
+print_status_database (const char *loc,
+		       const notmuch_database_t *database,
+		       notmuch_status_t status);
+
 #include "command-line-arguments.h"
 
 extern char *notmuch_requested_db_uuid;
diff --git a/status.c b/status.c
index 8fa81cb..45d3fb4 100644
--- a/status.c
+++ b/status.c
@@ -19,3 +19,20 @@ print_status_query (const char *loc,
     }
     return status;
 }
+
+notmuch_status_t
+print_status_database (const char *loc,
+		    const notmuch_database_t *notmuch,
+		    notmuch_status_t status)
+{
+    if (status) {
+	const char *msg;
+
+	fprintf (stderr, "%s: %s\n", loc,
+		 notmuch_status_to_string (status));
+	msg = notmuch_database_status_string (notmuch);
+	if (msg)
+	    fputs (msg, stderr);
+    }
+    return status;
+}
-- 
2.6.4

[WIP patch 8/9] lib: make string representation of metadata class public

[David Bremner]

This is needed for e.g. notmuch-dump
---
 lib/metadata.cc | 34 +++++++++++++++++++++++-----------
 lib/notmuch.h   |  5 +++++
 2 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/lib/metadata.cc b/lib/metadata.cc
index 79a0a9e..34b46e0 100644
--- a/lib/metadata.cc
+++ b/lib/metadata.cc
@@ -34,16 +34,28 @@ _notmuch_metadata_destroy (notmuch_metadata_t *list) {
     delete list->iterator;
     return 0;
 }
-static
+
+typedef struct prefix {
+    notmuch_metadata_class_t mclass;
+    const char *prefix;
+} prefix_t;
+
+static prefix_t METADATA_PREFIX[] = {
+    {NOTMUCH_METADATA_CONFIG, "C"},
+};
+
+#define ARRAY_SIZE(arr) (sizeof (arr) / sizeof (arr[0]))
+
 const char *
-_find_metadata_prefix (notmuch_metadata_class_t mclass)
+notmuch_metadata_prefix_string (notmuch_metadata_class_t mclass)
 {
-    switch (mclass) {
-    case NOTMUCH_METADATA_CONFIG:
-	return "C";
-    default:
-	return NULL;
+    unsigned int i;
+
+    for (i=0; i < ARRAY_SIZE(METADATA_PREFIX); i++) {
+	if (METADATA_PREFIX[0].mclass == mclass)
+	    return METADATA_PREFIX[i].prefix;
     }
+    return NULL;
 }
 
 notmuch_status_t _make_key(void *ctx, notmuch_metadata_class_t mclass,
@@ -55,7 +67,7 @@ notmuch_status_t _make_key(void *ctx, notmuch_metadata_class_t mclass,
     if (!out)
 	return NOTMUCH_STATUS_NULL_POINTER;
 
-    prefix = _find_metadata_prefix(mclass);
+    prefix = notmuch_metadata_prefix_string(mclass);
     if (!prefix)
 	return NOTMUCH_STATUS_UNSUPPORTED_OPERATION;
 
@@ -169,7 +181,7 @@ notmuch_database_get_all_metadata (notmuch_database_t *notmuch,
     notmuch_metadata_t *list = NULL;
     notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
 
-    const char *prefix = _find_metadata_prefix (mclass);
+    const char *prefix = notmuch_metadata_prefix_string (mclass);
 
     list = talloc (notmuch, notmuch_metadata_t);
     if (!list) {
@@ -207,7 +219,7 @@ notmuch_database_get_all_metadata (notmuch_database_t *notmuch,
 notmuch_bool_t
 notmuch_metadata_valid (notmuch_metadata_t *metadata)
 {
-    const char *prefix = _find_metadata_prefix (metadata->mclass);
+    const char *prefix = notmuch_metadata_prefix_string (metadata->mclass);
     if (*(metadata->iterator) == metadata->notmuch->xapian_db->metadata_keys_end())
 	return FALSE;
 
@@ -217,7 +229,7 @@ notmuch_metadata_valid (notmuch_metadata_t *metadata)
 const char *
 notmuch_metadata_key (notmuch_metadata_t *metadata)
 {
-    const char *prefix = _find_metadata_prefix (metadata->mclass);
+    const char *prefix = notmuch_metadata_prefix_string (metadata->mclass);
 
     if (metadata->current_key)
 	talloc_free (metadata->current_key);
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 6773253..657a7d6 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -1897,6 +1897,11 @@ notmuch_metadata_move_to_next (notmuch_metadata_t *metadata);
 void
 notmuch_metadata_destroy (notmuch_metadata_t * metadata);
 
+/**
+ * convert enum to string
+ */
+const char *
+notmuch_metadata_prefix_string (notmuch_metadata_class_t mclass);
 /* @} */
 
 NOTMUCH_END_DECLS
-- 
2.6.4

[WIP patch 9/9] CLI: add optional metadata to dump output.

[David Bremner]

This lacks at least documentation. Note that it changes the default dump
output format, but doesn't break existing notmuch-restore. It might
break user scripts though.
---
 notmuch-client.h      |  6 ++++++
 notmuch-dump.c        | 45 ++++++++++++++++++++++++++++++++++++++++++---
 notmuch-new.c         |  2 +-
 test/T590-metadata.sh |  8 ++++++++
 4 files changed, 57 insertions(+), 4 deletions(-)

diff --git a/notmuch-client.h b/notmuch-client.h
index 7c9a1ea..8ee6e0e 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -443,11 +443,17 @@ typedef enum dump_formats {
     DUMP_FORMAT_SUP
 } dump_format_t;
 
+typedef enum dump_includes {
+    DUMP_INCLUDE_TAGS=1,
+    DUMP_INCLUDE_METADATA=2,
+} dump_include_t;
+
 int
 notmuch_database_dump (notmuch_database_t *notmuch,
 		       const char *output_file_name,
 		       const char *query_str,
 		       dump_format_t output_format,
+		       dump_include_t include,
 		       notmuch_bool_t gzip_output);
 
 /* If status is non-zero (i.e. error) print appropriate
diff --git a/notmuch-dump.c b/notmuch-dump.c
index 829781f..a88b5d5 100644
--- a/notmuch-dump.c
+++ b/notmuch-dump.c
@@ -23,16 +23,47 @@
 #include "string-util.h"
 #include <zlib.h>
 
+static notmuch_status_t
+database_dump_metadata(notmuch_database_t *notmuch, gzFile output)
+{
+    notmuch_metadata_class_t mclass;
+    notmuch_metadata_t *meta;
+    notmuch_status_t status;
+
+    for (mclass = NOTMUCH_METADATA_FIRST_CLASS; mclass < NOTMUCH_METADATA_LAST_CLASS; mclass++) {
+	status = notmuch_database_get_all_metadata (notmuch, NOTMUCH_METADATA_CONFIG, &meta);
+	if (status)
+	    return status;
+
+	for (; notmuch_metadata_valid (meta); notmuch_metadata_move_to_next (meta)) {
+	    /* FIXME hexencode key and values */
+	    gzprintf(output, "#@ %s %s %s\n",
+		     notmuch_metadata_prefix_string(mclass),
+		     notmuch_metadata_key (meta), notmuch_metadata_value (meta));
+	}
+	notmuch_metadata_destroy (meta);
+    }
+    return NOTMUCH_STATUS_SUCCESS;
+}
 
 static int
 database_dump_file (notmuch_database_t *notmuch, gzFile output,
-		    const char *query_str, int output_format)
+		    const char *query_str, int output_format, int include)
 {
     notmuch_query_t *query;
     notmuch_messages_t *messages;
     notmuch_message_t *message;
     notmuch_tags_t *tags;
 
+    if (include | DUMP_INCLUDE_METADATA) {
+	if (print_status_database ("notmuch dump", notmuch,
+				   database_dump_metadata(notmuch,output)))
+	    return EXIT_FAILURE;
+    }
+
+    if (! (include & DUMP_INCLUDE_TAGS))
+	return EXIT_SUCCESS;
+
     if (! query_str)
 	query_str = "";
 
@@ -130,6 +161,7 @@ notmuch_database_dump (notmuch_database_t *notmuch,
 		       const char *output_file_name,
 		       const char *query_str,
 		       dump_format_t output_format,
+		       dump_include_t include,
 		       notmuch_bool_t gzip_output)
 {
     gzFile output = NULL;
@@ -164,7 +196,7 @@ notmuch_database_dump (notmuch_database_t *notmuch,
 	goto DONE;
     }
 
-    ret = database_dump_file (notmuch, output, query_str, output_format);
+    ret = database_dump_file (notmuch, output, query_str, output_format, include);
     if (ret) goto DONE;
 
     ret = gzflush (output, Z_FINISH);
@@ -226,6 +258,7 @@ notmuch_dump_command (notmuch_config_t *config, int argc, char *argv[])
     int opt_index;
 
     int output_format = DUMP_FORMAT_BATCH_TAG;
+    int include = 0;
     notmuch_bool_t gzip_output = 0;
 
     notmuch_opt_desc_t options[] = {
@@ -233,6 +266,9 @@ notmuch_dump_command (notmuch_config_t *config, int argc, char *argv[])
 	  (notmuch_keyword_t []){ { "sup", DUMP_FORMAT_SUP },
 				  { "batch-tag", DUMP_FORMAT_BATCH_TAG },
 				  { 0, 0 } } },
+	{ NOTMUCH_OPT_KEYWORD_FLAGS, &include, "include", 'i',
+	  (notmuch_keyword_t []){ { "metadata", DUMP_INCLUDE_METADATA },
+				  { "tags", DUMP_INCLUDE_TAGS} } },
 	{ NOTMUCH_OPT_STRING, &output_file_name, "output", 'o', 0  },
 	{ NOTMUCH_OPT_BOOLEAN, &gzip_output, "gzip", 'z', 0 },
 	{ NOTMUCH_OPT_INHERIT, (void *) &notmuch_shared_options, NULL, 0, 0 },
@@ -245,6 +281,9 @@ notmuch_dump_command (notmuch_config_t *config, int argc, char *argv[])
 
     notmuch_process_shared_options (argv[0]);
 
+    if (include == 0)
+	include = DUMP_INCLUDE_METADATA | DUMP_INCLUDE_TAGS;
+
     if (opt_index < argc) {
 	query_str = query_string_from_args (notmuch, argc - opt_index, argv + opt_index);
 	if (query_str == NULL) {
@@ -254,7 +293,7 @@ notmuch_dump_command (notmuch_config_t *config, int argc, char *argv[])
     }
 
     ret = notmuch_database_dump (notmuch, output_file_name, query_str,
-				 output_format, gzip_output);
+				 output_format, include, gzip_output);
 
     notmuch_database_destroy (notmuch);
 
diff --git a/notmuch-new.c b/notmuch-new.c
index e503776..1f8050d 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -1041,7 +1041,7 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])
 	    }
 
 	    if (notmuch_database_dump (notmuch, backup_name, "",
-				       DUMP_FORMAT_BATCH_TAG, TRUE)) {
+				       DUMP_FORMAT_BATCH_TAG, DUMP_INCLUDE_METADATA | DUMP_INCLUDE_TAGS, TRUE)) {
 		fprintf (stderr, "Backup failed. Aborting upgrade.");
 		return EXIT_FAILURE;
 	    }
diff --git a/test/T590-metadata.sh b/test/T590-metadata.sh
index 4d55298..45a49be 100755
--- a/test/T590-metadata.sh
+++ b/test/T590-metadata.sh
@@ -77,4 +77,12 @@ val = testvalue2
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "dump metadata"
+notmuch dump --include=metadata >OUTPUT
+cat <<'EOF' >EXPECTED
+#@ C testkey1 testvalue1
+#@ C testkey2 testvalue2
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_done
-- 
2.6.4

Re: WIP: add metadata to dump output

[Tomi Ollila]

On Sun, Jan 10 2016, David Bremner <david@tethera.net> wrote:

> It seems (at least to me) that xapian metadata is the right way store
> certain configuration data, including tag aliases [1] and perhaps some
> non-CLI specific configuration. On the other hand we don't want to
> have things lost if we dump and restore a database. Hence this series,
> which is a start at dumping and restore such config.
>
> The main idea here is that various classes of metadata can be defined
> by using prefixes, in exactly the same way as tags are defined for
> documents. This will hopefully help prevent e.g. config from stomping
> on tag aliases.
>
> The first 6 patches impliment iterators for simple "queries" on
> metadata. They are probably split a bit fine, but that's the way I
> developed them.
>
> The last 3 impliment the printing of metadata in dump output. In order
> to be upwardly compatible, it uses the old dodge of hiding things in
> comments. In fact the comment syntax (# in first column) was never
> well documented; this does mean that the notmuch dump output can be
> tested without breaking the current restore tests. I threw an @ in to
> help autodetection of formats; obviously this is not foolproof. On the
> other hand, I don't know how much people currently rely on comments in
> dump files, since notmuch doesn't generate them.
>
> There's lots of bikes to shed here. Probably the most important bits
> are the library API, the dump output format, and of course the ever
> tricky command line argument names.

Generally this series looks pretty good. IMO this could have gone with
way less separate patches -- It would have made the review easier,
now I had to go back to previous mails just to look context. But,
anyone who disagrees w/ this make David know (in any appropriate 
channel so my opinion does not get too emphasized ;D)

The first thing that came into my mind was this naming of
*_FIRST_CLASS and *_LAST_CLASS in enum values. the naming
is inconsistent in sense that first is first, but last is last + 1.
Unfortunately there is nothing we can do with that as these *_LAST_*
are used in other enums too so we just have to live with it. 

In last in this series there is
+typedef enum dump_includes {
+    DUMP_INCLUDE_TAGS=1,
+    DUMP_INCLUDE_METADATA=2,
+} dump_include_t;

-- spacing around ' = ' missing -- I did not see other whitespace errors
(not that there might not be those, though, as we know David ;)

One bug I found:

+    for (mclass = NOTMUCH_METADATA_FIRST_CLASS; mclass < NOTMUCH_METADATA_LAST_CLASS; mclass++) {
+	status = notmuch_database_get_all_metadata (notmuch, NOTMUCH_METADATA_CONFIG, &meta);

(mclass should be there). Currently as there is only that one in the enum
there is no problem -- also for the same reason current test can not 
notice this. If this were not fixed, this would be noticed in the future
by that particular test - unless it is changed erronelously ;)


Anyway, good stuff in general...


Tomi

> Getting the memory ownership semantics is tricky, especially with the
> mix of C++ objects and talloc. So I'd appreciate a critical eye on
> those bits of metadata.cc.

uh puh -- maybe I look that again (hmm, have to apply the patch series as
all of the metadata.cc is not in one patch ;/