unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* Restore relative path handling for database.path
@ 2021-05-07 11:30 David Bremner
  2021-05-07 11:30 ` [PATCH 1/9] test: add known broken test for relative database path in new David Bremner
                   ` (9 more replies)
  0 siblings, 10 replies; 17+ messages in thread
From: David Bremner @ 2021-05-07 11:30 UTC (permalink / raw)
  To: notmuch

Release 0.32 broke (at least) `notmuch new` if database.path has a
relative value.

Although support for relative paths was undocumented, it was
introduced as an easier (to implement) alternative to ~
expansion. Restore the pre-0.32 functionality, and treat path config
items introduced in 0.32 consistently.

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

* [PATCH 1/9] test: add known broken test for relative database path in new
  2021-05-07 11:30 Restore relative path handling for database.path David Bremner
@ 2021-05-07 11:30 ` David Bremner
  2021-05-07 12:19   ` Tomi Ollila
  2021-05-07 11:30 ` [PATCH 2/9] lib/config: canonicalize paths relative to $HOME David Bremner
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 17+ messages in thread
From: David Bremner @ 2021-05-07 11:30 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

This test highlights a bug introduced in 0.32. The new split between
path and mail_root does not properly canonicalize relative paths in
the latter.
---
 test/T050-new.sh | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/test/T050-new.sh b/test/T050-new.sh
index 2985e24c..5faf6839 100755
--- a/test/T050-new.sh
+++ b/test/T050-new.sh
@@ -394,6 +394,19 @@ exit status: 75
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "Relative database path expanded in new"
+test_subtest_known_broken
+ln -s `pwd`/mail home/Maildir
+notmuch config set database.path Maildir
+generate_message
+NOTMUCH_NEW > OUTPUT
+cat <<EOF >EXPECTED
+Added 1 new message to the database.
+EOF
+notmuch config set database.path ${MAIL_DIR}
+rm  home/Maildir
+test_expect_equal_file EXPECTED OUTPUT
+
 add_email_corpus broken
 test_begin_subtest "reference loop does not crash"
 test_expect_code 0 "notmuch show --format=json id:mid-loop-12@example.org id:mid-loop-21@example.org > OUTPUT"
-- 
2.30.2

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

* [PATCH 2/9] lib/config: canonicalize paths relative to $HOME.
  2021-05-07 11:30 Restore relative path handling for database.path David Bremner
  2021-05-07 11:30 ` [PATCH 1/9] test: add known broken test for relative database path in new David Bremner
@ 2021-05-07 11:30 ` David Bremner
  2021-05-07 12:28   ` Tomi Ollila
  2021-05-07 11:30 ` [PATCH 3/9] test: add known broken test for relative setting of mail_root David Bremner
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 17+ messages in thread
From: David Bremner @ 2021-05-07 11:30 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

Prior to 0.32, notmuch had the (undocumented) behaviour that it
expanded a relative value of database.path with respect to $HOME. In
0.32 this was special cased for database.path but broken for
database.mail_root, which causes problems for at least notmuch-new
when database.path is set to a relative path.

The change in T030-config.sh reflects a user visible, but hopefully
harmless behaviour change; the expanded form of the paths will now be
printed by notmuch config.
---
 lib/config.cc       | 22 +++++++++++++++++++++-
 test/T030-config.sh |  6 +++---
 test/T050-new.sh    |  1 -
 3 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/lib/config.cc b/lib/config.cc
index 50bcf6a2..e08c6bf7 100644
--- a/lib/config.cc
+++ b/lib/config.cc
@@ -387,6 +387,23 @@ notmuch_config_pairs_destroy (notmuch_config_pairs_t *pairs)
     talloc_free (pairs);
 }
 
+static char *
+_expand_path (void *ctx, const char *key, const char *val)
+{
+    char *expanded_val;
+
+    if ((strcmp (key, "database.path") == 0 ||
+	 strcmp (key, "database.mail_root") == 0 ||
+	 strcmp (key, "database.hook_dir") == 0 ||
+	 strcmp (key, "database.backup_path") == 0 ) &&
+	val[0] != '/')
+	expanded_val = talloc_asprintf (ctx, "%s/%s", getenv("HOME"), val);
+    else
+	expanded_val = talloc_strdup (ctx, val);
+
+    return expanded_val;
+}
+
 notmuch_status_t
 _notmuch_config_load_from_file (notmuch_database_t *notmuch,
 				GKeyFile *file)
@@ -407,14 +424,17 @@ _notmuch_config_load_from_file (notmuch_database_t *notmuch,
 	keys = g_key_file_get_keys (file, *grp, NULL, NULL);
 	for (gchar **keys_p = keys; *keys_p; keys_p++) {
 	    char *absolute_key = talloc_asprintf (notmuch, "%s.%s", *grp,  *keys_p);
+	    char *normalized_val;
 	    val = g_key_file_get_value (file, *grp, *keys_p, NULL);
 	    if (! val) {
 		status = NOTMUCH_STATUS_FILE_ERROR;
 		goto DONE;
 	    }
-	    _notmuch_string_map_set (notmuch->config, absolute_key, val);
+	    normalized_val = _expand_path (notmuch, absolute_key, val);
+	    _notmuch_string_map_set (notmuch->config, absolute_key, normalized_val);
 	    g_free (val);
 	    talloc_free (absolute_key);
+	    talloc_free (normalized_val);
 	    if (status)
 		goto DONE;
 	}
diff --git a/test/T030-config.sh b/test/T030-config.sh
index b22d8f29..7a1660e9 100755
--- a/test/T030-config.sh
+++ b/test/T030-config.sh
@@ -117,12 +117,12 @@ test_expect_equal "$(notmuch config get database.path)" \
 
 ln -s `pwd`/mail home/Maildir
 add_email_corpus
-test_begin_subtest "Relative database path expanded in open"
+test_begin_subtest "Relative database path expanded"
 notmuch config set database.path Maildir
-path=$(notmuch config get database.path)
+path=$(notmuch config get database.path | notmuch_dir_sanitize)
 count=$(notmuch count '*')
 test_expect_equal "${path} ${count}" \
-		  "Maildir 52"
+		  "CWD/home/Maildir 52"
 
 test_begin_subtest "Add config to database"
 notmuch new
diff --git a/test/T050-new.sh b/test/T050-new.sh
index 5faf6839..33ad7f5a 100755
--- a/test/T050-new.sh
+++ b/test/T050-new.sh
@@ -395,7 +395,6 @@ EOF
 test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "Relative database path expanded in new"
-test_subtest_known_broken
 ln -s `pwd`/mail home/Maildir
 notmuch config set database.path Maildir
 generate_message
-- 
2.30.2

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

* [PATCH 3/9] test: add known broken test for relative setting of mail_root
  2021-05-07 11:30 Restore relative path handling for database.path David Bremner
  2021-05-07 11:30 ` [PATCH 1/9] test: add known broken test for relative database path in new David Bremner
  2021-05-07 11:30 ` [PATCH 2/9] lib/config: canonicalize paths relative to $HOME David Bremner
@ 2021-05-07 11:30 ` David Bremner
  2021-05-07 11:30 ` [PATCH 4/9] lib/config: expand relative paths when reading from database David Bremner
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: David Bremner @ 2021-05-07 11:30 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

The behaviour should not change depending on where the configuration
is stored.
---
 test/T050-new.sh | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/test/T050-new.sh b/test/T050-new.sh
index 33ad7f5a..f28497bf 100755
--- a/test/T050-new.sh
+++ b/test/T050-new.sh
@@ -406,6 +406,19 @@ notmuch config set database.path ${MAIL_DIR}
 rm  home/Maildir
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "Relative mail root (in db) expanded in new"
+test_subtest_known_broken
+ln -s `pwd`/mail home/Maildir
+notmuch config set --database database.mail_root Maildir
+generate_message
+NOTMUCH_NEW > OUTPUT
+cat <<EOF >EXPECTED
+Added 1 new message to the database.
+EOF
+notmuch config set database.mail_root
+rm  home/Maildir
+test_expect_equal_file EXPECTED OUTPUT
+
 add_email_corpus broken
 test_begin_subtest "reference loop does not crash"
 test_expect_code 0 "notmuch show --format=json id:mid-loop-12@example.org id:mid-loop-21@example.org > OUTPUT"
-- 
2.30.2

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

* [PATCH 4/9] lib/config: expand relative paths when reading from database
  2021-05-07 11:30 Restore relative path handling for database.path David Bremner
                   ` (2 preceding siblings ...)
  2021-05-07 11:30 ` [PATCH 3/9] test: add known broken test for relative setting of mail_root David Bremner
@ 2021-05-07 11:30 ` David Bremner
  2021-05-07 12:30   ` Tomi Ollila
  2021-05-07 11:30 ` [PATCH 5/9] test: test relative paths for database.hook_dir David Bremner
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 17+ messages in thread
From: David Bremner @ 2021-05-07 11:30 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

This makes the treatment of relative paths consistent between the
database and config files.
---
 lib/config.cc    | 8 +++++---
 test/T050-new.sh | 1 -
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/lib/config.cc b/lib/config.cc
index e08c6bf7..74339694 100644
--- a/lib/config.cc
+++ b/lib/config.cc
@@ -46,6 +46,7 @@ struct _notmuch_config_pairs {
 };
 
 static const char *_notmuch_config_key_to_string (notmuch_config_key_t key);
+static char *_expand_path (void *ctx, const char * key, const char *val);
 
 static int
 _notmuch_config_list_destroy (notmuch_config_list_t *list)
@@ -257,9 +258,10 @@ _notmuch_config_load_from_database (notmuch_database_t *notmuch)
 	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));
+	const char *key= notmuch_config_list_key (list);
+	char *normalized_val = _expand_path (list, key, notmuch_config_list_value (list));
+	_notmuch_string_map_append (notmuch->config, key, normalized_val);
+	talloc_free (normalized_val);
     }
 
     return status;
diff --git a/test/T050-new.sh b/test/T050-new.sh
index f28497bf..dfabcf03 100755
--- a/test/T050-new.sh
+++ b/test/T050-new.sh
@@ -407,7 +407,6 @@ rm  home/Maildir
 test_expect_equal_file EXPECTED OUTPUT
 
 test_begin_subtest "Relative mail root (in db) expanded in new"
-test_subtest_known_broken
 ln -s `pwd`/mail home/Maildir
 notmuch config set --database database.mail_root Maildir
 generate_message
-- 
2.30.2

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

* [PATCH 5/9] test: test relative paths for database.hook_dir
  2021-05-07 11:30 Restore relative path handling for database.path David Bremner
                   ` (3 preceding siblings ...)
  2021-05-07 11:30 ` [PATCH 4/9] lib/config: expand relative paths when reading from database David Bremner
@ 2021-05-07 11:30 ` David Bremner
  2021-05-07 12:34   ` Tomi Ollila
  2021-05-07 11:30 ` [PATCH 6/9] test: test explicit configuration of backup directory David Bremner
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 17+ messages in thread
From: David Bremner @ 2021-05-07 11:30 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

---
 test/T400-hooks.sh | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/test/T400-hooks.sh b/test/T400-hooks.sh
index 3a2df2f4..00c99337 100755
--- a/test/T400-hooks.sh
+++ b/test/T400-hooks.sh
@@ -43,7 +43,7 @@ add_message
 # create maildir structure for notmuch-insert
 mkdir -p "$MAIL_DIR"/{cur,new,tmp}
 
-for config in traditional profile explicit XDG split; do
+for config in traditional profile explicit relative XDG split; do
     unset NOTMUCH_PROFILE
     notmuch config set database.hook_dir
     notmuch config set database.path ${MAIL_DIR}
@@ -63,6 +63,11 @@ for config in traditional profile explicit XDG split; do
 	    mkdir -p $HOOK_DIR
 	    notmuch config set database.hook_dir $HOOK_DIR
 	    ;;
+	relative)
+	    HOOK_DIR=${HOME}/.notmuch-hooks
+	    mkdir -p $HOOK_DIR
+	    notmuch config set database.hook_dir .notmuch-hooks
+	    ;;
 	XDG)
 	    HOOK_DIR=${HOME}/.config/notmuch/default/hooks
 	    ;;
-- 
2.30.2

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

* [PATCH 6/9] test: test explicit configuration of backup directory
  2021-05-07 11:30 Restore relative path handling for database.path David Bremner
                   ` (4 preceding siblings ...)
  2021-05-07 11:30 ` [PATCH 5/9] test: test relative paths for database.hook_dir David Bremner
@ 2021-05-07 11:30 ` David Bremner
  2021-05-07 11:30 ` [PATCH 7/9] doc: document (tersely) the intended behaviour of relative paths David Bremner
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 17+ messages in thread
From: David Bremner @ 2021-05-07 11:30 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

Including the relative path that was broken until a recent commit.
---
 test/T530-upgrade.sh | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/test/T530-upgrade.sh b/test/T530-upgrade.sh
index cce29f45..5f0de2ed 100755
--- a/test/T530-upgrade.sh
+++ b/test/T530-upgrade.sh
@@ -54,4 +54,23 @@ for key in 'from/subject/message-ID in database' \
     restore_database
 done
 
+test_begin_subtest "upgrade with configured backup dir"
+notmuch config set database.backup_dir ${HOME}/backups
+delete_feature 'modification tracking'
+notmuch new | grep Backing | notmuch_dir_sanitize | sed 's/dump-[0-9T]*/dump-XXX/' > OUTPUT
+cat <<EOF > EXPECTED
+Backing up tags to CWD/home/backups/dump-XXX.gz...
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+test_begin_subtest "upgrade with relative configured backup dir"
+notmuch config set database.backup_dir ${HOME}/backups
+delete_feature 'modification tracking'
+notmuch new | grep Backing | notmuch_dir_sanitize | sed 's/dump-[0-9T]*/dump-XXX/' > OUTPUT
+cat <<EOF > EXPECTED
+Backing up tags to CWD/home/backups/dump-XXX.gz...
+EOF
+test_expect_equal_file EXPECTED OUTPUT
+
+
 test_done
-- 
2.30.2

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

* [PATCH 7/9] doc: document (tersely) the intended behaviour of relative paths.
  2021-05-07 11:30 Restore relative path handling for database.path David Bremner
                   ` (5 preceding siblings ...)
  2021-05-07 11:30 ` [PATCH 6/9] test: test explicit configuration of backup directory David Bremner
@ 2021-05-07 11:30 ` David Bremner
  2021-05-07 15:31   ` David Edmondson
  2021-05-07 11:30 ` [PATCH 8/9] doc: document database.backup_dir David Bremner
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 17+ messages in thread
From: David Bremner @ 2021-05-07 11:30 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

---
 doc/man1/notmuch-config.rst | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/doc/man1/notmuch-config.rst b/doc/man1/notmuch-config.rst
index 32290a38..5cb6e203 100644
--- a/doc/man1/notmuch-config.rst
+++ b/doc/man1/notmuch-config.rst
@@ -44,7 +44,9 @@ configuration file and corresponding database.
     characters. In a multiple-value item (a list), the values are
     separated by semicolon characters.
 
-The available configuration items are described below.
+The available configuration items are described below. Non-absolute
+paths are expanded relative to `$HOME` for items in section
+**database**.
 
 **database.path**
     Notmuch will store its database here, (in
-- 
2.30.2

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

* [PATCH 8/9] doc: document database.backup_dir
  2021-05-07 11:30 Restore relative path handling for database.path David Bremner
                   ` (6 preceding siblings ...)
  2021-05-07 11:30 ` [PATCH 7/9] doc: document (tersely) the intended behaviour of relative paths David Bremner
@ 2021-05-07 11:30 ` David Bremner
  2021-05-07 11:30 ` [PATCH 9/9] NEWS: start NEWS for 0.32.1 David Bremner
  2021-05-10 14:42 ` Restore relative path handling for database.path David Bremner
  9 siblings, 0 replies; 17+ messages in thread
From: David Bremner @ 2021-05-07 11:30 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

Most users will not need to change this, but documenting it helps
preserve the interface.
---
 doc/man1/notmuch-config.rst | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/doc/man1/notmuch-config.rst b/doc/man1/notmuch-config.rst
index 5cb6e203..a6cfeb41 100644
--- a/doc/man1/notmuch-config.rst
+++ b/doc/man1/notmuch-config.rst
@@ -65,6 +65,14 @@ paths are expanded relative to `$HOME` for items in section
     Default: For compatibility with older configurations, the value of
     database.path is used if **database.mail\_root** is unset.
 
+**database.backup_dir**
+    Directory to store tag dumps when upgrading database.
+
+    History: this configuration value was introduced in notmuch 0.32.
+
+    Default: A sibling directory of the Xapian database called
+    `backups`.
+
 **database.hook_dir**
 
     Directory containing hooks run by notmuch commands. See
-- 
2.30.2

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

* [PATCH 9/9] NEWS: start NEWS for 0.32.1
  2021-05-07 11:30 Restore relative path handling for database.path David Bremner
                   ` (7 preceding siblings ...)
  2021-05-07 11:30 ` [PATCH 8/9] doc: document database.backup_dir David Bremner
@ 2021-05-07 11:30 ` David Bremner
  2021-05-10 14:42 ` Restore relative path handling for database.path David Bremner
  9 siblings, 0 replies; 17+ messages in thread
From: David Bremner @ 2021-05-07 11:30 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

---
 NEWS | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/NEWS b/NEWS
index 8cb9b345..44a18951 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,10 @@
+Notmuch 0.32.1 (UNRELEASED)
+===========================
+
+Restore handling of relative values for `database.path` that was
+broken by 0.32. Extend this handling to `database.mail_root`,
+`database.backup_dir`, and `database.hook_dir`.
+
 Notmuch 0.32 (2021-05-02)
 =========================
 
-- 
2.30.2

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

* Re: [PATCH 1/9] test: add known broken test for relative database path in new
  2021-05-07 11:30 ` [PATCH 1/9] test: add known broken test for relative database path in new David Bremner
@ 2021-05-07 12:19   ` Tomi Ollila
  0 siblings, 0 replies; 17+ messages in thread
From: Tomi Ollila @ 2021-05-07 12:19 UTC (permalink / raw)
  To: David Bremner, notmuch; +Cc: David Bremner

On Fri, May 07 2021, David Bremner wrote:

> This test highlights a bug introduced in 0.32. The new split between
> path and mail_root does not properly canonicalize relative paths in
> the latter.
> ---
>  test/T050-new.sh | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/test/T050-new.sh b/test/T050-new.sh
> index 2985e24c..5faf6839 100755
> --- a/test/T050-new.sh
> +++ b/test/T050-new.sh
> @@ -394,6 +394,19 @@ exit status: 75
>  EOF
>  test_expect_equal_file EXPECTED OUTPUT
>  
> +test_begin_subtest "Relative database path expanded in new"
> +test_subtest_known_broken
> +ln -s `pwd`/mail home/Maildir

$PWD

> +notmuch config set database.path Maildir
> +generate_message
> +NOTMUCH_NEW > OUTPUT
> +cat <<EOF >EXPECTED
> +Added 1 new message to the database.
> +EOF
> +notmuch config set database.path ${MAIL_DIR}
> +rm  home/Maildir

2 spaces

> +test_expect_equal_file EXPECTED OUTPUT
> +
>  add_email_corpus broken
>  test_begin_subtest "reference loop does not crash"
>  test_expect_code 0 "notmuch show --format=json id:mid-loop-12@example.org id:mid-loop-21@example.org > OUTPUT"
> -- 
> 2.30.2

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

* Re: [PATCH 2/9] lib/config: canonicalize paths relative to $HOME.
  2021-05-07 11:30 ` [PATCH 2/9] lib/config: canonicalize paths relative to $HOME David Bremner
@ 2021-05-07 12:28   ` Tomi Ollila
  2021-05-07 16:16     ` Tomi Ollila
  0 siblings, 1 reply; 17+ messages in thread
From: Tomi Ollila @ 2021-05-07 12:28 UTC (permalink / raw)
  To: David Bremner, notmuch; +Cc: David Bremner

On Fri, May 07 2021, David Bremner wrote:

> Prior to 0.32, notmuch had the (undocumented) behaviour that it
> expanded a relative value of database.path with respect to $HOME. In
> 0.32 this was special cased for database.path but broken for
> database.mail_root, which causes problems for at least notmuch-new
> when database.path is set to a relative path.
>
> The change in T030-config.sh reflects a user visible, but hopefully
> harmless behaviour change; the expanded form of the paths will now be
> printed by notmuch config.
> ---
>  lib/config.cc       | 22 +++++++++++++++++++++-
>  test/T030-config.sh |  6 +++---
>  test/T050-new.sh    |  1 -
>  3 files changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/lib/config.cc b/lib/config.cc
> index 50bcf6a2..e08c6bf7 100644
> --- a/lib/config.cc
> +++ b/lib/config.cc
> @@ -387,6 +387,23 @@ notmuch_config_pairs_destroy (notmuch_config_pairs_t *pairs)
>      talloc_free (pairs);
>  }
>  
> +static char *
> +_expand_path (void *ctx, const char *key, const char *val)
> +{
> +    char *expanded_val;
> +
> +    if ((strcmp (key, "database.path") == 0 ||
> +	 strcmp (key, "database.mail_root") == 0 ||
> +	 strcmp (key, "database.hook_dir") == 0 ||
> +	 strcmp (key, "database.backup_path") == 0 ) &&
> +	val[0] != '/')

check val[0] first then one does not need to waste time scanning
through all the other possible values for 'key'. that is trivial
"optimization", other possible but not worth the effort...

> +	expanded_val = talloc_asprintf (ctx, "%s/%s", getenv("HOME"), val);
> +    else
> +	expanded_val = talloc_strdup (ctx, val);
> +
> +    return expanded_val;
> +}
> +
>  notmuch_status_t
>  _notmuch_config_load_from_file (notmuch_database_t *notmuch,
>  				GKeyFile *file)
> @@ -407,14 +424,17 @@ _notmuch_config_load_from_file (notmuch_database_t *notmuch,
>  	keys = g_key_file_get_keys (file, *grp, NULL, NULL);
>  	for (gchar **keys_p = keys; *keys_p; keys_p++) {
>  	    char *absolute_key = talloc_asprintf (notmuch, "%s.%s", *grp,  *keys_p);
> +	    char *normalized_val;
>  	    val = g_key_file_get_value (file, *grp, *keys_p, NULL);
>  	    if (! val) {
>  		status = NOTMUCH_STATUS_FILE_ERROR;
>  		goto DONE;
>  	    }
> -	    _notmuch_string_map_set (notmuch->config, absolute_key, val);
> +	    normalized_val = _expand_path (notmuch, absolute_key, val);
> +	    _notmuch_string_map_set (notmuch->config, absolute_key, normalized_val);
>  	    g_free (val);
>  	    talloc_free (absolute_key);
> +	    talloc_free (normalized_val);
>  	    if (status)
>  		goto DONE;
>  	}
> diff --git a/test/T030-config.sh b/test/T030-config.sh
> index b22d8f29..7a1660e9 100755
> --- a/test/T030-config.sh
> +++ b/test/T030-config.sh
> @@ -117,12 +117,12 @@ test_expect_equal "$(notmuch config get database.path)" \
>  
>  ln -s `pwd`/mail home/Maildir
>  add_email_corpus
> -test_begin_subtest "Relative database path expanded in open"
> +test_begin_subtest "Relative database path expanded"
>  notmuch config set database.path Maildir
> -path=$(notmuch config get database.path)
> +path=$(notmuch config get database.path | notmuch_dir_sanitize)
>  count=$(notmuch count '*')
>  test_expect_equal "${path} ${count}" \
> -		  "Maildir 52"
> +		  "CWD/home/Maildir 52"
>  
>  test_begin_subtest "Add config to database"
>  notmuch new
> diff --git a/test/T050-new.sh b/test/T050-new.sh
> index 5faf6839..33ad7f5a 100755
> --- a/test/T050-new.sh
> +++ b/test/T050-new.sh
> @@ -395,7 +395,6 @@ EOF
>  test_expect_equal_file EXPECTED OUTPUT
>  
>  test_begin_subtest "Relative database path expanded in new"
> -test_subtest_known_broken
>  ln -s `pwd`/mail home/Maildir
>  notmuch config set database.path Maildir
>  generate_message
> -- 
> 2.30.2
> _______________________________________________
> notmuch mailing list -- notmuch@notmuchmail.org
> To unsubscribe send an email to notmuch-leave@notmuchmail.org

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

* Re: [PATCH 4/9] lib/config: expand relative paths when reading from database
  2021-05-07 11:30 ` [PATCH 4/9] lib/config: expand relative paths when reading from database David Bremner
@ 2021-05-07 12:30   ` Tomi Ollila
  0 siblings, 0 replies; 17+ messages in thread
From: Tomi Ollila @ 2021-05-07 12:30 UTC (permalink / raw)
  To: David Bremner, notmuch; +Cc: David Bremner

On Fri, May 07 2021, David Bremner wrote:

First, previous test change had the same extra space and `pwd` -> $PWD...

> This makes the treatment of relative paths consistent between the
> database and config files.
> ---
>  lib/config.cc    | 8 +++++---
>  test/T050-new.sh | 1 -
>  2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/lib/config.cc b/lib/config.cc
> index e08c6bf7..74339694 100644
> --- a/lib/config.cc
> +++ b/lib/config.cc
> @@ -46,6 +46,7 @@ struct _notmuch_config_pairs {
>  };
>  
>  static const char *_notmuch_config_key_to_string (notmuch_config_key_t key);
> +static char *_expand_path (void *ctx, const char * key, const char *val);

*key

>  
>  static int
>  _notmuch_config_list_destroy (notmuch_config_list_t *list)
> @@ -257,9 +258,10 @@ _notmuch_config_load_from_database (notmuch_database_t *notmuch)
>  	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));
> +	const char *key= notmuch_config_list_key (list);

key =

> +	char *normalized_val = _expand_path (list, key, notmuch_config_list_value (list));
> +	_notmuch_string_map_append (notmuch->config, key, normalized_val);
> +	talloc_free (normalized_val);
>      }
>  
>      return status;
> diff --git a/test/T050-new.sh b/test/T050-new.sh
> index f28497bf..dfabcf03 100755
> --- a/test/T050-new.sh
> +++ b/test/T050-new.sh
> @@ -407,7 +407,6 @@ rm  home/Maildir
>  test_expect_equal_file EXPECTED OUTPUT
>  
>  test_begin_subtest "Relative mail root (in db) expanded in new"
> -test_subtest_known_broken
>  ln -s `pwd`/mail home/Maildir

that $PWD, out of changes in this message

>  notmuch config set --database database.mail_root Maildir
>  generate_message
> -- 
> 2.30.2

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

* Re: [PATCH 5/9] test: test relative paths for database.hook_dir
  2021-05-07 11:30 ` [PATCH 5/9] test: test relative paths for database.hook_dir David Bremner
@ 2021-05-07 12:34   ` Tomi Ollila
  0 siblings, 0 replies; 17+ messages in thread
From: Tomi Ollila @ 2021-05-07 12:34 UTC (permalink / raw)
  To: David Bremner, notmuch

On Fri, May 07 2021, David Bremner wrote:

> ---
>  test/T400-hooks.sh | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/test/T400-hooks.sh b/test/T400-hooks.sh
> index 3a2df2f4..00c99337 100755
> --- a/test/T400-hooks.sh
> +++ b/test/T400-hooks.sh
> @@ -43,7 +43,7 @@ add_message
>  # create maildir structure for notmuch-insert
>  mkdir -p "$MAIL_DIR"/{cur,new,tmp}
>  
> -for config in traditional profile explicit XDG split; do
> +for config in traditional profile explicit relative XDG split; do
>      unset NOTMUCH_PROFILE
>      notmuch config set database.hook_dir
>      notmuch config set database.path ${MAIL_DIR}
> @@ -63,6 +63,11 @@ for config in traditional profile explicit XDG split; do
>  	    mkdir -p $HOOK_DIR
>  	    notmuch config set database.hook_dir $HOOK_DIR
>  	    ;;
> +	relative)
> +	    HOOK_DIR=${HOME}/.notmuch-hooks
> +	    mkdir -p $HOOK_DIR

I'd drop the -p, to be sure $HOOK_DIR does not exist before test run
(if it existed, one notices when it fails (or not, we don't fail on error
:/ -- anyway noise to terminal/log (hopefully)


> +	    notmuch config set database.hook_dir .notmuch-hooks
> +	    ;;
>  	XDG)
>  	    HOOK_DIR=${HOME}/.config/notmuch/default/hooks
>  	    ;;
> -- 
> 2.30.2

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

* Re: [PATCH 7/9] doc: document (tersely) the intended behaviour of relative paths.
  2021-05-07 11:30 ` [PATCH 7/9] doc: document (tersely) the intended behaviour of relative paths David Bremner
@ 2021-05-07 15:31   ` David Edmondson
  0 siblings, 0 replies; 17+ messages in thread
From: David Edmondson @ 2021-05-07 15:31 UTC (permalink / raw)
  To: David Bremner, notmuch; +Cc: David Bremner

On Friday, 2021-05-07 at 08:30:27 -03, David Bremner wrote:

> ---
>  doc/man1/notmuch-config.rst | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/doc/man1/notmuch-config.rst b/doc/man1/notmuch-config.rst
> index 32290a38..5cb6e203 100644
> --- a/doc/man1/notmuch-config.rst
> +++ b/doc/man1/notmuch-config.rst
> @@ -44,7 +44,9 @@ configuration file and corresponding database.
>      characters. In a multiple-value item (a list), the values are
>      separated by semicolon characters.
>  
> -The available configuration items are described below.
> +The available configuration items are described below. Non-absolute
> +paths are expanded relative to `$HOME` for items in section

Maybe "presumed" rather than "expanded"?

> +**database**.
>  
>  **database.path**
>      Notmuch will store its database here, (in
> -- 
> 2.30.2
> _______________________________________________
> notmuch mailing list -- notmuch@notmuchmail.org
> To unsubscribe send an email to notmuch-leave@notmuchmail.org

dme.
-- 
Freedom is just a song by Wham!.

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

* Re: [PATCH 2/9] lib/config: canonicalize paths relative to $HOME.
  2021-05-07 12:28   ` Tomi Ollila
@ 2021-05-07 16:16     ` Tomi Ollila
  0 siblings, 0 replies; 17+ messages in thread
From: Tomi Ollila @ 2021-05-07 16:16 UTC (permalink / raw)
  To: David Bremner, notmuch; +Cc: David Bremner

On Fri, May 07 2021, Tomi Ollila wrote:

> On Fri, May 07 2021, David Bremner wrote:
>
>> Prior to 0.32, notmuch had the (undocumented) behaviour that it
>> expanded a relative value of database.path with respect to $HOME. In
>> 0.32 this was special cased for database.path but broken for
>> database.mail_root, which causes problems for at least notmuch-new
>> when database.path is set to a relative path.
>>
>> The change in T030-config.sh reflects a user visible, but hopefully
>> harmless behaviour change; the expanded form of the paths will now be
>> printed by notmuch config.
>> ---
>>  lib/config.cc       | 22 +++++++++++++++++++++-
>>  test/T030-config.sh |  6 +++---
>>  test/T050-new.sh    |  1 -
>>  3 files changed, 24 insertions(+), 5 deletions(-)
>>
>> diff --git a/lib/config.cc b/lib/config.cc
>> index 50bcf6a2..e08c6bf7 100644
>> --- a/lib/config.cc
>> +++ b/lib/config.cc
>> @@ -387,6 +387,23 @@ notmuch_config_pairs_destroy (notmuch_config_pairs_t *pairs)
>>      talloc_free (pairs);
>>  }
>>  
>> +static char *
>> +_expand_path (void *ctx, const char *key, const char *val)
>> +{
>> +    char *expanded_val;
>> +
>> +    if ((strcmp (key, "database.path") == 0 ||
>> +	 strcmp (key, "database.mail_root") == 0 ||
>> +	 strcmp (key, "database.hook_dir") == 0 ||
>> +	 strcmp (key, "database.backup_path") == 0 ) &&
>> +	val[0] != '/')
>
> check val[0] first then one does not need to waste time scanning
> through all the other possible values for 'key'. that is trivial
> "optimization", other possible but not worth the effort...

Forget this suggestion; changing order makes things worse -- probably
in values of other keys than these checked don't start with '/' so
there would be one extra check in most of the cases...

Tomi


>
>> +	expanded_val = talloc_asprintf (ctx, "%s/%s", getenv("HOME"), val);
>> +    else
>> +	expanded_val = talloc_strdup (ctx, val);
>> +
>> +    return expanded_val;
>> +}
>> +
>>  notmuch_status_t
>>  _notmuch_config_load_from_file (notmuch_database_t *notmuch,
>>  				GKeyFile *file)
>> @@ -407,14 +424,17 @@ _notmuch_config_load_from_file (notmuch_database_t *notmuch,
>>  	keys = g_key_file_get_keys (file, *grp, NULL, NULL);
>>  	for (gchar **keys_p = keys; *keys_p; keys_p++) {
>>  	    char *absolute_key = talloc_asprintf (notmuch, "%s.%s", *grp,  *keys_p);
>> +	    char *normalized_val;
>>  	    val = g_key_file_get_value (file, *grp, *keys_p, NULL);
>>  	    if (! val) {
>>  		status = NOTMUCH_STATUS_FILE_ERROR;
>>  		goto DONE;
>>  	    }
>> -	    _notmuch_string_map_set (notmuch->config, absolute_key, val);
>> +	    normalized_val = _expand_path (notmuch, absolute_key, val);
>> +	    _notmuch_string_map_set (notmuch->config, absolute_key, normalized_val);
>>  	    g_free (val);
>>  	    talloc_free (absolute_key);
>> +	    talloc_free (normalized_val);
>>  	    if (status)
>>  		goto DONE;
>>  	}
>> diff --git a/test/T030-config.sh b/test/T030-config.sh
>> index b22d8f29..7a1660e9 100755
>> --- a/test/T030-config.sh
>> +++ b/test/T030-config.sh
>> @@ -117,12 +117,12 @@ test_expect_equal "$(notmuch config get database.path)" \
>>  
>>  ln -s `pwd`/mail home/Maildir
>>  add_email_corpus
>> -test_begin_subtest "Relative database path expanded in open"
>> +test_begin_subtest "Relative database path expanded"
>>  notmuch config set database.path Maildir
>> -path=$(notmuch config get database.path)
>> +path=$(notmuch config get database.path | notmuch_dir_sanitize)
>>  count=$(notmuch count '*')
>>  test_expect_equal "${path} ${count}" \
>> -		  "Maildir 52"
>> +		  "CWD/home/Maildir 52"
>>  
>>  test_begin_subtest "Add config to database"
>>  notmuch new
>> diff --git a/test/T050-new.sh b/test/T050-new.sh
>> index 5faf6839..33ad7f5a 100755
>> --- a/test/T050-new.sh
>> +++ b/test/T050-new.sh
>> @@ -395,7 +395,6 @@ EOF
>>  test_expect_equal_file EXPECTED OUTPUT
>>  
>>  test_begin_subtest "Relative database path expanded in new"
>> -test_subtest_known_broken
>>  ln -s `pwd`/mail home/Maildir
>>  notmuch config set database.path Maildir
>>  generate_message
>> -- 
>> 2.30.2
>> _______________________________________________
>> notmuch mailing list -- notmuch@notmuchmail.org
>> To unsubscribe send an email to notmuch-leave@notmuchmail.org

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

* Re: Restore relative path handling for database.path
  2021-05-07 11:30 Restore relative path handling for database.path David Bremner
                   ` (8 preceding siblings ...)
  2021-05-07 11:30 ` [PATCH 9/9] NEWS: start NEWS for 0.32.1 David Bremner
@ 2021-05-10 14:42 ` David Bremner
  9 siblings, 0 replies; 17+ messages in thread
From: David Bremner @ 2021-05-10 14:42 UTC (permalink / raw)
  To: notmuch

David Bremner <david@tethera.net> writes:

> Release 0.32 broke (at least) `notmuch new` if database.path has a
> relative value.
>
> Although support for relative paths was undocumented, it was
> introduced as an easier (to implement) alternative to ~
> expansion. Restore the pre-0.32 functionality, and treat path config
> items introduced in 0.32 consistently.

This series has been applied to release and master

I'm still waiting for feedback on the tag-hook patch and the $HOME/mail
patch before doing a point release.

d

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

end of thread, other threads:[~2021-05-10 14:42 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-05-07 11:30 Restore relative path handling for database.path David Bremner
2021-05-07 11:30 ` [PATCH 1/9] test: add known broken test for relative database path in new David Bremner
2021-05-07 12:19   ` Tomi Ollila
2021-05-07 11:30 ` [PATCH 2/9] lib/config: canonicalize paths relative to $HOME David Bremner
2021-05-07 12:28   ` Tomi Ollila
2021-05-07 16:16     ` Tomi Ollila
2021-05-07 11:30 ` [PATCH 3/9] test: add known broken test for relative setting of mail_root David Bremner
2021-05-07 11:30 ` [PATCH 4/9] lib/config: expand relative paths when reading from database David Bremner
2021-05-07 12:30   ` Tomi Ollila
2021-05-07 11:30 ` [PATCH 5/9] test: test relative paths for database.hook_dir David Bremner
2021-05-07 12:34   ` Tomi Ollila
2021-05-07 11:30 ` [PATCH 6/9] test: test explicit configuration of backup directory David Bremner
2021-05-07 11:30 ` [PATCH 7/9] doc: document (tersely) the intended behaviour of relative paths David Bremner
2021-05-07 15:31   ` David Edmondson
2021-05-07 11:30 ` [PATCH 8/9] doc: document database.backup_dir David Bremner
2021-05-07 11:30 ` [PATCH 9/9] NEWS: start NEWS for 0.32.1 David Bremner
2021-05-10 14:42 ` Restore relative path handling for database.path David Bremner

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).