git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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;
>         }
Previous: Junio C HamanoNext: SZEDER Gábor
Message 4 of 8 in “Make error message after failing commit_lock_file() less confusing”
  1. Make error message after failing commit_lock_file() less confusingSZEDER Gábor, Nov 30, 2015
  2. Jeff KingDec 1, 2015
  3. Junio C HamanoDec 2, 2015
  4. Eric SunshineDec 2, 2015
  5. SZEDER GáborDec 3, 2015
  6. Make error message after failing commit_lock_file() less confusingSZEDER Gábor, Dec 3, 2015
  7. credential-store: don't pass strerror to die_errno()SZEDER Gábor, Dec 16, 2015
  8. Jeff KingDec 16, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.