Simão Afonso <simao.afonso@xxxxxxxxxxxxxxxxxxx> writes:
> +credentialStore.fileTimeout::
> + The length of time, in milliseconds, for git-credential-store to retry
> + when trying to lock the credentials file. Value 0 means not to retry at
> + all; -1 means to try indefinitely. Default is 1000 (i.e., retry for
> + 1s).
I do not remember what was said in the first round of the review,
but I wonder if this is the best name for users. I think it is good
enough, but do ".lockTimeout" or ".lockTimeoutMS" make it even
easier to grok, perhaps?
> diff --git a/builtin/credential-store.c b/builtin/credential-store.c
> index 5331ab151..82284176e 100644
> --- a/builtin/credential-store.c
> +++ b/builtin/credential-store.c
> @@ -1,4 +1,5 @@
> #include "builtin.h"
> +#include "config.h"
> #include "lockfile.h"
> #include "credential.h"
> #include "string-list.h"
> @@ -58,8 +59,11 @@ static void print_line(struct strbuf *buf)
> static void rewrite_credential_file(const char *fn, struct credential *c,
> struct strbuf *extra)
> {
> - if (hold_lock_file_for_update(&credential_lock, fn, 0) < 0)
> - die_errno("unable to get credential storage lock");
> + int timeout_ms = 1000;
> + git_config_get_int("credentialstore.filetimeout", &timeout_ms);
Please have a blank line before the first statement.
> +
> + if (hold_lock_file_for_update_timeout(&credential_lock, fn, 0, timeout_ms) < 0)
> + die_errno("unable to get credential storage lock in %d ms", timeout_ms);
Should this be die_errno()? Looking at lock_file_timeout(), I am
not sure if the value of errno is valid in all codepaths that return
failure.
In any case, the message should be markd with _() for translation.
Other than that, it looks good.
Thanks.
> if (extra)
> print_line(extra);
> parse_credential_file(fn, c, NULL, print_line);
