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