[PATCH 0/3] config: follow symlinks when saving

[PATCH 0/3] config: follow symlinks when saving

[Jani Nikula]

This is v4 of [1], adding tests (and some semi-related TODO file
updates). The actual code is the same as before.

This makes my symlink based dotfiles setup happy.

BR,
Jani.

[1] id:1362320310-10930-1-git-send-email-jani@nikula.org


Jani Nikula (3):
  test: add some config file tests
  cli: config: do not overwrite symlinks when saving config file
  TODO: remove some completed todo items from the list

 devel/TODO       |   14 --------------
 notmuch-config.c |   24 ++++++++++++++++++++----
 test/config      |   23 +++++++++++++++++++++++
 3 files changed, 43 insertions(+), 18 deletions(-)

-- 
1.7.10.4

[PATCH 1/3] test: add some config file tests

[Jani Nikula]

Test the --config=FILE option, and add a broken test for writing
config file through a symbolic link.
---
 test/config |   24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/test/config b/test/config
index cfa1f32..344eced 100755
--- a/test/config
+++ b/test/config
@@ -57,4 +57,28 @@ maildir.synchronize_flags=true
 foo.string=this is another string value
 foo.list=this;is another;list value;"
 
+test_begin_subtest "Top level --config=FILE option"
+cp "${NOTMUCH_CONFIG}" alt-config
+notmuch --config=alt-config config set user.name "Another Name"
+test_expect_equal "$(notmuch --config=alt-config config get user.name)" \
+    "Another Name"
+
+test_begin_subtest "Top level --config=FILE option changed the right file"
+test_expect_equal "$(notmuch config get user.name)" \
+    "Notmuch Test Suite"
+
+test_begin_subtest "Read config file through a symlink"
+ln -s alt-config alt-config-link
+test_expect_equal "$(notmuch --config=alt-config-link config get user.name)" \
+    "Another Name"
+
+test_begin_subtest "Write config file through a symlink"
+notmuch --config=alt-config-link config set user.name "Link Name"
+test_expect_equal "$(notmuch --config=alt-config-link config get user.name)" \
+    "Link Name"
+
+test_begin_subtest "Writing config file through symlink follows symlink"
+test_subtest_known_broken
+test_expect_equal "$(readlink alt-config-link)" "alt-config"
+
 test_done
-- 
1.7.10.4

[PATCH 2/3] cli: config: do not overwrite symlinks when saving config file

[Jani Nikula]

Use realpath to canonicalize the config path before writing.

Previously 'notmuch setup' and 'notmuch config set' overwrote the
config file even if it was a symbolic link.
---
 notmuch-config.c |   24 ++++++++++++++++++++----
 test/config      |    1 -
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/notmuch-config.c b/notmuch-config.c
index 66a1cdf..d9c2eb3 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -444,7 +444,7 @@ int
 notmuch_config_save (notmuch_config_t *config)
 {
     size_t length;
-    char *data;
+    char *data, *filename;
     GError *error = NULL;
 
     data = g_key_file_to_data (config->key_file, &length, NULL);
@@ -453,14 +453,30 @@ notmuch_config_save (notmuch_config_t *config)
 	return 1;
     }
 
-    if (! g_file_set_contents (config->filename, data, length, &error)) {
-	fprintf (stderr, "Error saving configuration to %s: %s\n",
-		 config->filename, error->message);
+    /* Try not to overwrite symlinks. */
+    filename = realpath (config->filename, NULL);
+    if (! filename) {
+	fprintf (stderr, "Error canonicalizing %s: %s\n", config->filename,
+		 strerror (errno));
+	g_free (data);
+	return 1;
+    }
+
+    if (! g_file_set_contents (filename, data, length, &error)) {
+	if (strcmp (filename, config->filename) != 0) {
+	    fprintf (stderr, "Error saving configuration to %s (-> %s): %s\n",
+		     config->filename, filename, error->message);
+	} else {
+	    fprintf (stderr, "Error saving configuration to %s: %s\n",
+		     filename, error->message);
+	}
 	g_error_free (error);
+	free (filename);
 	g_free (data);
 	return 1;
     }
 
+    free (filename);
     g_free (data);
     return 0;
 }
diff --git a/test/config b/test/config
index 344eced..ca4cf33 100755
--- a/test/config
+++ b/test/config
@@ -78,7 +78,6 @@ test_expect_equal "$(notmuch --config=alt-config-link config get user.name)" \
     "Link Name"
 
 test_begin_subtest "Writing config file through symlink follows symlink"
-test_subtest_known_broken
 test_expect_equal "$(readlink alt-config-link)" "alt-config"
 
 test_done
-- 
1.7.10.4

[PATCH 3/3] TODO: remove some completed todo items from the list

[Jani Nikula]

Bash completion has been rewritten. Configuration file saves follow
symlinks. There is --config=FILE top level option to specify
configuration file.
---
 devel/TODO |   14 --------------
 1 file changed, 14 deletions(-)

diff --git a/devel/TODO b/devel/TODO
index e4f36c2..f63385d 100644
--- a/devel/TODO
+++ b/devel/TODO
@@ -57,12 +57,6 @@ Automatically open a message when navigating to it with N or P.
 
 Change 'a' command in thread-view mode to only archive open messages.
 
-Completion
-----------
-Fix bash completion to complete multiple search options (both --first
-and *then* --max-threads), and also complete value for --sort=
-(oldest-first or newest-first).
-
 notmuch command-line tool
 -------------------------
 Add support to "notmuch search" and "notmuch show" to allow for
@@ -70,11 +64,6 @@ listing of duplicate messages, (distinct filenames with the same
 Message-ID). I'm not sure what the option should be named. Perhaps
 --with-duplicates ?
 
-"notmuch setup" should use realpath() before replacing the
-configuration file. The ensures that the final target file of any
-intermediate symbolic links is what is actually replaced, (rather than
-any symbolic link).
-
 Replace "notmuch reply" with "notmuch compose --reply <search-terms>".
 This would enable a plain "notmuch compose" to be used to construct an
 initial message, (which would then have the properly configured name
@@ -102,9 +91,6 @@ Fix "notmuch restore" to operate in a single pass much like "notmuch
 dump" does, rather than doing N searches into the database, each
 matching 1/N messages.
 
-Add a "-f <filename>" option to select an alternate configuration
-file.
-
 Allow configuration for filename patterns that should be ignored when
 indexing.
 
-- 
1.7.10.4

Re: [PATCH 0/3] config: follow symlinks when saving

[Tomi Ollila]

On Sun, Apr 07 2013, Jani Nikula <jani@nikula.org> wrote:

> This is v4 of [1], adding tests (and some semi-related TODO file
> updates). The actual code is the same as before.
>
> This makes my symlink based dotfiles setup happy.

Looks good and
 PASS   Read config file through a symlink
 PASS   Write config file through a symlink
 PASS   Writing config file through symlink follows symlink

>
> BR,
> Jani.

Tomi


>
> [1] id:1362320310-10930-1-git-send-email-jani@nikula.org
>
>
> Jani Nikula (3):
>   test: add some config file tests
>   cli: config: do not overwrite symlinks when saving config file
>   TODO: remove some completed todo items from the list
>
>  devel/TODO       |   14 --------------
>  notmuch-config.c |   24 ++++++++++++++++++++----
>  test/config      |   23 +++++++++++++++++++++++
>  3 files changed, 43 insertions(+), 18 deletions(-)
>
> -- 
> 1.7.10.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 1/3] test: add some config file tests

[David Bremner]

Jani Nikula <jani@nikula.org> writes:

> Test the --config=FILE option, and add a broken test for writing
> config file through a symbolic link.
> ---
>  test/config |   24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)

Series pushed,

d