[PATCH v1 0/2] Interpret a directory path without a leading slash as relative to $HOME.

classic Classic list List threaded Threaded
9 messages Options
David Edmondson David Edmondson
Reply | Threaded
Open this post in threaded view
|

[PATCH v1 0/2] Interpret a directory path without a leading slash as relative to $HOME.


Being able to specify a database path that works well on multiple
machines, where $HOME is not always the same, seems generally
useful.

There was a previous attempt to support ~ in the configuration file
(id:[hidden email]), but this
foundered on concerns about support for ~otheruser.

This patch attempts to avoid that problem by asserting that any
database lacking a leading / character is interpreted as being
relative to $HOME.

So in my case, I would set the path to Maildir:

[database]
path=Maildir

This would work in Linux (where my home directory is /home/dme) and on
macOS (where it is /Users/dme).

David Edmondson (2):
  test: Absolute and relative directory paths.
  notmuch: Database paths without a leading / are relative to $HOME

 notmuch-config.c    | 14 +++++++++++++-
 test/T030-config.sh | 10 ++++++++++
 2 files changed, 23 insertions(+), 1 deletion(-)

--
2.11.0

_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch
David Edmondson David Edmondson
Reply | Threaded
Open this post in threaded view
|

[PATCH v1 1/2] test: Absolute and relative directory paths.

---
 test/T030-config.sh | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/test/T030-config.sh b/test/T030-config.sh
index e91c3659..f36695c6 100755
--- a/test/T030-config.sh
+++ b/test/T030-config.sh
@@ -99,4 +99,14 @@ test_expect_equal "$(notmuch --config=alt-config-link config get user.name)" \
 test_begin_subtest "Writing config file through symlink follows symlink"
 test_expect_equal "$(readlink alt-config-link)" "alt-config"
 
+test_begin_subtest "Absolute database path returned"
+notmuch config set database.path ${HOME}/Maildir
+test_expect_equal "$(notmuch config get database.path)" \
+  "${HOME}/Maildir"
+
+test_begin_subtest "Relative database path properly expanded"
+notmuch config set database.path Maildir
+test_expect_equal "$(notmuch config get database.path)" \
+  "${HOME}/Maildir"
+
 test_done
--
2.11.0

_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch
David Edmondson David Edmondson
Reply | Threaded
Open this post in threaded view
|

[PATCH v1 2/2] notmuch: Database paths without a leading / are relative to $HOME

In reply to this post by David Edmondson
If the database path specified in the configuration file does *not*
start with a /, presume that it is relative to $HOME and modify the
path used to open the database accordingly.
---
 notmuch-config.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/notmuch-config.c b/notmuch-config.c
index e1b16609..bf77cc9d 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -660,7 +660,19 @@ _config_set_list (notmuch_config_t *config,
 const char *
 notmuch_config_get_database_path (notmuch_config_t *config)
 {
-    return _config_get (config, &config->database_path, "database", "path");
+    char *db_path = (char *)_config_get (config, &config->database_path, "database", "path");
+
+    if (db_path && *db_path != '/') {
+ /* If the path in the configuration file begins with any
+ * character other than /, presume that it is relative to
+ * $HOME and update as appropriate.
+ */
+ char *abs_path = talloc_asprintf (config, "%s/%s", getenv ("HOME"), db_path);
+ talloc_free (db_path);
+ db_path = config->database_path = abs_path;
+    }
+
+    return db_path;
 }
 
 void
--
2.11.0

_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch
David Bremner-2 David Bremner-2
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v1 1/2] test: Absolute and relative directory paths.

In reply to this post by David Edmondson
David Edmondson <[hidden email]> writes:

> ---
>  test/T030-config.sh | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/test/T030-config.sh b/test/T030-config.sh
> index e91c3659..f36695c6 100755
> --- a/test/T030-config.sh
> +++ b/test/T030-config.sh
> @@ -99,4 +99,14 @@ test_expect_equal "$(notmuch --config=alt-config-link config get user.name)" \
>  test_begin_subtest "Writing config file through symlink follows symlink"
>  test_expect_equal "$(readlink alt-config-link)" "alt-config"
>  
> +test_begin_subtest "Absolute database path returned"
> +notmuch config set database.path ${HOME}/Maildir
> +test_expect_equal "$(notmuch config get database.path)" \
> +  "${HOME}/Maildir"
> +
> +test_begin_subtest "Relative database path properly expanded"
> +notmuch config set database.path Maildir
> +test_expect_equal "$(notmuch config get database.path)" \
> +  "${HOME}/Maildir"
> +
>  test_done
> --

We want the test suite to pass after every commit, so you need to either
squash the second test into the next commit, or mark it with
test_subtest_known_broken. I'm fine with either option.

d
_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch
David Bremner-2 David Bremner-2
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v1 2/2] notmuch: Database paths without a leading / are relative to $HOME

In reply to this post by David Edmondson
David Edmondson <[hidden email]> writes:

> If the database path specified in the configuration file does *not*
> start with a /, presume that it is relative to $HOME and modify the
> path used to open the database accordingly.
> ---
>  notmuch-config.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/notmuch-config.c b/notmuch-config.c
> index e1b16609..bf77cc9d 100644
> --- a/notmuch-config.c
> +++ b/notmuch-config.c
> @@ -660,7 +660,19 @@ _config_set_list (notmuch_config_t *config,
>  const char *
>  notmuch_config_get_database_path (notmuch_config_t *config)
>  {
> -    return _config_get (config, &config->database_path, "database", "path");
> +    char *db_path = (char *)_config_get (config, &config->database_path, "database", "path");
> +
> +    if (db_path && *db_path != '/') {
> + /* If the path in the configuration file begins with any
> + * character other than /, presume that it is relative to
> + * $HOME and update as appropriate.
> + */
> + char *abs_path = talloc_asprintf (config, "%s/%s", getenv ("HOME"), db_path);
> + talloc_free (db_path);
> + db_path = config->database_path = abs_path;
> +    }

In the unlikely event that HOME is not set, it would be nicer to print a
message to that effect, rather than

  configuration file (null)/.notmuch-config not found.
  Try running 'notmuch setup' to create a configuration.

_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch
David Edmondson David Edmondson
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v1 1/2] test: Absolute and relative directory paths.

In reply to this post by David Bremner-2
On Friday, 2018-09-07 at 21:05:38 -03, David Bremner wrote:

> David Edmondson <[hidden email]> writes:
>
>> ---
>>  test/T030-config.sh | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/test/T030-config.sh b/test/T030-config.sh
>> index e91c3659..f36695c6 100755
>> --- a/test/T030-config.sh
>> +++ b/test/T030-config.sh
>> @@ -99,4 +99,14 @@ test_expect_equal "$(notmuch --config=alt-config-link config get user.name)" \
>>  test_begin_subtest "Writing config file through symlink follows symlink"
>>  test_expect_equal "$(readlink alt-config-link)" "alt-config"
>>  
>> +test_begin_subtest "Absolute database path returned"
>> +notmuch config set database.path ${HOME}/Maildir
>> +test_expect_equal "$(notmuch config get database.path)" \
>> +  "${HOME}/Maildir"
>> +
>> +test_begin_subtest "Relative database path properly expanded"
>> +notmuch config set database.path Maildir
>> +test_expect_equal "$(notmuch config get database.path)" \
>> +  "${HOME}/Maildir"
>> +
>>  test_done
>> --
>
> We want the test suite to pass after every commit, so you need to either
> squash the second test into the next commit, or mark it with
> test_subtest_known_broken. I'm fine with either option.

Would you be happy with me just re-arranging the order, so that the test
came after?

dme.
--
Sorry I'm not home right now, I'm walking into spiderwebs.
_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch
David Edmondson David Edmondson
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v1 2/2] notmuch: Database paths without a leading / are relative to $HOME

In reply to this post by David Bremner-2
On Friday, 2018-09-07 at 21:10:42 -03, David Bremner wrote:

> David Edmondson <[hidden email]> writes:
>
>> If the database path specified in the configuration file does *not*
>> start with a /, presume that it is relative to $HOME and modify the
>> path used to open the database accordingly.
>> ---
>>  notmuch-config.c | 14 +++++++++++++-
>>  1 file changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/notmuch-config.c b/notmuch-config.c
>> index e1b16609..bf77cc9d 100644
>> --- a/notmuch-config.c
>> +++ b/notmuch-config.c
>> @@ -660,7 +660,19 @@ _config_set_list (notmuch_config_t *config,
>>  const char *
>>  notmuch_config_get_database_path (notmuch_config_t *config)
>>  {
>> -    return _config_get (config, &config->database_path, "database", "path");
>> +    char *db_path = (char *)_config_get (config, &config->database_path, "database", "path");
>> +
>> +    if (db_path && *db_path != '/') {
>> + /* If the path in the configuration file begins with any
>> + * character other than /, presume that it is relative to
>> + * $HOME and update as appropriate.
>> + */
>> + char *abs_path = talloc_asprintf (config, "%s/%s", getenv ("HOME"), db_path);
>> + talloc_free (db_path);
>> + db_path = config->database_path = abs_path;
>> +    }
>
> In the unlikely event that HOME is not set, it would be nicer to print a
> message to that effect, rather than

I did think about that, but decided to follow existing practice in
notmuch, which doesn't check:

        config->filename = talloc_asprintf (config, "%s/.notmuch-config",
                                            getenv ("HOME"));

...and...

            path = talloc_asprintf (config, "%s/mail",
                                    getenv ("HOME"));

>   configuration file (null)/.notmuch-config not found.
>   Try running 'notmuch setup' to create a configuration.

dme.
--
It's funny, I spent my whole life wanting to be talked about.
_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch
David Bremner-2 David Bremner-2
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v1 1/2] test: Absolute and relative directory paths.

In reply to this post by David Edmondson
David Edmondson <[hidden email]> writes:

>
> Would you be happy with me just re-arranging the order, so that the test
> came after?

Also fine.

d
_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch
David Bremner-2 David Bremner-2
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH v1 2/2] notmuch: Database paths without a leading / are relative to $HOME

In reply to this post by David Edmondson
David Edmondson <[hidden email]> writes:

>
> I did think about that, but decided to follow existing practice in
> notmuch, which doesn't check:
>
>         config->filename = talloc_asprintf (config, "%s/.notmuch-config",
>                                             getenv ("HOME"));
>

OK, that's fair.

d
_______________________________________________
notmuch mailing list
[hidden email]
https://notmuchmail.org/mailman/listinfo/notmuch