[PATCH 1/2] test/libconfig: save and restore config file

[PATCH 1/2] test/libconfig: save and restore config file

[David Bremner]

Currently the config file is unusable for further tests requiring a
valid database path.
---
 test/T590-libconfig.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/test/T590-libconfig.sh b/test/T590-libconfig.sh
index eb303444..089b4e58 100755
--- a/test/T590-libconfig.sh
+++ b/test/T590-libconfig.sh
@@ -972,6 +972,7 @@ EOF
 test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "open: database parameter overrides implicit config"
+cp $NOTMUCH_CONFIG ${NOTMUCH_CONFIG}.bak
 notmuch config set database.path ${MAIL_DIR}/nonexistent
 cat c_head3 - c_tail3 <<'EOF' | test_C ${MAIL_DIR}
   const char *path = NULL;
@@ -982,6 +983,7 @@ cat c_head3 - c_tail3 <<'EOF' | test_C ${MAIL_DIR}
   path = notmuch_database_get_path (db);
   printf ("path: %s\n", path ? path : "(null)");
 EOF
+cp ${NOTMUCH_CONFIG}.bak ${NOTMUCH_CONFIG}
 cat <<EOF> EXPECTED
 == stdout ==
 status: 0
-- 
2.33.0

[PATCH 2/2] test/libconfig: add two tests for the config = "" case

[David Bremner]

If notmuch_database_open_with_config finds a database, but that
database is not in a legacy, non-split configuration, then it
currently incorrectly deduces the mail root and returns SUCCESS. Add
to two tests to demonstrate this bug.
---
 test/T590-libconfig.sh | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/test/T590-libconfig.sh b/test/T590-libconfig.sh
index 089b4e58..9cc79e8c 100755
--- a/test/T590-libconfig.sh
+++ b/test/T590-libconfig.sh
@@ -994,4 +994,44 @@ EOF
 notmuch_dir_sanitize < OUTPUT > OUTPUT.clean
 test_expect_equal_file EXPECTED OUTPUT.clean
 
+cat <<EOF > c_body
+  notmuch_status_t st = notmuch_database_open_with_config(NULL,
+							  NOTMUCH_DATABASE_MODE_READ_ONLY,
+							  "", NULL, &db, NULL);
+  printf ("status == SUCCESS: %d\n", st == NOTMUCH_STATUS_SUCCESS);
+  if (db) {
+    const char *mail_root = NULL;
+    mail_root = notmuch_config_get (db, NOTMUCH_CONFIG_MAIL_ROOT);
+    printf ("mail_root: %s\n", mail_root ? mail_root : "(null)");
+  }
+EOF
+
+cat <<EOF> EXPECTED.common
+== stdout ==
+status == SUCCESS: 0
+db == NULL: 1
+== stderr ==
+EOF
+
+test_begin_subtest "open/error: config=empty with no mail root in db "
+old_NOTMUCH_CONFIG=${NOTMUCH_CONFIG}
+unset NOTMUCH_CONFIG
+cat c_head3 c_body c_tail3 | test_C
+export NOTMUCH_CONFIG=${old_NOTMUCH_CONFIG}
+notmuch_dir_sanitize < OUTPUT > OUTPUT.clean
+test_expect_equal_file EXPECTED.common OUTPUT.clean
+
+test_begin_subtest "open/error: config=empty with no mail root in db (xdg)"
+test_subtest_known_broken
+old_NOTMUCH_CONFIG=${NOTMUCH_CONFIG}
+unset NOTMUCH_CONFIG
+backup_database
+mkdir -p home/.local/share/notmuch
+mv mail/.notmuch home/.local/share/notmuch/default
+cat c_head3 c_body c_tail3 | test_C
+restore_database
+export NOTMUCH_CONFIG=${old_NOTMUCH_CONFIG}
+notmuch_dir_sanitize < OUTPUT > OUTPUT.clean
+test_expect_equal_file EXPECTED.common OUTPUT.clean
+
 test_done
-- 
2.33.0

[PATCH 3/6] lib/open: use db struct as talloc ctx for choose_database_path

[David Bremner]

The extra talloc struct "local" was left over from before the notmuch
struct was allocated earlier. Having the notmuch struct available in
this function will allow more flexibility to track the configuration
variations (e.g. split vs. non-split).
---
 lib/open.cc | 27 +++++++++------------------
 1 file changed, 9 insertions(+), 18 deletions(-)

diff --git a/lib/open.cc b/lib/open.cc
index a91d22ef..320b690e 100644
--- a/lib/open.cc
+++ b/lib/open.cc
@@ -186,7 +186,7 @@ _db_dir_exists (const char *database_path, char **message)
 }
 
 static notmuch_status_t
-_choose_database_path (void *ctx,
+_choose_database_path (notmuch_database_t *notmuch,
 		       const char *profile,
 		       GKeyFile *key_file,
 		       const char **database_path,
@@ -201,16 +201,16 @@ _choose_database_path (void *ctx,
 	char *path = g_key_file_get_string (key_file, "database", "path", NULL);
 	if (path) {
 	    if (path[0] == '/')
-		*database_path = talloc_strdup (ctx, path);
+		*database_path = talloc_strdup (notmuch, path);
 	    else
-		*database_path = talloc_asprintf (ctx, "%s/%s", getenv ("HOME"), path);
+		*database_path = talloc_asprintf (notmuch, "%s/%s", getenv ("HOME"), path);
 	    g_free (path);
 	}
     }
     if (! *database_path) {
 	notmuch_status_t status;
 
-	*database_path = _xdg_dir (ctx, "XDG_DATA_HOME", ".local/share", profile);
+	*database_path = _xdg_dir (notmuch, "XDG_DATA_HOME", ".local/share", profile);
 	status = _db_dir_exists (*database_path, message);
 	if (status) {
 	    *database_path = NULL;
@@ -226,7 +226,7 @@ _choose_database_path (void *ctx,
     if (! *database_path) {
 	notmuch_status_t status;
 
-	*database_path = talloc_asprintf (ctx, "%s/mail", getenv ("HOME"));
+	*database_path = talloc_asprintf (notmuch, "%s/mail", getenv ("HOME"));
 	status = _db_dir_exists (*database_path, message);
 	if (status) {
 	    *database_path = NULL;
@@ -510,7 +510,6 @@ notmuch_database_open_with_config (const char *database_path,
 				   char **status_string)
 {
     notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
-    void *local = talloc_new (NULL);
     notmuch_database_t *notmuch = NULL;
     char *message = NULL;
     GKeyFile *key_file = NULL;
@@ -530,7 +529,7 @@ notmuch_database_open_with_config (const char *database_path,
 	goto DONE;
     }
 
-    if ((status = _choose_database_path (local, profile, key_file,
+    if ((status = _choose_database_path (notmuch, profile, key_file,
 					 &database_path, &split,
 					 &message)))
 	goto DONE;
@@ -549,8 +548,6 @@ notmuch_database_open_with_config (const char *database_path,
     status = _finish_open (notmuch, profile, mode, key_file, &message);
 
   DONE:
-    talloc_free (local);
-
     if (key_file)
 	g_key_file_free (key_file);
 
@@ -612,7 +609,6 @@ notmuch_database_create_with_config (const char *database_path,
     const char *notmuch_path = NULL;
     char *message = NULL;
     GKeyFile *key_file = NULL;
-    void *local = talloc_new (NULL);
     int err;
     bool split = false;
 
@@ -630,7 +626,7 @@ notmuch_database_create_with_config (const char *database_path,
 	goto DONE;
     }
 
-    if ((status = _choose_database_path (local, profile, key_file,
+    if ((status = _choose_database_path (notmuch, profile, key_file,
 					 &database_path, &split, &message)))
 	goto DONE;
 
@@ -654,7 +650,7 @@ notmuch_database_create_with_config (const char *database_path,
     if (split) {
 	notmuch_path = database_path;
     } else {
-	if (! (notmuch_path = talloc_asprintf (local, "%s/%s", database_path, ".notmuch"))) {
+	if (! (notmuch_path = talloc_asprintf (notmuch, "%s/%s", database_path, ".notmuch"))) {
 	    status = NOTMUCH_STATUS_OUT_OF_MEMORY;
 	    goto DONE;
 	}
@@ -711,8 +707,6 @@ notmuch_database_create_with_config (const char *database_path,
     }
 
   DONE:
-    talloc_free (local);
-
     if (key_file)
 	g_key_file_free (key_file);
 
@@ -812,7 +806,6 @@ notmuch_database_load_config (const char *database_path,
 			      char **status_string)
 {
     notmuch_status_t status = NOTMUCH_STATUS_SUCCESS, warning = NOTMUCH_STATUS_SUCCESS;
-    void *local = talloc_new (NULL);
     notmuch_database_t *notmuch = NULL;
     char *message = NULL;
     GKeyFile *key_file = NULL;
@@ -838,7 +831,7 @@ notmuch_database_load_config (const char *database_path,
 	goto DONE;
     }
 
-    status = _choose_database_path (local, profile, key_file,
+    status = _choose_database_path (notmuch, profile, key_file,
 				    &database_path, &split, &message);
     switch (status) {
     case NOTMUCH_STATUS_NO_DATABASE:
@@ -874,8 +867,6 @@ notmuch_database_load_config (const char *database_path,
 	goto DONE;
 
   DONE:
-    talloc_free (local);
-
     if (status_string)
 	*status_string = message;
 
-- 
2.34.1

[PATCH 4/6] lib/open: use notmuch->params to track split status

[David Bremner]

Persisting this status will allow us to use the information in other
compilation units, in particular when setting configuration defaults.
---
 lib/database-private.h |  7 ++++++-
 lib/open.cc            | 19 ++++++++-----------
 2 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/lib/database-private.h b/lib/database-private.h
index 8133e364..5db8882f 100644
--- a/lib/database-private.h
+++ b/lib/database-private.h
@@ -191,12 +191,17 @@ operator& (notmuch_field_flag_t a, notmuch_field_flag_t b)
 				    Xapian::QueryParser::FLAG_PURE_NOT)
 
 /*
- * Which parameters were explicit when the database was opened */
+ * explicit and implied parameters to open */
 typedef enum {
     NOTMUCH_PARAM_NONE		= 0,
+    /* database passed explicitely */
     NOTMUCH_PARAM_DATABASE	= 1 << 0,
+    /* config file passed explicitely */
     NOTMUCH_PARAM_CONFIG	= 1 << 1,
+    /* profile name passed explicitely */
     NOTMUCH_PARAM_PROFILE	= 1 << 2,
+    /* split (e.g. XDG) configuration */
+    NOTMUCH_PARAM_SPLIT		= 1 << 3,
 } notmuch_open_param_t;
 
 /*
diff --git a/lib/open.cc b/lib/open.cc
index 320b690e..bf1d7ae3 100644
--- a/lib/open.cc
+++ b/lib/open.cc
@@ -190,7 +190,6 @@ _choose_database_path (notmuch_database_t *notmuch,
 		       const char *profile,
 		       GKeyFile *key_file,
 		       const char **database_path,
-		       bool *split,
 		       char **message)
 {
     if (! *database_path) {
@@ -215,7 +214,7 @@ _choose_database_path (notmuch_database_t *notmuch,
 	if (status) {
 	    *database_path = NULL;
 	} else {
-	    *split = true;
+	    notmuch->params |= NOTMUCH_PARAM_SPLIT;
 	}
     }
 
@@ -513,7 +512,6 @@ notmuch_database_open_with_config (const char *database_path,
     notmuch_database_t *notmuch = NULL;
     char *message = NULL;
     GKeyFile *key_file = NULL;
-    bool split = false;
 
     _notmuch_init ();
 
@@ -530,7 +528,7 @@ notmuch_database_open_with_config (const char *database_path,
     }
 
     if ((status = _choose_database_path (notmuch, profile, key_file,
-					 &database_path, &split,
+					 &database_path,
 					 &message)))
 	goto DONE;
 
@@ -610,7 +608,6 @@ notmuch_database_create_with_config (const char *database_path,
     char *message = NULL;
     GKeyFile *key_file = NULL;
     int err;
-    bool split = false;
 
     _notmuch_init ();
 
@@ -627,7 +624,7 @@ notmuch_database_create_with_config (const char *database_path,
     }
 
     if ((status = _choose_database_path (notmuch, profile, key_file,
-					 &database_path, &split, &message)))
+					 &database_path, &message)))
 	goto DONE;
 
     status = _db_dir_exists (database_path, &message);
@@ -636,18 +633,19 @@ notmuch_database_create_with_config (const char *database_path,
 
     _set_database_path (notmuch, database_path);
 
-    if (key_file && ! split) {
+    if (key_file && ! (notmuch->params & NOTMUCH_PARAM_SPLIT)) {
 	char *mail_root = notmuch_canonicalize_file_name (
 	    g_key_file_get_string (key_file, "database", "mail_root", NULL));
 	char *db_path = notmuch_canonicalize_file_name (database_path);
 
-	split = (mail_root && (0 != strcmp (mail_root, db_path)));
+	if (mail_root && (0 != strcmp (mail_root, db_path)))
+	    notmuch->params |= NOTMUCH_PARAM_SPLIT;
 
 	free (mail_root);
 	free (db_path);
     }
 
-    if (split) {
+    if (notmuch->params & NOTMUCH_PARAM_SPLIT) {
 	notmuch_path = database_path;
     } else {
 	if (! (notmuch_path = talloc_asprintf (notmuch, "%s/%s", database_path, ".notmuch"))) {
@@ -809,7 +807,6 @@ notmuch_database_load_config (const char *database_path,
     notmuch_database_t *notmuch = NULL;
     char *message = NULL;
     GKeyFile *key_file = NULL;
-    bool split = false;
 
     _notmuch_init ();
 
@@ -832,7 +829,7 @@ notmuch_database_load_config (const char *database_path,
     }
 
     status = _choose_database_path (notmuch, profile, key_file,
-				    &database_path, &split, &message);
+				    &database_path, &message);
     switch (status) {
     case NOTMUCH_STATUS_NO_DATABASE:
     case NOTMUCH_STATUS_SUCCESS:
-- 
2.34.1

[PATCH 5/6] lib/config: make sure the config map exists when loading defaults

[David Bremner]

We should not rely on one of the other "_notmuch_config_load_*"
functions being called before this one.
---
 lib/config.cc | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/config.cc b/lib/config.cc
index 7a2882de..8f6ef110 100644
--- a/lib/config.cc
+++ b/lib/config.cc
@@ -658,6 +658,9 @@ _notmuch_config_load_defaults (notmuch_database_t *notmuch)
 {
     notmuch_config_key_t key;
 
+    if (notmuch->config == NULL)
+	notmuch->config = _notmuch_string_map_create (notmuch);
+
     for (key = NOTMUCH_CONFIG_FIRST;
 	 key < NOTMUCH_CONFIG_LAST;
 	 key = notmuch_config_key_t (key + 1)) {
-- 
2.34.1

[PATCH 6/6] lib/open: no default mail root in split configurations

[David Bremner]

If we know the configuration is split, but there is no mail root
defined, this indicates a (lack of) configuration error. Currently
this can only arise in XDG configurations.
---
 bindings/python-cffi/notmuch2/_build.py | 1 +
 lib/config.cc                           | 6 +++++-
 lib/database.cc                         | 2 ++
 lib/notmuch.h                           | 4 ++++
 test/T590-libconfig.sh                  | 1 -
 5 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/bindings/python-cffi/notmuch2/_build.py b/bindings/python-cffi/notmuch2/_build.py
index 45eb20c0..a55b484f 100644
--- a/bindings/python-cffi/notmuch2/_build.py
+++ b/bindings/python-cffi/notmuch2/_build.py
@@ -54,6 +54,7 @@ ffibuilder.cdef(
         NOTMUCH_STATUS_NO_DATABASE,
         NOTMUCH_STATUS_DATABASE_EXISTS,
         NOTMUCH_STATUS_BAD_QUERY_SYNTAX,
+        NOTMUCH_STATUS_NO_MAIL_ROOT,
         NOTMUCH_STATUS_LAST_STATUS
     } notmuch_status_t;
     typedef enum {
diff --git a/lib/config.cc b/lib/config.cc
index 8f6ef110..f61eb636 100644
--- a/lib/config.cc
+++ b/lib/config.cc
@@ -657,6 +657,7 @@ notmuch_status_t
 _notmuch_config_load_defaults (notmuch_database_t *notmuch)
 {
     notmuch_config_key_t key;
+    notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
 
     if (notmuch->config == NULL)
 	notmuch->config = _notmuch_string_map_create (notmuch);
@@ -669,11 +670,14 @@ _notmuch_config_load_defaults (notmuch_database_t *notmuch)
 
 	val = _notmuch_string_map_get (notmuch->config, key_string);
 	if (! val) {
+	    if (key == NOTMUCH_CONFIG_MAIL_ROOT && (notmuch->params & NOTMUCH_PARAM_SPLIT))
+		status = NOTMUCH_STATUS_NO_MAIL_ROOT;
+
 	    _notmuch_string_map_set (notmuch->config, key_string, _notmuch_config_default (notmuch,
 											   key));
 	}
     }
-    return NOTMUCH_STATUS_SUCCESS;
+    return status;
 }
 
 const char *
diff --git a/lib/database.cc b/lib/database.cc
index 6ef56d56..0effe978 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -311,6 +311,8 @@ notmuch_status_to_string (notmuch_status_t status)
 	return "Database exists, not recreated";
     case NOTMUCH_STATUS_BAD_QUERY_SYNTAX:
 	return "Syntax error in query";
+    case NOTMUCH_STATUS_NO_MAIL_ROOT:
+	return "No mail root found";
     default:
     case NOTMUCH_STATUS_LAST_STATUS:
 	return "Unknown error status value";
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 1b2bdf3f..fef98b4b 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -224,6 +224,10 @@ typedef enum {
      * Syntax error in query
      */
     NOTMUCH_STATUS_BAD_QUERY_SYNTAX,
+    /**
+     * No mail root could be deduced from parameters and environment
+     */
+    NOTMUCH_STATUS_NO_MAIL_ROOT,
     /**
      * Not an actual status value. Just a way to find out how many
      * valid status values there are.
diff --git a/test/T590-libconfig.sh b/test/T590-libconfig.sh
index 9cc79e8c..0b5291ab 100755
--- a/test/T590-libconfig.sh
+++ b/test/T590-libconfig.sh
@@ -1022,7 +1022,6 @@ notmuch_dir_sanitize < OUTPUT > OUTPUT.clean
 test_expect_equal_file EXPECTED.common OUTPUT.clean
 
 test_begin_subtest "open/error: config=empty with no mail root in db (xdg)"
-test_subtest_known_broken
 old_NOTMUCH_CONFIG=${NOTMUCH_CONFIG}
 unset NOTMUCH_CONFIG
 backup_database
-- 
2.34.1

Re: [PATCH 1/2] test/libconfig: save and restore config file

[David Bremner]

David Bremner <david@tethera.net> writes:

> Currently the config file is unusable for further tests requiring a
> valid database path.

Series applied to master.

d