unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / Atom feed
* Initial implementation of merged config
@ 2020-08-08 14:16 David Bremner
  2020-08-08 14:16 ` [PATCH 01/19] test: use keys with group 'test' in T590-libconfig David Bremner
                   ` (18 more replies)
  0 siblings, 19 replies; 22+ messages in thread
From: David Bremner @ 2020-08-08 14:16 UTC (permalink / raw)
  To: notmuch

This follow-up to

     id:20200703134338.3311659-1-david@tethera.net

is a first draft of a rewrite of the notmuch configuration system. It
aims to remove several annoyances including:

- not being able to have certain configuration items in a plain text config file
- bindings support for configuration being somewhat half-baked
- not supporting XDG_foo

Not all of that promise is realized (or at least tested) in this
initial draft. On the other hand, hopefully a bunch of new tests in
T590-libconfig.sh make the intended API more clear. 

This patch implements the logic for finding config files:

     [PATCH 12/19] WIP: adding fallbacks for NULL config_path

Something similar needs to be done for finding database files.

In the last two commands you can see roughly how much
work is needed to convert to the style of config handling

     [PATCH 18/19] WIP converting notmuch search to new style config
     [PATCH 19/19] WIP: switch notmuch-show to new config framework

The is some awkardness due to converting one subcommand at a time: the
subcommand table uses a fixed function signature and we can't change
that until all the subcommands are updated. So we end up re-opening the config file in the modified subcommands.

Some design questions people might like to give feedback on

- the switch in notmuch-search.c to using the string
  "search.exclude_tags" directly to fetch tags.  I'm hoping that a
  reduction in boilerplate compensates for a decrease in "type safety"
  here, compared to the existing
  notmuch_config_get_search_exclude_tags. I'm hoping substantial
  portions of notmuch-config.c can disappear during this rewrite.

- the use of simple keys for the new merged notmuch_config_{get,set}_*
  in lib/config.cc, rather than the section, pair file suggested by the glib key_file format. 


My rough plan is to do a release 0.31 with all the cleanup we have
accumulated since 0.30, and target these changes for 0.32

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 01/19] test: use keys with group 'test' in T590-libconfig
  2020-08-08 14:16 Initial implementation of merged config David Bremner
@ 2020-08-08 14:16 ` David Bremner
  2020-08-08 14:16 ` [PATCH 02/19] lib: add stub for notmuch_database_open_with_config David Bremner
                   ` (17 subsequent siblings)
  18 siblings, 0 replies; 22+ messages in thread
From: David Bremner @ 2020-08-08 14:16 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

In a future commit we want to interoperate better with glib KeyFiles,
which need groups for all keys.
---
 test/T590-libconfig.sh | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/test/T590-libconfig.sh b/test/T590-libconfig.sh
index 360e45b0..8c34acf9 100755
--- a/test/T590-libconfig.sh
+++ b/test/T590-libconfig.sh
@@ -28,18 +28,18 @@ EOF
 test_begin_subtest "notmuch_database_{set,get}_config"
 cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
 {
-   EXPECT0(notmuch_database_set_config (db, "testkey1", "testvalue1"));
-   EXPECT0(notmuch_database_set_config (db, "testkey2", "testvalue2"));
-   EXPECT0(notmuch_database_get_config (db, "testkey1", &val));
-   printf("testkey1 = %s\n", val);
-   EXPECT0(notmuch_database_get_config (db, "testkey2", &val));
-   printf("testkey2 = %s\n", val);
+   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 ==
-testkey1 = testvalue1
-testkey2 = testvalue2
+test.key1 = testvalue1
+test.key2 = testvalue2
 == stderr ==
 EOF
 test_expect_equal_file EXPECTED OUTPUT
@@ -93,8 +93,8 @@ EOF
 cat <<'EOF' >EXPECTED
 == stdout ==
 aaabefore beforeval
-testkey1 testvalue1
-testkey2 testvalue2
+test.key1 testvalue1
+test.key2 testvalue2
 zzzafter afterval
 == stderr ==
 EOF
@@ -115,8 +115,8 @@ EOF
 cat <<'EOF' >EXPECTED
 == stdout ==
 aaabefore 1
-testkey1 1
-testkey2 1
+test.key1 1
+test.key2 1
 zzzafter 1
 == stderr ==
 EOF
@@ -126,7 +126,7 @@ test_begin_subtest "notmuch_database_get_config_list: one prefix"
 cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
 {
    notmuch_config_list_t *list;
-   EXPECT0(notmuch_database_get_config_list (db, "testkey", &list));
+   EXPECT0(notmuch_database_get_config_list (db, "test.key", &list));
    for (; notmuch_config_list_valid (list); notmuch_config_list_move_to_next (list)) {
       printf("%s %s\n", notmuch_config_list_key (list), notmuch_config_list_value(list));
    }
@@ -135,8 +135,8 @@ cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
 EOF
 cat <<'EOF' >EXPECTED
 == stdout ==
-testkey1 testvalue1
-testkey2 testvalue2
+test.key1 testvalue1
+test.key2 testvalue2
 == stderr ==
 EOF
 test_expect_equal_file EXPECTED OUTPUT
@@ -152,8 +152,8 @@ cat <<'EOF' >EXPECTED
 #notmuch-dump batch-tag:3 config
 #@ aaabefore beforeval
 #@ key%20with%20spaces value,%20with,%20spaces%21
-#@ testkey1 testvalue1
-#@ testkey2 testvalue2
+#@ test.key1 testvalue1
+#@ test.key2 testvalue2
 #@ zzzafter afterval
 EOF
 test_expect_equal_file EXPECTED OUTPUT
@@ -162,7 +162,7 @@ test_begin_subtest "restore config"
 notmuch dump --include=config >EXPECTED
 cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
 {
-    EXPECT0(notmuch_database_set_config (db, "testkey1", "mutatedvalue"));
+    EXPECT0(notmuch_database_set_config (db, "test.key1", "mutatedvalue"));
 }
 EOF
 notmuch restore --include=config <EXPECTED
-- 
2.28.0

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 02/19] lib: add stub for notmuch_database_open_with_config
  2020-08-08 14:16 Initial implementation of merged config David Bremner
  2020-08-08 14:16 ` [PATCH 01/19] test: use keys with group 'test' in T590-libconfig David Bremner
@ 2020-08-08 14:16 ` David Bremner
  2020-08-09  7:19   ` Reto
  2020-08-08 14:16 ` [PATCH 03/19] lib: cache configuration information from database David Bremner
                   ` (16 subsequent siblings)
  18 siblings, 1 reply; 22+ messages in thread
From: David Bremner @ 2020-08-08 14:16 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

Initially document the intended API and copy the code from
notmuch_database_open_verbose. Most of the documented functionality is
not there yet.
---
 lib/database.cc |  21 ++++++--
 lib/notmuch.h   | 138 ++++++++++++++++++++++++++++++++++++++----------
 2 files changed, 127 insertions(+), 32 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 75189685..acc686c0 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -918,6 +918,18 @@ notmuch_database_open_verbose (const char *path,
 			       notmuch_database_mode_t mode,
 			       notmuch_database_t **database,
 			       char **status_string)
+{
+    return notmuch_database_open_with_config (path, mode, "", NULL,
+					      database, status_string);
+}
+
+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);
@@ -929,19 +941,19 @@ notmuch_database_open_verbose (const char *path,
     unsigned int i, version;
     static int initialized = 0;
 
-    if (path == NULL) {
+    if (database_path == NULL) {
 	message = strdup ("Error: Cannot open a database for a NULL path.\n");
 	status = NOTMUCH_STATUS_NULL_POINTER;
 	goto DONE;
     }
 
-    if (path[0] != '/') {
+    if (database_path[0] != '/') {
 	message = strdup ("Error: Database path must be absolute.\n");
 	status = NOTMUCH_STATUS_PATH_ERROR;
 	goto DONE;
     }
 
-    if (! (notmuch_path = talloc_asprintf (local, "%s/%s", path, ".notmuch"))) {
+    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;
@@ -975,7 +987,7 @@ notmuch_database_open_verbose (const char *path,
     notmuch = talloc_zero (NULL, notmuch_database_t);
     notmuch->exception_reported = false;
     notmuch->status_string = NULL;
-    notmuch->path = talloc_strdup (notmuch, path);
+    notmuch->path = talloc_strdup (notmuch, database_path);
 
     strip_trailing (notmuch->path, '/');
 
@@ -1101,7 +1113,6 @@ notmuch_database_open_verbose (const char *path,
 
     return status;
 }
-
 notmuch_status_t
 notmuch_database_close (notmuch_database_t *notmuch)
 {
diff --git a/lib/notmuch.h b/lib/notmuch.h
index f3cb0fe2..e61634a1 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -301,52 +301,136 @@ typedef enum {
 } notmuch_database_mode_t;
 
 /**
- * Open an existing notmuch database located at 'path'.
+ * Deprecated alias for notmuch_database_open_with_config with
+ * config_path=error_message=NULL
+ * @deprecated Deprecated as of libnotmuch 5.4 (notmuch 0.32)
+ */
+/* NOTMUCH_DEPRECATED(5, 4) */
+notmuch_status_t
+notmuch_database_open (const char *path,
+		       notmuch_database_mode_t mode,
+		       notmuch_database_t **database);
+/**
+ * Deprecated alias for notmuch_database_open_with_config with
+ * config_path=NULL
+ *
+ * @deprecated Deprecated as of libnotmuch 5.4 (notmuch 0.32)
+ *
+ */
+/* NOTMUCH_DEPRECATED(5, 4) */
+notmuch_status_t
+notmuch_database_open_verbose (const char *path,
+			       notmuch_database_mode_t mode,
+			       notmuch_database_t **database,
+			       char **error_message);
+
+/**
+ * Open an existing notmuch database located at 'database_path', using
+ * configuration in 'config_path'.
+ *
+ * @param[in]	database_path
+ * @parblock
+ * Path to existing database.
+ *
+ * A notmuch database is a Xapian database containing appropriate
+ * metadata.
  *
  * The database should have been created at some time in the past,
  * (not necessarily by this process), by calling
- * notmuch_database_create with 'path'. By default the database should be
- * opened for reading only. In order to write to the database you need to
- * pass the NOTMUCH_DATABASE_MODE_READ_WRITE mode.
+ * notmuch_database_create.
+ *
+ * If 'database_path' is NULL, use the location specified
+ *
+ * - in the environment variable NOTMUCH_DATABASE, if non-empty
+ *
+ * - by $XDG_DATA_HOME/notmuch/$PROFILE where XDG_DATA_HOME defaults
+ *   to "$HOME/.local/share" and PROFILE as as discussed in
+ *   'profile'
+ *
+ * If 'database_path' is non-NULL, but does not appear to be a Xapian
+ * database, check for a directory '.notmuch/xapian' below
+ * 'database_path' (this is the behavior of
+ * notmuch_database_open_verbose pre-0.32).
+ *
+ * @endparblock
+ * @param[in]	mode
+ * @parblock
+ * Mode to open database. Use one of #NOTMUCH_DATABASE_MODE_READ_WRITE
+ * or #NOTMUCH_DATABASE_MODE_READ_WRITE
+ *
+ * @endparblock
+ * @param[in]  config_path
+ * @parblock
+ * Path to config file.
+ *
+ * Config file is key-value, with mandatory sections. See
+ * <em>notmuch-config(5)</em> for more information. The key-value pair
+ * overrides the corresponding configuration data stored in the
+ * database (see <em>notmuch_database_get_config</em>)
+ *
+ * If <em>config_path</em> is NULL use the path specified
  *
- * An existing notmuch database can be identified by the presence of a
- * directory named ".notmuch" below 'path'.
+ * - in environment variable <em>NOTMUCH_CONFIG</em>, if non-empty
+ *
+ * - by  <em>XDG_CONFIG_HOME</em>/notmuch/ where
+ *   XDG_CONFIG_HOME defaults to "$HOME/.config".
+ *
+ * - by $HOME/.notmuch-config
+ *
+ * If <em>config_path</em> is "" (empty string) then do not
+ * open any configuration file.
+ * @endparblock
+ * @param[in] profile:
+ * @parblock
+ * Name of profile (configuration/database variant).
+ *
+ * If non-NULL, append to the directory / file path determined for
+ * <em>config_path</em> and <em>database_path</em>.
+ *
+ * If NULL then use
+ * - environment variable NOTMUCH_PROFILE if defined,
+ * - otherwise "default" for directories and "" (empty string) for paths.
+ *
+ * @endparblock
+ * @param[out] database
+ * @parblock
+ * Pointer to database object. May not be NULL.
  *
  * The caller should call notmuch_database_destroy when finished with
  * this database.
  *
  * In case of any failure, this function returns an error status and
- * sets *database to NULL (after printing an error message on stderr).
+ * sets *database to NULL.
  *
- * Return value:
+ * @endparblock
+ * @param[out] error_message
+ * If non-NULL, store error message from opening the database.
+ * Any such message is allocated by \a malloc(3) and should be freed
+ * by the caller.
  *
- * NOTMUCH_STATUS_SUCCESS: Successfully opened the database.
+ * @retval NOTMUCH_STATUS_SUCCESS: Successfully opened the database.
  *
- * NOTMUCH_STATUS_NULL_POINTER: The given 'path' argument is NULL.
+ * @retval NOTMUCH_STATUS_NULL_POINTER: The given \a database
+ * argument is NULL.
  *
- * NOTMUCH_STATUS_OUT_OF_MEMORY: Out of memory.
+ * @retval NOTMUCH_STATUS_OUT_OF_MEMORY: Out of memory.
  *
- * NOTMUCH_STATUS_FILE_ERROR: An error occurred trying to open the
- *	database file (such as permission denied, or file not found,
+ * @retval NOTMUCH_STATUS_FILE_ERROR: An error occurred trying to open the
+ *	database or config file (such as permission denied, or file not found,
  *	etc.), or the database version is unknown.
  *
- * NOTMUCH_STATUS_XAPIAN_EXCEPTION: A Xapian exception occurred.
- */
-notmuch_status_t
-notmuch_database_open (const char *path,
-		       notmuch_database_mode_t mode,
-		       notmuch_database_t **database);
-/**
- * Like notmuch_database_open, except optionally return an error
- * message. This message is allocated by malloc and should be freed by
- * the caller.
+ * @retval NOTMUCH_STATUS_XAPIAN_EXCEPTION: A Xapian exception occurred.
+ *
+ * @since libnotmuch 5.4 (notmuch 0.32)
  */
 
 notmuch_status_t
-notmuch_database_open_verbose (const char *path,
-			       notmuch_database_mode_t mode,
-			       notmuch_database_t **database,
-			       char **error_message);
+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 **error_message);
 
 /**
  * Retrieve last status string for given database.
-- 
2.28.0

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 03/19] lib: cache configuration information from database
  2020-08-08 14:16 Initial implementation of merged config David Bremner
  2020-08-08 14:16 ` [PATCH 01/19] test: use keys with group 'test' in T590-libconfig David Bremner
  2020-08-08 14:16 ` [PATCH 02/19] lib: add stub for notmuch_database_open_with_config David Bremner
@ 2020-08-08 14:16 ` David Bremner
  2020-08-08 14:16 ` [PATCH 04/19] WIP: add notmuch_config_get David Bremner
                   ` (15 subsequent siblings)
  18 siblings, 0 replies; 22+ messages in thread
From: David Bremner @ 2020-08-08 14:16 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

The main goal is to allow configuration information to be temporarily
overridden by a separate config file.
---
 lib/config.cc          | 25 +++++++++++++++++++++++++
 lib/database-private.h |  3 +++
 lib/database.cc        |  4 ++++
 lib/notmuch-private.h  |  3 +++
 4 files changed, 35 insertions(+)

diff --git a/lib/config.cc b/lib/config.cc
index dae0ff0e..2d4c3801 100644
--- a/lib/config.cc
+++ b/lib/config.cc
@@ -194,3 +194,28 @@ notmuch_config_list_destroy (notmuch_config_list_t *list)
 {
     talloc_free (list);
 }
+
+notmuch_status_t
+_notmuch_config_load_from_database (notmuch_database_t *notmuch)
+{
+    notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
+    notmuch_config_list_t *list;
+
+    if (notmuch->config == NULL)
+	notmuch->config = _notmuch_string_map_create (notmuch);
+
+    if (unlikely(notmuch->config == NULL))
+	return NOTMUCH_STATUS_OUT_OF_MEMORY;
+
+    status = notmuch_database_get_config_list (notmuch, "", &list);
+    if (status)
+	return status;
+
+    for (; notmuch_config_list_valid (list); notmuch_config_list_move_to_next (list)) {
+	_notmuch_string_map_append (notmuch->config,
+				    notmuch_config_list_key (list),
+				    notmuch_config_list_value (list));
+    }
+
+    return status;
+}
diff --git a/lib/database-private.h b/lib/database-private.h
index 041602cd..bfeef869 100644
--- a/lib/database-private.h
+++ b/lib/database-private.h
@@ -226,6 +226,9 @@ struct _notmuch_database {
      * here, but at least they are small */
     notmuch_string_map_t *user_prefix;
     notmuch_string_map_t *user_header;
+
+    /* Cached and possibly overridden configuration */
+    notmuch_string_map_t *config;
 };
 
 /* Prior to database version 3, features were implied by the database
diff --git a/lib/database.cc b/lib/database.cc
index acc686c0..350abb11 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -987,6 +987,7 @@ notmuch_database_open_with_config (const char *database_path,
     notmuch = talloc_zero (NULL, notmuch_database_t);
     notmuch->exception_reported = false;
     notmuch->status_string = NULL;
+    notmuch->config = NULL;
     notmuch->path = talloc_strdup (notmuch, database_path);
 
     strip_trailing (notmuch->path, '/');
@@ -1085,6 +1086,9 @@ notmuch_database_open_with_config (const char *database_path,
 	    }
 	}
 	status = _setup_user_query_fields (notmuch);
+
+	if (status == NOTMUCH_STATUS_SUCCESS)
+	    status = _notmuch_config_load_from_database (notmuch);
     } catch (const Xapian::Error &error) {
 	IGNORE_RESULT (asprintf (&message, "A Xapian exception occurred opening database: %s\n",
 				 error.get_msg ().c_str ()));
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 57ec7f72..0f26b371 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -699,6 +699,9 @@ struct _notmuch_indexopts {
 
 #define EMPTY_STRING(s) ((s)[0] == '\0')
 
+/* config.cc */
+notmuch_status_t
+_notmuch_config_load_from_database (notmuch_database_t * db);
 NOTMUCH_END_DECLS
 
 #ifdef __cplusplus
-- 
2.28.0

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 04/19] WIP: add notmuch_config_get
  2020-08-08 14:16 Initial implementation of merged config David Bremner
                   ` (2 preceding siblings ...)
  2020-08-08 14:16 ` [PATCH 03/19] lib: cache configuration information from database David Bremner
@ 2020-08-08 14:16 ` David Bremner
  2020-08-08 14:16 ` [PATCH 05/19] WIP: add notmuch_config_set David Bremner
                   ` (14 subsequent siblings)
  18 siblings, 0 replies; 22+ messages in thread
From: David Bremner @ 2020-08-08 14:16 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

this just accesses the cached config data

API is to be decided; maybe we should return a status value.
---
 lib/config.cc          |  6 ++++++
 lib/notmuch.h          |  2 ++
 test/T590-libconfig.sh | 15 +++++++++++++++
 3 files changed, 23 insertions(+)

diff --git a/lib/config.cc b/lib/config.cc
index 2d4c3801..0a91ec03 100644
--- a/lib/config.cc
+++ b/lib/config.cc
@@ -219,3 +219,9 @@ _notmuch_config_load_from_database (notmuch_database_t *notmuch)
 
     return status;
 }
+
+const char *
+notmuch_config_get (notmuch_database_t *notmuch, const char *key) {
+
+    return _notmuch_string_map_get (notmuch->config, key);
+}
diff --git a/lib/notmuch.h b/lib/notmuch.h
index e61634a1..777116bb 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -2397,6 +2397,8 @@ notmuch_config_list_move_to_next (notmuch_config_list_t *config_list);
 void
 notmuch_config_list_destroy (notmuch_config_list_t *config_list);
 
+const char *
+notmuch_config_get (notmuch_database_t *notmuch, const char *key);
 
 /**
  * get the current default indexing options for a given database.
diff --git a/test/T590-libconfig.sh b/test/T590-libconfig.sh
index 8c34acf9..8a4519e9 100755
--- a/test/T590-libconfig.sh
+++ b/test/T590-libconfig.sh
@@ -169,4 +169,19 @@ notmuch restore --include=config <EXPECTED
 notmuch dump --include=config >OUTPUT
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "notmuch_config_get"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+{
+   printf("test.key1 = %s\n", notmuch_config_get (db, "test.key1"));
+   printf("test.key2 = %s\n", notmuch_config_get (db, "test.key2"));
+}
+EOF
+cat <<'EOF' >EXPECTED
+== stdout ==
+test.key1 = testvalue1
+test.key2 = testvalue2
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_done
-- 
2.28.0

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 05/19] WIP: add notmuch_config_set
  2020-08-08 14:16 Initial implementation of merged config David Bremner
                   ` (3 preceding siblings ...)
  2020-08-08 14:16 ` [PATCH 04/19] WIP: add notmuch_config_get David Bremner
@ 2020-08-08 14:16 ` David Bremner
  2020-08-08 14:16 ` [PATCH 06/19] WIP: initial retrieval of database path from config file David Bremner
                   ` (13 subsequent siblings)
  18 siblings, 0 replies; 22+ messages in thread
From: David Bremner @ 2020-08-08 14:16 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

To be decided: is the write_through paramater a good idea? Should we
simplify the API?
---
 lib/config.cc          | 16 ++++++++++++++++
 lib/notmuch-private.h  |  5 +++++
 lib/notmuch.h          |  5 +++++
 lib/string-map.c       | 16 ++++++++++++++++
 test/T590-libconfig.sh | 30 ++++++++++++++++++++++++++++++
 5 files changed, 72 insertions(+)

diff --git a/lib/config.cc b/lib/config.cc
index 0a91ec03..2cfd2882 100644
--- a/lib/config.cc
+++ b/lib/config.cc
@@ -225,3 +225,19 @@ notmuch_config_get (notmuch_database_t *notmuch, const char *key) {
 
     return _notmuch_string_map_get (notmuch->config, key);
 }
+
+notmuch_status_t
+notmuch_config_set (notmuch_database_t *notmuch,
+		    const char *key,
+		    const char *val,
+		    notmuch_bool_t write_through)
+{
+    notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
+
+    _notmuch_string_map_set (notmuch->config, key, val);
+
+    if (write_through)
+	status = notmuch_database_set_config (notmuch, key, val);
+
+    return status;
+}
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index 0f26b371..b545fc46 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -638,6 +638,11 @@ _notmuch_string_map_append (notmuch_string_map_t *map,
 			    const char *key,
 			    const char *value);
 
+void
+_notmuch_string_map_set (notmuch_string_map_t *map,
+			    const char *key,
+			    const char *value);
+
 const char *
 _notmuch_string_map_get (notmuch_string_map_t *map, const char *key);
 
diff --git a/lib/notmuch.h b/lib/notmuch.h
index 777116bb..e147d5e6 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -2400,6 +2400,11 @@ notmuch_config_list_destroy (notmuch_config_list_t *config_list);
 const char *
 notmuch_config_get (notmuch_database_t *notmuch, const char *key);
 
+notmuch_status_t
+notmuch_config_set (notmuch_database_t *notmuch, const char *key,
+		    const char *val,
+		    notmuch_bool_t write_through);
+
 /**
  * get the current default indexing options for a given database.
  *
diff --git a/lib/string-map.c b/lib/string-map.c
index a88404c7..9774dbe8 100644
--- a/lib/string-map.c
+++ b/lib/string-map.c
@@ -143,6 +143,22 @@ bsearch_first (notmuch_string_pair_t *array, size_t len, const char *key, bool e
 
 }
 
+void
+_notmuch_string_map_set (notmuch_string_map_t *map, const char *key, const char *val)
+{
+    notmuch_string_pair_t *pair;
+
+    /* this means that calling string_map_set invalidates iterators */
+    _notmuch_string_map_sort (map);
+    pair = bsearch_first (map->pairs, map->length, key, true);
+    if (! pair)
+	_notmuch_string_map_append (map, key, val);
+    else {
+	talloc_free (pair->value);
+	pair->value = talloc_strdup (map->pairs, val);
+    }
+}
+
 const char *
 _notmuch_string_map_get (notmuch_string_map_t *map, const char *key)
 {
diff --git a/test/T590-libconfig.sh b/test/T590-libconfig.sh
index 8a4519e9..c2bce4a2 100755
--- a/test/T590-libconfig.sh
+++ b/test/T590-libconfig.sh
@@ -184,4 +184,34 @@ test.key2 = testvalue2
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+backup_database
+test_begin_subtest "notmuch_config_set"
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+{
+   char *val;
+   printf("test.key1 = %s\n", notmuch_config_get (db, "test.key1"));
+   EXPECT0(notmuch_config_set (db, "test.key1", "overridden", FALSE));
+   printf("test.key1 = %s\n", notmuch_config_get (db, "test.key1"));
+   printf("test.key2 = %s\n", notmuch_config_get (db, "test.key2"));
+   EXPECT0(notmuch_database_get_config (db, "test.key1", &val));
+   printf("test.key1 (db) = %s\n", val);
+   EXPECT0(notmuch_config_set (db, "test.key2", "overridden2", TRUE));
+   printf("test.key2 = %s\n", notmuch_config_get (db, "test.key2"));
+   EXPECT0(notmuch_database_get_config (db, "test.key2", &val));
+   printf("test.key2 (db) = %s\n", val);
+}
+EOF
+cat <<'EOF' >EXPECTED
+== stdout ==
+test.key1 = testvalue1
+test.key1 = overridden
+test.key2 = testvalue2
+test.key1 (db) = testvalue1
+test.key2 = overridden2
+test.key2 (db) = overridden2
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+restore_database
+
 test_done
-- 
2.28.0

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 06/19] WIP: initial retrieval of database path from config file
  2020-08-08 14:16 Initial implementation of merged config David Bremner
                   ` (4 preceding siblings ...)
  2020-08-08 14:16 ` [PATCH 05/19] WIP: add notmuch_config_set David Bremner
@ 2020-08-08 14:16 ` David Bremner
  2020-08-08 14:16 ` [PATCH 07/19] test/libconfig; use n_database_open_with_config David Bremner
                   ` (12 subsequent siblings)
  18 siblings, 0 replies; 22+ messages in thread
From: David Bremner @ 2020-08-08 14:16 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

---
 lib/database.cc | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/lib/database.cc b/lib/database.cc
index 350abb11..2ab9170d 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -935,12 +935,27 @@ notmuch_database_open_with_config (const char *database_path,
     void *local = talloc_new (NULL);
     notmuch_database_t *notmuch = NULL;
     char *notmuch_path, *xapian_path, *incompat_features;
+    char *configured_database_path = NULL;
     char *message = NULL;
     struct stat st;
     int err;
     unsigned int i, version;
+    GKeyFile *key_file = NULL;
     static int initialized = 0;
 
+    /* XXX TODO: default locations for NULL case, handle profiles */
+    if (config_path != NULL && ! EMPTY_STRING (config_path)) {
+	key_file = g_key_file_new ();
+	if (! g_key_file_load_from_file (key_file, config_path, G_KEY_FILE_NONE, NULL)) {
+	    status = NOTMUCH_STATUS_FILE_ERROR;
+	    goto DONE;
+	}
+	configured_database_path = g_key_file_get_value (key_file, "database", "path", NULL);
+    }
+
+    if (database_path == NULL)
+	database_path = configured_database_path;
+
     if (database_path == NULL) {
 	message = strdup ("Error: Cannot open a database for a NULL path.\n");
 	status = NOTMUCH_STATUS_NULL_POINTER;
@@ -1100,6 +1115,9 @@ notmuch_database_open_with_config (const char *database_path,
   DONE:
     talloc_free (local);
 
+    if (key_file)
+	g_key_file_free (key_file);
+
     if (message) {
 	if (status_string)
 	    *status_string = message;
-- 
2.28.0

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 07/19] test/libconfig; use n_database_open_with_config
  2020-08-08 14:16 Initial implementation of merged config David Bremner
                   ` (5 preceding siblings ...)
  2020-08-08 14:16 ` [PATCH 06/19] WIP: initial retrieval of database path from config file David Bremner
@ 2020-08-08 14:16 ` David Bremner
  2020-08-08 14:16 ` [PATCH 08/19] WIP: add _notmuch_config_load_from_file David Bremner
                   ` (11 subsequent siblings)
  18 siblings, 0 replies; 22+ messages in thread
From: David Bremner @ 2020-08-08 14:16 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

This allows testing the "override database config with file
functionality". It also requires passing a config file explicitly to
each test program.
---
 test/T590-libconfig.sh | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/test/T590-libconfig.sh b/test/T590-libconfig.sh
index c2bce4a2..a34eae0b 100755
--- a/test/T590-libconfig.sh
+++ b/test/T590-libconfig.sh
@@ -16,7 +16,12 @@ int main (int argc, char** argv)
    char *val;
    notmuch_status_t stat;
 
-   EXPECT0(notmuch_database_open (argv[1], NOTMUCH_DATABASE_MODE_READ_WRITE, &db));
+   EXPECT0(notmuch_database_open_with_config (argv[1],
+                                              NOTMUCH_DATABASE_MODE_READ_WRITE,
+                                              argv[2],
+                                              NULL,
+                                              &db,
+                                              NULL));
 
 EOF
 
@@ -26,7 +31,7 @@ cat <<EOF > c_tail
 EOF
 
 test_begin_subtest "notmuch_database_{set,get}_config"
-cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} ${NOTMUCH_CONFIG}
 {
    EXPECT0(notmuch_database_set_config (db, "test.key1", "testvalue1"));
    EXPECT0(notmuch_database_set_config (db, "test.key2", "testvalue2"));
@@ -46,7 +51,7 @@ test_expect_equal_file EXPECTED OUTPUT
 
 
 test_begin_subtest "notmuch_database_get_config_list: empty list"
-cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} ${NOTMUCH_CONFIG}
 {
    notmuch_config_list_t *list;
    EXPECT0(notmuch_database_get_config_list (db, "nonexistent", &list));
@@ -78,7 +83,7 @@ EOF
 test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "notmuch_database_get_config_list: all pairs"
-cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} ${NOTMUCH_CONFIG}
 {
    notmuch_config_list_t *list;
    EXPECT0(notmuch_database_set_config (db, "zzzafter", "afterval"));
@@ -142,7 +147,7 @@ EOF
 test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "dump config"
-cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} ${NOTMUCH_CONFIG}
 {
     EXPECT0(notmuch_database_set_config (db, "key with spaces", "value, with, spaces!"));
 }
@@ -160,7 +165,7 @@ test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "restore config"
 notmuch dump --include=config >EXPECTED
-cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} ${NOTMUCH_CONFIG}
 {
     EXPECT0(notmuch_database_set_config (db, "test.key1", "mutatedvalue"));
 }
@@ -170,7 +175,7 @@ notmuch dump --include=config >OUTPUT
 test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "notmuch_config_get"
-cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} ${NOTMUCH_CONFIG}
 {
    printf("test.key1 = %s\n", notmuch_config_get (db, "test.key1"));
    printf("test.key2 = %s\n", notmuch_config_get (db, "test.key2"));
@@ -186,7 +191,7 @@ test_expect_equal_file EXPECTED OUTPUT
 
 backup_database
 test_begin_subtest "notmuch_config_set"
-cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} ${NOTMUCH_CONFIG}
 {
    char *val;
    printf("test.key1 = %s\n", notmuch_config_get (db, "test.key1"));
-- 
2.28.0

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 08/19] WIP: add _notmuch_config_load_from_file
  2020-08-08 14:16 Initial implementation of merged config David Bremner
                   ` (6 preceding siblings ...)
  2020-08-08 14:16 ` [PATCH 07/19] test/libconfig; use n_database_open_with_config David Bremner
@ 2020-08-08 14:16 ` David Bremner
  2020-08-08 14:16 ` [PATCH 09/19] lib: factor out feature name related code David Bremner
                   ` (10 subsequent siblings)
  18 siblings, 0 replies; 22+ messages in thread
From: David Bremner @ 2020-08-08 14:16 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

---
 lib/config.cc          | 34 ++++++++++++++++++++++++++++++++++
 lib/database.cc        |  4 ++++
 lib/notmuch-private.h  |  3 +++
 test/T590-libconfig.sh | 23 ++++++++++++++++++++++-
 4 files changed, 63 insertions(+), 1 deletion(-)

diff --git a/lib/config.cc b/lib/config.cc
index 2cfd2882..f5def3aa 100644
--- a/lib/config.cc
+++ b/lib/config.cc
@@ -241,3 +241,37 @@ notmuch_config_set (notmuch_database_t *notmuch,
 
     return status;
 }
+
+notmuch_status_t
+_notmuch_config_load_from_file (notmuch_database_t *notmuch,
+				GKeyFile *file)
+{
+    notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
+    gchar **groups,**keys, *val;
+
+    if (notmuch->config == NULL)
+	notmuch->config = _notmuch_string_map_create (notmuch);
+
+    if (unlikely(notmuch->config == NULL)) {
+	status = NOTMUCH_STATUS_OUT_OF_MEMORY;
+	goto DONE;
+    }
+
+    for (groups = g_key_file_get_groups (file, NULL); *groups; groups++) {
+	for (keys = g_key_file_get_keys (file, *groups, NULL, NULL); *keys; keys++) {
+	    char *absolute_key = talloc_asprintf(notmuch, "%s.%s", *groups,  *keys);
+	    val = g_key_file_get_value (file, *groups, *keys, NULL);
+	    if (! val) {
+		status = NOTMUCH_STATUS_FILE_ERROR;
+		goto DONE;
+	    }
+	    status = notmuch_config_set (notmuch, absolute_key, val, FALSE);
+	    talloc_free (absolute_key);
+	    if (status)
+		goto DONE;
+	}
+    }
+
+ DONE:
+    return status;
+}
diff --git a/lib/database.cc b/lib/database.cc
index 2ab9170d..93fef107 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -1104,6 +1104,10 @@ notmuch_database_open_with_config (const char *database_path,
 
 	if (status == NOTMUCH_STATUS_SUCCESS)
 	    status = _notmuch_config_load_from_database (notmuch);
+
+	if (status == NOTMUCH_STATUS_SUCCESS && key_file)
+	    status = _notmuch_config_load_from_file (notmuch, key_file);
+
     } catch (const Xapian::Error &error) {
 	IGNORE_RESULT (asprintf (&message, "A Xapian exception occurred opening database: %s\n",
 				 error.get_msg ().c_str ()));
diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h
index b545fc46..192a0771 100644
--- a/lib/notmuch-private.h
+++ b/lib/notmuch-private.h
@@ -707,6 +707,9 @@ struct _notmuch_indexopts {
 /* config.cc */
 notmuch_status_t
 _notmuch_config_load_from_database (notmuch_database_t * db);
+
+notmuch_status_t
+_notmuch_config_load_from_file (notmuch_database_t * db, GKeyFile *file);
 NOTMUCH_END_DECLS
 
 #ifdef __cplusplus
diff --git a/test/T590-libconfig.sh b/test/T590-libconfig.sh
index a34eae0b..a303319d 100755
--- a/test/T590-libconfig.sh
+++ b/test/T590-libconfig.sh
@@ -128,7 +128,7 @@ EOF
 test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "notmuch_database_get_config_list: one prefix"
-cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} ${NOTMUCH_CONFIG}
 {
    notmuch_config_list_t *list;
    EXPECT0(notmuch_database_get_config_list (db, "test.key", &list));
@@ -219,4 +219,25 @@ EOF
 test_expect_equal_file EXPECTED OUTPUT
 restore_database
 
+backup_database
+test_begin_subtest "override config from file"
+notmuch config set test.key1 overridden
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} ${NOTMUCH_CONFIG}
+{
+   printf("test.key1 = %s\n", notmuch_config_get (db, "test.key1"));
+   EXPECT0(notmuch_database_get_config (db, "test.key1", &val));
+   printf("test.key1 (db) = %s\n", val);
+   printf("test.key2 = %s\n", notmuch_config_get (db, "test.key2"));
+}
+EOF
+cat <<'EOF' >EXPECTED
+== stdout ==
+test.key1 = overridden
+test.key1 (db) = testvalue1
+test.key2 = testvalue2
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+restore_database
+
 test_done
-- 
2.28.0

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 09/19] lib: factor out feature name related code.
  2020-08-08 14:16 Initial implementation of merged config David Bremner
                   ` (7 preceding siblings ...)
  2020-08-08 14:16 ` [PATCH 08/19] WIP: add _notmuch_config_load_from_file David Bremner
@ 2020-08-08 14:16 ` David Bremner
  2020-08-08 14:16 ` [PATCH 10/19] lib: factor out prefix related code to its own file David Bremner
                   ` (9 subsequent siblings)
  18 siblings, 0 replies; 22+ messages in thread
From: David Bremner @ 2020-08-08 14:16 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

database.cc is uncomfortably large, and some of the static data
structures do not need to be shared as much as they are.

This is a somewhat small piece to factor out, but it will turn out to
be helpful to further refactoring.
---
 lib/Makefile.local     |   3 +-
 lib/database-private.h |  14 +++++
 lib/database.cc        | 116 +----------------------------------------
 lib/features.cc        | 114 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 131 insertions(+), 116 deletions(-)
 create mode 100644 lib/features.cc

diff --git a/lib/Makefile.local b/lib/Makefile.local
index 5dc057c0..b95ae216 100644
--- a/lib/Makefile.local
+++ b/lib/Makefile.local
@@ -59,7 +59,8 @@ libnotmuch_cxx_srcs =		\
 	$(dir)/config.cc	\
 	$(dir)/regexp-fields.cc	\
 	$(dir)/thread.cc \
-	$(dir)/thread-fp.cc
+	$(dir)/thread-fp.cc     \
+	$(dir)/features.cc
 
 libnotmuch_modules := $(libnotmuch_c_srcs:.c=.o) $(libnotmuch_cxx_srcs:.cc=.o)
 
diff --git a/lib/database-private.h b/lib/database-private.h
index bfeef869..3b365c41 100644
--- a/lib/database-private.h
+++ b/lib/database-private.h
@@ -32,6 +32,8 @@
 
 #include "notmuch-private.h"
 
+#define ARRAY_SIZE(arr) (sizeof (arr) / sizeof (arr[0]))
+
 #ifdef SILENCE_XAPIAN_DEPRECATION_WARNINGS
 #define XAPIAN_DEPRECATED(D) D
 #endif
@@ -266,4 +268,16 @@ _notmuch_database_find_doc_ids (notmuch_database_t *notmuch,
 				const char *value,
 				Xapian::PostingIterator *begin,
 				Xapian::PostingIterator *end);
+
+#define NOTMUCH_DATABASE_VERSION 3
+
+/* features.cc */
+
+_notmuch_features
+_notmuch_database_parse_features (const void *ctx, const char *features, unsigned int version,
+				  char mode, char **incompat_out);
+
+char *
+_notmuch_database_print_features (const void *ctx, unsigned int features);
+
 #endif
diff --git a/lib/database.cc b/lib/database.cc
index 93fef107..ceba6dd5 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -39,8 +39,6 @@
 
 using namespace std;
 
-#define ARRAY_SIZE(arr) (sizeof (arr) / sizeof (arr[0]))
-
 typedef struct {
     const char *name;
     const char *prefix;
@@ -458,41 +456,6 @@ _notmuch_database_prefix (notmuch_database_t *notmuch, const char *name)
     return NULL;
 }
 
-static const struct {
-    /* NOTMUCH_FEATURE_* value. */
-    _notmuch_features value;
-    /* Feature name as it appears in the database.  This name should
-     * be appropriate for displaying to the user if an older version
-     * of notmuch doesn't support this feature. */
-    const char *name;
-    /* Compatibility flags when this feature is declared. */
-    const char *flags;
-} feature_names[] = {
-    { NOTMUCH_FEATURE_FILE_TERMS,
-      "multiple paths per message", "rw" },
-    { NOTMUCH_FEATURE_DIRECTORY_DOCS,
-      "relative directory paths", "rw" },
-    /* Header values are not required for reading a database because a
-     * reader can just refer to the message file. */
-    { NOTMUCH_FEATURE_FROM_SUBJECT_ID_VALUES,
-      "from/subject/message-ID in database", "w" },
-    { NOTMUCH_FEATURE_BOOL_FOLDER,
-      "exact folder:/path: search", "rw" },
-    { NOTMUCH_FEATURE_GHOSTS,
-      "mail documents for missing messages", "w" },
-    /* Knowledge of the index mime-types are not required for reading
-     * a database because a reader will just be unable to query
-     * them. */
-    { NOTMUCH_FEATURE_INDEXED_MIMETYPES,
-      "indexed MIME types", "w" },
-    { NOTMUCH_FEATURE_LAST_MOD,
-      "modification tracking", "w" },
-    /* Existing databases will work fine for all queries not involving
-     * 'body:' */
-    { NOTMUCH_FEATURE_UNPREFIX_BODY_ONLY,
-      "index body and headers separately", "w" },
-};
-
 const char *
 notmuch_status_to_string (notmuch_status_t status)
 {
@@ -817,83 +780,6 @@ _notmuch_database_new_revision (notmuch_database_t *notmuch)
     return new_revision;
 }
 
-/* Parse a database features string from the given database version.
- * Returns the feature bit set.
- *
- * For version < 3, this ignores the features string and returns a
- * hard-coded set of features.
- *
- * If there are unrecognized features that are required to open the
- * database in mode (which should be 'r' or 'w'), return a
- * comma-separated list of unrecognized but required features in
- * *incompat_out suitable for presenting to the user.  *incompat_out
- * will be allocated from ctx.
- */
-static _notmuch_features
-_parse_features (const void *ctx, const char *features, unsigned int version,
-		 char mode, char **incompat_out)
-{
-    _notmuch_features res = static_cast<_notmuch_features>(0);
-    unsigned int namelen, i;
-    size_t llen = 0;
-    const char *flags;
-
-    /* Prior to database version 3, features were implied by the
-     * version number. */
-    if (version == 0)
-	return NOTMUCH_FEATURES_V0;
-    else if (version == 1)
-	return NOTMUCH_FEATURES_V1;
-    else if (version == 2)
-	return NOTMUCH_FEATURES_V2;
-
-    /* Parse the features string */
-    while ((features = strtok_len_c (features + llen, "\n", &llen)) != NULL) {
-	flags = strchr (features, '\t');
-	if (! flags || flags > features + llen)
-	    continue;
-	namelen = flags - features;
-
-	for (i = 0; i < ARRAY_SIZE (feature_names); ++i) {
-	    if (strlen (feature_names[i].name) == namelen &&
-		strncmp (feature_names[i].name, features, namelen) == 0) {
-		res |= feature_names[i].value;
-		break;
-	    }
-	}
-
-	if (i == ARRAY_SIZE (feature_names) && incompat_out) {
-	    /* Unrecognized feature */
-	    const char *have = strchr (flags, mode);
-	    if (have && have < features + llen) {
-		/* This feature is required to access this database in
-		 * 'mode', but we don't understand it. */
-		if (! *incompat_out)
-		    *incompat_out = talloc_strdup (ctx, "");
-		*incompat_out = talloc_asprintf_append_buffer (
-		    *incompat_out, "%s%.*s", **incompat_out ? ", " : "",
-		    namelen, features);
-	    }
-	}
-    }
-
-    return res;
-}
-
-static char *
-_print_features (const void *ctx, unsigned int features)
-{
-    unsigned int i;
-    char *res = talloc_strdup (ctx, "");
-
-    for (i = 0; i < ARRAY_SIZE (feature_names); ++i)
-	if (features & feature_names[i].value)
-	    res = talloc_asprintf_append_buffer (
-		res, "%s\t%s\n", feature_names[i].name, feature_names[i].flags);
-
-    return res;
-}
-
 notmuch_status_t
 notmuch_database_open (const char *path,
 		       notmuch_database_mode_t mode,
@@ -1711,7 +1597,7 @@ notmuch_database_upgrade (notmuch_database_t *notmuch,
     }
 
     status = NOTMUCH_STATUS_SUCCESS;
-    db->set_metadata ("features", _print_features (local, notmuch->features));
+    db->set_metadata ("features", _notmuch_database_print_features (local, notmuch->features));
     db->set_metadata ("version", STRINGIFY (NOTMUCH_DATABASE_VERSION));
 
   DONE:
diff --git a/lib/features.cc b/lib/features.cc
new file mode 100644
index 00000000..8def2461
--- /dev/null
+++ b/lib/features.cc
@@ -0,0 +1,114 @@
+#include "database-private.h"
+
+static const struct {
+    /* NOTMUCH_FEATURE_* value. */
+    _notmuch_features value;
+    /* Feature name as it appears in the database.  This name should
+     * be appropriate for displaying to the user if an older version
+     * of notmuch doesn't support this feature. */
+    const char *name;
+    /* Compatibility flags when this feature is declared. */
+    const char *flags;
+} feature_names[] = {
+    { NOTMUCH_FEATURE_FILE_TERMS,
+      "multiple paths per message", "rw" },
+    { NOTMUCH_FEATURE_DIRECTORY_DOCS,
+      "relative directory paths", "rw" },
+    /* Header values are not required for reading a database because a
+     * reader can just refer to the message file. */
+    { NOTMUCH_FEATURE_FROM_SUBJECT_ID_VALUES,
+      "from/subject/message-ID in database", "w" },
+    { NOTMUCH_FEATURE_BOOL_FOLDER,
+      "exact folder:/path: search", "rw" },
+    { NOTMUCH_FEATURE_GHOSTS,
+      "mail documents for missing messages", "w" },
+    /* Knowledge of the index mime-types are not required for reading
+     * a database because a reader will just be unable to query
+     * them. */
+    { NOTMUCH_FEATURE_INDEXED_MIMETYPES,
+      "indexed MIME types", "w" },
+    { NOTMUCH_FEATURE_LAST_MOD,
+      "modification tracking", "w" },
+    /* Existing databases will work fine for all queries not involving
+     * 'body:' */
+    { NOTMUCH_FEATURE_UNPREFIX_BODY_ONLY,
+      "index body and headers separately", "w" },
+};
+
+char *
+_notmuch_database_print_features (const void *ctx, unsigned int features)
+{
+    unsigned int i;
+    char *res = talloc_strdup (ctx, "");
+
+    for (i = 0; i < ARRAY_SIZE (feature_names); ++i)
+	if (features & feature_names[i].value)
+	    res = talloc_asprintf_append_buffer (
+		res, "%s\t%s\n", feature_names[i].name, feature_names[i].flags);
+
+    return res;
+}
+
+
+/* Parse a database features string from the given database version.
+ * Returns the feature bit set.
+ *
+ * For version < 3, this ignores the features string and returns a
+ * hard-coded set of features.
+ *
+ * If there are unrecognized features that are required to open the
+ * database in mode (which should be 'r' or 'w'), return a
+ * comma-separated list of unrecognized but required features in
+ * *incompat_out suitable for presenting to the user.  *incompat_out
+ * will be allocated from ctx.
+ */
+_notmuch_features
+_notmuch_database_parse_features (const void *ctx, const char *features, unsigned int version,
+		 char mode, char **incompat_out)
+{
+    _notmuch_features res = static_cast<_notmuch_features>(0);
+    unsigned int namelen, i;
+    size_t llen = 0;
+    const char *flags;
+
+    /* Prior to database version 3, features were implied by the
+     * version number. */
+    if (version == 0)
+	return NOTMUCH_FEATURES_V0;
+    else if (version == 1)
+	return NOTMUCH_FEATURES_V1;
+    else if (version == 2)
+	return NOTMUCH_FEATURES_V2;
+
+    /* Parse the features string */
+    while ((features = strtok_len_c (features + llen, "\n", &llen)) != NULL) {
+	flags = strchr (features, '\t');
+	if (! flags || flags > features + llen)
+	    continue;
+	namelen = flags - features;
+
+	for (i = 0; i < ARRAY_SIZE (feature_names); ++i) {
+	    if (strlen (feature_names[i].name) == namelen &&
+		strncmp (feature_names[i].name, features, namelen) == 0) {
+		res |= feature_names[i].value;
+		break;
+	    }
+	}
+
+	if (i == ARRAY_SIZE (feature_names) && incompat_out) {
+	    /* Unrecognized feature */
+	    const char *have = strchr (flags, mode);
+	    if (have && have < features + llen) {
+		/* This feature is required to access this database in
+		 * 'mode', but we don't understand it. */
+		if (! *incompat_out)
+		    *incompat_out = talloc_strdup (ctx, "");
+		*incompat_out = talloc_asprintf_append_buffer (
+		    *incompat_out, "%s%.*s", **incompat_out ? ", " : "",
+		    namelen, features);
+	    }
+	}
+    }
+
+    return res;
+}
-- 
2.28.0

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 10/19] lib: factor out prefix related code to its own file
  2020-08-08 14:16 Initial implementation of merged config David Bremner
                   ` (8 preceding siblings ...)
  2020-08-08 14:16 ` [PATCH 09/19] lib: factor out feature name related code David Bremner
@ 2020-08-08 14:16 ` David Bremner
  2020-08-08 14:16 ` [PATCH 11/19] lib: factor out notmuch_database_open* related code to " David Bremner
                   ` (8 subsequent siblings)
  18 siblings, 0 replies; 22+ messages in thread
From: David Bremner @ 2020-08-08 14:16 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

Reduce the size of database.cc, and limit the scope of prefix_table,
make sure it's accessed via a well-defined internal API.
---
 lib/Makefile.local     |   4 +-
 lib/database-private.h |   7 ++
 lib/database.cc        | 204 ++-------------------------------------
 lib/prefix.cc          | 210 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 229 insertions(+), 196 deletions(-)
 create mode 100644 lib/prefix.cc

diff --git a/lib/Makefile.local b/lib/Makefile.local
index b95ae216..15e71ef8 100644
--- a/lib/Makefile.local
+++ b/lib/Makefile.local
@@ -60,7 +60,9 @@ libnotmuch_cxx_srcs =		\
 	$(dir)/regexp-fields.cc	\
 	$(dir)/thread.cc \
 	$(dir)/thread-fp.cc     \
-	$(dir)/features.cc
+	$(dir)/features.cc	\
+	$(dir)/prefix.cc
+
 
 libnotmuch_modules := $(libnotmuch_c_srcs:.c=.o) $(libnotmuch_cxx_srcs:.cc=.o)
 
diff --git a/lib/database-private.h b/lib/database-private.h
index 3b365c41..d83cf0d0 100644
--- a/lib/database-private.h
+++ b/lib/database-private.h
@@ -280,4 +280,11 @@ _notmuch_database_parse_features (const void *ctx, const char *features, unsigne
 char *
 _notmuch_database_print_features (const void *ctx, unsigned int features);
 
+/* prefix.cc */
+notmuch_status_t
+_notmuch_database_setup_standard_query_fields (notmuch_database_t *notmuch);
+
+notmuch_status_t
+_notmuch_database_setup_user_query_fields (notmuch_database_t *notmuch);
+
 #endif
diff --git a/lib/database.cc b/lib/database.cc
index ceba6dd5..0620de5a 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -263,80 +263,6 @@ _notmuch_database_mode (notmuch_database_t *notmuch)
  *			same thread.
  */
 
-/* With these prefix values we follow the conventions published here:
- *
- * https://xapian.org/docs/omega/termprefixes.html
- *
- * as much as makes sense. Note that I took some liberty in matching
- * the reserved prefix values to notmuch concepts, (for example, 'G'
- * is documented as "newsGroup (or similar entity - e.g. a web forum
- * name)", for which I think the thread is the closest analogue in
- * notmuch. This in spite of the fact that we will eventually be
- * storing mailing-list messages where 'G' for "mailing list name"
- * might be even a closer analogue. I'm treating the single-character
- * prefixes preferentially for core notmuch concepts (which will be
- * nearly universal to all mail messages).
- */
-
-static const
-prefix_t prefix_table[] = {
-    /* name			term prefix	flags */
-    { "type",                   "T",            NOTMUCH_FIELD_NO_FLAGS },
-    { "reference",              "XREFERENCE",   NOTMUCH_FIELD_NO_FLAGS },
-    { "replyto",                "XREPLYTO",     NOTMUCH_FIELD_NO_FLAGS },
-    { "directory",              "XDIRECTORY",   NOTMUCH_FIELD_NO_FLAGS },
-    { "file-direntry",          "XFDIRENTRY",   NOTMUCH_FIELD_NO_FLAGS },
-    { "directory-direntry",     "XDDIRENTRY",   NOTMUCH_FIELD_NO_FLAGS },
-    { "body",                   "",             NOTMUCH_FIELD_EXTERNAL |
-      NOTMUCH_FIELD_PROBABILISTIC },
-    { "thread",                 "G",            NOTMUCH_FIELD_EXTERNAL |
-      NOTMUCH_FIELD_PROCESSOR },
-    { "tag",                    "K",            NOTMUCH_FIELD_EXTERNAL |
-      NOTMUCH_FIELD_PROCESSOR },
-    { "is",                     "K",            NOTMUCH_FIELD_EXTERNAL |
-      NOTMUCH_FIELD_PROCESSOR },
-    { "id",                     "Q",            NOTMUCH_FIELD_EXTERNAL },
-    { "mid",                    "Q",            NOTMUCH_FIELD_EXTERNAL |
-      NOTMUCH_FIELD_PROCESSOR },
-    { "path",                   "P",            NOTMUCH_FIELD_EXTERNAL |
-      NOTMUCH_FIELD_PROCESSOR },
-    { "property",               "XPROPERTY",    NOTMUCH_FIELD_EXTERNAL },
-    /*
-     * Unconditionally add ':' to reduce potential ambiguity with
-     * overlapping prefixes and/or terms that start with capital
-     * letters. See Xapian document termprefixes.html for related
-     * discussion.
-     */
-    { "folder",                 "XFOLDER:",     NOTMUCH_FIELD_EXTERNAL |
-      NOTMUCH_FIELD_PROCESSOR },
-    { "date",                   NULL,           NOTMUCH_FIELD_EXTERNAL |
-      NOTMUCH_FIELD_PROCESSOR },
-    { "query",                  NULL,           NOTMUCH_FIELD_EXTERNAL |
-      NOTMUCH_FIELD_PROCESSOR },
-    { "from",                   "XFROM",        NOTMUCH_FIELD_EXTERNAL |
-      NOTMUCH_FIELD_PROBABILISTIC |
-      NOTMUCH_FIELD_PROCESSOR },
-    { "to",                     "XTO",          NOTMUCH_FIELD_EXTERNAL |
-      NOTMUCH_FIELD_PROBABILISTIC },
-    { "attachment",             "XATTACHMENT",  NOTMUCH_FIELD_EXTERNAL |
-      NOTMUCH_FIELD_PROBABILISTIC },
-    { "mimetype",               "XMIMETYPE",    NOTMUCH_FIELD_EXTERNAL |
-      NOTMUCH_FIELD_PROBABILISTIC },
-    { "subject",                "XSUBJECT",     NOTMUCH_FIELD_EXTERNAL |
-      NOTMUCH_FIELD_PROBABILISTIC |
-      NOTMUCH_FIELD_PROCESSOR },
-};
-
-static void
-_setup_query_field_default (const prefix_t *prefix, notmuch_database_t *notmuch)
-{
-    if (prefix->prefix)
-	notmuch->query_parser->add_prefix ("", prefix->prefix);
-    if (prefix->flags & NOTMUCH_FIELD_PROBABILISTIC)
-	notmuch->query_parser->add_prefix (prefix->name, prefix->prefix);
-    else
-	notmuch->query_parser->add_boolean_prefix (prefix->name, prefix->prefix);
-}
 
 notmuch_string_map_iterator_t *
 _notmuch_database_user_headers (notmuch_database_t *notmuch)
@@ -344,118 +270,6 @@ _notmuch_database_user_headers (notmuch_database_t *notmuch)
     return _notmuch_string_map_iterator_create (notmuch->user_header, "", false);
 }
 
-const char *
-_user_prefix (void *ctx, const char *name)
-{
-    return talloc_asprintf (ctx, "XU%s:", name);
-}
-
-static notmuch_status_t
-_setup_user_query_fields (notmuch_database_t *notmuch)
-{
-    notmuch_config_list_t *list;
-    notmuch_status_t status;
-
-    notmuch->user_prefix = _notmuch_string_map_create (notmuch);
-    if (notmuch->user_prefix == NULL)
-	return NOTMUCH_STATUS_OUT_OF_MEMORY;
-
-    notmuch->user_header = _notmuch_string_map_create (notmuch);
-    if (notmuch->user_header == NULL)
-	return NOTMUCH_STATUS_OUT_OF_MEMORY;
-
-    status = notmuch_database_get_config_list (notmuch, CONFIG_HEADER_PREFIX, &list);
-    if (status)
-	return status;
-
-    for (; notmuch_config_list_valid (list); notmuch_config_list_move_to_next (list)) {
-
-	prefix_t query_field;
-
-	const char *key = notmuch_config_list_key (list)
-			  + sizeof (CONFIG_HEADER_PREFIX) - 1;
-
-	_notmuch_string_map_append (notmuch->user_prefix,
-				    key,
-				    _user_prefix (notmuch, key));
-
-	_notmuch_string_map_append (notmuch->user_header,
-				    key,
-				    notmuch_config_list_value (list));
-
-	query_field.name = talloc_strdup (notmuch, key);
-	query_field.prefix = _user_prefix (notmuch, key);
-	query_field.flags = NOTMUCH_FIELD_PROBABILISTIC
-			    | NOTMUCH_FIELD_EXTERNAL;
-
-	_setup_query_field_default (&query_field, notmuch);
-    }
-
-    notmuch_config_list_destroy (list);
-
-    return NOTMUCH_STATUS_SUCCESS;
-}
-
-static void
-_setup_query_field (const prefix_t *prefix, notmuch_database_t *notmuch)
-{
-    if (prefix->flags & NOTMUCH_FIELD_PROCESSOR) {
-	Xapian::FieldProcessor *fp;
-
-	if (STRNCMP_LITERAL (prefix->name, "date") == 0)
-	    fp = (new DateFieldProcessor(NOTMUCH_VALUE_TIMESTAMP))->release ();
-	else if (STRNCMP_LITERAL(prefix->name, "query") == 0)
-	    fp = (new QueryFieldProcessor (*notmuch->query_parser, notmuch))->release ();
-	else if (STRNCMP_LITERAL (prefix->name, "thread") == 0)
-	    fp = (new ThreadFieldProcessor (*notmuch->query_parser, notmuch))->release ();
-	else
-	    fp = (new RegexpFieldProcessor (prefix->name, prefix->flags,
-					    *notmuch->query_parser, notmuch))->release ();
-
-	/* we treat all field-processor fields as boolean in order to get the raw input */
-	if (prefix->prefix)
-	    notmuch->query_parser->add_prefix ("", prefix->prefix);
-	notmuch->query_parser->add_boolean_prefix (prefix->name, fp);
-    } else {
-	_setup_query_field_default (prefix, notmuch);
-    }
-}
-
-const char *
-_find_prefix (const char *name)
-{
-    unsigned int i;
-
-    for (i = 0; i < ARRAY_SIZE (prefix_table); i++) {
-	if (strcmp (name, prefix_table[i].name) == 0)
-	    return prefix_table[i].prefix;
-    }
-
-    INTERNAL_ERROR ("No prefix exists for '%s'\n", name);
-
-    return "";
-}
-
-/* Like find prefix, but include the possibility of user defined
- * prefixes specific to this database */
-
-const char *
-_notmuch_database_prefix (notmuch_database_t *notmuch, const char *name)
-{
-    unsigned int i;
-
-    /*XXX TODO: reduce code duplication */
-    for (i = 0; i < ARRAY_SIZE (prefix_table); i++) {
-	if (strcmp (name, prefix_table[i].name) == 0)
-	    return prefix_table[i].prefix;
-    }
-
-    if (notmuch->user_prefix)
-	return _notmuch_string_map_get (notmuch->user_prefix, name);
-
-    return NULL;
-}
-
 const char *
 notmuch_status_to_string (notmuch_status_t status)
 {
@@ -825,7 +639,7 @@ notmuch_database_open_with_config (const char *database_path,
     char *message = NULL;
     struct stat st;
     int err;
-    unsigned int i, version;
+    unsigned version;
     GKeyFile *key_file = NULL;
     static int initialized = 0;
 
@@ -926,7 +740,7 @@ notmuch_database_open_with_config (const char *database_path,
 
 	/* Check features. */
 	incompat_features = NULL;
-	notmuch->features = _parse_features (
+	notmuch->features = _notmuch_database_parse_features (
 	    local, notmuch->xapian_db->get_metadata ("features").c_str (),
 	    version, mode == NOTMUCH_DATABASE_MODE_READ_WRITE ? 'w' : 'r',
 	    &incompat_features);
@@ -980,13 +794,13 @@ notmuch_database_open_with_config (const char *database_path,
 	notmuch->query_parser->add_rangeprocessor (notmuch->date_range_processor);
 	notmuch->query_parser->add_rangeprocessor (notmuch->last_mod_range_processor);
 
-	for (i = 0; i < ARRAY_SIZE (prefix_table); i++) {
-	    const prefix_t *prefix = &prefix_table[i];
-	    if (prefix->flags & NOTMUCH_FIELD_EXTERNAL) {
-		_setup_query_field (prefix, notmuch);
-	    }
-	}
-	status = _setup_user_query_fields (notmuch);
+	status = _notmuch_database_setup_standard_query_fields (notmuch);
+	if (status)
+	    goto DONE;
+
+	status = _notmuch_database_setup_user_query_fields (notmuch);
+	if (status)
+	    goto DONE;
 
 	if (status == NOTMUCH_STATUS_SUCCESS)
 	    status = _notmuch_config_load_from_database (notmuch);
diff --git a/lib/prefix.cc b/lib/prefix.cc
new file mode 100644
index 00000000..dd7b193d
--- /dev/null
+++ b/lib/prefix.cc
@@ -0,0 +1,210 @@
+#include "database-private.h"
+#include "query-fp.h"
+#include "thread-fp.h"
+#include "regexp-fields.h"
+#include "parse-time-vrp.h"
+
+typedef struct {
+    const char *name;
+    const char *prefix;
+    notmuch_field_flag_t flags;
+} prefix_t;
+
+/* With these prefix values we follow the conventions published here:
+ *
+ * https://xapian.org/docs/omega/termprefixes.html
+ *
+ * as much as makes sense. Note that I took some liberty in matching
+ * the reserved prefix values to notmuch concepts, (for example, 'G'
+ * is documented as "newsGroup (or similar entity - e.g. a web forum
+ * name)", for which I think the thread is the closest analogue in
+ * notmuch. This in spite of the fact that we will eventually be
+ * storing mailing-list messages where 'G' for "mailing list name"
+ * might be even a closer analogue. I'm treating the single-character
+ * prefixes preferentially for core notmuch concepts (which will be
+ * nearly universal to all mail messages).
+ */
+
+static const
+prefix_t prefix_table[] = {
+    /* name			term prefix	flags */
+    { "type",                   "T",            NOTMUCH_FIELD_NO_FLAGS },
+    { "reference",              "XREFERENCE",   NOTMUCH_FIELD_NO_FLAGS },
+    { "replyto",                "XREPLYTO",     NOTMUCH_FIELD_NO_FLAGS },
+    { "directory",              "XDIRECTORY",   NOTMUCH_FIELD_NO_FLAGS },
+    { "file-direntry",          "XFDIRENTRY",   NOTMUCH_FIELD_NO_FLAGS },
+    { "directory-direntry",     "XDDIRENTRY",   NOTMUCH_FIELD_NO_FLAGS },
+    { "body",                   "",             NOTMUCH_FIELD_EXTERNAL |
+      NOTMUCH_FIELD_PROBABILISTIC },
+    { "thread",                 "G",            NOTMUCH_FIELD_EXTERNAL |
+      NOTMUCH_FIELD_PROCESSOR },
+    { "tag",                    "K",            NOTMUCH_FIELD_EXTERNAL |
+      NOTMUCH_FIELD_PROCESSOR },
+    { "is",                     "K",            NOTMUCH_FIELD_EXTERNAL |
+      NOTMUCH_FIELD_PROCESSOR },
+    { "id",                     "Q",            NOTMUCH_FIELD_EXTERNAL },
+    { "mid",                    "Q",            NOTMUCH_FIELD_EXTERNAL |
+      NOTMUCH_FIELD_PROCESSOR },
+    { "path",                   "P",            NOTMUCH_FIELD_EXTERNAL |
+      NOTMUCH_FIELD_PROCESSOR },
+    { "property",               "XPROPERTY",    NOTMUCH_FIELD_EXTERNAL },
+    /*
+     * Unconditionally add ':' to reduce potential ambiguity with
+     * overlapping prefixes and/or terms that start with capital
+     * letters. See Xapian document termprefixes.html for related
+     * discussion.
+     */
+    { "folder",                 "XFOLDER:",     NOTMUCH_FIELD_EXTERNAL |
+      NOTMUCH_FIELD_PROCESSOR },
+    { "date",                   NULL,           NOTMUCH_FIELD_EXTERNAL |
+      NOTMUCH_FIELD_PROCESSOR },
+    { "query",                  NULL,           NOTMUCH_FIELD_EXTERNAL |
+      NOTMUCH_FIELD_PROCESSOR },
+    { "from",                   "XFROM",        NOTMUCH_FIELD_EXTERNAL |
+      NOTMUCH_FIELD_PROBABILISTIC |
+      NOTMUCH_FIELD_PROCESSOR },
+    { "to",                     "XTO",          NOTMUCH_FIELD_EXTERNAL |
+      NOTMUCH_FIELD_PROBABILISTIC },
+    { "attachment",             "XATTACHMENT",  NOTMUCH_FIELD_EXTERNAL |
+      NOTMUCH_FIELD_PROBABILISTIC },
+    { "mimetype",               "XMIMETYPE",    NOTMUCH_FIELD_EXTERNAL |
+      NOTMUCH_FIELD_PROBABILISTIC },
+    { "subject",                "XSUBJECT",     NOTMUCH_FIELD_EXTERNAL |
+      NOTMUCH_FIELD_PROBABILISTIC |
+      NOTMUCH_FIELD_PROCESSOR },
+};
+
+static const char *
+_user_prefix (void *ctx, const char *name)
+{
+    return talloc_asprintf (ctx, "XU%s:", name);
+}
+
+const char *
+_find_prefix (const char *name)
+{
+    unsigned int i;
+
+    for (i = 0; i < ARRAY_SIZE (prefix_table); i++) {
+	if (strcmp (name, prefix_table[i].name) == 0)
+	    return prefix_table[i].prefix;
+    }
+
+    INTERNAL_ERROR ("No prefix exists for '%s'\n", name);
+
+    return "";
+}
+
+/* Like find prefix, but include the possibility of user defined
+ * prefixes specific to this database */
+
+const char *
+_notmuch_database_prefix (notmuch_database_t *notmuch, const char *name)
+{
+    unsigned int i;
+
+    /*XXX TODO: reduce code duplication */
+    for (i = 0; i < ARRAY_SIZE (prefix_table); i++) {
+	if (strcmp (name, prefix_table[i].name) == 0)
+	    return prefix_table[i].prefix;
+    }
+
+    if (notmuch->user_prefix)
+	return _notmuch_string_map_get (notmuch->user_prefix, name);
+
+    return NULL;
+}
+
+static void
+_setup_query_field_default (const prefix_t *prefix, notmuch_database_t *notmuch)
+{
+    if (prefix->prefix)
+	notmuch->query_parser->add_prefix ("", prefix->prefix);
+    if (prefix->flags & NOTMUCH_FIELD_PROBABILISTIC)
+	notmuch->query_parser->add_prefix (prefix->name, prefix->prefix);
+    else
+	notmuch->query_parser->add_boolean_prefix (prefix->name, prefix->prefix);
+}
+
+static void
+_setup_query_field (const prefix_t *prefix, notmuch_database_t *notmuch)
+{
+    if (prefix->flags & NOTMUCH_FIELD_PROCESSOR) {
+	Xapian::FieldProcessor *fp;
+
+	if (STRNCMP_LITERAL (prefix->name, "date") == 0)
+	    fp = (new DateFieldProcessor(NOTMUCH_VALUE_TIMESTAMP))->release ();
+	else if (STRNCMP_LITERAL(prefix->name, "query") == 0)
+	    fp = (new QueryFieldProcessor (*notmuch->query_parser, notmuch))->release ();
+	else if (STRNCMP_LITERAL (prefix->name, "thread") == 0)
+	    fp = (new ThreadFieldProcessor (*notmuch->query_parser, notmuch))->release ();
+	else
+	    fp = (new RegexpFieldProcessor (prefix->name, prefix->flags,
+					    *notmuch->query_parser, notmuch))->release ();
+
+	/* we treat all field-processor fields as boolean in order to get the raw input */
+	if (prefix->prefix)
+	    notmuch->query_parser->add_prefix ("", prefix->prefix);
+	notmuch->query_parser->add_boolean_prefix (prefix->name, fp);
+    } else {
+	_setup_query_field_default (prefix, notmuch);
+    }
+}
+
+notmuch_status_t
+_notmuch_database_setup_standard_query_fields (notmuch_database_t *notmuch)
+{
+    for (unsigned int i = 0; i < ARRAY_SIZE (prefix_table); i++) {
+	const prefix_t *prefix = &prefix_table[i];
+	if (prefix->flags & NOTMUCH_FIELD_EXTERNAL) {
+	    _setup_query_field (prefix, notmuch);
+	}
+    }
+    return NOTMUCH_STATUS_SUCCESS;
+}
+
+notmuch_status_t
+_notmuch_database_setup_user_query_fields (notmuch_database_t *notmuch)
+{
+    notmuch_config_list_t *list;
+    notmuch_status_t status;
+
+    notmuch->user_prefix = _notmuch_string_map_create (notmuch);
+    if (notmuch->user_prefix == NULL)
+	return NOTMUCH_STATUS_OUT_OF_MEMORY;
+
+    notmuch->user_header = _notmuch_string_map_create (notmuch);
+    if (notmuch->user_header == NULL)
+	return NOTMUCH_STATUS_OUT_OF_MEMORY;
+
+    status = notmuch_database_get_config_list (notmuch, CONFIG_HEADER_PREFIX, &list);
+    if (status)
+	return status;
+
+    for (; notmuch_config_list_valid (list); notmuch_config_list_move_to_next (list)) {
+
+	prefix_t query_field;
+
+	const char *key = notmuch_config_list_key (list)
+			  + sizeof (CONFIG_HEADER_PREFIX) - 1;
+
+	_notmuch_string_map_append (notmuch->user_prefix,
+				    key,
+				    _user_prefix (notmuch, key));
+
+	_notmuch_string_map_append (notmuch->user_header,
+				    key,
+				    notmuch_config_list_value (list));
+
+	query_field.name = talloc_strdup (notmuch, key);
+	query_field.prefix = _user_prefix (notmuch, key);
+	query_field.flags = NOTMUCH_FIELD_PROBABILISTIC
+			    | NOTMUCH_FIELD_EXTERNAL;
+
+	_setup_query_field_default (&query_field, notmuch);
+    }
+
+    notmuch_config_list_destroy (list);
+
+    return NOTMUCH_STATUS_SUCCESS;
+}
-- 
2.28.0

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 11/19] lib: factor out notmuch_database_open* related code to own file
  2020-08-08 14:16 Initial implementation of merged config David Bremner
                   ` (9 preceding siblings ...)
  2020-08-08 14:16 ` [PATCH 10/19] lib: factor out prefix related code to its own file David Bremner
@ 2020-08-08 14:16 ` David Bremner
  2020-08-08 14:16 ` [PATCH 12/19] WIP: adding fallbacks for NULL config_path David Bremner
                   ` (7 subsequent siblings)
  18 siblings, 0 replies; 22+ messages in thread
From: David Bremner @ 2020-08-08 14:16 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

Reduce the size of database.cc, and prepare for adding missing features to
notmuch_database_open_with_config (to match API documentation already
in notmuch.h).
---
 lib/Makefile.local |   3 +-
 lib/database.cc    | 255 ---------------------------------------------
 lib/open.cc        | 253 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 255 insertions(+), 256 deletions(-)
 create mode 100644 lib/open.cc

diff --git a/lib/Makefile.local b/lib/Makefile.local
index 15e71ef8..01d3b34b 100644
--- a/lib/Makefile.local
+++ b/lib/Makefile.local
@@ -61,7 +61,8 @@ libnotmuch_cxx_srcs =		\
 	$(dir)/thread.cc \
 	$(dir)/thread-fp.cc     \
 	$(dir)/features.cc	\
-	$(dir)/prefix.cc
+	$(dir)/prefix.cc	\
+	$(dir)/open.cc
 
 
 libnotmuch_modules := $(libnotmuch_c_srcs:.c=.o) $(libnotmuch_cxx_srcs:.cc=.o)
diff --git a/lib/database.cc b/lib/database.cc
index 0620de5a..0f4e2ff9 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -19,10 +19,6 @@
  */
 
 #include "database-private.h"
-#include "parse-time-vrp.h"
-#include "query-fp.h"
-#include "thread-fp.h"
-#include "regexp-fields.h"
 #include "string-util.h"
 
 #include <iostream>
@@ -50,12 +46,6 @@ typedef struct {
 #define STRINGIFY(s) _SUB_STRINGIFY (s)
 #define _SUB_STRINGIFY(s) #s
 
-#if HAVE_XAPIAN_DB_RETRY_LOCK
-#define DB_ACTION (Xapian::DB_CREATE_OR_OPEN | Xapian::DB_RETRY_LOCK)
-#else
-#define DB_ACTION Xapian::DB_CREATE_OR_OPEN
-#endif
-
 #define LOG_XAPIAN_EXCEPTION(message, error) _log_xapian_exception (__location__, message, error)
 
 static void
@@ -594,251 +584,6 @@ _notmuch_database_new_revision (notmuch_database_t *notmuch)
     return new_revision;
 }
 
-notmuch_status_t
-notmuch_database_open (const char *path,
-		       notmuch_database_mode_t mode,
-		       notmuch_database_t **database)
-{
-    char *status_string = NULL;
-    notmuch_status_t status;
-
-    status = notmuch_database_open_verbose (path, mode, database,
-					    &status_string);
-
-    if (status_string) {
-	fputs (status_string, stderr);
-	free (status_string);
-    }
-
-    return status;
-}
-
-notmuch_status_t
-notmuch_database_open_verbose (const char *path,
-			       notmuch_database_mode_t mode,
-			       notmuch_database_t **database,
-			       char **status_string)
-{
-    return notmuch_database_open_with_config (path, mode, "", NULL,
-					      database, status_string);
-}
-
-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, *xapian_path, *incompat_features;
-    char *configured_database_path = NULL;
-    char *message = NULL;
-    struct stat st;
-    int err;
-    unsigned version;
-    GKeyFile *key_file = NULL;
-    static int initialized = 0;
-
-    /* XXX TODO: default locations for NULL case, handle profiles */
-    if (config_path != NULL && ! EMPTY_STRING (config_path)) {
-	key_file = g_key_file_new ();
-	if (! g_key_file_load_from_file (key_file, config_path, G_KEY_FILE_NONE, NULL)) {
-	    status = NOTMUCH_STATUS_FILE_ERROR;
-	    goto DONE;
-	}
-	configured_database_path = g_key_file_get_value (key_file, "database", "path", NULL);
-    }
-
-    if (database_path == NULL)
-	database_path = configured_database_path;
-
-    if (database_path == NULL) {
-	message = strdup ("Error: Cannot open a database for a NULL path.\n");
-	status = NOTMUCH_STATUS_NULL_POINTER;
-	goto DONE;
-    }
-
-    if (database_path[0] != '/') {
-	message = strdup ("Error: Database path must be absolute.\n");
-	status = NOTMUCH_STATUS_PATH_ERROR;
-	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;
-	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 ();
-#endif
-
-    /* Initialize gmime */
-    if (! initialized) {
-	g_mime_init ();
-	initialized = 1;
-    }
-
-    notmuch = talloc_zero (NULL, notmuch_database_t);
-    notmuch->exception_reported = false;
-    notmuch->status_string = NULL;
-    notmuch->config = NULL;
-    notmuch->path = talloc_strdup (notmuch, database_path);
-
-    strip_trailing (notmuch->path, '/');
-
-    notmuch->writable_xapian_db = NULL;
-    notmuch->atomic_nesting = 0;
-    notmuch->view = 1;
-    try {
-	string last_thread_id;
-	string last_mod;
-
-	if (mode == NOTMUCH_DATABASE_MODE_READ_WRITE) {
-	    notmuch->writable_xapian_db = new Xapian::WritableDatabase (xapian_path,
-									DB_ACTION);
-	    notmuch->xapian_db = notmuch->writable_xapian_db;
-	} else {
-	    notmuch->xapian_db = new Xapian::Database (xapian_path);
-	}
-
-	/* Check version.  As of database version 3, we represent
-	 * changes in terms of features, so assume a version bump
-	 * means a dramatically incompatible change. */
-	version = notmuch_database_get_version (notmuch);
-	if (version > NOTMUCH_DATABASE_VERSION) {
-	    IGNORE_RESULT (asprintf (&message,
-				     "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));
-	    notmuch_database_destroy (notmuch);
-	    notmuch = NULL;
-	    status = NOTMUCH_STATUS_FILE_ERROR;
-	    goto DONE;
-	}
-
-	/* Check features. */
-	incompat_features = NULL;
-	notmuch->features = _notmuch_database_parse_features (
-	    local, notmuch->xapian_db->get_metadata ("features").c_str (),
-	    version, mode == NOTMUCH_DATABASE_MODE_READ_WRITE ? 'w' : 'r',
-	    &incompat_features);
-	if (incompat_features) {
-	    IGNORE_RESULT (asprintf (&message,
-				     "Error: Notmuch database at %s\n"
-				     "       requires features (%s)\n"
-				     "       not supported by this version of notmuch.\n",
-				     notmuch_path, incompat_features));
-	    notmuch_database_destroy (notmuch);
-	    notmuch = NULL;
-	    status = NOTMUCH_STATUS_FILE_ERROR;
-	    goto DONE;
-	}
-
-	notmuch->last_doc_id = notmuch->xapian_db->get_lastdocid ();
-	last_thread_id = notmuch->xapian_db->get_metadata ("last_thread_id");
-	if (last_thread_id.empty ()) {
-	    notmuch->last_thread_id = 0;
-	} else {
-	    const char *str;
-	    char *end;
-
-	    str = last_thread_id.c_str ();
-	    notmuch->last_thread_id = strtoull (str, &end, 16);
-	    if (*end != '\0')
-		INTERNAL_ERROR ("Malformed database last_thread_id: %s", str);
-	}
-
-	/* Get current highest revision number. */
-	last_mod = notmuch->xapian_db->get_value_upper_bound (
-	    NOTMUCH_VALUE_LAST_MOD);
-	if (last_mod.empty ())
-	    notmuch->revision = 0;
-	else
-	    notmuch->revision = Xapian::sortable_unserialise (last_mod);
-	notmuch->uuid = talloc_strdup (
-	    notmuch, notmuch->xapian_db->get_uuid ().c_str ());
-
-	notmuch->query_parser = new Xapian::QueryParser;
-	notmuch->term_gen = new Xapian::TermGenerator;
-	notmuch->term_gen->set_stemmer (Xapian::Stem ("english"));
-	notmuch->value_range_processor = new Xapian::NumberRangeProcessor (NOTMUCH_VALUE_TIMESTAMP);
-	notmuch->date_range_processor = new ParseTimeRangeProcessor (NOTMUCH_VALUE_TIMESTAMP, "date:");
-	notmuch->last_mod_range_processor = new Xapian::NumberRangeProcessor (NOTMUCH_VALUE_LAST_MOD, "lastmod:");
-	notmuch->query_parser->set_default_op (Xapian::Query::OP_AND);
-	notmuch->query_parser->set_database (*notmuch->xapian_db);
-	notmuch->query_parser->set_stemmer (Xapian::Stem ("english"));
-	notmuch->query_parser->set_stemming_strategy (Xapian::QueryParser::STEM_SOME);
-	notmuch->query_parser->add_rangeprocessor (notmuch->value_range_processor);
-	notmuch->query_parser->add_rangeprocessor (notmuch->date_range_processor);
-	notmuch->query_parser->add_rangeprocessor (notmuch->last_mod_range_processor);
-
-	status = _notmuch_database_setup_standard_query_fields (notmuch);
-	if (status)
-	    goto DONE;
-
-	status = _notmuch_database_setup_user_query_fields (notmuch);
-	if (status)
-	    goto DONE;
-
-	if (status == NOTMUCH_STATUS_SUCCESS)
-	    status = _notmuch_config_load_from_database (notmuch);
-
-	if (status == NOTMUCH_STATUS_SUCCESS && key_file)
-	    status = _notmuch_config_load_from_file (notmuch, key_file);
-
-    } catch (const Xapian::Error &error) {
-	IGNORE_RESULT (asprintf (&message, "A Xapian exception occurred opening database: %s\n",
-				 error.get_msg ().c_str ()));
-	notmuch_database_destroy (notmuch);
-	notmuch = NULL;
-	status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
-    }
-
-  DONE:
-    talloc_free (local);
-
-    if (key_file)
-	g_key_file_free (key_file);
-
-    if (message) {
-	if (status_string)
-	    *status_string = message;
-	else
-	    free (message);
-    }
-
-    if (database)
-	*database = notmuch;
-    else
-	talloc_free (notmuch);
-
-    if (notmuch)
-	notmuch->open = true;
-
-    return status;
-}
 notmuch_status_t
 notmuch_database_close (notmuch_database_t *notmuch)
 {
diff --git a/lib/open.cc b/lib/open.cc
new file mode 100644
index 00000000..9b7da161
--- /dev/null
+++ b/lib/open.cc
@@ -0,0 +1,253 @@
+#include <unistd.h>
+#include "database-private.h"
+#include "parse-time-vrp.h"
+
+#if HAVE_XAPIAN_DB_RETRY_LOCK
+#define DB_ACTION (Xapian::DB_CREATE_OR_OPEN | Xapian::DB_RETRY_LOCK)
+#else
+#define DB_ACTION Xapian::DB_CREATE_OR_OPEN
+#endif
+
+notmuch_status_t
+notmuch_database_open (const char *path,
+		       notmuch_database_mode_t mode,
+		       notmuch_database_t **database)
+{
+    char *status_string = NULL;
+    notmuch_status_t status;
+
+    status = notmuch_database_open_verbose (path, mode, database,
+					    &status_string);
+
+    if (status_string) {
+	fputs (status_string, stderr);
+	free (status_string);
+    }
+
+    return status;
+}
+
+notmuch_status_t
+notmuch_database_open_verbose (const char *path,
+			       notmuch_database_mode_t mode,
+			       notmuch_database_t **database,
+			       char **status_string)
+{
+    return notmuch_database_open_with_config (path, mode, "", NULL,
+					      database, status_string);
+}
+
+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, *xapian_path, *incompat_features;
+    char *configured_database_path = NULL;
+    char *message = NULL;
+    struct stat st;
+    int err;
+    unsigned  version;
+    GKeyFile *key_file = NULL;
+    static int initialized = 0;
+
+    /* XXX TODO: default locations for NULL case, handle profiles */
+    if (config_path != NULL && ! EMPTY_STRING (config_path)) {
+	key_file = g_key_file_new ();
+	if (! g_key_file_load_from_file (key_file, config_path, G_KEY_FILE_NONE, NULL)) {
+	    status = NOTMUCH_STATUS_FILE_ERROR;
+	    goto DONE;
+	}
+	configured_database_path = g_key_file_get_value (key_file, "database", "path", NULL);
+    }
+
+    if (database_path == NULL)
+	database_path = configured_database_path;
+
+    if (database_path == NULL) {
+	message = strdup ("Error: Cannot open a database for a NULL path.\n");
+	status = NOTMUCH_STATUS_NULL_POINTER;
+	goto DONE;
+    }
+
+    if (database_path[0] != '/') {
+	message = strdup ("Error: Database path must be absolute.\n");
+	status = NOTMUCH_STATUS_PATH_ERROR;
+	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;
+	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 ();
+#endif
+
+    /* Initialize gmime */
+    if (! initialized) {
+	g_mime_init ();
+	initialized = 1;
+    }
+
+    notmuch = talloc_zero (NULL, notmuch_database_t);
+    notmuch->exception_reported = false;
+    notmuch->status_string = NULL;
+    notmuch->config = NULL;
+    notmuch->path = talloc_strdup (notmuch, database_path);
+
+    strip_trailing (notmuch->path, '/');
+
+    notmuch->writable_xapian_db = NULL;
+    notmuch->atomic_nesting = 0;
+    notmuch->view = 1;
+    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,
+									DB_ACTION);
+	    notmuch->xapian_db = notmuch->writable_xapian_db;
+	} else {
+	    notmuch->xapian_db = new Xapian::Database (xapian_path);
+	}
+
+	/* Check version.  As of database version 3, we represent
+	 * changes in terms of features, so assume a version bump
+	 * means a dramatically incompatible change. */
+	version = notmuch_database_get_version (notmuch);
+	if (version > NOTMUCH_DATABASE_VERSION) {
+	    IGNORE_RESULT (asprintf (&message,
+				     "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));
+	    notmuch_database_destroy (notmuch);
+	    notmuch = NULL;
+	    status = NOTMUCH_STATUS_FILE_ERROR;
+	    goto DONE;
+	}
+
+	/* Check features. */
+	incompat_features = NULL;
+	notmuch->features = _notmuch_database_parse_features (
+	    local, notmuch->xapian_db->get_metadata ("features").c_str (),
+	    version, mode == NOTMUCH_DATABASE_MODE_READ_WRITE ? 'w' : 'r',
+	    &incompat_features);
+	if (incompat_features) {
+	    IGNORE_RESULT (asprintf (&message,
+				     "Error: Notmuch database at %s\n"
+				     "       requires features (%s)\n"
+				     "       not supported by this version of notmuch.\n",
+				     notmuch_path, incompat_features));
+	    notmuch_database_destroy (notmuch);
+	    notmuch = NULL;
+	    status = NOTMUCH_STATUS_FILE_ERROR;
+	    goto DONE;
+	}
+
+	notmuch->last_doc_id = notmuch->xapian_db->get_lastdocid ();
+	last_thread_id = notmuch->xapian_db->get_metadata ("last_thread_id");
+	if (last_thread_id.empty ()) {
+	    notmuch->last_thread_id = 0;
+	} else {
+	    const char *str;
+	    char *end;
+
+	    str = last_thread_id.c_str ();
+	    notmuch->last_thread_id = strtoull (str, &end, 16);
+	    if (*end != '\0')
+		INTERNAL_ERROR ("Malformed database last_thread_id: %s", str);
+	}
+
+	/* Get current highest revision number. */
+	last_mod = notmuch->xapian_db->get_value_upper_bound (
+	    NOTMUCH_VALUE_LAST_MOD);
+	if (last_mod.empty ())
+	    notmuch->revision = 0;
+	else
+	    notmuch->revision = Xapian::sortable_unserialise (last_mod);
+	notmuch->uuid = talloc_strdup (
+	    notmuch, notmuch->xapian_db->get_uuid ().c_str ());
+
+	notmuch->query_parser = new Xapian::QueryParser;
+	notmuch->term_gen = new Xapian::TermGenerator;
+	notmuch->term_gen->set_stemmer (Xapian::Stem ("english"));
+	notmuch->value_range_processor = new Xapian::NumberRangeProcessor (NOTMUCH_VALUE_TIMESTAMP);
+	notmuch->date_range_processor = new ParseTimeRangeProcessor (NOTMUCH_VALUE_TIMESTAMP, "date:");
+	notmuch->last_mod_range_processor = new Xapian::NumberRangeProcessor (NOTMUCH_VALUE_LAST_MOD, "lastmod:");
+	notmuch->query_parser->set_default_op (Xapian::Query::OP_AND);
+	notmuch->query_parser->set_database (*notmuch->xapian_db);
+	notmuch->query_parser->set_stemmer (Xapian::Stem ("english"));
+	notmuch->query_parser->set_stemming_strategy (Xapian::QueryParser::STEM_SOME);
+	notmuch->query_parser->add_rangeprocessor (notmuch->value_range_processor);
+	notmuch->query_parser->add_rangeprocessor (notmuch->date_range_processor);
+	notmuch->query_parser->add_rangeprocessor (notmuch->last_mod_range_processor);
+
+	status = _notmuch_database_setup_standard_query_fields (notmuch);
+	if (status)
+	    goto DONE;
+	
+	status = _notmuch_database_setup_user_query_fields (notmuch);
+
+	if (status == NOTMUCH_STATUS_SUCCESS)
+	    status = _notmuch_config_load_from_database (notmuch);
+
+	if (status == NOTMUCH_STATUS_SUCCESS && key_file)
+	    status = _notmuch_config_load_from_file (notmuch, key_file);
+
+    } catch (const Xapian::Error &error) {
+	IGNORE_RESULT (asprintf (&message, "A Xapian exception occurred opening database: %s\n",
+				 error.get_msg ().c_str ()));
+	notmuch_database_destroy (notmuch);
+	notmuch = NULL;
+	status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
+    }
+
+  DONE:
+    talloc_free (local);
+
+    if (key_file)
+	g_key_file_free (key_file);
+
+    if (message) {
+	if (status_string)
+	    *status_string = message;
+	else
+	    free (message);
+    }
+
+    if (database)
+	*database = notmuch;
+    else
+	talloc_free (notmuch);
+
+    if (notmuch)
+	notmuch->open = true;
+
+    return status;
+}
-- 
2.28.0

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 12/19] WIP: adding fallbacks for NULL config_path
  2020-08-08 14:16 Initial implementation of merged config David Bremner
                   ` (10 preceding siblings ...)
  2020-08-08 14:16 ` [PATCH 11/19] lib: factor out notmuch_database_open* related code to " David Bremner
@ 2020-08-08 14:16 ` David Bremner
  2020-08-08 14:16 ` [PATCH 13/19] lib/config: delay setting talloc destructor David Bremner
                   ` (6 subsequent siblings)
  18 siblings, 0 replies; 22+ messages in thread
From: David Bremner @ 2020-08-08 14:16 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

still need to test combinations of NULL and non-NULL arguments
---
 lib/open.cc            |  95 ++++++++++++++++++++++++----
 test/T590-libconfig.sh | 138 +++++++++++++++++++++++++++++++++++++----
 2 files changed, 209 insertions(+), 24 deletions(-)

diff --git a/lib/open.cc b/lib/open.cc
index 9b7da161..213dc21e 100644
--- a/lib/open.cc
+++ b/lib/open.cc
@@ -37,6 +37,82 @@ notmuch_database_open_verbose (const char *path,
 					      database, status_string);
 }
 
+static const char *
+_xdg_dir (void *ctx,
+	  const char *xdg_root_variable,
+	  const char *xdg_prefix,
+	  const char *profile_name)
+{
+    const char *xdg_root = getenv (xdg_root_variable);
+    const char *home = getenv ("HOME");
+
+    if (! xdg_root) {
+	if (! home) return NULL;
+
+	xdg_root = talloc_asprintf (ctx,
+				    "%s/%s",
+				    home,
+				    xdg_prefix);
+    }
+
+    if (! profile_name)
+	profile_name = getenv ("NOTMUCH_PROFILE");
+
+    if (! profile_name)
+	profile_name = "default";
+
+    return talloc_asprintf (ctx,
+			    "%s/notmuch/%s",
+			    xdg_root,
+			    profile_name);
+}
+
+static notmuch_status_t
+_load_key_file (const char *path,
+		const char *profile,
+		GKeyFile **key_file)
+{
+    notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
+    void *local = talloc_new (NULL);
+
+    if (path && EMPTY_STRING (path))
+	goto DONE;
+
+    if (! path)
+	path = getenv ("NOTMUCH_CONFIG");
+
+    if (! path) {
+	const char *dir = _xdg_dir (local, "XDG_CONFIG_HOME", ".config", profile);
+
+	if (dir) {
+	    path = talloc_asprintf (local, "%s/config", dir);
+	    if (access (path, R_OK) !=0)
+		path = NULL;
+	}
+    }
+
+    if (! path) {
+	const char *home = getenv ("HOME");
+
+	path = talloc_asprintf (local, "%s/.notmuch-config", home);
+
+	if (! profile)
+	    profile = getenv ("NOTMUCH_PROFILE");
+
+	if (profile)
+	    path = talloc_asprintf (local, "%s.%s", path, profile);
+    }
+
+    *key_file = g_key_file_new ();
+    if (! g_key_file_load_from_file (*key_file, path, G_KEY_FILE_NONE, NULL)) {
+	status = NOTMUCH_STATUS_FILE_ERROR;
+    }
+
+DONE:
+    talloc_free (local);
+    return status;
+}
+
 notmuch_status_t
 notmuch_database_open_with_config (const char *database_path,
 				   notmuch_database_mode_t mode,
@@ -49,7 +125,6 @@ notmuch_database_open_with_config (const char *database_path,
     void *local = talloc_new (NULL);
     notmuch_database_t *notmuch = NULL;
     char *notmuch_path, *xapian_path, *incompat_features;
-    char *configured_database_path = NULL;
     char *message = NULL;
     struct stat st;
     int err;
@@ -57,18 +132,14 @@ notmuch_database_open_with_config (const char *database_path,
     GKeyFile *key_file = NULL;
     static int initialized = 0;
 
-    /* XXX TODO: default locations for NULL case, handle profiles */
-    if (config_path != NULL && ! EMPTY_STRING (config_path)) {
-	key_file = g_key_file_new ();
-	if (! g_key_file_load_from_file (key_file, config_path, G_KEY_FILE_NONE, NULL)) {
-	    status = NOTMUCH_STATUS_FILE_ERROR;
-	    goto DONE;
-	}
-	configured_database_path = g_key_file_get_value (key_file, "database", "path", NULL);
+    status = _load_key_file (config_path, profile, &key_file);
+    if (status) {
+	message = strdup ("Error: cannot load config file");
+	goto DONE;
     }
-
-    if (database_path == NULL)
-	database_path = configured_database_path;
+	
+    if (! database_path && key_file)
+	database_path = g_key_file_get_value (key_file, "database", "path", NULL);
 
     if (database_path == NULL) {
 	message = strdup ("Error: Cannot open a database for a NULL path.\n");
diff --git a/test/T590-libconfig.sh b/test/T590-libconfig.sh
index a303319d..cc814efa 100755
--- a/test/T590-libconfig.sh
+++ b/test/T590-libconfig.sh
@@ -15,14 +15,21 @@ int main (int argc, char** argv)
    notmuch_database_t *db;
    char *val;
    notmuch_status_t stat;
+   char *msg = NULL;
 
-   EXPECT0(notmuch_database_open_with_config (argv[1],
+   for (int i=1; i<argc; i++)
+      if (strcmp (argv[i], "%NULL%") == 0) argv[i] = NULL;
+
+   stat = notmuch_database_open_with_config (argv[1],
                                               NOTMUCH_DATABASE_MODE_READ_WRITE,
                                               argv[2],
-                                              NULL,
+                                              argv[3],
                                               &db,
-                                              NULL));
-
+                                              &msg);
+   if (stat != NOTMUCH_STATUS_SUCCESS) {
+     fprintf (stderr, "error opening database: %d %s\n", stat, msg ? msg : "");
+     exit (1);
+   }
 EOF
 
 cat <<EOF > c_tail
@@ -51,7 +58,7 @@ test_expect_equal_file EXPECTED OUTPUT
 
 
 test_begin_subtest "notmuch_database_get_config_list: empty list"
-cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} ${NOTMUCH_CONFIG}
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} ${NOTMUCH_CONFIG} %NULL%
 {
    notmuch_config_list_t *list;
    EXPECT0(notmuch_database_get_config_list (db, "nonexistent", &list));
@@ -83,7 +90,7 @@ EOF
 test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "notmuch_database_get_config_list: all pairs"
-cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} ${NOTMUCH_CONFIG}
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} ${NOTMUCH_CONFIG} %NULL%
 {
    notmuch_config_list_t *list;
    EXPECT0(notmuch_database_set_config (db, "zzzafter", "afterval"));
@@ -128,7 +135,7 @@ EOF
 test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "notmuch_database_get_config_list: one prefix"
-cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} ${NOTMUCH_CONFIG}
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} ${NOTMUCH_CONFIG} %NULL%
 {
    notmuch_config_list_t *list;
    EXPECT0(notmuch_database_get_config_list (db, "test.key", &list));
@@ -147,7 +154,7 @@ EOF
 test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "dump config"
-cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} ${NOTMUCH_CONFIG}
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} ${NOTMUCH_CONFIG} %NULL%
 {
     EXPECT0(notmuch_database_set_config (db, "key with spaces", "value, with, spaces!"));
 }
@@ -165,7 +172,7 @@ test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "restore config"
 notmuch dump --include=config >EXPECTED
-cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} ${NOTMUCH_CONFIG}
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} ${NOTMUCH_CONFIG} %NULL%
 {
     EXPECT0(notmuch_database_set_config (db, "test.key1", "mutatedvalue"));
 }
@@ -175,7 +182,7 @@ notmuch dump --include=config >OUTPUT
 test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "notmuch_config_get"
-cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} ${NOTMUCH_CONFIG}
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} ${NOTMUCH_CONFIG} %NULL%
 {
    printf("test.key1 = %s\n", notmuch_config_get (db, "test.key1"));
    printf("test.key2 = %s\n", notmuch_config_get (db, "test.key2"));
@@ -191,7 +198,7 @@ test_expect_equal_file EXPECTED OUTPUT
 
 backup_database
 test_begin_subtest "notmuch_config_set"
-cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} ${NOTMUCH_CONFIG}
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} ${NOTMUCH_CONFIG} %NULL%
 {
    char *val;
    printf("test.key1 = %s\n", notmuch_config_get (db, "test.key1"));
@@ -221,8 +228,32 @@ restore_database
 
 backup_database
 test_begin_subtest "override config from file"
+ovconfig=${TMP_DIRECTORY}/override-config
+cp ${NOTMUCH_CONFIG} "${ovconfig}"
+notmuch --config=${ovconfig} config set test.key1 overridden
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} "${ovconfig}" %NULL%
+{
+   printf("test.key1 = %s\n", notmuch_config_get (db, "test.key1"));
+   EXPECT0(notmuch_database_get_config (db, "test.key1", &val));
+   printf("test.key1 (db) = %s\n", val);
+   printf("test.key2 = %s\n", notmuch_config_get (db, "test.key2"));
+}
+EOF
+cat <<'EOF' >EXPECTED
+== stdout ==
+test.key1 = overridden
+test.key1 (db) = testvalue1
+test.key2 = testvalue2
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+restore_database
+
+backup_database
+test_begin_subtest "override config from \${NOTMUCH_CONFIG}"
 notmuch config set test.key1 overridden
-cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} ${NOTMUCH_CONFIG}
+# second argument omitted to make argv[2] == NULL
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR}
 {
    printf("test.key1 = %s\n", notmuch_config_get (db, "test.key1"));
    EXPECT0(notmuch_database_get_config (db, "test.key1", &val));
@@ -230,6 +261,7 @@ cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} ${NOTMUCH_CONFIG}
    printf("test.key2 = %s\n", notmuch_config_get (db, "test.key2"));
 }
 EOF
+notmuch config set test.key1
 cat <<'EOF' >EXPECTED
 == stdout ==
 test.key1 = overridden
@@ -240,4 +272,86 @@ EOF
 test_expect_equal_file EXPECTED OUTPUT
 restore_database
 
+backup_database
+test_begin_subtest "override config from \${HOME}/.notmuch-config"
+ovconfig=${HOME}/.notmuch-config
+cp ${NOTMUCH_CONFIG} ${ovconfig}
+old_NOTMUCH_CONFIG=${NOTMUCH_CONFIG}
+unset NOTMUCH_CONFIG
+notmuch --config=${ovconfig} config set test.key1 overridden-home
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} %NULL% %NULL%
+{
+   printf("test.key1 = %s\n", notmuch_config_get (db, "test.key1"));
+   EXPECT0(notmuch_database_get_config (db, "test.key1", &val));
+   printf("test.key1 (db) = %s\n", val);
+   printf("test.key2 = %s\n", notmuch_config_get (db, "test.key2"));
+}
+EOF
+rm -f ${ovconfig}
+NOTMUCH_CONFIG=${old_NOTMUCH_CONFIG}
+cat <<'EOF' >EXPECTED
+== stdout ==
+test.key1 = overridden-home
+test.key1 (db) = testvalue1
+test.key2 = testvalue2
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+restore_database
+
+backup_database
+test_begin_subtest "override config from \${XDG_CONFIG_HOME}/notmuch"
+ovconfig=${HOME}/.config/notmuch/default/config
+mkdir -p $(dirname ${ovconfig})
+cp ${NOTMUCH_CONFIG} ${ovconfig}
+old_NOTMUCH_CONFIG=${NOTMUCH_CONFIG}
+unset NOTMUCH_CONFIG
+notmuch --config=${ovconfig} config set test.key1 overridden-home
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} %NULL% %NULL%
+{
+   printf("test.key1 = %s\n", notmuch_config_get (db, "test.key1"));
+   EXPECT0(notmuch_database_get_config (db, "test.key1", &val));
+   printf("test.key1 (db) = %s\n", val);
+   printf("test.key2 = %s\n", notmuch_config_get (db, "test.key2"));
+}
+EOF
+rm -f ${ovconfig}
+NOTMUCH_CONFIG=${old_NOTMUCH_CONFIG}
+cat <<'EOF' >EXPECTED
+== stdout ==
+test.key1 = overridden-home
+test.key1 (db) = testvalue1
+test.key2 = testvalue2
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+restore_database
+
+backup_database
+test_begin_subtest "override config from \${HOME}/.notmuch-config.work (via args)"
+ovconfig=${HOME}/.notmuch-config.work
+cp ${NOTMUCH_CONFIG} ${ovconfig}
+old_NOTMUCH_CONFIG=${NOTMUCH_CONFIG}
+unset NOTMUCH_CONFIG
+notmuch --config=${ovconfig} config set test.key1 overridden-home
+cat c_head - c_tail <<'EOF' | test_C ${MAIL_DIR} %NULL% work
+{
+   printf("test.key1 = %s\n", notmuch_config_get (db, "test.key1"));
+   EXPECT0(notmuch_database_get_config (db, "test.key1", &val));
+   printf("test.key1 (db) = %s\n", val);
+   printf("test.key2 = %s\n", notmuch_config_get (db, "test.key2"));
+}
+EOF
+#rm -f ${ovconfig}
+NOTMUCH_CONFIG=${old_NOTMUCH_CONFIG}
+cat <<'EOF' >EXPECTED
+== stdout ==
+test.key1 = overridden-home
+test.key1 (db) = testvalue1
+test.key2 = testvalue2
+== stderr ==
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+restore_database
+
 test_done
-- 
2.28.0

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 13/19] lib/config: delay setting talloc destructor
  2020-08-08 14:16 Initial implementation of merged config David Bremner
                   ` (11 preceding siblings ...)
  2020-08-08 14:16 ` [PATCH 12/19] WIP: adding fallbacks for NULL config_path David Bremner
@ 2020-08-08 14:16 ` David Bremner
  2020-08-08 14:16 ` [PATCH 14/19] WIP: add strsplit_len David Bremner
                   ` (5 subsequent siblings)
  18 siblings, 0 replies; 22+ messages in thread
From: David Bremner @ 2020-08-08 14:16 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

If Xapian has thrown an exception, it is not safe to invoke the
destuctor when freeing the list struct.
---
 lib/config.cc | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/lib/config.cc b/lib/config.cc
index f5def3aa..ca7ac2a8 100644
--- a/lib/config.cc
+++ b/lib/config.cc
@@ -113,7 +113,6 @@ notmuch_database_get_config_list (notmuch_database_t *notmuch,
 	goto DONE;
     }
 
-    talloc_set_destructor (list, _notmuch_config_list_destroy);
     list->notmuch = notmuch;
     list->current_key = NULL;
     list->current_val = NULL;
@@ -133,8 +132,15 @@ notmuch_database_get_config_list (notmuch_database_t *notmuch,
     *out = list;
 
   DONE:
-    if (status && list)
-	talloc_free (list);
+    if (status) {
+	if (list) {
+	    talloc_free (list);
+	    if (status != NOTMUCH_STATUS_XAPIAN_EXCEPTION)
+		_notmuch_config_list_destroy (list);
+	}
+    }  else {
+	talloc_set_destructor (list, _notmuch_config_list_destroy);
+    }
 
     return status;
 }
-- 
2.28.0

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 14/19] WIP: add strsplit_len
  2020-08-08 14:16 Initial implementation of merged config David Bremner
                   ` (12 preceding siblings ...)
  2020-08-08 14:16 ` [PATCH 13/19] lib/config: delay setting talloc destructor David Bremner
@ 2020-08-08 14:16 ` David Bremner
  2020-08-08 14:16 ` [PATCH 15/19] WIP: add config values iterator David Bremner
                   ` (4 subsequent siblings)
  18 siblings, 0 replies; 22+ messages in thread
From: David Bremner @ 2020-08-08 14:16 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

---
 util/string-util.c | 23 +++++++++++++++++++++++
 util/string-util.h | 14 ++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/util/string-util.c b/util/string-util.c
index de8430b2..27f8a26b 100644
--- a/util/string-util.c
+++ b/util/string-util.c
@@ -24,6 +24,7 @@
 
 #include <ctype.h>
 #include <errno.h>
+#include <stdbool.h>
 
 char *
 strtok_len (char *s, const char *delim, size_t *len)
@@ -37,6 +38,28 @@ strtok_len (char *s, const char *delim, size_t *len)
     return *len ? s : NULL;
 }
 
+const char *
+strsplit_len (const char *s, char delim, size_t *len)
+{
+    bool escaping = false;
+    size_t count = 0;
+
+    /* Skip initial unescaped delimiters */
+    while (*s && *s == delim)
+	s++;
+
+    while (s[count] && (escaping || s[count] != delim)) {
+	escaping = (s[count] == '\\');
+	count++;
+    }
+
+    if (count==0)
+	return NULL;
+
+    *len = count;
+    return s;
+}
+
 const char *
 strtok_len_c (const char *s, const char *delim, size_t *len)
 {
diff --git a/util/string-util.h b/util/string-util.h
index fb95a740..80647c5f 100644
--- a/util/string-util.h
+++ b/util/string-util.h
@@ -26,6 +26,20 @@ char *strtok_len (char *s, const char *delim, size_t *len);
 /* Const version of strtok_len. */
 const char *strtok_len_c (const char *s, const char *delim, size_t *len);
 
+/* Simplified version of strtok_len, with a single delimiter.
+ * Handles escaping delimiters with \
+ * Usage pattern:
+ *
+ * const char *tok = input;
+ * const char *delim = ';';
+ * size_t tok_len = 0;
+ *
+ * while ((tok = strsplit_len (tok + tok_len, delim, &tok_len)) != NULL) {
+ *     // do stuff with string tok of length tok_len
+ * }
+ */
+const char *strsplit_len (const char *s, char delim, size_t *len);
+
 /* Return a talloced string with str sanitized.
  *
  * Whitespace characters (tabs and newlines) are replaced with spaces,
-- 
2.28.0

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 15/19] WIP: add config values iterator
  2020-08-08 14:16 Initial implementation of merged config David Bremner
                   ` (13 preceding siblings ...)
  2020-08-08 14:16 ` [PATCH 14/19] WIP: add strsplit_len David Bremner
@ 2020-08-08 14:16 ` David Bremner
  2020-08-08 14:16 ` [PATCH 16/19] test: add regression test for searching with alternate config David Bremner
                   ` (3 subsequent siblings)
  18 siblings, 0 replies; 22+ messages in thread
From: David Bremner @ 2020-08-08 14:16 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

---
 lib/config.cc | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 lib/notmuch.h | 17 +++++++++++++++++
 2 files changed, 66 insertions(+)

diff --git a/lib/config.cc b/lib/config.cc
index ca7ac2a8..dd042ab2 100644
--- a/lib/config.cc
+++ b/lib/config.cc
@@ -31,6 +31,11 @@ struct _notmuch_config_list {
     char *current_val;
 };
 
+struct _notmuch_config_values {
+    const char *iterator;
+    size_t tok_len;
+};
+
 static int
 _notmuch_config_list_destroy (notmuch_config_list_t *list)
 {
@@ -232,6 +237,50 @@ notmuch_config_get (notmuch_database_t *notmuch, const char *key) {
     return _notmuch_string_map_get (notmuch->config, key);
 }
 
+notmuch_config_values_t *
+notmuch_config_get_values (notmuch_database_t *notmuch, const char *key)
+{
+    notmuch_config_values_t *values;
+
+    const char *str;
+    str  = _notmuch_string_map_get (notmuch->config, key);
+    if (! str)
+	return NULL;
+
+    values = talloc (notmuch, notmuch_config_values_t);
+    if (unlikely(! values))
+	return NULL;
+
+    values->iterator = str;
+    values->tok_len = 0;
+    return values;
+}
+
+notmuch_bool_t
+notmuch_config_values_valid (notmuch_config_values_t *values) {
+    if (! values)
+	return false;
+
+    return (values->iterator != NULL);
+}
+
+const char *
+notmuch_config_values_get (notmuch_config_values_t *values) {
+    return talloc_strndup (values, values->iterator, values->tok_len);
+}
+
+void
+notmuch_config_values_move_to_next (notmuch_config_values_t *values) {
+    values->iterator += values->tok_len;
+    values->iterator = strsplit_len (values->iterator, ';', &(values->tok_len));
+}
+
+void
+notmuch_config_values_destroy (notmuch_config_values_t *values) {
+    talloc_free (values);
+}
+
+
 notmuch_status_t
 notmuch_config_set (notmuch_database_t *notmuch,
 		    const char *key,
diff --git a/lib/notmuch.h b/lib/notmuch.h
index e147d5e6..7ae69265 100644
--- a/lib/notmuch.h
+++ b/lib/notmuch.h
@@ -236,6 +236,7 @@ typedef struct _notmuch_tags notmuch_tags_t;
 typedef struct _notmuch_directory notmuch_directory_t;
 typedef struct _notmuch_filenames notmuch_filenames_t;
 typedef struct _notmuch_config_list notmuch_config_list_t;
+typedef struct _notmuch_config_values notmuch_config_values_t;
 typedef struct _notmuch_indexopts notmuch_indexopts_t;
 #endif /* __DOXYGEN__ */
 
@@ -2405,6 +2406,22 @@ notmuch_config_set (notmuch_database_t *notmuch, const char *key,
 		    const char *val,
 		    notmuch_bool_t write_through);
 
+
+notmuch_config_values_t *
+notmuch_config_get_values (notmuch_database_t *notmuch, const char *key);
+
+notmuch_bool_t
+notmuch_config_values_valid (notmuch_config_values_t *values);
+
+const char *
+notmuch_config_values_get (notmuch_config_values_t *values);
+
+void
+notmuch_config_values_move_to_next (notmuch_config_values_t *values);
+
+void
+notmuch_config_values_destroy (notmuch_config_values_t *values);
+
 /**
  * get the current default indexing options for a given database.
  *
-- 
2.28.0

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 16/19] test: add regression test for searching with alternate config
  2020-08-08 14:16 Initial implementation of merged config David Bremner
                   ` (14 preceding siblings ...)
  2020-08-08 14:16 ` [PATCH 15/19] WIP: add config values iterator David Bremner
@ 2020-08-08 14:16 ` David Bremner
  2020-08-08 14:16 ` [PATCH 17/19] cli/config: add accessor for config file name David Bremner
                   ` (2 subsequent siblings)
  18 siblings, 0 replies; 22+ messages in thread
From: David Bremner @ 2020-08-08 14:16 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

Make sure upcoming changes to config handling do not break command
line specification.
---
 test/T140-excludes.sh | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/test/T140-excludes.sh b/test/T140-excludes.sh
index 0cf69975..cef07095 100755
--- a/test/T140-excludes.sh
+++ b/test/T140-excludes.sh
@@ -39,6 +39,16 @@ deleted_id=$gen_msg_id
 output=$(notmuch search subject:deleted | notmuch_search_sanitize)
 test_expect_equal "$output" "thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Not deleted (inbox unread)"
 
+test_begin_subtest "Search, exclude \"deleted\" messages; alternate config file"
+cp ${NOTMUCH_CONFIG} alt-config
+notmuch config set search.exclude_tags
+notmuch --config=alt-config search subject:deleted | notmuch_search_sanitize > OUTPUT
+cp alt-config ${NOTMUCH_CONFIG}
+cat <<EOF > EXPECTED
+thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Not deleted (inbox unread)
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
 test_begin_subtest "Search, exclude \"deleted\" messages from message search"
 output=$(notmuch search --output=messages subject:deleted | notmuch_search_sanitize)
 test_expect_equal "$output" "id:$not_deleted_id"
-- 
2.28.0

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 17/19] cli/config: add accessor for config file name
  2020-08-08 14:16 Initial implementation of merged config David Bremner
                   ` (15 preceding siblings ...)
  2020-08-08 14:16 ` [PATCH 16/19] test: add regression test for searching with alternate config David Bremner
@ 2020-08-08 14:16 ` David Bremner
  2020-08-08 14:16 ` [PATCH 18/19] WIP converting notmuch search to new style config David Bremner
  2020-08-08 14:16 ` [PATCH 19/19] WIP: switch notmuch-show to new config framework David Bremner
  18 siblings, 0 replies; 22+ messages in thread
From: David Bremner @ 2020-08-08 14:16 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

This is intended for use in temporary code transitioning to the new
configuration system. The name is chosen to avoid cluttering the
notmuch_config_* namespace further with non-library functions.
---
 notmuch-client.h | 2 ++
 notmuch-config.c | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/notmuch-client.h b/notmuch-client.h
index ebd43e8d..2ec84678 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -332,6 +332,8 @@ void
 notmuch_config_set_search_exclude_tags (notmuch_config_t *config,
 					const char *list[],
 					size_t length);
+const char *
+_notmuch_config_get_path (notmuch_config_t *config);
 
 int
 notmuch_run_hook (const char *db_path, const char *hook);
diff --git a/notmuch-config.c b/notmuch-config.c
index 19c2ddb3..c7b05d8c 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -510,6 +510,9 @@ notmuch_config_close (notmuch_config_t *config)
     talloc_free (config);
 }
 
+const char *_notmuch_config_get_path (notmuch_config_t *config) {
+    return config->filename;
+}
 /* Save any changes made to the notmuch configuration.
  *
  * Any comments originally in the file will be preserved.
-- 
2.28.0

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 18/19] WIP converting notmuch search to new style config
  2020-08-08 14:16 Initial implementation of merged config David Bremner
                   ` (16 preceding siblings ...)
  2020-08-08 14:16 ` [PATCH 17/19] cli/config: add accessor for config file name David Bremner
@ 2020-08-08 14:16 ` David Bremner
  2020-08-08 14:16 ` [PATCH 19/19] WIP: switch notmuch-show to new config framework David Bremner
  18 siblings, 0 replies; 22+ messages in thread
From: David Bremner @ 2020-08-08 14:16 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

this breaks passing the config file as a command line argument, but
that is not currently tested for notmuch-search
---
 notmuch-search.c | 45 +++++++++++++++++++++++++++------------------
 1 file changed, 27 insertions(+), 18 deletions(-)

diff --git a/notmuch-search.c b/notmuch-search.c
index 2805d960..4a5a57c9 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -52,6 +52,7 @@ typedef enum {
 
 typedef struct {
     notmuch_database_t *notmuch;
+    void *talloc_ctx;
     int format_sel;
     sprinter_t *format;
     int exclude;
@@ -677,28 +678,32 @@ do_search_tags (const search_context_t *ctx)
 }
 
 static int
-_notmuch_search_prepare (search_context_t *ctx, notmuch_config_t *config, int argc, char *argv[])
+_notmuch_search_prepare (search_context_t *ctx,
+			 const char  *config_path,
+			 int argc, char *argv[])
 {
     char *query_str;
-    unsigned int i;
     char *status_string = NULL;
 
+    if (! ctx->talloc_ctx)
+	ctx->talloc_ctx = talloc_new (NULL);
+
     switch (ctx->format_sel) {
     case NOTMUCH_FORMAT_TEXT:
-	ctx->format = sprinter_text_create (config, stdout);
+	ctx->format = sprinter_text_create (ctx->talloc_ctx, stdout);
 	break;
     case NOTMUCH_FORMAT_TEXT0:
 	if (ctx->output == OUTPUT_SUMMARY) {
 	    fprintf (stderr, "Error: --format=text0 is not compatible with --output=summary.\n");
 	    return EXIT_FAILURE;
 	}
-	ctx->format = sprinter_text0_create (config, stdout);
+	ctx->format = sprinter_text0_create (ctx->talloc_ctx, stdout);
 	break;
     case NOTMUCH_FORMAT_JSON:
-	ctx->format = sprinter_json_create (config, stdout);
+	ctx->format = sprinter_json_create (ctx->talloc_ctx, stdout);
 	break;
     case NOTMUCH_FORMAT_SEXP:
-	ctx->format = sprinter_sexp_create (config, stdout);
+	ctx->format = sprinter_sexp_create (ctx->talloc_ctx, stdout);
 	break;
     default:
 	/* this should never happen */
@@ -707,9 +712,12 @@ _notmuch_search_prepare (search_context_t *ctx, notmuch_config_t *config, int ar
 
     notmuch_exit_if_unsupported_format ();
 
-    if (notmuch_database_open_verbose (
-	    notmuch_config_get_database_path (config),
-	    NOTMUCH_DATABASE_MODE_READ_ONLY, &ctx->notmuch, &status_string)) {
+    if (notmuch_database_open_with_config (
+	    NULL,
+	    NOTMUCH_DATABASE_MODE_READ_ONLY,
+	    config_path,
+	    NULL,
+	    &ctx->notmuch, &status_string)) {
 
 	if (status_string) {
 	    fputs (status_string, stderr);
@@ -748,21 +756,20 @@ _notmuch_search_prepare (search_context_t *ctx, notmuch_config_t *config, int ar
     }
 
     if (ctx->exclude != NOTMUCH_EXCLUDE_FALSE) {
-	const char **search_exclude_tags;
-	size_t search_exclude_tags_length;
+	notmuch_config_values_t *exclude_tags;
 	notmuch_status_t status;
 
-	search_exclude_tags = notmuch_config_get_search_exclude_tags (
-	    config, &search_exclude_tags_length);
+	for (exclude_tags = notmuch_config_get_values (ctx->notmuch, "search.exclude_tags");
+	     notmuch_config_values_valid (exclude_tags);
+	     notmuch_config_values_move_to_next (exclude_tags)) {
 
-	for (i = 0; i < search_exclude_tags_length; i++) {
-	    status = notmuch_query_add_tag_exclude (ctx->query, search_exclude_tags[i]);
+	    status = notmuch_query_add_tag_exclude (ctx->query,
+						    notmuch_config_values_get (exclude_tags));
 	    if (status && status != NOTMUCH_STATUS_IGNORED) {
 		print_status_query ("notmuch search", ctx->query, status);
 		return EXIT_FAILURE;
 	    }
 	}
-
 	notmuch_query_set_omit_excluded (ctx->query, ctx->exclude);
     }
 
@@ -845,7 +852,8 @@ notmuch_search_command (notmuch_config_t *config, int argc, char *argv[])
 	return EXIT_FAILURE;
     }
 
-    if (_notmuch_search_prepare (ctx, config,
+    if (_notmuch_search_prepare (ctx,
+				 _notmuch_config_get_path (config),
 				 argc - opt_index, argv + opt_index))
 	return EXIT_FAILURE;
 
@@ -911,7 +919,8 @@ notmuch_address_command (notmuch_config_t *config, int argc, char *argv[])
 	return EXIT_FAILURE;
     }
 
-    if (_notmuch_search_prepare (ctx, config,
+    if (_notmuch_search_prepare (ctx,
+				 _notmuch_config_get_path (config),
 				 argc - opt_index, argv + opt_index))
 	return EXIT_FAILURE;
 
-- 
2.28.0

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 19/19] WIP: switch notmuch-show to new config framework
  2020-08-08 14:16 Initial implementation of merged config David Bremner
                   ` (17 preceding siblings ...)
  2020-08-08 14:16 ` [PATCH 18/19] WIP converting notmuch search to new style config David Bremner
@ 2020-08-08 14:16 ` David Bremner
  18 siblings, 0 replies; 22+ messages in thread
From: David Bremner @ 2020-08-08 14:16 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

---
 notmuch-show.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/notmuch-show.c b/notmuch-show.c
index dd836add..094cd689 100644
--- a/notmuch-show.c
+++ b/notmuch-show.c
@@ -1234,6 +1234,7 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[])
     bool entire_thread_set = false;
     bool single_message;
     bool unthreaded = FALSE;
+    char *status_string = NULL;
 
     notmuch_opt_desc_t options[] = {
 	{ .opt_keyword = &format, .name = "format", .keywords =
@@ -1323,7 +1324,20 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[])
 	fprintf (stderr, "Warning: --include-html only implemented for format=text, format=json and format=sexp\n");
     }
 
-    query_string = query_string_from_args (config, argc - opt_index, argv + opt_index);
+    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))
+	return EXIT_FAILURE;
+
+    notmuch_exit_if_unmatched_db_uuid (notmuch);
+
+    query_string = query_string_from_args (notmuch, argc - opt_index, argv + opt_index);
     if (query_string == NULL) {
 	fprintf (stderr, "Out of memory\n");
 	return EXIT_FAILURE;
@@ -1334,15 +1348,6 @@ notmuch_show_command (notmuch_config_t *config, int argc, char *argv[])
 	return EXIT_FAILURE;
     }
 
-    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 (notmuch_config_get_database_path (config),
-			       mode, &notmuch))
-	return EXIT_FAILURE;
-
-    notmuch_exit_if_unmatched_db_uuid (notmuch);
-
     query = notmuch_query_create (notmuch, query_string);
     if (query == NULL) {
 	fprintf (stderr, "Out of memory\n");
-- 
2.28.0

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 02/19] lib: add stub for notmuch_database_open_with_config
  2020-08-08 14:16 ` [PATCH 02/19] lib: add stub for notmuch_database_open_with_config David Bremner
@ 2020-08-09  7:19   ` Reto
  2020-08-09 10:18     ` David Bremner
  0 siblings, 1 reply; 22+ messages in thread
From: Reto @ 2020-08-09  7:19 UTC (permalink / raw)
  To: David Bremner; +Cc: notmuch

On Sat, Aug 08, 2020 at 11:16:36AM -0300, David Bremner wrote:
> +/* NOTMUCH_DEPRECATED(5, 4) */
> +notmuch_status_t
> +notmuch_database_open_verbose (const char *path,
> +			       notmuch_database_mode_t mode,
> +			       notmuch_database_t **database,
> +			       char **error_message);
> +
> +/**
> + * Open an existing notmuch database located at 'database_path', using
> + * configuration in 'config_path'.
> + *
> + * @param[in]	database_path
> + * @parblock
> + * Path to existing database.
> + *
> + * A notmuch database is a Xapian database containing appropriate
> + * metadata.
>   *
>   * The database should have been created at some time in the past,
>   * (not necessarily by this process), by calling
> - * notmuch_database_create with 'path'. By default the database should be
> - * opened for reading only. In order to write to the database you need to
> - * pass the NOTMUCH_DATABASE_MODE_READ_WRITE mode.
> + * notmuch_database_create.
> + *
> + * If 'database_path' is NULL, use the location specified
> + *
> + * - in the environment variable NOTMUCH_DATABASE, if non-empty
> + *
> + * - by $XDG_DATA_HOME/notmuch/$PROFILE where XDG_DATA_HOME defaults
> + *   to "$HOME/.local/share" and PROFILE as as discussed in
> + *   'profile'
> + *
> + * If 'database_path' is non-NULL, but does not appear to be a Xapian
> + * database, check for a directory '.notmuch/xapian' below
> + * 'database_path' (this is the behavior of
> + * notmuch_database_open_verbose pre-0.32).
> + *
> + * @endparblock
> + * @param[in]	mode
> + * @parblock
> + * Mode to open database. Use one of #NOTMUCH_DATABASE_MODE_READ_WRITE
> + * or #NOTMUCH_DATABASE_MODE_READ_WRITE

I think you want to have NOTMUCH_DATABASE_MODE_READ_ONLY here?

Greetings,
Reto

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 02/19] lib: add stub for notmuch_database_open_with_config
  2020-08-09  7:19   ` Reto
@ 2020-08-09 10:18     ` David Bremner
  0 siblings, 0 replies; 22+ messages in thread
From: David Bremner @ 2020-08-09 10:18 UTC (permalink / raw)
  To: Reto; +Cc: notmuch

Reto <reto@labrat.space> writes:

>> + * Mode to open database. Use one of #NOTMUCH_DATABASE_MODE_READ_WRITE
>> + * or #NOTMUCH_DATABASE_MODE_READ_WRITE
>
> I think you want to have NOTMUCH_DATABASE_MODE_READ_ONLY here?
>
> Greetings,
> Reto

yes, you are correct.

d

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2020-08-09 10:18 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-08 14:16 Initial implementation of merged config David Bremner
2020-08-08 14:16 ` [PATCH 01/19] test: use keys with group 'test' in T590-libconfig David Bremner
2020-08-08 14:16 ` [PATCH 02/19] lib: add stub for notmuch_database_open_with_config David Bremner
2020-08-09  7:19   ` Reto
2020-08-09 10:18     ` David Bremner
2020-08-08 14:16 ` [PATCH 03/19] lib: cache configuration information from database David Bremner
2020-08-08 14:16 ` [PATCH 04/19] WIP: add notmuch_config_get David Bremner
2020-08-08 14:16 ` [PATCH 05/19] WIP: add notmuch_config_set David Bremner
2020-08-08 14:16 ` [PATCH 06/19] WIP: initial retrieval of database path from config file David Bremner
2020-08-08 14:16 ` [PATCH 07/19] test/libconfig; use n_database_open_with_config David Bremner
2020-08-08 14:16 ` [PATCH 08/19] WIP: add _notmuch_config_load_from_file David Bremner
2020-08-08 14:16 ` [PATCH 09/19] lib: factor out feature name related code David Bremner
2020-08-08 14:16 ` [PATCH 10/19] lib: factor out prefix related code to its own file David Bremner
2020-08-08 14:16 ` [PATCH 11/19] lib: factor out notmuch_database_open* related code to " David Bremner
2020-08-08 14:16 ` [PATCH 12/19] WIP: adding fallbacks for NULL config_path David Bremner
2020-08-08 14:16 ` [PATCH 13/19] lib/config: delay setting talloc destructor David Bremner
2020-08-08 14:16 ` [PATCH 14/19] WIP: add strsplit_len David Bremner
2020-08-08 14:16 ` [PATCH 15/19] WIP: add config values iterator David Bremner
2020-08-08 14:16 ` [PATCH 16/19] test: add regression test for searching with alternate config David Bremner
2020-08-08 14:16 ` [PATCH 17/19] cli/config: add accessor for config file name David Bremner
2020-08-08 14:16 ` [PATCH 18/19] WIP converting notmuch search to new style config David Bremner
2020-08-08 14:16 ` [PATCH 19/19] WIP: switch notmuch-show to new config framework David Bremner

unofficial mirror of notmuch@notmuchmail.org

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://yhetil.org/notmuch/0 notmuch/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 notmuch notmuch/ https://yhetil.org/notmuch \
		notmuch@notmuchmail.org
	public-inbox-index notmuch

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.yhetil.org/yhetil.mail.notmuch.general
	nntp://news.gmane.io/gmane.mail.notmuch.general


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git