Re: [PATCH v2 5/5] compact: provide user more information on after-compaction failures

[Jani Nikula <jani@nikula.org>]

On Wed, 13 Nov 2013, Tomi Ollila <tomi.ollila@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@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch