unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* WARNING: database upgrade coming
@ 2014-03-09 15:01 David Bremner
  2014-03-12 11:25 ` David Bremner
  2014-03-17 17:17 ` Jameson Graef Rollins
  0 siblings, 2 replies; 18+ messages in thread
From: David Bremner @ 2014-03-09 15:01 UTC (permalink / raw)
  To: Notmuch list


I will fairly soon push some version of jani's patches changing folder:
matching to master. This will require an irreversible database upgrade.

Of course we believe everything is going to go smoothly, but you
probably should not run this upcoming version of notmuch on your live
mail store until you understand the implications for your workflow.

It never hurts to backup your tags before upgrading notmuch. We've never
had a data loss yet, but it _is_ software.

d

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

* Re: WARNING: database upgrade coming
  2014-03-09 15:01 WARNING: database upgrade coming David Bremner
@ 2014-03-12 11:25 ` David Bremner
  2014-03-17 17:17 ` Jameson Graef Rollins
  1 sibling, 0 replies; 18+ messages in thread
From: David Bremner @ 2014-03-12 11:25 UTC (permalink / raw)
  To: Notmuch list

David Bremner <david@tethera.net> writes:

> I will fairly soon push some version of jani's patches changing folder:
> matching to master. This will require an irreversible database upgrade.

This is pushed. One additional warning, the upgrade takes some time (for
my 300k messages, about 20 minutes), so don't do it when you desperately
need to run notmuch for other purposes.


d

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

* Re: WARNING: database upgrade coming
  2014-03-09 15:01 WARNING: database upgrade coming David Bremner
  2014-03-12 11:25 ` David Bremner
@ 2014-03-17 17:17 ` Jameson Graef Rollins
  2014-03-17 19:12   ` Jani Nikula
  1 sibling, 1 reply; 18+ messages in thread
From: Jameson Graef Rollins @ 2014-03-17 17:17 UTC (permalink / raw)
  To: David Bremner, Notmuch list

[-- Attachment #1: Type: text/plain, Size: 1166 bytes --]

On Sun, Mar 09 2014, David Bremner <david@tethera.net> wrote:
> I will fairly soon push some version of jani's patches changing folder:
> matching to master. This will require an irreversible database upgrade.
>
> Of course we believe everything is going to go smoothly, but you
> probably should not run this upcoming version of notmuch on your live
> mail store until you understand the implications for your workflow.
>
> It never hurts to backup your tags before upgrading notmuch. We've never
> had a data loss yet, but it _is_ software.

Hey, David et. al.  I recently did an upgrade to notmuch/master after
being out of the loop for a while and not noticing that a database
upgrade was pending.  The next notmuch new informed me that a database
upgrade was required, but it went ahead and did the upgrade unprompted.
Things went fine, but it did worry me a bit that I didn't know an
upgrade was pending and didn't have a chance to do a backup before it
happened.

It would be nice if notmuch new would prompt users when an database
upgrade is pending, to give the user an option to abort before
proceeding so that they can do a backup if they'd like.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 818 bytes --]

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

* Re: WARNING: database upgrade coming
  2014-03-17 17:17 ` Jameson Graef Rollins
@ 2014-03-17 19:12   ` Jani Nikula
  2014-03-17 21:19     ` David Bremner
  0 siblings, 1 reply; 18+ messages in thread
From: Jani Nikula @ 2014-03-17 19:12 UTC (permalink / raw)
  To: Jameson Graef Rollins, David Bremner, Notmuch list


Hi Jamie -

On Mon, 17 Mar 2014, Jameson Graef Rollins <jrollins@finestructure.net> wrote:
> Hey, David et. al.  I recently did an upgrade to notmuch/master after
> being out of the loop for a while and not noticing that a database
> upgrade was pending.  The next notmuch new informed me that a database
> upgrade was required, but it went ahead and did the upgrade unprompted.
> Things went fine, but it did worry me a bit that I didn't know an
> upgrade was pending and didn't have a chance to do a backup before it
> happened.

FWIW it should always be safe to interrupt the upgrade; I know we don't
inform the user about this.

> It would be nice if notmuch new would prompt users when an database
> upgrade is pending, to give the user an option to abort before
> proceeding so that they can do a backup if they'd like.

Please see id:87lhy4f6pr.fsf@nikula.org for a summary on some of the
pros and cons that have been discussed about prompting the user.


BR,
Jani.

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

* Re: WARNING: database upgrade coming
  2014-03-17 19:12   ` Jani Nikula
@ 2014-03-17 21:19     ` David Bremner
  2014-03-17 21:29       ` Jameson Graef Rollins
  2014-03-17 21:31       ` WARNING: database upgrade coming Jani Nikula
  0 siblings, 2 replies; 18+ messages in thread
From: David Bremner @ 2014-03-17 21:19 UTC (permalink / raw)
  To: Jani Nikula, Jameson Graef Rollins, Notmuch list

Jani Nikula <jani@nikula.org> writes:

>
> FWIW it should always be safe to interrupt the upgrade; I know we don't
> inform the user about this.
>

With that in mind, would it be reasonable/worthwhile to print a 5 second (or so)
countdown before running the upgrade? But then people who run it
non-interactively would still automagically get the upgrade, just 5
seconds later.

d

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

* Re: WARNING: database upgrade coming
  2014-03-17 21:19     ` David Bremner
@ 2014-03-17 21:29       ` Jameson Graef Rollins
  2014-03-23  2:26         ` conservative database upgrade control David Bremner
  2014-03-17 21:31       ` WARNING: database upgrade coming Jani Nikula
  1 sibling, 1 reply; 18+ messages in thread
From: Jameson Graef Rollins @ 2014-03-17 21:29 UTC (permalink / raw)
  To: David Bremner, Jani Nikula, Notmuch list

[-- Attachment #1: Type: text/plain, Size: 787 bytes --]

On Mon, Mar 17 2014, David Bremner <david@tethera.net> wrote:
> Jani Nikula <jani@nikula.org> writes:
>> FWIW it should always be safe to interrupt the upgrade; I know we don't
>> inform the user about this.
>
> With that in mind, would it be reasonable/worthwhile to print a 5 second (or so)
> countdown before running the upgrade? But then people who run it
> non-interactively would still automagically get the upgrade, just 5
> seconds later.

My vote would be that anything invasive like this should be done with an
explicit "OK" from the user.  In otherwords, when a database upgrade is
due, non-interactive usage of notmuch new should fail until the user has
run notmuch new interactively and acknowledged that a db upgrade will be
happening and that they're ok with it.

jamie.


[-- Attachment #2: Type: application/pgp-signature, Size: 818 bytes --]

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

* Re: WARNING: database upgrade coming
  2014-03-17 21:19     ` David Bremner
  2014-03-17 21:29       ` Jameson Graef Rollins
@ 2014-03-17 21:31       ` Jani Nikula
  2014-03-18  6:06         ` Rob Browning
  1 sibling, 1 reply; 18+ messages in thread
From: Jani Nikula @ 2014-03-17 21:31 UTC (permalink / raw)
  To: David Bremner, Jameson Graef Rollins, Notmuch list

On Mon, 17 Mar 2014, David Bremner <david@tethera.net> wrote:
> Jani Nikula <jani@nikula.org> writes:
>
>>
>> FWIW it should always be safe to interrupt the upgrade; I know we don't
>> inform the user about this.
>>
>
> With that in mind, would it be reasonable/worthwhile to print a 5 second (or so)
> countdown before running the upgrade? But then people who run it
> non-interactively would still automagically get the upgrade, just 5
> seconds later.

Something like this? Just insert text that makes sense to the user. ;)

Jani.


diff --git a/notmuch-new.c b/notmuch-new.c
index 82acf695353e..f256a3142eb0 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -989,8 +989,11 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])
 	    return EXIT_FAILURE;
 
 	if (notmuch_database_needs_upgrade (notmuch)) {
-	    if (add_files_state.verbosity >= VERBOSITY_NORMAL)
+	    if (add_files_state.verbosity >= VERBOSITY_NORMAL) {
 		printf ("Welcome to a new version of notmuch! Your database will now be upgraded.\n");
+		sleep (5);
+		printf ("Now really.\n");
+	    }
 	    gettimeofday (&add_files_state.tv_start, NULL);
 	    notmuch_database_upgrade (notmuch,
 				      add_files_state.verbosity >= VERBOSITY_NORMAL ? upgrade_print_progress : NULL,

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

* Re: WARNING: database upgrade coming
  2014-03-17 21:31       ` WARNING: database upgrade coming Jani Nikula
@ 2014-03-18  6:06         ` Rob Browning
  2014-03-18  6:57           ` Tomi Ollila
  0 siblings, 1 reply; 18+ messages in thread
From: Rob Browning @ 2014-03-18  6:06 UTC (permalink / raw)
  To: Jani Nikula, David Bremner, Jameson Graef Rollins, Notmuch list

Jani Nikula <jani@nikula.org> writes:

> Something like this? Just insert text that makes sense to the user. ;)

If you decide to go this route, I wonder if it might also be worth
adding the text about interruption being OK.

-- 
Rob Browning
rlb @defaultvalue.org and @debian.org
GPG as of 2011-07-10 E6A9 DA3C C9FD 1FF8 C676 D2C4 C0F0 39E9 ED1B 597A
GPG as of 2002-11-03 14DD 432F AE39 534D B592 F9A0 25C8 D377 8C7E 73A4

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

* Re: WARNING: database upgrade coming
  2014-03-18  6:06         ` Rob Browning
@ 2014-03-18  6:57           ` Tomi Ollila
  2014-03-18 11:55             ` Stewart Smith
  0 siblings, 1 reply; 18+ messages in thread
From: Tomi Ollila @ 2014-03-18  6:57 UTC (permalink / raw)
  To: Rob Browning, Jani Nikula, David Bremner, Jameson Graef Rollins,
	Notmuch list

On Tue, Mar 18 2014, Rob Browning <rlb@defaultvalue.org> wrote:

> Jani Nikula <jani@nikula.org> writes:
>
>> Something like this? Just insert text that makes sense to the user. ;)
>
> If you decide to go this route, I wonder if it might also be worth
> adding the text about interruption being OK.

In addition it could just start the upgrade, and if the total time of
upgrade is less than 10 seconds it would wait until that time gets
up and then replace the old database with the new one.

But very informative and verbose messages of how safe the interruption
of database is would be in order...

Some ideas to bikeshed with:

"The database upgrade is done in a new database; at the end of the updrade
the current database is replaced with the new one -- Interrupting updrade
(with Ctrl-C) leaves you with the current database."

> -- 
> Rob Browning

Tomi

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

* Re: WARNING: database upgrade coming
  2014-03-18  6:57           ` Tomi Ollila
@ 2014-03-18 11:55             ` Stewart Smith
  0 siblings, 0 replies; 18+ messages in thread
From: Stewart Smith @ 2014-03-18 11:55 UTC (permalink / raw)
  To: Tomi Ollila, Rob Browning, Jani Nikula, David Bremner,
	Jameson Graef Rollins, Notmuch list

[-- Attachment #1: Type: text/plain, Size: 541 bytes --]

Tomi Ollila <tomi.ollila@iki.fi> writes:
> Some ideas to bikeshed with:
>
> "The database upgrade is done in a new database; at the end of the updrade
> the current database is replaced with the new one -- Interrupting updrade
> (with Ctrl-C) leaves you with the current database."

In a condition where free space on filesystem is less than size of
database... things could get interesting, right? At the very least it's
probably not worth even attempting the upgrade unless there's a --force
or something.

-- 
Stewart Smith

[-- Attachment #2: Type: application/pgp-signature, Size: 818 bytes --]

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

* conservative database upgrade control
  2014-03-17 21:29       ` Jameson Graef Rollins
@ 2014-03-23  2:26         ` David Bremner
  2014-03-23  2:26           ` [PATCH 1/2] config: add key database.upgrades David Bremner
  2014-03-23  2:26           ` [PATCH 2/2] notmuch-new: block database upgrades in default configuration David Bremner
  0 siblings, 2 replies; 18+ messages in thread
From: David Bremner @ 2014-03-23  2:26 UTC (permalink / raw)
  To: notmuch

Here is a conservative approach that doesn't require a lot of new
code.

The advantage over a command line argument is that those who prefer
the current setup are only irritated once.

Several of the other ideas require adding heuristics to e.g. test if
notmuch is running in a terminal or that there is enough
diskspace. Those sound hard to test.

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

* [PATCH 1/2] config: add key database.upgrades
  2014-03-23  2:26         ` conservative database upgrade control David Bremner
@ 2014-03-23  2:26           ` David Bremner
  2014-03-23 13:12             ` Jani Nikula
  2014-03-23  2:26           ` [PATCH 2/2] notmuch-new: block database upgrades in default configuration David Bremner
  1 sibling, 1 reply; 18+ messages in thread
From: David Bremner @ 2014-03-23  2:26 UTC (permalink / raw)
  To: notmuch

The intent is to allow the user to enable or disable automatic
database upgrades.

This doesn't do anything yet.
---
 doc/man1/notmuch-config.rst |  4 ++++
 notmuch-client.h            |  7 +++++++
 notmuch-config.c            | 34 +++++++++++++++++++++++++++++-----
 test/T030-config.sh         |  1 +
 test/T040-setup.sh          |  1 +
 5 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/doc/man1/notmuch-config.rst b/doc/man1/notmuch-config.rst
index 3c9a568..fc7fab6 100644
--- a/doc/man1/notmuch-config.rst
+++ b/doc/man1/notmuch-config.rst
@@ -49,6 +49,10 @@ The available configuration items are described below.
         within a sub-directory of the path configured here named
         ``.notmuch``.
 
+    **database.upgrades**
+        Set to ``yes`` to enable automatic in-place upgrades of the notmuch
+	database.
+
     **user.name**
         Your full name.
 
diff --git a/notmuch-client.h b/notmuch-client.h
index 278b498..3891a82 100644
--- a/notmuch-client.h
+++ b/notmuch-client.h
@@ -275,6 +275,13 @@ notmuch_config_set_database_path (notmuch_config_t *config,
 				  const char *database_path);
 
 const char *
+notmuch_config_get_database_upgrades (notmuch_config_t *config);
+
+void
+notmuch_config_set_database_upgrades (notmuch_config_t *config,
+				  const char *database_upgrades);
+
+const char *
 notmuch_config_get_user_name (notmuch_config_t *config);
 
 void
diff --git a/notmuch-config.c b/notmuch-config.c
index 8d28653..8f4ce99 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -32,11 +32,16 @@ static const char toplevel_config_comment[] =
 static const char database_config_comment[] =
     " Database configuration\n"
     "\n"
-    " The only value supported here is 'path' which should be the top-level\n"
-    " directory where your mail currently exists and to where mail will be\n"
-    " delivered in the future. Files should be individual email messages.\n"
-    " Notmuch will store its database within a sub-directory of the path\n"
-    " configured here named \".notmuch\".\n";
+    "The following options are supported here:\n"
+    "\n"
+    "\tpath       The top-level directory where your mail currently exists\n"
+    "\t           and to where mail will be  delivered in the future.  Files\n"
+    "\t           should be individual email messages.  Notmuch will store\n"
+    "\t           its database within a sub-directory (named .notmuch) of\n"
+    "\t           the path configured here.\n"
+    "\n"
+    "\tupgrades   Set to 'yes' to enable automatic database upgrades.\n"
+    "\n";
 
 static const char new_config_comment[] =
     " Configuration for \"notmuch new\"\n"
@@ -107,6 +112,7 @@ struct _notmuch_config {
     notmuch_bool_t is_new;
 
     char *database_path;
+    char *database_upgrades;
     char *user_name;
     char *user_primary_email;
     const char **user_other_email;
@@ -265,6 +271,7 @@ notmuch_config_open (void *ctx,
 
     config->is_new = FALSE;
     config->database_path = NULL;
+    config->database_upgrades = NULL;
     config->user_name = NULL;
     config->user_primary_email = NULL;
     config->user_other_email = NULL;
@@ -328,6 +335,10 @@ notmuch_config_open (void *ctx,
 	talloc_free (path);
     }
 
+    if (notmuch_config_get_database_upgrades (config) == NULL) {
+	notmuch_config_set_database_upgrades (config, "no");
+    }
+
     if (notmuch_config_get_user_name (config) == NULL) {
 	char *name = get_name_from_passwd_file (config);
 	notmuch_config_set_user_name (config, name);
@@ -582,6 +593,19 @@ notmuch_config_set_database_path (notmuch_config_t *config,
 }
 
 const char *
+notmuch_config_get_database_upgrades (notmuch_config_t *config)
+{
+    return _config_get (config, &config->database_upgrades, "database", "upgrades");
+}
+
+void
+notmuch_config_set_database_upgrades (notmuch_config_t *config,
+				  const char *database_upgrades)
+{
+    _config_set (config, &config->database_upgrades, "database", "upgrades", database_upgrades);
+}
+
+const char *
 notmuch_config_get_user_name (notmuch_config_t *config)
 {
     return _config_get (config, &config->user_name, "user", "name");
diff --git a/test/T030-config.sh b/test/T030-config.sh
index ca4cf33..d095f0e 100755
--- a/test/T030-config.sh
+++ b/test/T030-config.sh
@@ -47,6 +47,7 @@ notmuch config set database.path "/canonical/path"
 output=$(notmuch config list)
 test_expect_equal "$output" "\
 database.path=/canonical/path
+database.upgrades=no
 user.name=Notmuch Test Suite
 user.primary_email=test_suite@notmuchmail.org
 user.other_email=test_suite_other@notmuchmail.org;test_suite@otherdomain.org
diff --git a/test/T040-setup.sh b/test/T040-setup.sh
index 124ef1c..46fd327 100755
--- a/test/T040-setup.sh
+++ b/test/T040-setup.sh
@@ -16,6 +16,7 @@ EOF
 output=$(notmuch --config=new-notmuch-config config list)
 test_expect_equal "$output" "\
 database.path=/path/to/maildir
+database.upgrades=no
 user.name=Test Suite
 user.primary_email=test.suite@example.com
 user.other_email=another.suite@example.com;
-- 
1.9.0

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

* [PATCH 2/2] notmuch-new: block database upgrades in default configuration.
  2014-03-23  2:26         ` conservative database upgrade control David Bremner
  2014-03-23  2:26           ` [PATCH 1/2] config: add key database.upgrades David Bremner
@ 2014-03-23  2:26           ` David Bremner
  2014-03-23 14:17             ` Jani Nikula
  1 sibling, 1 reply; 18+ messages in thread
From: David Bremner @ 2014-03-23  2:26 UTC (permalink / raw)
  To: notmuch

People who prefer the current behaviour can set the configuration
variable and forget it.
---
 notmuch-new.c        |  9 +++++++++
 test/T530-upgrade.sh | 11 +++++++++++
 2 files changed, 20 insertions(+)

diff --git a/notmuch-new.c b/notmuch-new.c
index 82acf69..c6a15b0 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -989,6 +989,15 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])
 	    return EXIT_FAILURE;
 
 	if (notmuch_database_needs_upgrade (notmuch)) {
+	    if (strcasecmp(notmuch_config_get_database_upgrades (config), "yes") != 0) {
+		fprintf (stderr,
+			 "Database upgrade needed. Please enable with\n\n"
+			 "  notmuch config set database.upgrades yes\n\n"
+			 "after taking appropriate precautions.\n");
+
+		return EXIT_FAILURE;
+	    }
+
 	    if (add_files_state.verbosity >= VERBOSITY_NORMAL)
 		printf ("Welcome to a new version of notmuch! Your database will now be upgraded.\n");
 	    gettimeofday (&add_files_state.tv_start, NULL);
diff --git a/test/T530-upgrade.sh b/test/T530-upgrade.sh
index 67bbf31..00dcb1e 100755
--- a/test/T530-upgrade.sh
+++ b/test/T530-upgrade.sh
@@ -25,8 +25,19 @@ test_begin_subtest "path: search does not work with old database version"
 output=$(notmuch search path:foo)
 test_expect_equal "$output" ""
 
+test_begin_subtest "database upgrade blocked by default config"
+output=$(notmuch new 2>&1)
+test_expect_equal "$output" "\
+Database upgrade needed. Please enable with
+
+  notmuch config set database.upgrades yes
+
+after taking appropriate precautions."
+
 test_begin_subtest "database upgrade from format version 1"
+notmuch config set database.upgrades yes
 output=$(notmuch new)
+
 test_expect_equal "$output" "\
 Welcome to a new version of notmuch! Your database will now be upgraded.
 Your notmuch database has now been upgraded to database format version 2.
-- 
1.9.0

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

* Re: [PATCH 1/2] config: add key database.upgrades
  2014-03-23  2:26           ` [PATCH 1/2] config: add key database.upgrades David Bremner
@ 2014-03-23 13:12             ` Jani Nikula
  0 siblings, 0 replies; 18+ messages in thread
From: Jani Nikula @ 2014-03-23 13:12 UTC (permalink / raw)
  To: David Bremner, notmuch

On Sun, 23 Mar 2014, David Bremner <david@tethera.net> wrote:
> The intent is to allow the user to enable or disable automatic
> database upgrades.
>
> This doesn't do anything yet.
> ---
>  doc/man1/notmuch-config.rst |  4 ++++
>  notmuch-client.h            |  7 +++++++
>  notmuch-config.c            | 34 +++++++++++++++++++++++++++++-----
>  test/T030-config.sh         |  1 +
>  test/T040-setup.sh          |  1 +
>  5 files changed, 42 insertions(+), 5 deletions(-)
>
> diff --git a/doc/man1/notmuch-config.rst b/doc/man1/notmuch-config.rst
> index 3c9a568..fc7fab6 100644
> --- a/doc/man1/notmuch-config.rst
> +++ b/doc/man1/notmuch-config.rst
> @@ -49,6 +49,10 @@ The available configuration items are described below.
>          within a sub-directory of the path configured here named
>          ``.notmuch``.
>  
> +    **database.upgrades**
> +        Set to ``yes`` to enable automatic in-place upgrades of the notmuch
> +	database.
> +
>      **user.name**
>          Your full name.
>  
> diff --git a/notmuch-client.h b/notmuch-client.h
> index 278b498..3891a82 100644
> --- a/notmuch-client.h
> +++ b/notmuch-client.h
> @@ -275,6 +275,13 @@ notmuch_config_set_database_path (notmuch_config_t *config,
>  				  const char *database_path);
>  
>  const char *
> +notmuch_config_get_database_upgrades (notmuch_config_t *config);
> +
> +void
> +notmuch_config_set_database_upgrades (notmuch_config_t *config,
> +				  const char *database_upgrades);

I'm opposed to adding a config option for this, but if you're going to
do it anyway, please at least make it a bool. See synchronize flags
option.

BR,
Jani.

> +
> +const char *
>  notmuch_config_get_user_name (notmuch_config_t *config);
>  
>  void
> diff --git a/notmuch-config.c b/notmuch-config.c
> index 8d28653..8f4ce99 100644
> --- a/notmuch-config.c
> +++ b/notmuch-config.c
> @@ -32,11 +32,16 @@ static const char toplevel_config_comment[] =
>  static const char database_config_comment[] =
>      " Database configuration\n"
>      "\n"
> -    " The only value supported here is 'path' which should be the top-level\n"
> -    " directory where your mail currently exists and to where mail will be\n"
> -    " delivered in the future. Files should be individual email messages.\n"
> -    " Notmuch will store its database within a sub-directory of the path\n"
> -    " configured here named \".notmuch\".\n";
> +    "The following options are supported here:\n"
> +    "\n"
> +    "\tpath       The top-level directory where your mail currently exists\n"
> +    "\t           and to where mail will be  delivered in the future.  Files\n"
> +    "\t           should be individual email messages.  Notmuch will store\n"
> +    "\t           its database within a sub-directory (named .notmuch) of\n"
> +    "\t           the path configured here.\n"
> +    "\n"
> +    "\tupgrades   Set to 'yes' to enable automatic database upgrades.\n"
> +    "\n";
>  
>  static const char new_config_comment[] =
>      " Configuration for \"notmuch new\"\n"
> @@ -107,6 +112,7 @@ struct _notmuch_config {
>      notmuch_bool_t is_new;
>  
>      char *database_path;
> +    char *database_upgrades;
>      char *user_name;
>      char *user_primary_email;
>      const char **user_other_email;
> @@ -265,6 +271,7 @@ notmuch_config_open (void *ctx,
>  
>      config->is_new = FALSE;
>      config->database_path = NULL;
> +    config->database_upgrades = NULL;
>      config->user_name = NULL;
>      config->user_primary_email = NULL;
>      config->user_other_email = NULL;
> @@ -328,6 +335,10 @@ notmuch_config_open (void *ctx,
>  	talloc_free (path);
>      }
>  
> +    if (notmuch_config_get_database_upgrades (config) == NULL) {
> +	notmuch_config_set_database_upgrades (config, "no");
> +    }
> +
>      if (notmuch_config_get_user_name (config) == NULL) {
>  	char *name = get_name_from_passwd_file (config);
>  	notmuch_config_set_user_name (config, name);
> @@ -582,6 +593,19 @@ notmuch_config_set_database_path (notmuch_config_t *config,
>  }
>  
>  const char *
> +notmuch_config_get_database_upgrades (notmuch_config_t *config)
> +{
> +    return _config_get (config, &config->database_upgrades, "database", "upgrades");
> +}
> +
> +void
> +notmuch_config_set_database_upgrades (notmuch_config_t *config,
> +				  const char *database_upgrades)
> +{
> +    _config_set (config, &config->database_upgrades, "database", "upgrades", database_upgrades);
> +}
> +
> +const char *
>  notmuch_config_get_user_name (notmuch_config_t *config)
>  {
>      return _config_get (config, &config->user_name, "user", "name");
> diff --git a/test/T030-config.sh b/test/T030-config.sh
> index ca4cf33..d095f0e 100755
> --- a/test/T030-config.sh
> +++ b/test/T030-config.sh
> @@ -47,6 +47,7 @@ notmuch config set database.path "/canonical/path"
>  output=$(notmuch config list)
>  test_expect_equal "$output" "\
>  database.path=/canonical/path
> +database.upgrades=no
>  user.name=Notmuch Test Suite
>  user.primary_email=test_suite@notmuchmail.org
>  user.other_email=test_suite_other@notmuchmail.org;test_suite@otherdomain.org
> diff --git a/test/T040-setup.sh b/test/T040-setup.sh
> index 124ef1c..46fd327 100755
> --- a/test/T040-setup.sh
> +++ b/test/T040-setup.sh
> @@ -16,6 +16,7 @@ EOF
>  output=$(notmuch --config=new-notmuch-config config list)
>  test_expect_equal "$output" "\
>  database.path=/path/to/maildir
> +database.upgrades=no
>  user.name=Test Suite
>  user.primary_email=test.suite@example.com
>  user.other_email=another.suite@example.com;
> -- 
> 1.9.0
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

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

* Re: [PATCH 2/2] notmuch-new: block database upgrades in default configuration.
  2014-03-23  2:26           ` [PATCH 2/2] notmuch-new: block database upgrades in default configuration David Bremner
@ 2014-03-23 14:17             ` Jani Nikula
  2014-03-23 14:35               ` David Bremner
  2014-03-23 23:54               ` Jameson Graef Rollins
  0 siblings, 2 replies; 18+ messages in thread
From: Jani Nikula @ 2014-03-23 14:17 UTC (permalink / raw)
  To: David Bremner, notmuch

On Sun, 23 Mar 2014, David Bremner <david@tethera.net> wrote:
> People who prefer the current behaviour can set the configuration
> variable and forget it.
> ---
>  notmuch-new.c        |  9 +++++++++
>  test/T530-upgrade.sh | 11 +++++++++++
>  2 files changed, 20 insertions(+)
>
> diff --git a/notmuch-new.c b/notmuch-new.c
> index 82acf69..c6a15b0 100644
> --- a/notmuch-new.c
> +++ b/notmuch-new.c
> @@ -989,6 +989,15 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])
>  	    return EXIT_FAILURE;
>  
>  	if (notmuch_database_needs_upgrade (notmuch)) {
> +	    if (strcasecmp(notmuch_config_get_database_upgrades (config), "yes") != 0) {
> +		fprintf (stderr,
> +			 "Database upgrade needed. Please enable with\n\n"
> +			 "  notmuch config set database.upgrades yes\n\n"
> +			 "after taking appropriate precautions.\n");

"and if you're paranoid, please switch off the same option afterwards"?

We really have very few configuration options, and so far none of them
are such that we could make the decision for the user. I think in this
case pushing the responsibility to the user would be just *us* being
paranoid. What does that tell our users?

I also think the user should consider doing backups before upgrading
notmuch in the first place. I am generally more concerned about the user
doing a database dump of the old database version using the new notmuch
than doing the upgrade. (Although in this case I think we're fine unless
the user decides 'notmuch dump folder:important' is enough.)

BR,
Jani.


> +
> +		return EXIT_FAILURE;
> +	    }
> +
>  	    if (add_files_state.verbosity >= VERBOSITY_NORMAL)
>  		printf ("Welcome to a new version of notmuch! Your database will now be upgraded.\n");
>  	    gettimeofday (&add_files_state.tv_start, NULL);
> diff --git a/test/T530-upgrade.sh b/test/T530-upgrade.sh
> index 67bbf31..00dcb1e 100755
> --- a/test/T530-upgrade.sh
> +++ b/test/T530-upgrade.sh
> @@ -25,8 +25,19 @@ test_begin_subtest "path: search does not work with old database version"
>  output=$(notmuch search path:foo)
>  test_expect_equal "$output" ""
>  
> +test_begin_subtest "database upgrade blocked by default config"
> +output=$(notmuch new 2>&1)
> +test_expect_equal "$output" "\
> +Database upgrade needed. Please enable with
> +
> +  notmuch config set database.upgrades yes
> +
> +after taking appropriate precautions."
> +
>  test_begin_subtest "database upgrade from format version 1"
> +notmuch config set database.upgrades yes
>  output=$(notmuch new)
> +
>  test_expect_equal "$output" "\
>  Welcome to a new version of notmuch! Your database will now be upgraded.
>  Your notmuch database has now been upgraded to database format version 2.
> -- 
> 1.9.0
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

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

* Re: [PATCH 2/2] notmuch-new: block database upgrades in default configuration.
  2014-03-23 14:17             ` Jani Nikula
@ 2014-03-23 14:35               ` David Bremner
  2014-03-23 14:39                 ` David Bremner
  2014-03-23 23:54               ` Jameson Graef Rollins
  1 sibling, 1 reply; 18+ messages in thread
From: David Bremner @ 2014-03-23 14:35 UTC (permalink / raw)
  To: Jani Nikula, notmuch

Jani Nikula <jani@nikula.org> writes:

> We really have very few configuration options, and so far none of them
> are such that we could make the decision for the user. I think in this
> case pushing the responsibility to the user would be just *us* being
> paranoid. What does that tell our users?

I don't care either way, I just want to do a release. It's easier to err
on the side of being paranoid. So please collectively sort out what you
want, and if you want something different other than the status quo or
this, please send patches ASAP.

d

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

* Re: [PATCH 2/2] notmuch-new: block database upgrades in default configuration.
  2014-03-23 14:35               ` David Bremner
@ 2014-03-23 14:39                 ` David Bremner
  0 siblings, 0 replies; 18+ messages in thread
From: David Bremner @ 2014-03-23 14:39 UTC (permalink / raw)
  To: Jani Nikula, notmuch

David Bremner <david@tethera.net> writes:

> Jani Nikula <jani@nikula.org> writes:
>
>> We really have very few configuration options, and so far none of them
>> are such that we could make the decision for the user. I think in this
>> case pushing the responsibility to the user would be just *us* being
>> paranoid. What does that tell our users?
>
> I don't care either way, I just want to do a release. It's easier to err
> on the side of being paranoid. So please collectively sort out what you
> want, and if you want something different other than the status quo or
> this, please send patches ASAP.

Umm, of course by "this", I mean some polished version of the series.

d

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

* Re: [PATCH 2/2] notmuch-new: block database upgrades in default configuration.
  2014-03-23 14:17             ` Jani Nikula
  2014-03-23 14:35               ` David Bremner
@ 2014-03-23 23:54               ` Jameson Graef Rollins
  1 sibling, 0 replies; 18+ messages in thread
From: Jameson Graef Rollins @ 2014-03-23 23:54 UTC (permalink / raw)
  To: Jani Nikula, David Bremner, notmuch

[-- Attachment #1: Type: text/plain, Size: 1058 bytes --]

On Sun, Mar 23 2014, Jani Nikula <jani@nikula.org> wrote:
> We really have very few configuration options, and so far none of them
> are such that we could make the decision for the user. I think in this
> case pushing the responsibility to the user would be just *us* being
> paranoid. What does that tell our users?
>
> I also think the user should consider doing backups before upgrading
> notmuch in the first place. I am generally more concerned about the user
> doing a database dump of the old database version using the new notmuch
> than doing the upgrade. (Although in this case I think we're fine unless
> the user decides 'notmuch dump folder:important' is enough.)

Sorry for the delay responding to this.

I really think the right way to handle a database upgrade is:

* fail notmuch new (with appropriate message) if run in quiet mode
* prompt user yes/no to upgrade database otherwise

This will allow all users to acknowledge the upgrade and do the
appropriate thing, regardless of configuration, without adding a new
config option.

jamie.

[-- Attachment #2: Type: application/pgp-signature, Size: 818 bytes --]

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

end of thread, other threads:[~2014-03-23 23:54 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-09 15:01 WARNING: database upgrade coming David Bremner
2014-03-12 11:25 ` David Bremner
2014-03-17 17:17 ` Jameson Graef Rollins
2014-03-17 19:12   ` Jani Nikula
2014-03-17 21:19     ` David Bremner
2014-03-17 21:29       ` Jameson Graef Rollins
2014-03-23  2:26         ` conservative database upgrade control David Bremner
2014-03-23  2:26           ` [PATCH 1/2] config: add key database.upgrades David Bremner
2014-03-23 13:12             ` Jani Nikula
2014-03-23  2:26           ` [PATCH 2/2] notmuch-new: block database upgrades in default configuration David Bremner
2014-03-23 14:17             ` Jani Nikula
2014-03-23 14:35               ` David Bremner
2014-03-23 14:39                 ` David Bremner
2014-03-23 23:54               ` Jameson Graef Rollins
2014-03-17 21:31       ` WARNING: database upgrade coming Jani Nikula
2014-03-18  6:06         ` Rob Browning
2014-03-18  6:57           ` Tomi Ollila
2014-03-18 11:55             ` Stewart Smith

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).