From: Tomi Ollila <tomi.ollila@iki.fi>
To: notmuch@notmuchmail.org
Subject: Re: compactor adjustments
In-Reply-To: <1384192538-15291-1-git-send-email-tomi.ollila@iki.fi>
References: <1384192538-15291-1-git-send-email-tomi.ollila@iki.fi>
User-Agent: Notmuch/0.16+119~g219c55f (http://notmuchmail.org) Emacs/24.3.1
	(x86_64-unknown-linux-gnu)
Date: Tue, 12 Nov 2013 20:58:24 +0200
Message-ID: <m2fvr1tpkf.fsf@guru.guru-group.fi>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: list

On Mon, Nov 11 2013, Tomi Ollila <tomi.ollila@iki.fi> wrote:

> Hi
>
> I think these changes would be good to have in use before notmuch compact
> is in wider usage.
>
> All tests pass. I plan to test all of these code paths manually tomorrow.
> If anyone comes up with good plan how to add automatic tests I'll add
> those too (I also think myself, haven't got any good ones yet).

I made this change to the code:

diff --git a/lib/database.cc b/lib/database.cc
index 40272dc..ad7002b 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -933,2 +933,2 @@ notmuch_database_compact (const char* path,
-    } catch (Xapian::InvalidArgumentError e) {
-       fprintf (stderr, "Error while compacting: %s\n", e.get_msg().c_str());
+    } catch (Xapian::Error &error) {
+       fprintf (stderr, "Error while compacting: %s\n", error.get_msg().c_str());

Now it is consistent with other code and also doesn't crash on other errors

I did the following tests:


$ pwd
/path/to/.notmuch


$ mkdir xapian.old
$ notmuch compact
Compacting database...
Backup path already exists: /path/to/.notmuch/xapian.old
Compaction failed: Something went wrong trying to read or write a file
zsh: exit 1     ./notmuch compact


$ : tried to make compact fail by doing:
$ : mkdir xapian.compact / touch xapian.compact + chmod 000 xapian.compact
$ : notmuch compact worked OK after the above.


$ chmod 555 .
$ notmuch compact
Compacting database...
Error while compacting: /path/to/.notmuch/xapian.compact:
cannot create directory
Compaction failed: A Xapian exception occurred
zsh: exit 1     ./notmuch compact


$ chmod 755 .
$ notmuch compact --backup=/tmp/exdev
Compacting database...
...
...
...
Error moving old database out of the way:
Old database: /path/to/.notmuch/xapian
Backup database: /tmp/exdev
Error: Invalid cross-device link
Compaction failed: Something went wrong trying to read or write a file
zsh: exit 1     ./notmuch compact --backup=/tmp/exdev


$ : here I tried all I could think of (that doesn't include modifying
$ : permissions on the fly (perhaps in lib/database.cc code!), but
$ : could not get  if (rename(compact_xapian_path, xapian_path)) {
$ : fail. i.e. it is possible although highly unlikely (which is good). 


$ chmod 555 xapian
Compacting database...
...
...
...
Error removing backup database: Permission denied

Please remove the backup database with

   rm -rf '/path/to/.notmuch/xapian.old'

Compaction failed: Something went wrong trying to read or write a file
zsh: exit 1     ./notmuch compact


$ chmod 755 xapian
$ notmuch compact --backup=xapian.backup
Compacting database...
...
...
...
The old database has been moved to xapian.backup.
Done.
$ ls xapian.backup
flintlock       postlist.baseB  record.baseB    termlist.baseB
iamchert        postlist.DB     record.DB       termlist.DB
postlist.baseA  record.baseA    termlist.baseA


---

Unless there are other comments I'll make new patch series where
 } catch (Xapian::Error &error) { and submit it for inclusion to 0.17.
we'd better have somewhat decent 'notmuch compact' from day one.


Tomi