v4 flexible path location

v4 flexible path location

[David Bremner]

Since v3 [1]

- rebase against the uncrustified version (unfortunately this makes an
  interdiff too noisy to be helpful).
- improve (?) the documentation of database.mail_root in notmuch-config(1).
- tweak commit messages
- move the call to _init_libs from [10/22] to [9/22]

I'm going to mark this version as ready to apply, so object now, or
grumble later.

[1]: id:20210307172734.1091264-1-david@tethera.net

[PATCH 09/22] lib/open: factor out library initialization
[PATCH 10/22] lib/open: reuse directory checks from n_d_c_with_config

[PATCH 01/22] test/setup: add check for config exists, but no database.

[David Bremner]

This code path is not currently tested, and will need updating if the
location of the xapian database changes.
---
 test/T040-setup.sh | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/test/T040-setup.sh b/test/T040-setup.sh
index 1223ebf7..5eb281d7 100755
--- a/test/T040-setup.sh
+++ b/test/T040-setup.sh
@@ -34,4 +34,20 @@ built_with.compact=something
 built_with.field_processor=something
 built_with.retry_lock=something"
 
+test_begin_subtest "notmuch with a config but without a database suggests notmuch new"
+notmuch 2>&1 | notmuch_dir_sanitize > OUTPUT
+cat <<EOF > EXPECTED
+Notmuch is configured, but there's not yet a database at
+
+	MAIL_DIR/.notmuch
+
+You probably want to run "notmuch new" now to create that database.
+
+Note that the first run of "notmuch new" can take a very long time
+and that the resulting database will use roughly the same amount of
+storage space as the email being indexed.
+
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_done
-- 
2.30.1

[PATCH 02/22] lib: publish API for notmuch_database_reopen

[David Bremner]

Include the (currently unused) mode argument which will specify which
mode to re-open the database in. Functionality and docs to be
finalized in a followup commit.
---
 lib/database.cc       | 3 ++-
 lib/message.cc        | 3 ++-
 lib/notmuch-private.h | 3 ---
 lib/notmuch.h         | 7 +++++++
 4 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index f27616da..82ac5f20 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -518,7 +518,8 @@ notmuch_database_close (notmuch_database_t *notmuch)
 }
 
 notmuch_status_t
-_notmuch_database_reopen (notmuch_database_t *notmuch)
+notmuch_database_reopen (notmuch_database_t *notmuch,
+			 unused(notmuch_database_mode_t mode))
 {
     if (_notmuch_database_mode (notmuch) != NOTMUCH_DATABASE_MODE_READ_ONLY)
 	return NOTMUCH_STATUS_UNSUPPORTED_OPERATION;
diff --git a/lib/message.cc b/lib/message.cc
index dc4b47dc..73a7805f 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -459,7 +459,8 @@ _notmuch_message_ensure_metadata (notmuch_message_t *message, void *field)
 	    /* all the way without an exception */
 	    break;
 	} catch (const Xapian::DatabaseModifiedError &error) {
-	    notmuch_status_t status = _notmuch_database_reopen (message->notmuch);
+	    notmuch_status_t status = notmuch_database_reopen (message->notmuch,
+							       NOTMUCH_DATABASE_MODE_READ_ONLY);
 	    if (status != NOTMUCH_STATUS_SUCCESS)
 		INTERNAL_ERROR ("unhandled error from notmuch_database_reopen: %s\n",
 				notmuch_status_to_string (status));
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 3c52cfc2..ef814378 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -204,9 +204,6 @@ _notmuch_message_id_compressed (void *ctx, const char *message_id);
 notmuch_status_t
 _notmuch_database_ensure_writable (notmuch_database_t *notmuch);
 
-notmuch_status_t
-_notmuch_database_reopen (notmuch_database_t *notmuch);
-
 void
 _notmuch_database_log (notmuch_database_t *notmuch,
 		       const char *format, ...);
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 58c568d5..db86ffc0 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -879,6 +879,13 @@ notmuch_database_find_message_by_filename (notmuch_database_t *notmuch,
 notmuch_tags_t *
 notmuch_database_get_all_tags (notmuch_database_t *db);
 
+/**
+ * Reopen an open notmuch database.
+ *
+ */
+notmuch_status_t
+notmuch_database_reopen (notmuch_database_t *db, notmuch_database_mode_t mode);
+
 /**
  * Create a new query for 'database'.
  *
-- 
2.30.1

[PATCH 03/22] lib: save path of xapian database in notmuch struct.

[David Bremner]

This will allow re-opening in a different mode (read/write
vs. read-only) with current Xapian API. It will also prove useful when
updating the compact functions to support more flexible database
location.
---
 lib/database-private.h |  4 ++++
 lib/open.cc            | 19 ++++++++++---------
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/lib/database-private.h b/lib/database-private.h
index d83cf0d0..d936b216 100644
--- a/lib/database-private.h
+++ b/lib/database-private.h
@@ -189,8 +189,12 @@ operator& (notmuch_field_flag_t a, notmuch_field_flag_t b)
 struct _notmuch_database {
     bool exception_reported;
 
+    /* Path to database parent directory and or/mail root */
     char *path;
 
+    /* Path to actual database */
+    const char *xapian_path;
+
     int atomic_nesting;
     /* true if changes have been made in this atomic section */
     bool atomic_dirty;
diff --git a/lib/open.cc b/lib/open.cc
index 0c965d0d..12d3613f 100644
--- a/lib/open.cc
+++ b/lib/open.cc
@@ -199,7 +199,7 @@ notmuch_database_open_with_config (const char *database_path,
     notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
     void *local = talloc_new (NULL);
     notmuch_database_t *notmuch = NULL;
-    char *notmuch_path, *xapian_path, *incompat_features;
+    char *notmuch_path, *incompat_features;
     char *message = NULL;
     struct stat st;
     int err;
@@ -225,12 +225,6 @@ notmuch_database_open_with_config (const char *database_path,
 	goto DONE;
     }
 
-    if (! (xapian_path = talloc_asprintf (local, "%s/%s", notmuch_path, "xapian"))) {
-	message = strdup ("Out of memory\n");
-	status = NOTMUCH_STATUS_OUT_OF_MEMORY;
-	goto DONE;
-    }
-
     /* Initialize the GLib type system and threads */
 #if ! GLIB_CHECK_VERSION (2, 35, 1)
     g_type_init ();
@@ -252,16 +246,23 @@ notmuch_database_open_with_config (const char *database_path,
     notmuch->writable_xapian_db = NULL;
     notmuch->atomic_nesting = 0;
     notmuch->view = 1;
+
+    if (! (notmuch->xapian_path = talloc_asprintf (notmuch, "%s/%s", notmuch_path, "xapian"))) {
+	message = strdup ("Out of memory\n");
+	status = NOTMUCH_STATUS_OUT_OF_MEMORY;
+	goto DONE;
+    }
+
     try {
 	std::string last_thread_id;
 	std::string last_mod;
 
 	if (mode == NOTMUCH_DATABASE_MODE_READ_WRITE) {
-	    notmuch->writable_xapian_db = new Xapian::WritableDatabase (xapian_path,
+	    notmuch->writable_xapian_db = new Xapian::WritableDatabase (notmuch->xapian_path,
 									DB_ACTION);
 	    notmuch->xapian_db = notmuch->writable_xapian_db;
 	} else {
-	    notmuch->xapian_db = new Xapian::Database (xapian_path);
+	    notmuch->xapian_db = new Xapian::Database (notmuch->xapian_path);
 	}
 
 	/* Check version.  As of database version 3, we represent
-- 
2.30.1

[PATCH 04/22] lib: support reopening databases for write access.

[David Bremner]

In the future Xapian will apparently support this more conveniently
for the cases other than READ_ONLY => READ_ONLY

Conceptually this function seems to fit better in lib/open.cc;
database.cc is still large enough that moving the function makes
sense.
---
 lib/database.cc     |  23 --------
 lib/notmuch.h       |   6 +++
 lib/open.cc         |  46 ++++++++++++++++
 test/T595-reopen.sh | 125 ++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 177 insertions(+), 23 deletions(-)
 create mode 100755 test/T595-reopen.sh

diff --git a/lib/database.cc b/lib/database.cc
index 82ac5f20..fc13b53b 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -517,29 +517,6 @@ notmuch_database_close (notmuch_database_t *notmuch)
     return status;
 }
 
-notmuch_status_t
-notmuch_database_reopen (notmuch_database_t *notmuch,
-			 unused(notmuch_database_mode_t mode))
-{
-    if (_notmuch_database_mode (notmuch) != NOTMUCH_DATABASE_MODE_READ_ONLY)
-	return NOTMUCH_STATUS_UNSUPPORTED_OPERATION;
-
-    try {
-	notmuch->xapian_db->reopen ();
-    } catch (const Xapian::Error &error) {
-	if (! notmuch->exception_reported) {
-	    _notmuch_database_log (notmuch, "Error: A Xapian exception reopening database: %s\n",
-				   error.get_msg ().c_str ());
-	    notmuch->exception_reported = true;
-	}
-	return NOTMUCH_STATUS_XAPIAN_EXCEPTION;
-    }
-
-    notmuch->view++;
-
-    return NOTMUCH_STATUS_SUCCESS;
-}
-
 static int
 unlink_cb (const char *path,
 	   unused (const struct stat *sb),
diff --git a/lib/notmuch.h b/lib/notmuch.h
index db86ffc0..b5688087 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -882,6 +882,12 @@ notmuch_database_get_all_tags (notmuch_database_t *db);
 /**
  * Reopen an open notmuch database.
  *
+ * @param [in] db	open notmuch database
+ * @param [in] mode	mode (read only or read-write) for reopened database.
+ *
+ * @retval #NOTMUCH_STATUS_SUCCESS
+ * @retval #NOTMUCH_STATUS_ILLEGAL_ARGUMENT	The passed database was not open.
+ * @retval #NOTMUCH_STATUS_XAPIAN_EXCEPTION	A Xapian exception occured
  */
 notmuch_status_t
 notmuch_database_reopen (notmuch_database_t *db, notmuch_database_mode_t mode);
diff --git a/lib/open.cc b/lib/open.cc
index 12d3613f..d05a19a7 100644
--- a/lib/open.cc
+++ b/lib/open.cc
@@ -511,3 +511,49 @@ notmuch_database_create_with_config (const char *database_path,
 	talloc_free (notmuch);
     return status;
 }
+
+notmuch_status_t
+notmuch_database_reopen (notmuch_database_t *notmuch,
+			 notmuch_database_mode_t new_mode)
+{
+    notmuch_database_mode_t cur_mode = _notmuch_database_mode (notmuch);
+
+    if (notmuch->xapian_db == NULL) {
+	_notmuch_database_log (notmuch, "Cannot reopen closed or nonexistent database\n");
+	return NOTMUCH_STATUS_ILLEGAL_ARGUMENT;
+    }
+
+    try {
+	if (cur_mode == new_mode &&
+	    new_mode == NOTMUCH_DATABASE_MODE_READ_ONLY) {
+	    notmuch->xapian_db->reopen ();
+	} else {
+	    notmuch->xapian_db->close ();
+
+	    delete notmuch->xapian_db;
+	    notmuch->xapian_db = NULL;
+	    /* no need to free the same object twice */
+	    notmuch->writable_xapian_db = NULL;
+
+	    if (new_mode == NOTMUCH_DATABASE_MODE_READ_WRITE) {
+		notmuch->writable_xapian_db = new Xapian::WritableDatabase (notmuch->xapian_path,
+									    DB_ACTION);
+		notmuch->xapian_db = notmuch->writable_xapian_db;
+	    } else {
+		notmuch->xapian_db = new Xapian::Database (notmuch->xapian_path,
+							   DB_ACTION);
+	    }
+	}
+    } catch (const Xapian::Error &error) {
+	if (! notmuch->exception_reported) {
+	    _notmuch_database_log (notmuch, "Error: A Xapian exception reopening database: %s\n",
+				   error.get_msg ().c_str ());
+	    notmuch->exception_reported = true;
+	}
+	return NOTMUCH_STATUS_XAPIAN_EXCEPTION;
+    }
+
+    notmuch->view++;
+    notmuch->open = true;
+    return NOTMUCH_STATUS_SUCCESS;
+}
diff --git a/test/T595-reopen.sh b/test/T595-reopen.sh
new file mode 100755
index 00000000..7375e2ac
--- /dev/null
+++ b/test/T595-reopen.sh
@@ -0,0 +1,125 @@
+#!/usr/bin/env bash
+test_description="library reopen API"
+
+. $(dirname "$0")/test-lib.sh || exit 1
+
+add_email_corpus
+
+cat <<EOF > c_head
+#include <string.h>
+#include <stdlib.h>
+#include <notmuch-test.h>
+
+int main (int argc, char** argv)
+{
+   notmuch_database_t *db;
+   char *val;
+   notmuch_status_t stat;
+   notmuch_database_mode_t mode = NOTMUCH_DATABASE_MODE_READ_ONLY;
+
+   char *msg = NULL;
+
+   for (int i = 1; i < argc; i++)
+      if (strcmp (argv[i], "%NULL%") == 0) argv[i] = NULL;
+
+   if (argv[2] && (argv[2][0] == 'w' || argv[2][0] == 'W'))
+     mode = NOTMUCH_DATABASE_MODE_READ_WRITE;
+
+   stat = notmuch_database_open_with_config (argv[1],
+					     mode,
+					     argv[3],
+					     argv[4],
+					     &db,
+					     &msg);
+   if (stat != NOTMUCH_STATUS_SUCCESS) {
+     fprintf (stderr, "error opening database: %d %s\n", stat, msg ? msg : "");
+     exit (1);
+   }
+EOF
+
+cat <<EOF > c_tail
+   EXPECT0(notmuch_database_destroy(db));
+}
+EOF
+
+# The sequence of tests is important here
+
+test_begin_subtest "notmuch_database_reopen (read=>write)"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} read ${NOTMUCH_CONFIG}
+{
+   EXPECT0(notmuch_database_reopen (db, NOTMUCH_DATABASE_MODE_READ_WRITE));
+   EXPECT0(notmuch_database_set_config (db, "test.key1", "testvalue1"));
+   EXPECT0(notmuch_database_set_config (db, "test.key2", "testvalue2"));
+   EXPECT0(notmuch_database_get_config (db, "test.key1", &val));
+   printf("test.key1 = %s\n", val);
+   EXPECT0(notmuch_database_get_config (db, "test.key2", &val));
+   printf("test.key2 = %s\n", val);
+}
+EOF
+cat <<'EOF' >EXPECTED
+== stdout ==
+test.key1 = testvalue1
+test.key2 = testvalue2
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest "notmuch_database_reopen (read=>read)"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} read ${NOTMUCH_CONFIG}
+{
+   EXPECT0(notmuch_database_reopen (db, NOTMUCH_DATABASE_MODE_READ_ONLY));
+   EXPECT0(notmuch_database_get_config (db, "test.key1", &val));
+   printf("test.key1 = %s\n", val);
+   EXPECT0(notmuch_database_get_config (db, "test.key2", &val));
+   printf("test.key2 = %s\n", val);
+}
+EOF
+cat <<'EOF' >EXPECTED
+== stdout ==
+test.key1 = testvalue1
+test.key2 = testvalue2
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest "notmuch_database_reopen (write=>read)"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} write ${NOTMUCH_CONFIG}
+{
+   EXPECT0(notmuch_database_reopen (db, NOTMUCH_DATABASE_MODE_READ_ONLY));
+   EXPECT0(notmuch_database_get_config (db, "test.key1", &val));
+   printf("test.key1 = %s\n", val);
+   EXPECT0(notmuch_database_get_config (db, "test.key2", &val));
+   printf("test.key2 = %s\n", val);
+}
+EOF
+cat <<'EOF' >EXPECTED
+== stdout ==
+test.key1 = testvalue1
+test.key2 = testvalue2
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest "notmuch_database_reopen (write=>write)"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} write ${NOTMUCH_CONFIG}
+{
+   EXPECT0(notmuch_database_reopen (db, NOTMUCH_DATABASE_MODE_READ_WRITE));
+   EXPECT0(notmuch_database_set_config (db, "test.key3", "testvalue3"));
+   EXPECT0(notmuch_database_get_config (db, "test.key1", &val));
+   printf("test.key1 = %s\n", val);
+   EXPECT0(notmuch_database_get_config (db, "test.key2", &val));
+   printf("test.key2 = %s\n", val);
+   EXPECT0(notmuch_database_get_config (db, "test.key3", &val));
+   printf("test.key3 = %s\n", val);
+}
+EOF
+cat <<'EOF' >EXPECTED
+== stdout ==
+test.key1 = testvalue1
+test.key2 = testvalue2
+test.key3 = testvalue3
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_done
-- 
2.30.1

Re: [PATCH 04/22] lib: support reopening databases for write access.

[David Bremner]

David Bremner <david@tethera.net> writes:

> In the future Xapian will apparently support this more conveniently
> for the cases other than READ_ONLY => READ_ONLY
>
> Conceptually this function seems to fit better in lib/open.cc;
> database.cc is still large enough that moving the function makes
> sense.

I have applied the first 4 patches of the series to master, as I need
them for a bug fix for pre-new hooks.

d

[PATCH 05/22] CLI/show: complete conversion to new configuration framework.

[David Bremner]

In order to open the database in main() for this command, we may need
to re-open it in the (possibly less common) case where crypto options
require write access.
---
 notmuch-show.c           | 28 ++++++++--------------------
 notmuch.c                |  2 +-
 test/T035-read-config.sh |  2 --
 3 files changed, 9 insertions(+), 23 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index b6b53991..5fd1ce25 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -1225,10 +1225,9 @@ static const notmuch_show_format_t *formatters[] = {
 };
 
 int
-notmuch_show_command (notmuch_config_t *config, unused(notmuch_database_t *notmuch),
+notmuch_show_command (unused (notmuch_config_t *config), notmuch_database_t *notmuch,
 		      int argc, char *argv[])
 {
-    notmuch_database_t *notmuch;
     notmuch_query_t *query;
     char *query_string;
     int opt_index, ret;
@@ -1245,7 +1244,7 @@ notmuch_show_command (notmuch_config_t *config, unused(notmuch_database_t *notmu
     bool entire_thread_set = false;
     bool single_message;
     bool unthreaded = FALSE;
-    char *status_string = NULL;
+    notmuch_status_t status;
 
     notmuch_opt_desc_t options[] = {
 	{ .opt_keyword = &format, .name = "format", .keywords =
@@ -1336,26 +1335,15 @@ notmuch_show_command (notmuch_config_t *config, unused(notmuch_database_t *notmu
 		 "Warning: --include-html only implemented for format=text, format=json and format=sexp\n");
     }
 
-    notmuch_database_mode_t mode = NOTMUCH_DATABASE_MODE_READ_ONLY;
-
-    if (params.crypto.decrypt == NOTMUCH_DECRYPT_TRUE)
-	mode = NOTMUCH_DATABASE_MODE_READ_WRITE;
-    if (notmuch_database_open_with_config (NULL,
-					   mode,
-					   _notmuch_config_get_path (config),
-					   NULL,
-					   &notmuch,
-					   &status_string)) {
-	if (status_string) {
-	    fputs (status_string, stderr);
-	    free (status_string);
+    if (params.crypto.decrypt == NOTMUCH_DECRYPT_TRUE) {
+	status = notmuch_database_reopen (notmuch, NOTMUCH_DATABASE_MODE_READ_WRITE);
+	if (status) {
+	    fprintf (stderr, "Error reopening database for READ_WRITE: %s\n",
+                     notmuch_status_to_string (status));
+	    return EXIT_FAILURE;
 	}
-
-	return EXIT_FAILURE;
     }
 
-    config = NULL;
-
     notmuch_exit_if_unmatched_db_uuid (notmuch);
 
     query_string = query_string_from_args (notmuch, argc - opt_index, argv + opt_index);
diff --git a/notmuch.c b/notmuch.c
index 5e56209b..f2bf9c24 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -156,7 +156,7 @@ static command_t commands[] = {
       "Search for messages matching the given search terms." },
     { "address", notmuch_address_command, NOTMUCH_COMMAND_DATABASE_EARLY,
       "Get addresses from messages matching the given search terms." },
-    { "show", notmuch_show_command, NOTMUCH_COMMAND_CONFIG_OPEN,
+    { "show", notmuch_show_command, NOTMUCH_COMMAND_DATABASE_EARLY,
       "Show all messages matching the search terms." },
     { "count", notmuch_count_command, NOTMUCH_COMMAND_DATABASE_EARLY,
       "Count messages matching the search terms." },
diff --git a/test/T035-read-config.sh b/test/T035-read-config.sh
index 205d1736..ac0f420b 100755
--- a/test/T035-read-config.sh
+++ b/test/T035-read-config.sh
@@ -378,7 +378,6 @@ restore_database
 test_expect_equal "$output" "OK"
 
 test_begin_subtest "show with alternate config (xdg)"
-test_subtest_known_broken
 backup_database
 notmuch tag -- +foobar17 '*'
 xdg_config
@@ -389,7 +388,6 @@ restore_config
 test_expect_equal "$output" "OK"
 
 test_begin_subtest "show with alternate config (xdg+profile)"
-test_subtest_known_broken
 backup_database
 notmuch tag -- +foobar17 '*'
 xdg_config foobar17
-- 
2.30.1

[PATCH 06/22] lib/open: support NOTMUCH_DATABASE environment variable

[David Bremner]

The additional code is trivial, but making sure we get the priority of
various options correct takes a few tests.
---
 lib/open.cc            |  4 +++
 test/T590-libconfig.sh | 67 +++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 70 insertions(+), 1 deletion(-)

diff --git a/lib/open.cc b/lib/open.cc
index d05a19a7..6e46168a 100644
--- a/lib/open.cc
+++ b/lib/open.cc
@@ -168,6 +168,10 @@ _choose_database_path (void *ctx,
 	return status;
     }
 
+    if (! *database_path) {
+	*database_path = getenv ("NOTMUCH_DATABASE");
+    }
+
     if (! *database_path && *key_file) {
 	char *path = g_key_file_get_value (*key_file, "database", "path", NULL);
 	if (path) {
diff --git a/test/T590-libconfig.sh b/test/T590-libconfig.sh
index e7e6e08a..4e510e97 100755
--- a/test/T590-libconfig.sh
+++ b/test/T590-libconfig.sh
@@ -27,7 +27,7 @@ int main (int argc, char** argv)
                                               &db,
                                               &msg);
    if (stat != NOTMUCH_STATUS_SUCCESS) {
-     fprintf (stderr, "error opening database: %d %s\n", stat, msg ? msg : "");
+     fprintf (stderr, "error opening database\n%s\n%s\n", notmuch_status_to_string (stat), msg ? msg : "");
      exit (1);
    }
 EOF
@@ -505,4 +505,69 @@ EOF
 test_expect_equal_file EXPECTED OUTPUT
 restore_database
 
+test_begin_subtest "no config, fail to open database"
+old_NOTMUCH_CONFIG=${NOTMUCH_CONFIG}
+unset NOTMUCH_CONFIG
+cat c_head - c_tail <<'EOF' | test_C %NULL% '' %NULL%
+{
+   printf("NOT RUN");
+}
+EOF
+NOTMUCH_CONFIG=${old_NOTMUCH_CONFIG}
+cat <<'EOF' >EXPECTED
+== stdout ==
+== stderr ==
+error opening database
+Erroneous NULL pointer
+Error: Cannot open a database for a NULL path.
+
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest "open database from NOTMUCH_DATABASE"
+old_NOTMUCH_CONFIG=${NOTMUCH_CONFIG}
+unset NOTMUCH_CONFIG
+export NOTMUCH_DATABASE=${MAIL_DIR}
+cat c_head - c_tail <<'EOF' | test_C %NULL% '' %NULL%
+{
+   EXPECT0(notmuch_database_get_config (db, "test.key1", &val));
+   printf("test.key1 = %s\n", val);
+   EXPECT0(notmuch_database_get_config (db, "test.key2", &val));
+   printf("test.key2 = %s\n", val);
+}
+EOF
+NOTMUCH_CONFIG=${old_NOTMUCH_CONFIG}
+unset NOTMUCH_DATABASE
+cat <<'EOF' >EXPECTED
+== stdout ==
+test.key1 = testvalue1
+test.key2 = testvalue2
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest "NOTMUCH_DATABASE overrides config"
+old_path=$(notmuch config get database.path)
+notmuch config set database.path /nonexistent
+export NOTMUCH_DATABASE=${MAIL_DIR}
+cat c_head - c_tail <<'EOF' | test_C %NULL% '' %NULL%
+{
+   EXPECT0(notmuch_database_get_config (db, "test.key1", &val));
+   printf("test.key1 = %s\n", val);
+   EXPECT0(notmuch_database_get_config (db, "test.key2", &val));
+   printf("test.key2 = %s\n", val);
+}
+EOF
+NOTMUCH_CONFIG=${old_NOTMUCH_CONFIG}
+unset NOTMUCH_DATABASE
+cat <<'EOF' >EXPECTED
+== stdout ==
+test.key1 = testvalue1
+test.key2 = testvalue2
+== stderr ==
+EOF
+notmuch config set database.path "${old_path}"
+test_expect_equal_file EXPECTED OUTPUT
+
+
 test_done
-- 
2.30.1

[PATCH 07/22] lib/open: allocate notmuch_t struct early

[David Bremner]

This gives more flexibility in restructuring the database opening
code.
---
 lib/open.cc | 57 +++++++++++++++++++++++++++++++++--------------------
 1 file changed, 36 insertions(+), 21 deletions(-)

diff --git a/lib/open.cc b/lib/open.cc
index 6e46168a..de5f3467 100644
--- a/lib/open.cc
+++ b/lib/open.cc
@@ -192,6 +192,23 @@ _choose_database_path (void *ctx,
     return NOTMUCH_STATUS_SUCCESS;
 }
 
+notmuch_database_t *
+_alloc_notmuch ()
+{
+    notmuch_database_t *notmuch;
+
+    notmuch = talloc_zero (NULL, notmuch_database_t);
+    if (! notmuch)
+	return NULL;
+
+    notmuch->exception_reported = false;
+    notmuch->status_string = NULL;
+    notmuch->writable_xapian_db = NULL;
+    notmuch->atomic_nesting = 0;
+    notmuch->view = 1;
+    return notmuch;
+}
+
 notmuch_status_t
 notmuch_database_open_with_config (const char *database_path,
 				   notmuch_database_mode_t mode,
@@ -211,10 +228,19 @@ notmuch_database_open_with_config (const char *database_path,
     GKeyFile *key_file = NULL;
     static int initialized = 0;
 
-    if ((status = _choose_database_path (local, config_path, profile, &key_file, &database_path,
-					 &message)))
+    notmuch = _alloc_notmuch ();
+    if (! notmuch) {
+	status = NOTMUCH_STATUS_OUT_OF_MEMORY;
+	goto DONE;
+    }
+
+    if ((status = _choose_database_path (local, config_path, profile,
+					 &key_file, &database_path, &message)))
 	goto DONE;
 
+    notmuch->path = talloc_strdup (notmuch, database_path);
+    strip_trailing (notmuch->path, '/');
+
     if (! (notmuch_path = talloc_asprintf (local, "%s/%s", database_path, ".notmuch"))) {
 	message = strdup ("Out of memory\n");
 	status = NOTMUCH_STATUS_OUT_OF_MEMORY;
@@ -229,6 +255,12 @@ notmuch_database_open_with_config (const char *database_path,
 	goto DONE;
     }
 
+    if (! (notmuch->xapian_path = talloc_asprintf (local, "%s/%s", notmuch_path, "xapian"))) {
+	message = strdup ("Out of memory\n");
+	status = NOTMUCH_STATUS_OUT_OF_MEMORY;
+	goto DONE;
+    }
+
     /* Initialize the GLib type system and threads */
 #if ! GLIB_CHECK_VERSION (2, 35, 1)
     g_type_init ();
@@ -240,23 +272,6 @@ notmuch_database_open_with_config (const char *database_path,
 	initialized = 1;
     }
 
-    notmuch = talloc_zero (NULL, notmuch_database_t);
-    notmuch->exception_reported = false;
-    notmuch->status_string = NULL;
-    notmuch->path = talloc_strdup (notmuch, database_path);
-
-    strip_trailing (notmuch->path, '/');
-
-    notmuch->writable_xapian_db = NULL;
-    notmuch->atomic_nesting = 0;
-    notmuch->view = 1;
-
-    if (! (notmuch->xapian_path = talloc_asprintf (notmuch, "%s/%s", notmuch_path, "xapian"))) {
-	message = strdup ("Out of memory\n");
-	status = NOTMUCH_STATUS_OUT_OF_MEMORY;
-	goto DONE;
-    }
-
     try {
 	std::string last_thread_id;
 	std::string last_mod;
@@ -442,8 +457,8 @@ notmuch_database_create_with_config (const char *database_path,
     int err;
     void *local = talloc_new (NULL);
 
-    if ((status = _choose_database_path (local, config_path, profile, &key_file, &database_path,
-					 &message)))
+    if ((status = _choose_database_path (local, config_path, profile,
+					 &key_file, &database_path, &message)))
 	goto DONE;
 
     err = stat (database_path, &st);
-- 
2.30.1

[PATCH 08/22] lib: remove "path" from notmuch struct

[David Bremner]

This removes duplication between the struct element and the
configuration string_map entry. Create a simple wrapper for setting
the database path that makes sure the trailing / is stripped.
---
 lib/config.cc          |  3 +++
 lib/database-private.h |  3 ---
 lib/database.cc        |  2 +-
 lib/open.cc            | 14 ++++++++++++--
 4 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/lib/config.cc b/lib/config.cc
index 6993e67f..483a02ef 100644
--- a/lib/config.cc
+++ b/lib/config.cc
@@ -496,5 +496,8 @@ notmuch_config_set (notmuch_database_t *notmuch, notmuch_config_key_t key, const
 void
 _notmuch_config_cache (notmuch_database_t *notmuch, notmuch_config_key_t key, const char *val)
 {
+    if (notmuch->config == NULL)
+	notmuch->config = _notmuch_string_map_create (notmuch);
+
     _notmuch_string_map_set (notmuch->config, _notmuch_config_key_to_string (key), val);
 }
diff --git a/lib/database-private.h b/lib/database-private.h
index d936b216..2900382d 100644
--- a/lib/database-private.h
+++ b/lib/database-private.h
@@ -189,9 +189,6 @@ operator& (notmuch_field_flag_t a, notmuch_field_flag_t b)
 struct _notmuch_database {
     bool exception_reported;
 
-    /* Path to database parent directory and or/mail root */
-    char *path;
-
     /* Path to actual database */
     const char *xapian_path;
 
diff --git a/lib/database.cc b/lib/database.cc
index fc13b53b..580b2d58 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -752,7 +752,7 @@ notmuch_database_destroy (notmuch_database_t *notmuch)
 const char *
 notmuch_database_get_path (notmuch_database_t *notmuch)
 {
-    return notmuch->path;
+    return notmuch_config_get (notmuch, NOTMUCH_CONFIG_DATABASE_PATH);
 }
 
 unsigned int
diff --git a/lib/open.cc b/lib/open.cc
index de5f3467..a62d4dee 100644
--- a/lib/open.cc
+++ b/lib/open.cc
@@ -209,6 +209,17 @@ _alloc_notmuch ()
     return notmuch;
 }
 
+static void
+_set_database_path (notmuch_database_t *notmuch,
+		    const char *database_path)
+{
+    char *path = talloc_strdup (notmuch, database_path);
+
+    strip_trailing (path, '/');
+
+    _notmuch_config_cache (notmuch, NOTMUCH_CONFIG_DATABASE_PATH, path);
+}
+
 notmuch_status_t
 notmuch_database_open_with_config (const char *database_path,
 				   notmuch_database_mode_t mode,
@@ -238,8 +249,7 @@ notmuch_database_open_with_config (const char *database_path,
 					 &key_file, &database_path, &message)))
 	goto DONE;
 
-    notmuch->path = talloc_strdup (notmuch, database_path);
-    strip_trailing (notmuch->path, '/');
+    _set_database_path (notmuch, database_path);
 
     if (! (notmuch_path = talloc_asprintf (local, "%s/%s", database_path, ".notmuch"))) {
 	message = strdup ("Out of memory\n");
-- 
2.30.1

[PATCH 09/22] lib/open: factor out library initialization

[David Bremner]

This is slightly more tidy, but more importantly it allows for re-use
of this code in n_d_create_with_config. That re-use will be crucial
when we no longer call n_d_open_with_config from
n_d_create_with_config.
---
 lib/open.cc | 34 ++++++++++++++++++++++------------
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/lib/open.cc b/lib/open.cc
index a62d4dee..f48b97ae 100644
--- a/lib/open.cc
+++ b/lib/open.cc
@@ -220,6 +220,24 @@ _set_database_path (notmuch_database_t *notmuch,
     _notmuch_config_cache (notmuch, NOTMUCH_CONFIG_DATABASE_PATH, path);
 }
 
+static void
+_init_libs ()
+{
+
+    static int initialized = 0;
+
+    /* Initialize the GLib type system and threads */
+#if ! GLIB_CHECK_VERSION (2, 35, 1)
+    g_type_init ();
+#endif
+
+    /* Initialize gmime */
+    if (! initialized) {
+	g_mime_init ();
+	initialized = 1;
+    }
+}
+
 notmuch_status_t
 notmuch_database_open_with_config (const char *database_path,
 				   notmuch_database_mode_t mode,
@@ -237,7 +255,8 @@ notmuch_database_open_with_config (const char *database_path,
     int err;
     unsigned int version;
     GKeyFile *key_file = NULL;
-    static int initialized = 0;
+
+    _init_libs ();
 
     notmuch = _alloc_notmuch ();
     if (! notmuch) {
@@ -271,17 +290,6 @@ notmuch_database_open_with_config (const char *database_path,
 	goto DONE;
     }
 
-    /* Initialize the GLib type system and threads */
-#if ! GLIB_CHECK_VERSION (2, 35, 1)
-    g_type_init ();
-#endif
-
-    /* Initialize gmime */
-    if (! initialized) {
-	g_mime_init ();
-	initialized = 1;
-    }
-
     try {
 	std::string last_thread_id;
 	std::string last_mod;
@@ -467,6 +475,8 @@ notmuch_database_create_with_config (const char *database_path,
     int err;
     void *local = talloc_new (NULL);
 
+    _init_libs ();
+
     if ((status = _choose_database_path (local, config_path, profile,
 					 &key_file, &database_path, &message)))
 	goto DONE;
-- 
2.30.1

[PATCH 10/22] lib/open: reuse directory checks from n_d_c_with_config

[David Bremner]

Make checks more uniform between creating new databases and opening
existing ones.
---
 lib/open.cc              | 66 ++++++++++++++++++++++++++--------------
 test/T030-config.sh      |  2 +-
 test/T560-lib-error.sh   |  4 +--
 test/T750-user-header.sh |  2 +-
 4 files changed, 47 insertions(+), 27 deletions(-)

diff --git a/lib/open.cc b/lib/open.cc
index f48b97ae..0defa9c0 100644
--- a/lib/open.cc
+++ b/lib/open.cc
@@ -152,6 +152,29 @@ _load_key_file (const char *path,
     return status;
 }
 
+static notmuch_status_t
+_db_dir_exists (const char *database_path, char **message)
+{
+    struct stat st;
+    int err;
+
+    err = stat (database_path, &st);
+    if (err) {
+	IGNORE_RESULT (asprintf (message, "Error: Cannot open database at %s: %s.\n",
+				 database_path, strerror (errno)));
+	return NOTMUCH_STATUS_FILE_ERROR;
+    }
+
+    if (! S_ISDIR (st.st_mode)) {
+	IGNORE_RESULT (asprintf (message, "Error: Cannot open database at %s: "
+				 "Not a directory.\n",
+				 database_path));
+	return NOTMUCH_STATUS_FILE_ERROR;
+    }
+
+    return NOTMUCH_STATUS_SUCCESS;
+}
+
 static notmuch_status_t
 _choose_database_path (void *ctx,
 		       const char *config_path,
@@ -251,8 +274,6 @@ notmuch_database_open_with_config (const char *database_path,
     notmuch_database_t *notmuch = NULL;
     char *notmuch_path, *incompat_features;
     char *message = NULL;
-    struct stat st;
-    int err;
     unsigned int version;
     GKeyFile *key_file = NULL;
 
@@ -270,19 +291,19 @@ notmuch_database_open_with_config (const char *database_path,
 
     _set_database_path (notmuch, database_path);
 
+    status = _db_dir_exists (database_path, &message);
+    if (status)
+	goto DONE;
+
     if (! (notmuch_path = talloc_asprintf (local, "%s/%s", database_path, ".notmuch"))) {
 	message = strdup ("Out of memory\n");
 	status = NOTMUCH_STATUS_OUT_OF_MEMORY;
 	goto DONE;
     }
 
-    err = stat (notmuch_path, &st);
-    if (err) {
-	IGNORE_RESULT (asprintf (&message, "Error opening database at %s: %s\n",
-				 notmuch_path, strerror (errno)));
-	status = NOTMUCH_STATUS_FILE_ERROR;
+    status = _db_dir_exists (notmuch_path, &message);
+    if (status)
 	goto DONE;
-    }
 
     if (! (notmuch->xapian_path = talloc_asprintf (local, "%s/%s", notmuch_path, "xapian"))) {
 	message = strdup ("Out of memory\n");
@@ -471,9 +492,16 @@ notmuch_database_create_with_config (const char *database_path,
     char *notmuch_path = NULL;
     char *message = NULL;
     GKeyFile *key_file = NULL;
-    struct stat st;
-    int err;
     void *local = talloc_new (NULL);
+    int err;
+
+    _init_libs ();
+
+    notmuch = _alloc_notmuch ();
+    if (! notmuch) {
+	status = NOTMUCH_STATUS_OUT_OF_MEMORY;
+	goto DONE;
+    }
 
     _init_libs ();
 
@@ -481,21 +509,11 @@ notmuch_database_create_with_config (const char *database_path,
 					 &key_file, &database_path, &message)))
 	goto DONE;
 
-    err = stat (database_path, &st);
-    if (err) {
-	IGNORE_RESULT (asprintf (&message, "Error: Cannot create database at %s: %s.\n",
-				 database_path, strerror (errno)));
-	status = NOTMUCH_STATUS_FILE_ERROR;
+    status = _db_dir_exists (database_path, &message);
+    if (status)
 	goto DONE;
-    }
 
-    if (! S_ISDIR (st.st_mode)) {
-	IGNORE_RESULT (asprintf (&message, "Error: Cannot create database at %s: "
-				 "Not a directory.\n",
-				 database_path));
-	status = NOTMUCH_STATUS_FILE_ERROR;
-	goto DONE;
-    }
+    _set_database_path (notmuch, database_path);
 
     notmuch_path = talloc_asprintf (local, "%s/%s", database_path, ".notmuch");
 
@@ -503,6 +521,8 @@ notmuch_database_create_with_config (const char *database_path,
     if (err) {
 	if (errno == EEXIST) {
 	    status = NOTMUCH_STATUS_DATABASE_EXISTS;
+	    talloc_free (notmuch);
+	    notmuch = NULL;
 	} else {
 	    IGNORE_RESULT (asprintf (&message, "Error: Cannot create directory %s: %s.\n",
 				     notmuch_path, strerror (errno)));
diff --git a/test/T030-config.sh b/test/T030-config.sh
index 32efa945..6610bf8e 100755
--- a/test/T030-config.sh
+++ b/test/T030-config.sh
@@ -61,7 +61,7 @@ built_with.compact=something
 built_with.field_processor=something
 built_with.retry_lock=something
 ====
-Error opening database at MAIL_DIR/.notmuch: No such file or directory
+Error: Cannot open database at MAIL_DIR/.notmuch: No such file or directory.
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
diff --git a/test/T560-lib-error.sh b/test/T560-lib-error.sh
index ade376ef..03df69d9 100755
--- a/test/T560-lib-error.sh
+++ b/test/T560-lib-error.sh
@@ -76,7 +76,7 @@ EOF
 cat <<'EOF' >EXPECTED
 == stdout ==
 == stderr ==
-Error opening database at CWD/nonexistent/foo/.notmuch: No such file or directory
+Error: Cannot open database at CWD/nonexistent/foo: No such file or directory.
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
@@ -111,7 +111,7 @@ EOF
 cat <<'EOF' >EXPECTED
 == stdout ==
 == stderr ==
-Error: Cannot create database at CWD/nonexistent/foo: No such file or directory.
+Error: Cannot open database at CWD/nonexistent/foo: No such file or directory.
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
diff --git a/test/T750-user-header.sh b/test/T750-user-header.sh
index ff554b06..b19db571 100755
--- a/test/T750-user-header.sh
+++ b/test/T750-user-header.sh
@@ -5,7 +5,7 @@ test_description='indexing user specified headers'
 test_begin_subtest "error adding user header before initializing DB"
 notmuch config set index.header.List List-Id 2>&1 | notmuch_dir_sanitize > OUTPUT
 cat <<EOF > EXPECTED
-Error opening database at MAIL_DIR/.notmuch: No such file or directory
+Error: Cannot open database at MAIL_DIR/.notmuch: No such file or directory.
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
-- 
2.30.1

[PATCH 11/22] lib/open: factor out the second half of n_d_open_with_config

[David Bremner]

The idea is to allow reuse in n_d_create_with_config. This is
primarily code movement, with some changes in error messages to reduce
the number of input parameters.
---
 lib/open.cc | 115 ++++++++++++++++++++++++++++++----------------------
 1 file changed, 67 insertions(+), 48 deletions(-)

diff --git a/lib/open.cc b/lib/open.cc
index 0defa9c0..7673e875 100644
--- a/lib/open.cc
+++ b/lib/open.cc
@@ -261,55 +261,18 @@ _init_libs ()
     }
 }
 
-notmuch_status_t
-notmuch_database_open_with_config (const char *database_path,
-				   notmuch_database_mode_t mode,
-				   const char *config_path,
-				   const char *profile,
-				   notmuch_database_t **database,
-				   char **status_string)
+static notmuch_status_t
+_finish_open (notmuch_database_t *notmuch,
+	      const char *profile,
+	      notmuch_database_mode_t mode,
+	      GKeyFile *key_file,
+	      char **message_ptr)
 {
     notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
-    void *local = talloc_new (NULL);
-    notmuch_database_t *notmuch = NULL;
-    char *notmuch_path, *incompat_features;
+    char *incompat_features;
     char *message = NULL;
     unsigned int version;
-    GKeyFile *key_file = NULL;
-
-    _init_libs ();
-
-    notmuch = _alloc_notmuch ();
-    if (! notmuch) {
-	status = NOTMUCH_STATUS_OUT_OF_MEMORY;
-	goto DONE;
-    }
-
-    if ((status = _choose_database_path (local, config_path, profile,
-					 &key_file, &database_path, &message)))
-	goto DONE;
-
-    _set_database_path (notmuch, database_path);
-
-    status = _db_dir_exists (database_path, &message);
-    if (status)
-	goto DONE;
-
-    if (! (notmuch_path = talloc_asprintf (local, "%s/%s", database_path, ".notmuch"))) {
-	message = strdup ("Out of memory\n");
-	status = NOTMUCH_STATUS_OUT_OF_MEMORY;
-	goto DONE;
-    }
-
-    status = _db_dir_exists (notmuch_path, &message);
-    if (status)
-	goto DONE;
-
-    if (! (notmuch->xapian_path = talloc_asprintf (local, "%s/%s", notmuch_path, "xapian"))) {
-	message = strdup ("Out of memory\n");
-	status = NOTMUCH_STATUS_OUT_OF_MEMORY;
-	goto DONE;
-    }
+    const char *database_path = notmuch_database_get_path (notmuch);
 
     try {
 	std::string last_thread_id;
@@ -332,7 +295,7 @@ notmuch_database_open_with_config (const char *database_path,
 				     "Error: Notmuch database at %s\n"
 				     "       has a newer database format version (%u) than supported by this\n"
 				     "       version of notmuch (%u).\n",
-				     notmuch_path, version, NOTMUCH_DATABASE_VERSION));
+				     database_path, version, NOTMUCH_DATABASE_VERSION));
 	    notmuch_database_destroy (notmuch);
 	    notmuch = NULL;
 	    status = NOTMUCH_STATUS_FILE_ERROR;
@@ -342,7 +305,7 @@ notmuch_database_open_with_config (const char *database_path,
 	/* Check features. */
 	incompat_features = NULL;
 	notmuch->features = _notmuch_database_parse_features (
-	    local, notmuch->xapian_db->get_metadata ("features").c_str (),
+	    notmuch, notmuch->xapian_db->get_metadata ("features").c_str (),
 	    version, mode == NOTMUCH_DATABASE_MODE_READ_WRITE ? 'w' : 'r',
 	    &incompat_features);
 	if (incompat_features) {
@@ -350,7 +313,7 @@ notmuch_database_open_with_config (const char *database_path,
 				     "Error: Notmuch database at %s\n"
 				     "       requires features (%s)\n"
 				     "       not supported by this version of notmuch.\n",
-				     notmuch_path, incompat_features));
+				     database_path, incompat_features));
 	    notmuch_database_destroy (notmuch);
 	    notmuch = NULL;
 	    status = NOTMUCH_STATUS_FILE_ERROR;
@@ -430,6 +393,62 @@ notmuch_database_open_with_config (const char *database_path,
 	notmuch = NULL;
 	status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
     }
+  DONE:
+    if (message_ptr)
+	*message_ptr = message;
+    return status;
+}
+
+notmuch_status_t
+notmuch_database_open_with_config (const char *database_path,
+				   notmuch_database_mode_t mode,
+				   const char *config_path,
+				   const char *profile,
+				   notmuch_database_t **database,
+				   char **status_string)
+{
+    notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
+    void *local = talloc_new (NULL);
+    notmuch_database_t *notmuch = NULL;
+    char *notmuch_path;
+    char *message = NULL;
+    GKeyFile *key_file = NULL;
+
+    _init_libs ();
+
+    notmuch = _alloc_notmuch ();
+    if (! notmuch) {
+	status = NOTMUCH_STATUS_OUT_OF_MEMORY;
+	goto DONE;
+    }
+
+    if ((status = _choose_database_path (local, config_path, profile, &key_file, &database_path,
+					 &message)))
+	goto DONE;
+
+    _set_database_path (notmuch, database_path);
+
+    status = _db_dir_exists (database_path, &message);
+    if (status)
+	goto DONE;
+
+    if (! (notmuch_path = talloc_asprintf (local, "%s/%s", database_path, ".notmuch"))) {
+	message = strdup ("Out of memory\n");
+	status = NOTMUCH_STATUS_OUT_OF_MEMORY;
+	goto DONE;
+    }
+
+    status = _db_dir_exists (notmuch_path, &message);
+    if (status)
+	goto DONE;
+
+    if (! (notmuch->xapian_path = talloc_asprintf (local, "%s/%s", notmuch_path, "xapian"))) {
+	message = strdup ("Out of memory\n");
+	status = NOTMUCH_STATUS_OUT_OF_MEMORY;
+	goto DONE;
+    }
+
+    status = _finish_open (notmuch, profile, mode, key_file, &message);
 
   DONE:
     talloc_free (local);
-- 
2.30.1

[PATCH 12/22] lib/open: use _finish_open in n_d_create_with_config

[David Bremner]

This avoids reading the configuration file twice.
---
 lib/open.cc | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/lib/open.cc b/lib/open.cc
index 7673e875..54e71fec 100644
--- a/lib/open.cc
+++ b/lib/open.cc
@@ -442,8 +442,7 @@ notmuch_database_open_with_config (const char *database_path,
     if (status)
 	goto DONE;
 
-    if (! (notmuch->xapian_path = talloc_asprintf (local, "%s/%s", notmuch_path, "xapian"))) {
-	message = strdup ("Out of memory\n");
+    if (! (notmuch->xapian_path = talloc_asprintf (notmuch, "%s/%s", notmuch_path, "xapian"))) {
 	status = NOTMUCH_STATUS_OUT_OF_MEMORY;
 	goto DONE;
     }
@@ -550,12 +549,16 @@ notmuch_database_create_with_config (const char *database_path,
 	goto DONE;
     }
 
-    /* XXX this reads the config file twice, which is a bit wasteful */
-    status = notmuch_database_open_with_config (database_path,
-						NOTMUCH_DATABASE_MODE_READ_WRITE,
-						config_path,
-						profile,
-						&notmuch, &message);
+    if (! (notmuch->xapian_path = talloc_asprintf (notmuch, "%s/%s", notmuch_path, "xapian"))) {
+	status = NOTMUCH_STATUS_OUT_OF_MEMORY;
+	goto DONE;
+    }
+
+    status = _finish_open (notmuch,
+			   profile,
+			   NOTMUCH_DATABASE_MODE_READ_WRITE,
+			   key_file,
+			   &message);
     if (status)
 	goto DONE;
 
-- 
2.30.1

[PATCH 13/22] lib/open: Use check for existing database by trial opening

[David Bremner]

This is a bit heavyweight for now, but it will make more sense when we
check multiple locations for the Xapian database.
---
 lib/open.cc                | 61 +++++++++++++++++++++++++++++---------
 test/T360-symbol-hiding.sh |  2 +-
 2 files changed, 48 insertions(+), 15 deletions(-)

diff --git a/lib/open.cc b/lib/open.cc
index 54e71fec..956d50b9 100644
--- a/lib/open.cc
+++ b/lib/open.cc
@@ -232,6 +232,51 @@ _alloc_notmuch ()
     return notmuch;
 }
 
+static notmuch_status_t
+_trial_open (const char *xapian_path, char **message_ptr)
+{
+    try {
+	Xapian::Database db (xapian_path);
+    } catch (const Xapian::DatabaseOpeningError &error) {
+	IGNORE_RESULT (asprintf (message_ptr,
+				 "Cannot open Xapian database at %s: %s\n",
+				 xapian_path,
+				 error.get_msg ().c_str ()));
+	return NOTMUCH_STATUS_PATH_ERROR;
+    } catch (const Xapian::Error &error) {
+	IGNORE_RESULT (asprintf (message_ptr,
+				 "A Xapian exception occurred opening database: %s\n",
+				 error.get_msg ().c_str ()));
+	return NOTMUCH_STATUS_XAPIAN_EXCEPTION;
+    }
+    return NOTMUCH_STATUS_SUCCESS;
+}
+
+static notmuch_status_t
+_choose_xapian_path (void *ctx, const char *database_path, const char **xapian_path,
+		     char **message_ptr)
+{
+    notmuch_status_t status;
+    const char *trial_path, *notmuch_path;
+
+    status = _db_dir_exists (database_path, message_ptr);
+    if (status)
+	goto DONE;
+
+    notmuch_path = talloc_asprintf (ctx, "%s/.notmuch", database_path);
+    status = _db_dir_exists (notmuch_path, message_ptr);
+    if (status)
+	goto DONE;
+
+    trial_path = talloc_asprintf (ctx, "%s/xapian", notmuch_path);
+    status = _trial_open (trial_path, message_ptr);
+
+  DONE:
+    if (status == NOTMUCH_STATUS_SUCCESS)
+	*xapian_path = trial_path;
+    return status;
+}
+
 static void
 _set_database_path (notmuch_database_t *notmuch,
 		    const char *database_path)
@@ -410,7 +455,6 @@ notmuch_database_open_with_config (const char *database_path,
     notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
     void *local = talloc_new (NULL);
     notmuch_database_t *notmuch = NULL;
-    char *notmuch_path;
     char *message = NULL;
     GKeyFile *key_file = NULL;
 
@@ -426,27 +470,16 @@ notmuch_database_open_with_config (const char *database_path,
 					 &message)))
 	goto DONE;
 
-    _set_database_path (notmuch, database_path);
-
     status = _db_dir_exists (database_path, &message);
     if (status)
 	goto DONE;
 
-    if (! (notmuch_path = talloc_asprintf (local, "%s/%s", database_path, ".notmuch"))) {
-	message = strdup ("Out of memory\n");
-	status = NOTMUCH_STATUS_OUT_OF_MEMORY;
-	goto DONE;
-    }
+    _set_database_path (notmuch, database_path);
 
-    status = _db_dir_exists (notmuch_path, &message);
+    status = _choose_xapian_path (local, database_path, &notmuch->xapian_path, &message);
     if (status)
 	goto DONE;
 
-    if (! (notmuch->xapian_path = talloc_asprintf (notmuch, "%s/%s", notmuch_path, "xapian"))) {
-	status = NOTMUCH_STATUS_OUT_OF_MEMORY;
-	goto DONE;
-    }
-
     status = _finish_open (notmuch, profile, mode, key_file, &message);
 
   DONE:
diff --git a/test/T360-symbol-hiding.sh b/test/T360-symbol-hiding.sh
index 729b9d72..642457bf 100755
--- a/test/T360-symbol-hiding.sh
+++ b/test/T360-symbol-hiding.sh
@@ -17,7 +17,7 @@ $TEST_DIRECTORY/symbol-test ${PWD}/fakedb ${PWD}/nonexistent 2>&1 \
 	| notmuch_dir_sanitize | sed -e "s,\`,\',g" -e "s,No [^[:space:]]* database,No XXXXXX database,g" > OUTPUT
 
 cat <<EOF > EXPECTED
-A Xapian exception occurred opening database: Couldn't stat 'CWD/fakedb/.notmuch/xapian'
+Cannot open Xapian database at CWD/fakedb/.notmuch/xapian: Couldn't stat 'CWD/fakedb/.notmuch/xapian'
 caught No XXXXXX database found at path 'CWD/nonexistent'
 EOF
 test_expect_equal_file EXPECTED OUTPUT
-- 
2.30.1

[PATCH 14/22] lib: support splitting mail from database location.

[David Bremner]

Introduce a new configuration value for the mail root, and use it to
locate mail messages in preference to the database.path (which
previously implied the mail messages were also in this location.

Initially only a subset of the CLI is tested in a split
configuration. Further changes will be needed for the remainder of the
CLI to work in split configurations.
---
 doc/man1/notmuch-config.rst | 15 +++++--
 lib/config.cc               | 11 +++--
 lib/database.cc             |  2 +-
 lib/message-file.c          |  2 +-
 lib/message.cc              |  2 +-
 lib/notmuch.h               |  1 +
 lib/open.cc                 |  5 +++
 test/T055-path-config.sh    | 90 +++++++++++++++++++++++++++++++++++++
 test/T590-libconfig.sh      |  1 +
 9 files changed, 120 insertions(+), 9 deletions(-)
 create mode 100755 test/T055-path-config.sh

diff --git a/doc/man1/notmuch-config.rst b/doc/man1/notmuch-config.rst
index bc597957..223d2de5 100644
--- a/doc/man1/notmuch-config.rst
+++ b/doc/man1/notmuch-config.rst
@@ -43,12 +43,21 @@ configuration file and corresponding database.
 The available configuration items are described below.
 
 **database.path**
+    Notmuch will store its database here, (in
+    sub-directory named ``.notmuch`` if **database.mail\_root**
+    is unset).
+
+    Default: ``$MAILDIR`` variable if set, otherwise ``$HOME/mail``.
+
+**database.mail_root**
     The top-level directory where your mail currently exists and to
     where mail will be delivered in the future. Files should be
-    individual email messages. Notmuch will store its database within
-    a sub-directory of the path configured here named ``.notmuch``.
+    individual email messages.
 
-    Default: ``$MAILDIR`` variable if set, otherwise ``$HOME/mail``.
+    History: this configuration value was introduced in notmuch 0.32.
+
+    Default: For compatibility with older configurations, the value of
+    database.path is used if **database.mail\_root** is unset.
 
 **database.hook_dir**
 
diff --git a/lib/config.cc b/lib/config.cc
index 483a02ef..f2060e28 100644
--- a/lib/config.cc
+++ b/lib/config.cc
@@ -406,6 +406,8 @@ _notmuch_config_key_to_string (notmuch_config_key_t key)
     switch (key) {
     case NOTMUCH_CONFIG_DATABASE_PATH:
 	return "database.path";
+    case NOTMUCH_CONFIG_MAIL_ROOT:
+	return "database.mail_root";
     case NOTMUCH_CONFIG_HOOK_DIR:
 	return "database.hook_dir";
     case NOTMUCH_CONFIG_EXCLUDE_TAGS:
@@ -428,7 +430,7 @@ _notmuch_config_key_to_string (notmuch_config_key_t key)
 }
 
 static const char *
-_notmuch_config_default (void *ctx, notmuch_config_key_t key)
+_notmuch_config_default (notmuch_database_t *notmuch, notmuch_config_key_t key)
 {
     char *path;
 
@@ -436,11 +438,14 @@ _notmuch_config_default (void *ctx, notmuch_config_key_t key)
     case NOTMUCH_CONFIG_DATABASE_PATH:
 	path = getenv ("MAILDIR");
 	if (path)
-	    path = talloc_strdup (ctx, path);
+	    path = talloc_strdup (notmuch, path);
 	else
-	    path = talloc_asprintf (ctx, "%s/mail",
+	    path = talloc_asprintf (notmuch, "%s/mail",
 				    getenv ("HOME"));
 	return path;
+    case NOTMUCH_CONFIG_MAIL_ROOT:
+	/* by default, mail root is the same as database path */
+	return notmuch_database_get_path (notmuch);
     case NOTMUCH_CONFIG_EXCLUDE_TAGS:
 	return "";
     case NOTMUCH_CONFIG_NEW_TAGS:
diff --git a/lib/database.cc b/lib/database.cc
index 580b2d58..248d1dc2 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -1350,7 +1350,7 @@ _notmuch_database_relative_path (notmuch_database_t *notmuch,
     const char *db_path, *relative;
     unsigned int db_path_len;
 
-    db_path = notmuch_database_get_path (notmuch);
+    db_path = notmuch_config_get (notmuch, NOTMUCH_CONFIG_MAIL_ROOT);
     db_path_len = strlen (db_path);
 
     relative = path;
diff --git a/lib/message-file.c b/lib/message-file.c
index 15b0bfad..9e9b387f 100644
--- a/lib/message-file.c
+++ b/lib/message-file.c
@@ -64,7 +64,7 @@ _notmuch_message_file_open_ctx (notmuch_database_t *notmuch,
     if (unlikely (message == NULL))
 	return NULL;
 
-    const char *prefix = notmuch_database_get_path (notmuch);
+    const char *prefix = notmuch_config_get (notmuch, NOTMUCH_CONFIG_MAIL_ROOT);
 
     if (prefix == NULL)
 	goto FAIL;
diff --git a/lib/message.cc b/lib/message.cc
index 73a7805f..0c2eeab5 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -1102,7 +1102,7 @@ _notmuch_message_ensure_filename_list (notmuch_message_t *message)
 
 	*colon = '\0';
 
-	db_path = notmuch_database_get_path (message->notmuch);
+	db_path = notmuch_config_get (message->notmuch, NOTMUCH_CONFIG_MAIL_ROOT);
 
 	directory = _notmuch_database_get_directory_path (local,
 							  message->notmuch,
diff --git a/lib/notmuch.h b/lib/notmuch.h
index b5688087..c9b01719 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -2474,6 +2474,7 @@ notmuch_config_list_destroy (notmuch_config_list_t *config_list);
 typedef enum _notmuch_config_key {
     NOTMUCH_CONFIG_FIRST,
     NOTMUCH_CONFIG_DATABASE_PATH = NOTMUCH_CONFIG_FIRST,
+    NOTMUCH_CONFIG_MAIL_ROOT,
     NOTMUCH_CONFIG_HOOK_DIR,
     NOTMUCH_CONFIG_EXCLUDE_TAGS,
     NOTMUCH_CONFIG_NEW_TAGS,
diff --git a/lib/open.cc b/lib/open.cc
index 956d50b9..fe42f7f0 100644
--- a/lib/open.cc
+++ b/lib/open.cc
@@ -263,6 +263,11 @@ _choose_xapian_path (void *ctx, const char *database_path, const char **xapian_p
     if (status)
 	goto DONE;
 
+    trial_path = talloc_asprintf (ctx, "%s/xapian", database_path);
+    status = _trial_open (trial_path, message_ptr);
+    if (status != NOTMUCH_STATUS_PATH_ERROR)
+	goto DONE;
+
     notmuch_path = talloc_asprintf (ctx, "%s/.notmuch", database_path);
     status = _db_dir_exists (notmuch_path, message_ptr);
     if (status)
diff --git a/test/T055-path-config.sh b/test/T055-path-config.sh
new file mode 100755
index 00000000..c6920ca9
--- /dev/null
+++ b/test/T055-path-config.sh
@@ -0,0 +1,90 @@
+#!/usr/bin/env bash
+test_description='Configuration of mail-root and database path'
+. $(dirname "$0")/test-lib.sh || exit 1
+
+backup_config () {
+    local test_name=$(basename $0 .sh)
+    cp ${NOTMUCH_CONFIG} notmuch-config-backup.${test_name}
+}
+
+restore_config () {
+    local test_name=$(basename $0 .sh)
+    export NOTMUCH_CONFIG="${TMP_DIRECTORY}/notmuch-config"
+    unset CONFIG_PATH
+    unset DATABASE_PATH
+    unset NOTMUCH_PROFILE
+    cp notmuch-config-backup.${test_name} ${NOTMUCH_CONFIG}
+}
+
+split_config () {
+    local dir
+    backup_config
+    dir="$TMP_DIRECTORY/database.$test_count"
+    rm -rf $dir
+    mkdir $dir
+    notmuch config set database.path $dir
+    notmuch config set database.mail_root $MAIL_DIR
+    DATABASE_PATH=$dir
+}
+
+
+
+for config in traditional split; do
+    # start each set of tests with a known set of messages
+    add_email_corpus
+
+    case $config in
+	traditional)
+	    backup_config
+	    ;;
+	split)
+	    split_config
+	    mv mail/.notmuch/xapian $DATABASE_PATH
+	    ;;
+    esac
+
+    test_begin_subtest "count ($config)"
+    output=$(notmuch count '*')
+    test_expect_equal "$output" '52'
+
+    test_begin_subtest "count+tag ($config)"
+    tag="tag${RANDOM}"
+    notmuch tag +$tag '*'
+    output=$(notmuch count tag:$tag)
+    notmuch tag -$tag '*'
+    test_expect_equal "$output" '52'
+
+    test_begin_subtest "address ($config)"
+    notmuch address --deduplicate=no --sort=newest-first --output=sender --output=recipients path:foo >OUTPUT
+    cat <<EOF >EXPECTED
+Carl Worth <cworth@cworth.org>
+notmuch@notmuchmail.org
+EOF
+    test_expect_equal_file EXPECTED OUTPUT
+
+    test_begin_subtest "dump ($config)"
+    notmuch dump is:attachment and is:signed | sort > OUTPUT
+    cat <<EOF > EXPECTED
+#notmuch-dump batch-tag:3 config,properties,tags
++attachment +inbox +signed +unread -- id:20091118005829.GB25380@dottiness.seas.harvard.edu
++attachment +inbox +signed +unread -- id:20091118010116.GC25380@dottiness.seas.harvard.edu
+EOF
+    test_expect_equal_file EXPECTED OUTPUT
+
+    test_begin_subtest "dump + tag + restore ($config)"
+    notmuch dump '*' > EXPECTED
+    notmuch tag -inbox '*'
+    notmuch restore < EXPECTED
+    notmuch dump > OUTPUT
+    test_expect_equal_file EXPECTED OUTPUT
+
+    test_begin_subtest "reindex ($config)"
+    notmuch search --output=messages '*' > EXPECTED
+    notmuch reindex '*'
+    notmuch search --output=messages '*' > OUTPUT
+    test_expect_equal_file EXPECTED OUTPUT
+
+    restore_config
+done
+
+test_done
diff --git a/test/T590-libconfig.sh b/test/T590-libconfig.sh
index 4e510e97..5cf70987 100755
--- a/test/T590-libconfig.sh
+++ b/test/T590-libconfig.sh
@@ -364,6 +364,7 @@ EOF
 cat <<'EOF' >EXPECTED
 == stdout ==
 MAIL_DIR
+MAIL_DIR
 MAIL_DIR/.notmuch/hooks
 
 inbox;unread
-- 
2.30.1

[PATCH 15/22] lib/open: check for split configuration when creating database.

[David Bremner]

The main functionality will be tested when notmuch-new is converted to
support split configuration. Here only the somewhat odd case of split
mail root which is actually symlinked to the database path is tested.
---
 lib/open.cc              | 53 ++++++++++++++++++++++++++++++----------
 test/T055-path-config.sh | 15 ++++++++++--
 2 files changed, 53 insertions(+), 15 deletions(-)

diff --git a/lib/open.cc b/lib/open.cc
index fe42f7f0..2816f72a 100644
--- a/lib/open.cc
+++ b/lib/open.cc
@@ -545,11 +545,12 @@ notmuch_database_create_with_config (const char *database_path,
 {
     notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
     notmuch_database_t *notmuch = NULL;
-    char *notmuch_path = NULL;
+    const char *notmuch_path = NULL;
     char *message = NULL;
     GKeyFile *key_file = NULL;
     void *local = talloc_new (NULL);
     int err;
+    bool split = false;
 
     _init_libs ();
 
@@ -571,20 +572,38 @@ notmuch_database_create_with_config (const char *database_path,
 
     _set_database_path (notmuch, database_path);
 
-    notmuch_path = talloc_asprintf (local, "%s/%s", database_path, ".notmuch");
+    if (key_file && ! split) {
+	char *mail_root = canonicalize_file_name (
+	    g_key_file_get_value (key_file, "database", "mail_root", NULL));
+	char *db_path = canonicalize_file_name (database_path);
 
-    err = mkdir (notmuch_path, 0755);
-    if (err) {
-	if (errno == EEXIST) {
-	    status = NOTMUCH_STATUS_DATABASE_EXISTS;
-	    talloc_free (notmuch);
-	    notmuch = NULL;
-	} else {
-	    IGNORE_RESULT (asprintf (&message, "Error: Cannot create directory %s: %s.\n",
-				     notmuch_path, strerror (errno)));
-	    status = NOTMUCH_STATUS_FILE_ERROR;
+	split = (mail_root && (0 != strcmp (mail_root, db_path)));
+
+	free (mail_root);
+	free (db_path);
+    }
+
+    if (split) {
+	notmuch_path = database_path;
+    } else {
+	if (! (notmuch_path = talloc_asprintf (local, "%s/%s", database_path, ".notmuch"))) {
+	    status = NOTMUCH_STATUS_OUT_OF_MEMORY;
+	    goto DONE;
+	}
+
+	err = mkdir (notmuch_path, 0755);
+	if (err) {
+	    if (errno == EEXIST) {
+		status = NOTMUCH_STATUS_DATABASE_EXISTS;
+		talloc_free (notmuch);
+		notmuch = NULL;
+	    } else {
+		IGNORE_RESULT (asprintf (&message, "Error: Cannot create directory %s: %s.\n",
+					 notmuch_path, strerror (errno)));
+		status = NOTMUCH_STATUS_FILE_ERROR;
+	    }
+	    goto DONE;
 	}
-	goto DONE;
     }
 
     if (! (notmuch->xapian_path = talloc_asprintf (notmuch, "%s/%s", notmuch_path, "xapian"))) {
@@ -592,6 +611,14 @@ notmuch_database_create_with_config (const char *database_path,
 	goto DONE;
     }
 
+    status = _trial_open (notmuch->xapian_path, &message);
+    if (status == NOTMUCH_STATUS_SUCCESS) {
+	notmuch_database_destroy (notmuch);
+	notmuch = NULL;
+	status = NOTMUCH_STATUS_DATABASE_EXISTS;
+	goto DONE;
+    }
+
     status = _finish_open (notmuch,
 			   profile,
 			   NOTMUCH_DATABASE_MODE_READ_WRITE,
diff --git a/test/T055-path-config.sh b/test/T055-path-config.sh
index c6920ca9..19a8a4ab 100755
--- a/test/T055-path-config.sh
+++ b/test/T055-path-config.sh
@@ -27,9 +27,17 @@ split_config () {
     DATABASE_PATH=$dir
 }
 
+symlink_config () {
+    local dir
+    backup_config
+    dir="$TMP_DIRECTORY/link.$test_count"
+    ln -s $MAIL_DIR $dir
+    notmuch config set database.path $dir
+    notmuch config set database.mail_root $MAIL_DIR
+    unset DATABASE_PATH
+}
 
-
-for config in traditional split; do
+for config in traditional split symlink; do
     # start each set of tests with a known set of messages
     add_email_corpus
 
@@ -41,6 +49,9 @@ for config in traditional split; do
 	    split_config
 	    mv mail/.notmuch/xapian $DATABASE_PATH
 	    ;;
+	symlink)
+	    symlink_config
+	    ;;
     esac
 
     test_begin_subtest "count ($config)"
-- 
2.30.1

[PATCH 16/22] CLI/new: support split database and mail location

[David Bremner]

This adds new state variable for the mail root, and uses it most
places db_path was used. The notable exception is dumps from
backups. The latter will be dealt with properly in a future commit.
---
 notmuch-new.c            | 54 ++++++++++++++++++++++++++--------------
 test/T055-path-config.sh | 41 ++++++++++++++++++++++++++++++
 2 files changed, 76 insertions(+), 19 deletions(-)

diff --git a/notmuch-new.c b/notmuch-new.c
index 223d68bb..30062512 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -43,6 +43,7 @@ enum verbosity {
 
 typedef struct {
     const char *db_path;
+    const char *mail_root;
 
     int output_is_a_tty;
     enum verbosity verbosity;
@@ -307,18 +308,18 @@ _setup_ignore (notmuch_database_t *notmuch, add_files_state_t *state)
 }
 
 static char *
-_get_relative_path (const char *db_path, const char *dirpath, const char *entry)
+_get_relative_path (const char *mail_root, const char *dirpath, const char *entry)
 {
-    size_t db_path_len = strlen (db_path);
+    size_t mail_root_len = strlen (mail_root);
 
     /* paranoia? */
-    if (strncmp (dirpath, db_path, db_path_len) != 0) {
+    if (strncmp (dirpath, mail_root, mail_root_len) != 0) {
 	fprintf (stderr, "Warning: '%s' is not a subdirectory of '%s'\n",
-		 dirpath, db_path);
+		 dirpath, mail_root);
 	return NULL;
     }
 
-    dirpath += db_path_len;
+    dirpath += mail_root_len;
     while (*dirpath == '/')
 	dirpath++;
 
@@ -346,7 +347,7 @@ _entry_in_ignore_list (add_files_state_t *state, const char *dirpath,
     if (state->ignore_regex_length == 0)
 	return false;
 
-    path = _get_relative_path (state->db_path, dirpath, entry);
+    path = _get_relative_path (state->mail_root, dirpath, entry);
     if (! path)
 	return false;
 
@@ -1050,22 +1051,34 @@ _maybe_upgrade (notmuch_database_t *notmuch, add_files_state_t *state)
     if (notmuch_database_needs_upgrade (notmuch)) {
 	time_t now = time (NULL);
 	struct tm *gm_time = gmtime (&now);
+	struct stat st;
+	int err;
 	notmuch_status_t status;
 	char *dot_notmuch_path = talloc_asprintf (notmuch, "%s/%s", state->db_path, ".notmuch");
 
+	const char *backup_name;
+
+	err = stat (dot_notmuch_path, &st);
+	if (err) {
+	    if (errno == ENOENT) {
+		dot_notmuch_path = NULL;
+	    } else {
+		fprintf (stderr, "Failed to stat %s: %s\n", dot_notmuch_path, strerror (errno));
+		return EXIT_FAILURE;
+	    }
+	}
+
 	/* since dump files are written atomically, the amount of
 	 * harm from overwriting one within a second seems
 	 * relatively small. */
-
-	const char *backup_name =
-	    talloc_asprintf (notmuch, "%s/dump-%04d%02d%02dT%02d%02d%02d.gz",
-			     dot_notmuch_path,
-			     gm_time->tm_year + 1900,
-			     gm_time->tm_mon + 1,
-			     gm_time->tm_mday,
-			     gm_time->tm_hour,
-			     gm_time->tm_min,
-			     gm_time->tm_sec);
+	backup_name = talloc_asprintf (notmuch, "%s/dump-%04d%02d%02dT%02d%02d%02d.gz",
+				       dot_notmuch_path ? dot_notmuch_path : state->db_path,
+				       gm_time->tm_year + 1900,
+				       gm_time->tm_mon + 1,
+				       gm_time->tm_mday,
+				       gm_time->tm_hour,
+				       gm_time->tm_min,
+				       gm_time->tm_sec);
 
 	if (state->verbosity >= VERBOSITY_NORMAL) {
 	    printf ("Welcome to a new version of notmuch! Your database will now be upgraded.\n");
@@ -1108,7 +1121,7 @@ notmuch_new_command (unused(notmuch_config_t *config), notmuch_database_t *notmu
     };
     struct timeval tv_start;
     int ret = 0;
-    const char *db_path;
+    const char *db_path, *mail_root;
     struct sigaction action;
     _filename_node_t *f;
     int opt_index;
@@ -1153,6 +1166,9 @@ notmuch_new_command (unused(notmuch_config_t *config), notmuch_database_t *notmu
     db_path = notmuch_config_get (notmuch, NOTMUCH_CONFIG_DATABASE_PATH);
     add_files_state.db_path = db_path;
 
+    mail_root = notmuch_config_get (notmuch, NOTMUCH_CONFIG_MAIL_ROOT);
+    add_files_state.mail_root = mail_root;
+
     if (! _setup_ignore (notmuch, &add_files_state))
 	return EXIT_FAILURE;
 
@@ -1179,7 +1195,7 @@ notmuch_new_command (unused(notmuch_config_t *config), notmuch_database_t *notmu
 
     if (notmuch_database_get_revision (notmuch, NULL) == 0) {
 	int count = 0;
-	count_files (db_path, &count, &add_files_state);
+	count_files (mail_root, &count, &add_files_state);
 	if (interrupted)
 	    return EXIT_FAILURE;
 
@@ -1225,7 +1241,7 @@ notmuch_new_command (unused(notmuch_config_t *config), notmuch_database_t *notmu
 	timer_is_active = true;
     }
 
-    ret = add_files (notmuch, db_path, &add_files_state);
+    ret = add_files (notmuch, mail_root, &add_files_state);
     if (ret)
 	goto DONE;
 
diff --git a/test/T055-path-config.sh b/test/T055-path-config.sh
index 19a8a4ab..6df17f80 100755
--- a/test/T055-path-config.sh
+++ b/test/T055-path-config.sh
@@ -95,6 +95,47 @@ EOF
     notmuch search --output=messages '*' > OUTPUT
     test_expect_equal_file EXPECTED OUTPUT
 
+    test_begin_subtest "use existing database ($config)"
+    output=$(notmuch new)
+    test_expect_equal "$output" 'No new mail.'
+
+    test_begin_subtest "create database ($config)"
+    rm -rf $DATABASE_PATH/{.notmuch,}/xapian
+    notmuch new
+    output=$(notmuch count '*')
+    test_expect_equal "$output" '52'
+
+    test_begin_subtest "detect new files ($config)"
+    generate_message
+    generate_message
+    notmuch new
+    output=$(notmuch count '*')
+    test_expect_equal "$output" '54'
+
+    test_begin_subtest "Show a raw message ($config)"
+    add_message
+    notmuch show --format=raw id:$gen_msg_id > OUTPUT
+    test_expect_equal_file $gen_msg_filename OUTPUT
+    rm -f $gen_msg_filename
+
+    test_begin_subtest "reply ($config)"
+    add_message '[from]="Sender <sender@example.com>"' \
+		[to]=test_suite@notmuchmail.org \
+		[subject]=notmuch-reply-test \
+		'[date]="Tue, 05 Jan 2010 15:43:56 -0000"' \
+		'[body]="basic reply test"'
+    notmuch reply id:${gen_msg_id} 2>&1 > OUTPUT
+    cat <<EOF > EXPECTED
+From: Notmuch Test Suite <test_suite@notmuchmail.org>
+Subject: Re: notmuch-reply-test
+To: Sender <sender@example.com>
+In-Reply-To: <${gen_msg_id}>
+References: <${gen_msg_id}>
+
+On Tue, 05 Jan 2010 15:43:56 -0000, Sender <sender@example.com> wrote:
+> basic reply test
+EOF
+    test_expect_equal_file EXPECTED OUTPUT
     restore_config
 done
 
-- 
2.30.1

[PATCH 17/22] lib/open: support XDG_DATA_HOME as a fallback database location.

[David Bremner]

This changes some error reporting, either intentionally by reporting
the highest level missing directory, or by side effect from looking in
XDG locations when given null database location.
---
 lib/open.cc              | 13 +++++++++++--
 test/T055-path-config.sh | 35 +++++++++++++++++++++++++++++++++--
 test/T560-lib-error.sh   |  4 ++--
 test/T590-libconfig.sh   |  4 ++--
 4 files changed, 48 insertions(+), 8 deletions(-)

diff --git a/lib/open.cc b/lib/open.cc
index 2816f72a..3097121e 100644
--- a/lib/open.cc
+++ b/lib/open.cc
@@ -181,6 +181,7 @@ _choose_database_path (void *ctx,
 		       const char *profile,
 		       GKeyFile **key_file,
 		       const char **database_path,
+		       bool *split,
 		       char **message)
 {
     notmuch_status_t status;
@@ -203,6 +204,11 @@ _choose_database_path (void *ctx,
 	}
     }
 
+    if (! *database_path) {
+	*database_path = _xdg_dir (ctx, "XDG_DATA_HOME", ".local/share", profile);
+	*split = true;
+    }
+
     if (*database_path == NULL) {
 	*message = strdup ("Error: Cannot open a database for a NULL path.\n");
 	return NOTMUCH_STATUS_NULL_POINTER;
@@ -462,6 +468,7 @@ notmuch_database_open_with_config (const char *database_path,
     notmuch_database_t *notmuch = NULL;
     char *message = NULL;
     GKeyFile *key_file = NULL;
+    bool split = false;
 
     _init_libs ();
 
@@ -471,7 +478,8 @@ notmuch_database_open_with_config (const char *database_path,
 	goto DONE;
     }
 
-    if ((status = _choose_database_path (local, config_path, profile, &key_file, &database_path,
+    if ((status = _choose_database_path (local, config_path, profile,
+					 &key_file, &database_path, &split,
 					 &message)))
 	goto DONE;
 
@@ -563,7 +571,8 @@ notmuch_database_create_with_config (const char *database_path,
     _init_libs ();
 
     if ((status = _choose_database_path (local, config_path, profile,
-					 &key_file, &database_path, &message)))
+					 &key_file, &database_path, &split,
+					 &message)))
 	goto DONE;
 
     status = _db_dir_exists (database_path, &message);
diff --git a/test/T055-path-config.sh b/test/T055-path-config.sh
index 6df17f80..f9a8e200 100755
--- a/test/T055-path-config.sh
+++ b/test/T055-path-config.sh
@@ -37,8 +37,31 @@ symlink_config () {
     unset DATABASE_PATH
 }
 
-for config in traditional split symlink; do
-    # start each set of tests with a known set of messages
+xdg_config () {
+    local dir
+    local profile=${1:-default}
+
+    if [[ $profile != default ]]; then
+	export NOTMUCH_PROFILE=$profile
+    fi
+
+    backup_config
+    DATABASE_PATH="${HOME}/.local/share/notmuch/${profile}"
+    rm -rf $DATABASE_PATH
+    mkdir -p $DATABASE_PATH
+
+    config_dir="${HOME}/.config/notmuch/${profile}"
+    mkdir -p ${config_dir}
+    CONFIG_PATH=$config_dir/config
+    mv ${NOTMUCH_CONFIG} $CONFIG_PATH
+    unset NOTMUCH_CONFIG
+
+    notmuch --config=${CONFIG_PATH} config set database.mail_root ${TMP_DIRECTORY}/mail
+    notmuch --config=${CONFIG_PATH} config set database.path
+}
+
+for config in traditional split XDG XDG+profile symlink; do
+    #start each set of tests with an known set of messages
     add_email_corpus
 
     case $config in
@@ -49,6 +72,14 @@ for config in traditional split symlink; do
 	    split_config
 	    mv mail/.notmuch/xapian $DATABASE_PATH
 	    ;;
+	XDG)
+	    xdg_config
+	    mv mail/.notmuch/xapian $DATABASE_PATH
+	    ;;
+	XDG+profile)
+	    xdg_config ${RANDOM}
+	    mv mail/.notmuch/xapian $DATABASE_PATH
+	    ;;
 	symlink)
 	    symlink_config
 	    ;;
diff --git a/test/T560-lib-error.sh b/test/T560-lib-error.sh
index 03df69d9..89447e9a 100755
--- a/test/T560-lib-error.sh
+++ b/test/T560-lib-error.sh
@@ -22,7 +22,7 @@ EOF
 cat <<'EOF' >EXPECTED
 == stdout ==
 == stderr ==
-Error: Cannot open a database for a NULL path.
+Error: Cannot open database at CWD/home/.local/share/notmuch/default: No such file or directory.
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
@@ -93,7 +93,7 @@ EOF
 cat <<'EOF' >EXPECTED
 == stdout ==
 == stderr ==
-Error: Cannot open a database for a NULL path.
+Error: Cannot open database at CWD/home/.local/share/notmuch/default: No such file or directory.
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
diff --git a/test/T590-libconfig.sh b/test/T590-libconfig.sh
index 5cf70987..c21c139b 100755
--- a/test/T590-libconfig.sh
+++ b/test/T590-libconfig.sh
@@ -519,8 +519,8 @@ cat <<'EOF' >EXPECTED
 == stdout ==
 == stderr ==
 error opening database
-Erroneous NULL pointer
-Error: Cannot open a database for a NULL path.
+Something went wrong trying to read or write a file
+Error: Cannot open database at CWD/home/.local/share/notmuch/default: No such file or directory.
 
 EOF
 test_expect_equal_file EXPECTED OUTPUT
-- 
2.30.1

[PATCH 18/22] CLI/insert: support split database and mail root

[David Bremner]

The new test is in T055-path-config because it uses the helper
function split_config, and because it seems easier to put the
database path related tests in one place.
---
 notmuch-insert.c         | 12 +++---------
 test/T055-path-config.sh | 10 ++++++++++
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/notmuch-insert.c b/notmuch-insert.c
index cc5aeefc..865b6b69 100644
--- a/notmuch-insert.c
+++ b/notmuch-insert.c
@@ -449,7 +449,7 @@ notmuch_insert_command (unused(notmuch_config_t *config), notmuch_database_t *no
 {
     notmuch_status_t status, close_status;
     struct sigaction action;
-    const char *db_path;
+    const char *mail_root;
     notmuch_config_values_t *new_tags = NULL;
     tag_op_list_t *tag_ops;
     char *query_string = NULL;
@@ -481,13 +481,7 @@ notmuch_insert_command (unused(notmuch_config_t *config), notmuch_database_t *no
 
     notmuch_process_shared_options (argv[0]);
 
-
-    db_path = notmuch_config_get (notmuch, NOTMUCH_CONFIG_DATABASE_PATH);
-
-    if (! db_path)
-	INTERNAL_ERROR ("Unable to retrieve database path");
-    else
-	db_path = talloc_strdup (local, db_path);
+    mail_root = notmuch_config_get (notmuch, NOTMUCH_CONFIG_MAIL_ROOT);
 
     new_tags = notmuch_config_get_values (notmuch, NOTMUCH_CONFIG_NEW_TAGS);
 
@@ -533,7 +527,7 @@ notmuch_insert_command (unused(notmuch_config_t *config), notmuch_database_t *no
 	return EXIT_FAILURE;
     }
 
-    maildir = talloc_asprintf (local, "%s/%s", db_path, folder);
+    maildir = talloc_asprintf (local, "%s/%s", mail_root, folder);
     if (! maildir) {
 	fprintf (stderr, "Out of memory\n");
 	return EXIT_FAILURE;
diff --git a/test/T055-path-config.sh b/test/T055-path-config.sh
index f9a8e200..fb5174ac 100755
--- a/test/T055-path-config.sh
+++ b/test/T055-path-config.sh
@@ -167,6 +167,16 @@ On Tue, 05 Jan 2010 15:43:56 -0000, Sender <sender@example.com> wrote:
 > basic reply test
 EOF
     test_expect_equal_file EXPECTED OUTPUT
+    test_begin_subtest "insert+search ($config)"
+    generate_message \
+	"[subject]=\"insert-subject\"" \
+	"[date]=\"Sat, 01 Jan 2000 12:00:00 -0000\"" \
+	"[body]=\"insert-message\""
+    mkdir -p "$MAIL_DIR"/{cur,new,tmp}
+    notmuch insert < "$gen_msg_filename"
+    cur_msg_filename=$(notmuch search --output=files "subject:insert-subject")
+    test_expect_equal_file "$cur_msg_filename" "$gen_msg_filename"
+
     restore_config
 done
 
-- 
2.30.1

[PATCH 19/22] lib/compact: enable split config

[David Bremner]

This promotes _choose_xapian_path from static to extern linkage in
order to share between open.cc and database.cc.
---
 lib/database.cc          | 13 ++++---------
 lib/notmuch-private.h    |  5 +++++
 lib/open.cc              |  9 +++++----
 test/T055-path-config.sh |  7 +++++++
 4 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 248d1dc2..9743c1ca 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -610,11 +610,12 @@ notmuch_database_compact_db (notmuch_database_t *notmuch,
 			     void *closure)
 {
     void *local;
-    char *notmuch_path, *xapian_path, *compact_xapian_path;
+    const char *xapian_path, *compact_xapian_path;
     const char *path;
     notmuch_status_t ret = NOTMUCH_STATUS_SUCCESS;
     struct stat statbuf;
     bool keep_backup;
+    char *message;
 
     ret = _notmuch_database_ensure_writable (notmuch);
     if (ret)
@@ -628,15 +629,9 @@ notmuch_database_compact_db (notmuch_database_t *notmuch,
     if (! local)
 	return NOTMUCH_STATUS_OUT_OF_MEMORY;
 
-    if (! (notmuch_path = talloc_asprintf (local, "%s/%s", path, ".notmuch"))) {
-	ret = NOTMUCH_STATUS_OUT_OF_MEMORY;
-	goto DONE;
-    }
-
-    if (! (xapian_path = talloc_asprintf (local, "%s/%s", notmuch_path, "xapian"))) {
-	ret = NOTMUCH_STATUS_OUT_OF_MEMORY;
+    ret = _notmuch_choose_xapian_path (local, path, &xapian_path, &message);
+    if (ret)
 	goto DONE;
-    }
 
     if (! (compact_xapian_path = talloc_asprintf (local, "%s.compact", xapian_path))) {
 	ret = NOTMUCH_STATUS_OUT_OF_MEMORY;
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index ef814378..387f0c1e 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -724,6 +724,11 @@ _notmuch_config_load_defaults (notmuch_database_t *db);
 void
 _notmuch_config_cache (notmuch_database_t *db, notmuch_config_key_t key, const char *val);
 
+/* open.cc */
+notmuch_status_t
+_notmuch_choose_xapian_path (void *ctx, const char *database_path, const char **xapian_path,
+			     char **message);
+
 NOTMUCH_END_DECLS
 
 #ifdef __cplusplus
diff --git a/lib/open.cc b/lib/open.cc
index 3097121e..8321a3be 100644
--- a/lib/open.cc
+++ b/lib/open.cc
@@ -258,9 +258,9 @@ _trial_open (const char *xapian_path, char **message_ptr)
     return NOTMUCH_STATUS_SUCCESS;
 }
 
-static notmuch_status_t
-_choose_xapian_path (void *ctx, const char *database_path, const char **xapian_path,
-		     char **message_ptr)
+notmuch_status_t
+_notmuch_choose_xapian_path (void *ctx, const char *database_path,
+			     const char **xapian_path, char **message_ptr)
 {
     notmuch_status_t status;
     const char *trial_path, *notmuch_path;
@@ -489,7 +489,8 @@ notmuch_database_open_with_config (const char *database_path,
 
     _set_database_path (notmuch, database_path);
 
-    status = _choose_xapian_path (local, database_path, &notmuch->xapian_path, &message);
+    status = _notmuch_choose_xapian_path (local, database_path,
+					  &notmuch->xapian_path, &message);
     if (status)
 	goto DONE;
 
diff --git a/test/T055-path-config.sh b/test/T055-path-config.sh
index fb5174ac..e4812c82 100755
--- a/test/T055-path-config.sh
+++ b/test/T055-path-config.sh
@@ -177,6 +177,13 @@ EOF
     cur_msg_filename=$(notmuch search --output=files "subject:insert-subject")
     test_expect_equal_file "$cur_msg_filename" "$gen_msg_filename"
 
+
+    test_begin_subtest "compact+search ($config)"
+    notmuch search --output=messages '*' | sort > EXPECTED
+    notmuch compact
+    notmuch search --output=messages '*' | sort > OUTPUT
+    test_expect_equal_file EXPECTED OUTPUT
+
     restore_config
 done
 
-- 
2.30.1

[PATCH 20/22] lib/open: fix hook directory calculation in split configuration

[David Bremner]

Choose sibling directory of xapian database, as .notmuch may not
exist.

libgen.h is already used in debugger.c, so it is not a new dependency
/ potential portability problem.
---
 lib/open.cc        | 6 ++++--
 test/T400-hooks.sh | 9 ++++++++-
 2 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/lib/open.cc b/lib/open.cc
index 8321a3be..aad7fa77 100644
--- a/lib/open.cc
+++ b/lib/open.cc
@@ -1,4 +1,6 @@
 #include <unistd.h>
+#include <libgen.h>
+
 #include "database-private.h"
 #include "parse-time-vrp.h"
 
@@ -92,8 +94,8 @@ _choose_hook_dir (notmuch_database_t *notmuch,
     err = stat (hook_dir, &st);
     if (err) {
 	if (errno == ENOENT) {
-	    const char *database_path = notmuch_database_get_path (notmuch);
-	    hook_dir = talloc_asprintf (notmuch, "%s/.notmuch/hooks", database_path);
+	    char *notmuch_path = dirname (talloc_strdup (notmuch, notmuch->xapian_path));
+	    hook_dir = talloc_asprintf (notmuch, "%s/hooks", notmuch_path);
 	} else {
 	    IGNORE_RESULT (asprintf (message, "Error: Cannot stat %s: %s.\n",
 				     hook_dir, strerror (errno)));
diff --git a/test/T400-hooks.sh b/test/T400-hooks.sh
index a3dd4c63..8267d057 100755
--- a/test/T400-hooks.sh
+++ b/test/T400-hooks.sh
@@ -28,9 +28,10 @@ add_message
 # create maildir structure for notmuch-insert
 mkdir -p "$MAIL_DIR"/{cur,new,tmp}
 
-for config in traditional profile explicit XDG; do
+for config in traditional profile explicit XDG split; do
     unset NOTMUCH_PROFILE
     notmuch config set database.hook_dir
+    notmuch config set database.path ${MAIL_DIR}
     case $config in
 	traditional)
 	    HOOK_DIR=${MAIL_DIR}/.notmuch/hooks
@@ -50,6 +51,12 @@ for config in traditional profile explicit XDG; do
 	XDG)
 	    HOOK_DIR=${HOME}/.config/notmuch/default/hooks
 	    ;;
+	split)
+	    dir="$TMP_DIRECTORY/database.$test_count"
+	    notmuch config set database.path $dir
+	    notmuch config set database.mail_root $MAIL_DIR
+	    HOOK_DIR=${dir}/hooks
+	    ;;
     esac
 
     test_begin_subtest "pre-new is run [${config}]"
-- 
2.30.1

[PATCH 21/22] lib/config: add configuration variable for backup directory

[David Bremner]

Like the hook directory, we primarily need a way to communicate this
directory between various components, but we may as well let the user
configure it.

Most of the diff is generalizing choose_hook_dir to work for both
backup and hook directories.
---
 lib/config.cc          |  3 +++
 lib/notmuch.h          |  1 +
 lib/open.cc            | 47 ++++++++++++++++++++++++++++--------------
 test/T590-libconfig.sh |  1 +
 4 files changed, 37 insertions(+), 15 deletions(-)

diff --git a/lib/config.cc b/lib/config.cc
index f2060e28..8bd3c35a 100644
--- a/lib/config.cc
+++ b/lib/config.cc
@@ -410,6 +410,8 @@ _notmuch_config_key_to_string (notmuch_config_key_t key)
 	return "database.mail_root";
     case NOTMUCH_CONFIG_HOOK_DIR:
 	return "database.hook_dir";
+    case NOTMUCH_CONFIG_BACKUP_DIR:
+	return "database.backup_dir";
     case NOTMUCH_CONFIG_EXCLUDE_TAGS:
 	return "search.exclude_tags";
     case NOTMUCH_CONFIG_NEW_TAGS:
@@ -453,6 +455,7 @@ _notmuch_config_default (notmuch_database_t *notmuch, notmuch_config_key_t key)
     case NOTMUCH_CONFIG_SYNC_MAILDIR_FLAGS:
 	return "true";
     case NOTMUCH_CONFIG_HOOK_DIR:
+    case NOTMUCH_CONFIG_BACKUP_DIR:
     case NOTMUCH_CONFIG_NEW_IGNORE:
     case NOTMUCH_CONFIG_USER_NAME:
     case NOTMUCH_CONFIG_PRIMARY_EMAIL:
diff --git a/lib/notmuch.h b/lib/notmuch.h
index c9b01719..f9efd79e 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -2476,6 +2476,7 @@ typedef enum _notmuch_config_key {
     NOTMUCH_CONFIG_DATABASE_PATH = NOTMUCH_CONFIG_FIRST,
     NOTMUCH_CONFIG_MAIL_ROOT,
     NOTMUCH_CONFIG_HOOK_DIR,
+    NOTMUCH_CONFIG_BACKUP_DIR,
     NOTMUCH_CONFIG_EXCLUDE_TAGS,
     NOTMUCH_CONFIG_NEW_TAGS,
     NOTMUCH_CONFIG_NEW_IGNORE,
diff --git a/lib/open.cc b/lib/open.cc
index aad7fa77..c9390359 100644
--- a/lib/open.cc
+++ b/lib/open.cc
@@ -71,39 +71,43 @@ _xdg_dir (void *ctx,
 }
 
 static notmuch_status_t
-_choose_hook_dir (notmuch_database_t *notmuch,
-		  const char *profile,
-		  char **message)
+_choose_dir (notmuch_database_t *notmuch,
+	     const char *profile,
+	     notmuch_config_key_t key,
+	     const char *xdg_var,
+	     const char *xdg_subdir,
+	     const char *subdir,
+	     char **message = NULL)
 {
-    const char *config;
-    const char *hook_dir;
+    const char *parent;
+    const char *dir;
     struct stat st;
     int err;
 
-    hook_dir = notmuch_config_get (notmuch, NOTMUCH_CONFIG_HOOK_DIR);
+    dir = notmuch_config_get (notmuch, key);
 
-    if (hook_dir)
+    if (dir)
 	return NOTMUCH_STATUS_SUCCESS;
 
-    config = _xdg_dir (notmuch, "XDG_CONFIG_HOME", ".config", profile);
-    if (! config)
+    parent = _xdg_dir (notmuch, xdg_var, xdg_subdir, profile);
+    if (! parent)
 	return NOTMUCH_STATUS_PATH_ERROR;
 
-    hook_dir = talloc_asprintf (notmuch, "%s/hooks", config);
+    dir = talloc_asprintf (notmuch, "%s/%s", parent, subdir);
 
-    err = stat (hook_dir, &st);
+    err = stat (dir, &st);
     if (err) {
 	if (errno == ENOENT) {
 	    char *notmuch_path = dirname (talloc_strdup (notmuch, notmuch->xapian_path));
-	    hook_dir = talloc_asprintf (notmuch, "%s/hooks", notmuch_path);
+	    dir = talloc_asprintf (notmuch, "%s/%s", notmuch_path, subdir);
 	} else {
 	    IGNORE_RESULT (asprintf (message, "Error: Cannot stat %s: %s.\n",
-				     hook_dir, strerror (errno)));
+				     dir, strerror (errno)));
 	    return NOTMUCH_STATUS_FILE_ERROR;
 	}
     }
 
-    _notmuch_config_cache (notmuch, NOTMUCH_CONFIG_HOOK_DIR, hook_dir);
+    _notmuch_config_cache (notmuch, key, dir);
 
     return NOTMUCH_STATUS_SUCCESS;
 }
@@ -428,10 +432,23 @@ _finish_open (notmuch_database_t *notmuch,
 	if (status)
 	    goto DONE;
 
-	status = _choose_hook_dir (notmuch, profile, &message);
+	status = _choose_dir (notmuch, profile,
+			      NOTMUCH_CONFIG_HOOK_DIR,
+			      "XDG_CONFIG_HOME",
+			      ".config",
+			      "hooks",
+			      &message);
 	if (status)
 	    goto DONE;
 
+	status = _choose_dir (notmuch, profile,
+			      NOTMUCH_CONFIG_BACKUP_DIR,
+			      "XDG_DATA_HOME",
+			      ".local/share",
+			      "backups",
+			      &message);
+	if (status)
+	    goto DONE;
 	status = _notmuch_config_load_defaults (notmuch);
 	if (status)
 	    goto DONE;
diff --git a/test/T590-libconfig.sh b/test/T590-libconfig.sh
index c21c139b..310668a9 100755
--- a/test/T590-libconfig.sh
+++ b/test/T590-libconfig.sh
@@ -366,6 +366,7 @@ cat <<'EOF' >EXPECTED
 MAIL_DIR
 MAIL_DIR
 MAIL_DIR/.notmuch/hooks
+MAIL_DIR/.notmuch/backups
 
 inbox;unread
 NULL
-- 
2.30.1

[PATCH 22/22] CLI/new: use configuration variable for backup directory

[David Bremner]

The stat is essentially replaced by the mkdir for error detection
purposes.  This changes the default location for backups to make
things tidier, even in non-split configurations. Hopefully there is
not too many user scripts relying on the previous location.

Because the default location may not exist, replace the use of stat
for error detection with a call to mkdir.
---
 notmuch-new.c            | 18 ++++++------------
 test/T055-path-config.sh | 15 +++++++++++++++
 test/T530-upgrade.sh     |  2 +-
 3 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/notmuch-new.c b/notmuch-new.c
index 30062512..bae28109 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -1051,28 +1051,22 @@ _maybe_upgrade (notmuch_database_t *notmuch, add_files_state_t *state)
     if (notmuch_database_needs_upgrade (notmuch)) {
 	time_t now = time (NULL);
 	struct tm *gm_time = gmtime (&now);
-	struct stat st;
 	int err;
 	notmuch_status_t status;
-	char *dot_notmuch_path = talloc_asprintf (notmuch, "%s/%s", state->db_path, ".notmuch");
-
+	const char *backup_dir = notmuch_config_get (notmuch, NOTMUCH_CONFIG_BACKUP_DIR);
 	const char *backup_name;
 
-	err = stat (dot_notmuch_path, &st);
-	if (err) {
-	    if (errno == ENOENT) {
-		dot_notmuch_path = NULL;
-	    } else {
-		fprintf (stderr, "Failed to stat %s: %s\n", dot_notmuch_path, strerror (errno));
-		return EXIT_FAILURE;
-	    }
+	err = mkdir (backup_dir, 0755);
+	if (err && errno != EEXIST) {
+	    fprintf (stderr, "Failed to create %s: %s\n", backup_dir, strerror (errno));
+	    return EXIT_FAILURE;
 	}
 
 	/* since dump files are written atomically, the amount of
 	 * harm from overwriting one within a second seems
 	 * relatively small. */
 	backup_name = talloc_asprintf (notmuch, "%s/dump-%04d%02d%02dT%02d%02d%02d.gz",
-				       dot_notmuch_path ? dot_notmuch_path : state->db_path,
+				       backup_dir,
 				       gm_time->tm_year + 1900,
 				       gm_time->tm_mon + 1,
 				       gm_time->tm_mday,
diff --git a/test/T055-path-config.sh b/test/T055-path-config.sh
index e4812c82..d8828342 100755
--- a/test/T055-path-config.sh
+++ b/test/T055-path-config.sh
@@ -2,6 +2,8 @@
 test_description='Configuration of mail-root and database path'
 . $(dirname "$0")/test-lib.sh || exit 1
 
+test_require_external_prereq xapian-metdata
+
 backup_config () {
     local test_name=$(basename $0 .sh)
     cp ${NOTMUCH_CONFIG} notmuch-config-backup.${test_name}
@@ -13,6 +15,7 @@ restore_config () {
     unset CONFIG_PATH
     unset DATABASE_PATH
     unset NOTMUCH_PROFILE
+    unset XAPIAN_PATH
     cp notmuch-config-backup.${test_name} ${NOTMUCH_CONFIG}
 }
 
@@ -25,6 +28,7 @@ split_config () {
     notmuch config set database.path $dir
     notmuch config set database.mail_root $MAIL_DIR
     DATABASE_PATH=$dir
+    XAPIAN_PATH="$dir/xapian"
 }
 
 symlink_config () {
@@ -34,6 +38,7 @@ symlink_config () {
     ln -s $MAIL_DIR $dir
     notmuch config set database.path $dir
     notmuch config set database.mail_root $MAIL_DIR
+    XAPIAN_PATH="$MAIL_DIR/.notmuch/xapian"
     unset DATABASE_PATH
 }
 
@@ -56,6 +61,7 @@ xdg_config () {
     mv ${NOTMUCH_CONFIG} $CONFIG_PATH
     unset NOTMUCH_CONFIG
 
+    XAPIAN_PATH="${DATABASE_PATH}/xapian"
     notmuch --config=${CONFIG_PATH} config set database.mail_root ${TMP_DIRECTORY}/mail
     notmuch --config=${CONFIG_PATH} config set database.path
 }
@@ -67,6 +73,7 @@ for config in traditional split XDG XDG+profile symlink; do
     case $config in
 	traditional)
 	    backup_config
+	    XAPIAN_PATH="$MAIL_DIR/.notmuch/xapian"
 	    ;;
 	split)
 	    split_config
@@ -184,6 +191,14 @@ EOF
     notmuch search --output=messages '*' | sort > OUTPUT
     test_expect_equal_file EXPECTED OUTPUT
 
+    test_begin_subtest "upgrade backup ($config)"
+    features=$(xapian-metadata get $XAPIAN_PATH features | grep -v "^relative directory paths")
+    xapian-metadata set $XAPIAN_PATH features "$features"
+    output=$(notmuch new | grep Welcome)
+    test_expect_equal \
+	"$output" \
+	"Welcome to a new version of notmuch! Your database will now be upgraded."
+
     restore_config
 done
 
diff --git a/test/T530-upgrade.sh b/test/T530-upgrade.sh
index c599dacf..cce29f45 100755
--- a/test/T530-upgrade.sh
+++ b/test/T530-upgrade.sh
@@ -5,7 +5,7 @@ test_description='database upgrades'
 test_require_external_prereq xapian-metadata
 
 XAPIAN_PATH=$MAIL_DIR/.notmuch/xapian
-BACKUP_PATH=$MAIL_DIR/.notmuch
+BACKUP_PATH=$MAIL_DIR/.notmuch/backups
 
 delete_feature () {
     local key=$1
-- 
2.30.1

Re: v4 flexible path location

[David Bremner]

David Bremner <david@tethera.net> writes:

> Since v3 [1]
>
> - rebase against the uncrustified version (unfortunately this makes an
>   interdiff too noisy to be helpful).
> - improve (?) the documentation of database.mail_root in notmuch-config(1).
> - tweak commit messages
> - move the call to _init_libs from [10/22] to [9/22]
>
> I'm going to mark this version as ready to apply, so object now, or
> grumble later.

Rebasing this on top of the hook fix, I noticed the following

diff --git a/lib/open.cc b/lib/open.cc
index c9390359..8f572b0b 100644
--- a/lib/open.cc
+++ b/lib/open.cc
@@ -508,7 +508,7 @@ notmuch_database_open_with_config (const char *database_path,
 
     _set_database_path (notmuch, database_path);
 
-    status = _notmuch_choose_xapian_path (local, database_path,
+    status = _notmuch_choose_xapian_path (notmuch, database_path,
                                          &notmuch->xapian_path, &message);
     if (status)
        goto DONE;

Like all good memory bugs, this was mysteriously undetected until reopen
was called multiple times in the hook fix.

This is now fixed in my git repo. I'll hold off posting another series
for now.

d

Re: v4 flexible path location

[David Bremner]

David Bremner <david@tethera.net> writes:

> Since v3 [1]
>
> - rebase against the uncrustified version (unfortunately this makes an
>   interdiff too noisy to be helpful).
> - improve (?) the documentation of database.mail_root in notmuch-config(1).
> - tweak commit messages
> - move the call to _init_libs from [10/22] to [9/22]
>
> I'm going to mark this version as ready to apply, so object now, or
> grumble later.
>

As promised, I have now applied the updated version of this series to
master.

d