* [PATCH 1/2] test/libconfig: save and restore config file @ 2021-12-11 12:49 David Bremner 2021-12-11 12:49 ` [PATCH 2/2] test/libconfig: add two tests for the config = "" case David Bremner 2022-01-16 1:56 ` [PATCH 1/2] test/libconfig: save and restore config file David Bremner 0 siblings, 2 replies; 7+ messages in thread From: David Bremner @ 2021-12-11 12:49 UTC (permalink / raw) To: notmuch 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] test/libconfig: add two tests for the config = "" case 2021-12-11 12:49 [PATCH 1/2] test/libconfig: save and restore config file David Bremner @ 2021-12-11 12:49 ` David Bremner 2021-12-25 13:33 ` [PATCH 3/6] lib/open: use db struct as talloc ctx for choose_database_path David Bremner 2022-01-16 1:56 ` [PATCH 1/2] test/libconfig: save and restore config file David Bremner 1 sibling, 1 reply; 7+ messages in thread From: David Bremner @ 2021-12-11 12:49 UTC (permalink / raw) To: notmuch 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/6] lib/open: use db struct as talloc ctx for choose_database_path 2021-12-11 12:49 ` [PATCH 2/2] test/libconfig: add two tests for the config = "" case David Bremner @ 2021-12-25 13:33 ` David Bremner 2021-12-25 13:33 ` [PATCH 4/6] lib/open: use notmuch->params to track split status David Bremner ` (2 more replies) 0 siblings, 3 replies; 7+ messages in thread From: David Bremner @ 2021-12-25 13:33 UTC (permalink / raw) To: David Bremner, notmuch 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 4/6] lib/open: use notmuch->params to track split status 2021-12-25 13:33 ` [PATCH 3/6] lib/open: use db struct as talloc ctx for choose_database_path David Bremner @ 2021-12-25 13:33 ` David Bremner 2021-12-25 13:33 ` [PATCH 5/6] lib/config: make sure the config map exists when loading defaults David Bremner 2021-12-25 13:33 ` [PATCH 6/6] lib/open: no default mail root in split configurations David Bremner 2 siblings, 0 replies; 7+ messages in thread From: David Bremner @ 2021-12-25 13:33 UTC (permalink / raw) To: David Bremner, notmuch 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 5/6] lib/config: make sure the config map exists when loading defaults 2021-12-25 13:33 ` [PATCH 3/6] lib/open: use db struct as talloc ctx for choose_database_path David Bremner 2021-12-25 13:33 ` [PATCH 4/6] lib/open: use notmuch->params to track split status David Bremner @ 2021-12-25 13:33 ` David Bremner 2021-12-25 13:33 ` [PATCH 6/6] lib/open: no default mail root in split configurations David Bremner 2 siblings, 0 replies; 7+ messages in thread From: David Bremner @ 2021-12-25 13:33 UTC (permalink / raw) To: David Bremner, notmuch 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 6/6] lib/open: no default mail root in split configurations 2021-12-25 13:33 ` [PATCH 3/6] lib/open: use db struct as talloc ctx for choose_database_path David Bremner 2021-12-25 13:33 ` [PATCH 4/6] lib/open: use notmuch->params to track split status David Bremner 2021-12-25 13:33 ` [PATCH 5/6] lib/config: make sure the config map exists when loading defaults David Bremner @ 2021-12-25 13:33 ` David Bremner 2 siblings, 0 replies; 7+ messages in thread From: David Bremner @ 2021-12-25 13:33 UTC (permalink / raw) To: David Bremner, notmuch 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] test/libconfig: save and restore config file 2021-12-11 12:49 [PATCH 1/2] test/libconfig: save and restore config file David Bremner 2021-12-11 12:49 ` [PATCH 2/2] test/libconfig: add two tests for the config = "" case David Bremner @ 2022-01-16 1:56 ` David Bremner 1 sibling, 0 replies; 7+ messages in thread From: David Bremner @ 2022-01-16 1:56 UTC (permalink / raw) To: notmuch 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-01-16 1:57 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-12-11 12:49 [PATCH 1/2] test/libconfig: save and restore config file David Bremner 2021-12-11 12:49 ` [PATCH 2/2] test/libconfig: add two tests for the config = "" case David Bremner 2021-12-25 13:33 ` [PATCH 3/6] lib/open: use db struct as talloc ctx for choose_database_path David Bremner 2021-12-25 13:33 ` [PATCH 4/6] lib/open: use notmuch->params to track split status David Bremner 2021-12-25 13:33 ` [PATCH 5/6] lib/config: make sure the config map exists when loading defaults David Bremner 2021-12-25 13:33 ` [PATCH 6/6] lib/open: no default mail root in split configurations David Bremner 2022-01-16 1:56 ` [PATCH 1/2] test/libconfig: save and restore config file David Bremner
Code repositories for project(s) associated with this public inbox https://yhetil.org/notmuch.git/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).