Thread (7 messages) flat view 7 messages, 3 authors, 2016-06-15

Re: [PATCH] Make error message after failing commit_lock_file() less confusing

From: Eric Sunshine <hidden>
Date: 2016-06-15 23:07:21

On Mon, Nov 30, 2015 at 6:40 AM, SZEDER Gábor [off-list ref] wrote:
quoted hunk ↗ jump to hunk
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 <redacted>
---
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.
quoted hunk ↗ jump to hunk
 }

 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;
        }
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help