On Wed, 13 Nov 2013, Tomi Ollila <tomi.ollila at iki.fi> wrote:
> After database has been compacted, there are steps to put the new
> database into place -- and these steps may fail. In case such
> failure happens, provide better information how to resolve it.
I disagree with having a library spew all this information out. For each
case, I think it should be sufficient to just say what happened
(e.g. "rename a -> b failed" + strerror). I don't think a library's
error messages should be a tutorial on how to fix things.
We may need to amend notmuch compact man page though.
BR,
Jani.
>
> Thanks to Ben Gamari for most of the information content.
> ---
> lib/database.cc | 39 +++++++++++++++++++++++++++++++++++----
> 1 file changed, 35 insertions(+), 4 deletions(-)
>
> diff --git a/lib/database.cc b/lib/database.cc
> index 4b5ac64..a6daac6 100644
> --- a/lib/database.cc
> +++ b/lib/database.cc
> @@ -939,19 +939,50 @@ notmuch_database_compact (const char *path,
> }
>
> if (rename (xapian_path, backup_path)) {
> - fprintf (stderr, "Error moving old database out of the way\n");
> + fprintf (stderr, "Error moving old database out of the way:\n"
> + "Old database: %s\n"
> + "Backup database: %s\n"
> + "Error: %s\n", xapian_path, backup_path, strerror (errno));
> ret = NOTMUCH_STATUS_FILE_ERROR;
> goto DONE;
> }
>
> 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"
> + "Encountered error while moving the compacted database\n"
> + "\n"
> + " %s\n"
> + "\n"
> + "to\n"
> + "\n"
> + " %s\n"
> + "\n"
> + "Please identify the reason for this and move the compacted
> database\n"
> + "into place manually.\n"
> + "\n"
> + "Alternatively you can revert to the uncompacted database
> with\n"
> + "\n"
> + " mv '%s' '%s'\n"
> + "\n", compact_xapian_path, xapian_path,
> + backup_path, xapian_path);
> ret = NOTMUCH_STATUS_FILE_ERROR;
> goto DONE;
> }
>
> - if (! keep_backup)
> - rmtree (backup_path);
> + if (! keep_backup) {
> + if (rmtree (backup_path)) {
> + fprintf (stderr, "Error removing backup database: %s\n",
> + strerror (errno));
> + fprintf (stderr, "\n"
> + "Please remove the backup database with\n"
> + "\n"
> + " rm -rf '%s'\n" "\n", backup_path);
> + ret = NOTMUCH_STATUS_FILE_ERROR;
> + goto DONE;
> + }
> + }
>
> DONE:
> if (notmuch)
> --
> 1.8.3.1
>
> _______________________________________________
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
