* [PATCH 1/3] test: add known broken test for conflict with database parameter
2021-10-28 1:34 Fix overriding database path when opening a different file David Bremner
@ 2021-10-28 1:34 ` David Bremner
2021-10-28 1:45 ` David Bremner
2021-12-04 13:48 ` David Bremner
2021-10-28 1:34 ` [PATCH 2/3] lib/open: track which parameters are passed David Bremner
2021-10-28 1:34 ` [PATCH 3/3] lib/config: don't overwrite database.path if the caller passed it David Bremner
2 siblings, 2 replies; 6+ messages in thread
From: David Bremner @ 2021-10-28 1:34 UTC (permalink / raw)
To: notmuch
This is arguably user error: having configuration file with bad
settings in it (and/or having a bad NOTMUCH_CONFIG environment
variable). On the other hand returning a different path than was
actually opened is definitely a bug.
---
test/T590-libconfig.sh | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)
diff --git a/test/T590-libconfig.sh b/test/T590-libconfig.sh
index c9471dc4..eab70ae9 100755
--- a/test/T590-libconfig.sh
+++ b/test/T590-libconfig.sh
@@ -933,7 +933,7 @@ NOTMUCH_CONFIG="/nonexistent"
cat c_head3 - c_tail3 <<'EOF' | test_C ${MAIL_DIR}
notmuch_status_t st = notmuch_database_load_config(argv[1], NULL, NULL, &db, NULL);
EOF
-NOTMUCH_CONFIG=${old_NOTMUCH_CONFIG}
+export NOTMUCH_CONFIG=${old_NOTMUCH_CONFIG}
cat <<EOF> EXPECTED
== stdout ==
db == NULL: 0
@@ -952,4 +952,26 @@ db == NULL: 1
EOF
test_expect_equal_file EXPECTED OUTPUT
+test_begin_subtest "open: database parameter overrides implicit config"
+test_subtest_known_broken
+notmuch config set database.path ${MAIL_DIR}/nonexistent
+cat c_head3 - c_tail3 <<'EOF' | test_C ${MAIL_DIR}
+ const char *path = NULL;
+ notmuch_status_t st = notmuch_database_open_with_config(argv[1],
+ NOTMUCH_DATABASE_MODE_READ_ONLY,
+ NULL, NULL, &db, NULL);
+ printf ("status: %d\n", st);
+ path = notmuch_database_get_path (db);
+ printf ("path: %s\n", path ? path : "(null)");
+EOF
+cat <<EOF> EXPECTED
+== stdout ==
+status: 0
+path: MAIL_DIR
+db == NULL: 0
+== stderr ==
+EOF
+notmuch_dir_sanitize < OUTPUT > OUTPUT.clean
+test_expect_equal_file EXPECTED OUTPUT.clean
+
test_done
--
2.33.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] lib/open: track which parameters are passed
2021-10-28 1:34 Fix overriding database path when opening a different file David Bremner
2021-10-28 1:34 ` [PATCH 1/3] test: add known broken test for conflict with database parameter David Bremner
@ 2021-10-28 1:34 ` David Bremner
2021-10-28 1:34 ` [PATCH 3/3] lib/config: don't overwrite database.path if the caller passed it David Bremner
2 siblings, 0 replies; 6+ messages in thread
From: David Bremner @ 2021-10-28 1:34 UTC (permalink / raw)
To: notmuch
This will be used to fine tune the loading of configuration for
certain special configuration items (initially just "database.path").
---
lib/database-private.h | 36 ++++++++++++++++++++++++++++++++++++
lib/open.cc | 17 +++++++++++++----
2 files changed, 49 insertions(+), 4 deletions(-)
diff --git a/lib/database-private.h b/lib/database-private.h
index 8b9d67fe..8dd77281 100644
--- a/lib/database-private.h
+++ b/lib/database-private.h
@@ -190,6 +190,39 @@ operator& (notmuch_field_flag_t a, notmuch_field_flag_t b)
Xapian::QueryParser::FLAG_WILDCARD | \
Xapian::QueryParser::FLAG_PURE_NOT)
+/*
+ * Which parameters were explicit when the database was opened */
+typedef enum {
+ NOTMUCH_PARAM_NONE = 0,
+ NOTMUCH_PARAM_DATABASE = 1 << 0,
+ NOTMUCH_PARAM_CONFIG = 1 << 1,
+ NOTMUCH_PARAM_PROFILE = 1 << 2,
+} notmuch_open_param_t;
+
+/*
+ * define bitwise operators to hide casts */
+
+inline notmuch_open_param_t
+operator| (notmuch_open_param_t a, notmuch_open_param_t b)
+{
+ return static_cast<notmuch_open_param_t>(
+ static_cast<unsigned>(a) | static_cast<unsigned>(b));
+}
+
+inline notmuch_open_param_t&
+operator|= (notmuch_open_param_t &a, notmuch_open_param_t b)
+{
+ a = a | b;
+ return a;
+}
+
+inline notmuch_open_param_t
+operator& (notmuch_open_param_t a, notmuch_open_param_t b)
+{
+ return static_cast<notmuch_open_param_t>(
+ static_cast<unsigned>(a) & static_cast<unsigned>(b));
+}
+
struct _notmuch_database {
bool exception_reported;
@@ -249,6 +282,9 @@ struct _notmuch_database {
/* Cached and possibly overridden configuration */
notmuch_string_map_t *config;
+
+ /* Track what parameters were specified when opening */
+ notmuch_open_param_t params;
};
/* Prior to database version 3, features were implied by the database
diff --git a/lib/open.cc b/lib/open.cc
index ba32c2f1..a942383b 100644
--- a/lib/open.cc
+++ b/lib/open.cc
@@ -247,7 +247,7 @@ _choose_database_path (void *ctx,
}
static notmuch_database_t *
-_alloc_notmuch ()
+_alloc_notmuch (const char *database_path, const char *config_path, const char *profile)
{
notmuch_database_t *notmuch;
@@ -263,6 +263,15 @@ _alloc_notmuch ()
notmuch->transaction_count = 0;
notmuch->transaction_threshold = 0;
notmuch->view = 1;
+
+ notmuch->params = NOTMUCH_PARAM_NONE;
+ if (database_path)
+ notmuch->params |= NOTMUCH_PARAM_DATABASE;
+ if (config_path)
+ notmuch->params |= NOTMUCH_PARAM_CONFIG;
+ if (profile)
+ notmuch->params |= NOTMUCH_PARAM_PROFILE;
+
return notmuch;
}
@@ -510,7 +519,7 @@ notmuch_database_open_with_config (const char *database_path,
_notmuch_init ();
- notmuch = _alloc_notmuch ();
+ notmuch = _alloc_notmuch (database_path, config_path, profile);
if (! notmuch) {
status = NOTMUCH_STATUS_OUT_OF_MEMORY;
goto DONE;
@@ -610,7 +619,7 @@ notmuch_database_create_with_config (const char *database_path,
_notmuch_init ();
- notmuch = _alloc_notmuch ();
+ notmuch = _alloc_notmuch (database_path, config_path, profile);
if (! notmuch) {
status = NOTMUCH_STATUS_OUT_OF_MEMORY;
goto DONE;
@@ -812,7 +821,7 @@ notmuch_database_load_config (const char *database_path,
_notmuch_init ();
- notmuch = _alloc_notmuch ();
+ notmuch = _alloc_notmuch (database_path, config_path, profile);
if (! notmuch) {
status = NOTMUCH_STATUS_OUT_OF_MEMORY;
goto DONE;
--
2.33.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] lib/config: don't overwrite database.path if the caller passed it
2021-10-28 1:34 Fix overriding database path when opening a different file David Bremner
2021-10-28 1:34 ` [PATCH 1/3] test: add known broken test for conflict with database parameter David Bremner
2021-10-28 1:34 ` [PATCH 2/3] lib/open: track which parameters are passed David Bremner
@ 2021-10-28 1:34 ` David Bremner
2 siblings, 0 replies; 6+ messages in thread
From: David Bremner @ 2021-10-28 1:34 UTC (permalink / raw)
To: notmuch
If the user passed a path, and we opened it, then we consider that
definitive definition of "database.path". This makes libnotmuch
respond more gracefully to certain erroneous combinations of
NOTMUCH_CONFIG settings and config file contents.
---
lib/config.cc | 17 ++++++++++++++++-
test/T590-libconfig.sh | 1 -
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/lib/config.cc b/lib/config.cc
index 8775b00a..e502858d 100644
--- a/lib/config.cc
+++ b/lib/config.cc
@@ -259,7 +259,15 @@ _notmuch_config_load_from_database (notmuch_database_t *notmuch)
for (; notmuch_config_list_valid (list); notmuch_config_list_move_to_next (list)) {
const char *key = notmuch_config_list_key (list);
- char *normalized_val = _expand_path (list, key, notmuch_config_list_value (list));
+ char *normalized_val = NULL;
+
+ /* If we opened from a given path, do not overwrite it */
+ if (strcmp (key, "database.path") == 0 &&
+ (notmuch->params & NOTMUCH_PARAM_DATABASE) &&
+ notmuch->xapian_db)
+ continue;
+
+ normalized_val = _expand_path (list, key, notmuch_config_list_value (list));
_notmuch_string_map_append (notmuch->config, key, normalized_val);
talloc_free (normalized_val);
}
@@ -432,6 +440,13 @@ _notmuch_config_load_from_file (notmuch_database_t *notmuch,
status = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
}
+
+ /* If we opened from a given path, do not overwrite it */
+ if (strcmp (absolute_key, "database.path") == 0 &&
+ (notmuch->params & NOTMUCH_PARAM_DATABASE) &&
+ notmuch->xapian_db)
+ continue;
+
normalized_val = _expand_path (notmuch, absolute_key, val);
_notmuch_string_map_set (notmuch->config, absolute_key, normalized_val);
g_free (val);
diff --git a/test/T590-libconfig.sh b/test/T590-libconfig.sh
index eab70ae9..30bdb1b0 100755
--- a/test/T590-libconfig.sh
+++ b/test/T590-libconfig.sh
@@ -953,7 +953,6 @@ EOF
test_expect_equal_file EXPECTED OUTPUT
test_begin_subtest "open: database parameter overrides implicit config"
-test_subtest_known_broken
notmuch config set database.path ${MAIL_DIR}/nonexistent
cat c_head3 - c_tail3 <<'EOF' | test_C ${MAIL_DIR}
const char *path = NULL;
--
2.33.0
^ permalink raw reply related [flat|nested] 6+ messages in thread