* [PATCH 2/4] compact: Give user more feedback on failure renaming
2013-10-28 22:23 [PATCH 1/4] test: Add compact test Ben Gamari
@ 2013-10-28 22:23 ` Ben Gamari
2013-10-28 22:23 ` [PATCH 3/4] compact: Provide user with more error feedback Ben Gamari
` (2 subsequent siblings)
3 siblings, 0 replies; 11+ messages in thread
From: Ben Gamari @ 2013-10-28 22:23 UTC (permalink / raw)
To: notmuch
Provide the user with instructions after we fail to move the old
un-compacted database out of the way.
Signed-off-by: Ben Gamari <bgamari.foss@gmail.com>
---
lib/database.cc | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/lib/database.cc b/lib/database.cc
index 06f1c0a..57c2292 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -922,7 +922,14 @@ notmuch_database_compact (const char* path,
if (old_xapian_path != NULL) {
if (rename(xapian_path, old_xapian_path)) {
- fprintf (stderr, "Error moving old database out of the way\n");
+ fprintf (stderr, "Error moving old database out of the way: %s\n",
+ strerror(errno));
+ fprintf (stderr, "\n");
+ fprintf (stderr, "Old database: %s\n", xapian_path);
+ fprintf (stderr, "Compacted database: %s\n", compact_xapian_path);
+ fprintf (stderr, "\n");
+ fprintf (stderr, "At this point it's probably best to remove the compacted database,\n");
+ fprintf (stderr, "find the cause of this error, and try compacting again.\n");
ret = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
}
--
1.8.3.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/4] compact: Provide user with more error feedback
2013-10-28 22:23 [PATCH 1/4] test: Add compact test Ben Gamari
2013-10-28 22:23 ` [PATCH 2/4] compact: Give user more feedback on failure renaming Ben Gamari
@ 2013-10-28 22:23 ` Ben Gamari
2013-10-28 22:23 ` [PATCH 4/4] database: Handle error while deleting uncompacted database Ben Gamari
2013-10-29 11:03 ` [PATCH 1/4] test: Add compact test Tomi Ollila
3 siblings, 0 replies; 11+ messages in thread
From: Ben Gamari @ 2013-10-28 22:23 UTC (permalink / raw)
To: notmuch
Provide instructions on what to do when we couldn't move the compacted
database into place.
Signed-off-by: Ben Gamari <bgamari.foss@gmail.com>
---
lib/database.cc | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/lib/database.cc b/lib/database.cc
index 57c2292..34753ab 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -938,7 +938,25 @@ notmuch_database_compact (const char* path,
}
if (rename(compact_xapian_path, xapian_path)) {
- fprintf (stderr, "Error moving compacted database\n");
+ fprintf (stderr, "Error moving compacted database into place: %s\n", strerror(errno));
+ fprintf (stderr, "\n");
+ fprintf (stderr, "Encountered error while moving the compacted database\n");
+ fprintf (stderr, "\n");
+ fprintf (stderr, " %s\n", compact_xapian_path);
+ fprintf (stderr, "\n");
+ fprintf (stderr, "to\n");
+ fprintf (stderr, "\n");
+ fprintf (stderr, " %s\n", xapian_path);
+ fprintf (stderr, "\n");
+ fprintf (stderr, "Please identify the reason for this and move the compacted database\n");
+ fprintf (stderr, "into place manually.\n");
+ if (old_xapian_path != NULL) {
+ fprintf (stderr, "\n");
+ fprintf (stderr, "Alternatively you can revert to the uncompacted database with\n");
+ fprintf (stderr, "\n");
+ fprintf (stderr, " mv %s %s\n", old_xapian_path, xapian_path);
+ fprintf (stderr, "\n");
+ }
ret = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
}
--
1.8.3.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 4/4] database: Handle error while deleting uncompacted database
2013-10-28 22:23 [PATCH 1/4] test: Add compact test Ben Gamari
2013-10-28 22:23 ` [PATCH 2/4] compact: Give user more feedback on failure renaming Ben Gamari
2013-10-28 22:23 ` [PATCH 3/4] compact: Provide user with more error feedback Ben Gamari
@ 2013-10-28 22:23 ` Ben Gamari
2013-11-02 12:23 ` David Bremner
2013-10-29 11:03 ` [PATCH 1/4] test: Add compact test Tomi Ollila
3 siblings, 1 reply; 11+ messages in thread
From: Ben Gamari @ 2013-10-28 22:23 UTC (permalink / raw)
To: notmuch
We never checked to ensure that the rmtree() of the old database
succeeded.
Signed-off-by: Ben Gamari <bgamari.foss@gmail.com>
---
lib/database.cc | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/lib/database.cc b/lib/database.cc
index 34753ab..bfc5dac 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -934,7 +934,19 @@ notmuch_database_compact (const char* path,
goto DONE;
}
} else {
- rmtree(xapian_path);
+ if (rmtree(xapian_path)) {
+ fprintf (stderr, "Error removing old database: %s\n",
+ strerror(errno));
+ fprintf (stderr, "\n");
+ fprintf (stderr, "Old database: %s\n", xapian_path);
+ fprintf (stderr, "\n");
+ fprintf (stderr, "Please remove the old database and move the compacted one in to place manually with\n");
+ fprintf (stderr, "\n");
+ fprintf (stderr, " mv %s %s\n", compact_xapian_path, xapian_path);
+ fprintf (stderr, "\n");
+ ret = NOTMUCH_STATUS_FILE_ERROR;
+ goto DONE;
+ }
}
if (rename(compact_xapian_path, xapian_path)) {
--
1.8.3.2
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] test: Add compact test
2013-10-28 22:23 [PATCH 1/4] test: Add compact test Ben Gamari
` (2 preceding siblings ...)
2013-10-28 22:23 ` [PATCH 4/4] database: Handle error while deleting uncompacted database Ben Gamari
@ 2013-10-29 11:03 ` Tomi Ollila
2013-10-31 4:10 ` Ben Gamari
3 siblings, 1 reply; 11+ messages in thread
From: Tomi Ollila @ 2013-10-29 11:03 UTC (permalink / raw)
To: Ben Gamari, notmuch
On Tue, Oct 29 2013, Ben Gamari <bgamari.foss@gmail.com> wrote:
> Signed-off-by: Ben Gamari <bgamari.foss@gmail.com>
> ---
Patches 2, 3 & 4 Looks OK To Me. Thanks. A few comments on this patch
inline:
> test/compact | 35 +++++++++++++++++++++++++++++++++++
> test/notmuch-test | 1 +
> 2 files changed, 36 insertions(+)
> create mode 100755 test/compact
>
> diff --git a/test/compact b/test/compact
> new file mode 100755
> index 0000000..54e85ab
> --- /dev/null
> +++ b/test/compact
> @@ -0,0 +1,35 @@
> +#!/usr/bin/env bash
> +test_description='"notmuch compact"'
> +. ./test-lib.sh
> +
> +add_message '[subject]=One'
> +add_message '[subject]=Two'
> +add_message '[subject]=Three'
> +
> +notmuch tag +tag1 \*
> +notmuch tag +tag2 subject:Two
> +notmuch tag -tag1 +tag3 subject:Three
> +
> +test_begin_subtest "Compacting"
> +notmuch compact
> +test_expect_success "compact" "notmuch compact"
test_expect_success executes "$2" ("notmuch compact" in this case)
do you mean to run notmuch 'compact twice' ?
> +notmuch search \*
> +output=$(notmuch search \* | notmuch_search_sanitize)
Now do you mean to run 'notmuch search \*' twice ?
> +test_expect_equal "$output" "\
> +thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; One (inbox tag1 unread)
> +thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; Two (inbox tag1 tag2 unread)
> +thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; Three (inbox tag3 unread)"
> +
> +test_begin_subtest "Restoring backup"
> +rm -Rf ${TEST_TMPDIR}/mail/xapian
> +mv ${TEST_TMPDIR}/mail/xapian.old ${TEST_TMPDIR}/mail/xapian
> +
> +notmuch search \*
> +output=$(notmuch search \* | notmuch_search_sanitize)
again... ?
... actually David Bremner had even more insightful comments in
id:874n8cw2yq.fsf@zancas.localnet to these same issues :D
Tomi
> +test_expect_equal "$output" "\
> +thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; One (inbox tag1 unread)
> +thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; Two (inbox tag1 tag2 unread)
> +thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; Three (inbox tag3 unread)"
> +
> +test_done
> diff --git a/test/notmuch-test b/test/notmuch-test
> index aa28bb0..ec94baf 100755
> --- a/test/notmuch-test
> +++ b/test/notmuch-test
> @@ -19,6 +19,7 @@ cd $(dirname "$0")
> TESTS="
> basic
> help-test
> + compact
> config
> setup
> new
> --
> 1.8.3.2
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/4] test: Add compact test
2013-10-29 11:03 ` [PATCH 1/4] test: Add compact test Tomi Ollila
@ 2013-10-31 4:10 ` Ben Gamari
0 siblings, 0 replies; 11+ messages in thread
From: Ben Gamari @ 2013-10-31 4:10 UTC (permalink / raw)
To: Tomi Ollila, notmuch
[-- Attachment #1: Type: text/plain, Size: 2242 bytes --]
Tomi Ollila <tomi.ollila@iki.fi> writes:
> On Tue, Oct 29 2013, Ben Gamari <bgamari.foss@gmail.com> wrote:
>
>> Signed-off-by: Ben Gamari <bgamari.foss@gmail.com>
>> ---
>
> Patches 2, 3 & 4 Looks OK To Me. Thanks. A few comments on this patch
> inline:
>
Thanks again for the review!
>> test/compact | 35 +++++++++++++++++++++++++++++++++++
>> test/notmuch-test | 1 +
>> 2 files changed, 36 insertions(+)
>> create mode 100755 test/compact
>>
>> diff --git a/test/compact b/test/compact
>> new file mode 100755
>> index 0000000..54e85ab
>> --- /dev/null
>> +++ b/test/compact
>> @@ -0,0 +1,35 @@
>> +#!/usr/bin/env bash
>> +test_description='"notmuch compact"'
>> +. ./test-lib.sh
>> +
>> +add_message '[subject]=One'
>> +add_message '[subject]=Two'
>> +add_message '[subject]=Three'
>> +
>> +notmuch tag +tag1 \*
>> +notmuch tag +tag2 subject:Two
>> +notmuch tag -tag1 +tag3 subject:Three
>> +
>> +test_begin_subtest "Compacting"
>> +notmuch compact
>> +test_expect_success "compact" "notmuch compact"
>
> test_expect_success executes "$2" ("notmuch compact" in this case)
> do you mean to run notmuch 'compact twice' ?
>
It's been a while but I suspect I just never cleaned up the patch after
coming to this realization. Anyways, it's fixed in the revised patch
coming shortly.
>> +notmuch search \*
>> +output=$(notmuch search \* | notmuch_search_sanitize)
>
> Now do you mean to run 'notmuch search \*' twice ?
>
>> +test_expect_equal "$output" "\
>> +thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; One (inbox tag1 unread)
>> +thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; Two (inbox tag1 tag2 unread)
>> +thread:XXX 2001-01-05 [1/1] Notmuch Test Suite; Three (inbox tag3 unread)"
>> +
>> +test_begin_subtest "Restoring backup"
>> +rm -Rf ${TEST_TMPDIR}/mail/xapian
>> +mv ${TEST_TMPDIR}/mail/xapian.old ${TEST_TMPDIR}/mail/xapian
>> +
>> +notmuch search \*
>> +output=$(notmuch search \* | notmuch_search_sanitize)
>
> again... ?
>
>
> ... actually David Bremner had even more insightful comments in
> id:874n8cw2yq.fsf@zancas.localnet to these same issues :D
>
Hmm, it seems I overlooked these. Thanks for the reference.
Cheers,
- Ben
[-- Attachment #2: Type: application/pgp-signature, Size: 489 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread