Re: [PATCH] Make error message after failing commit_lock_file() less confusing
- From
Eric Sunshine <sunshine@sunshineco.com>
- Date
- Dec 2, 2015, 04:28 UTC
- Message-ID
- <CAPig+cROdVgf=EL3jKyyPpLiRA_JyHD8P336-YDCWhK8nGSQaQ@mail.gmail.com>
- In-Reply-To
- <1448883653-9140-1-git-send-email-szeder@ira.uka.de>
On Mon, Nov 30, 2015 at 6:40 AM, SZEDER Gábor <szeder@ira.uka.de> wrote:
Show 33 quoted lines
> The error message after a failing commit_lock_file() call sometimes
> looks like this, causing confusion:
>
> $ git remote add remote git@server.com/repo.git
> error: could not commit config file .git/config
> # Huh?!
> # I didn't want to commit anything, especially not my config file!
>
> While in the narrow context of the lockfile module using the verb
> 'commit' in the error message makes perfect sense, in the broader
> context of git the word 'commit' already has a very specific meaning,
> hence the confusion.
>
> Reword these error messages to say "could not write" instead of "could
> not commit".
>
> While at it, include strerror in the error messages after writing the
> config file or the credential store fails to provide some information
> about the cause of the failure, and update the style of the error
> message after writing the reflog fails to match surrounding error
> messages (i.e. no '' around the pathname and no () around the error
> description).
>
> Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
> ---
> diff --git a/config.c b/config.c
> @@ -64,7 +64,8 @@ static void rewrite_credential_file(const char *fn, struct credential *c,
> print_line(extra);
> parse_credential_file(fn, c, NULL, print_line);
> if (commit_lock_file(&credential_lock) < 0)
> - die_errno("unable to commit credential store");
> + die_errno("unable to write credential store: %s",
> + strerror(errno));Hmm, this is already calling die_errno(), so adding another strerror() to the mix seems superfluous.
Show 11 quoted lines
> }
>
> static void store_credential_file(const char *fn, struct credential *c)
> diff --git a/fast-import.c b/fast-import.c
> @@ -1824,7 +1824,7 @@ static void dump_marks(void)
>
> dump_marks_helper(f, 0, marks);
> if (commit_lock_file(&mark_lock)) {
> - failure |= error("Unable to commit marks file %s: %s",
> + failure |= error("Unable to write file %s: %s",
> export_marks_file, strerror(errno));Since you're already doing some normalization of the error messages with this patch, do you want to drop capitalization on this one?
> return; > }