On Mon, Jan 09, 2012 at 11:57:33PM -0500, Jeff King wrote:

Subject: [PATCH] credential-cache: report more daemon connection errors

Originally, this code remained relatively silent when we
failed to connect to the cache. The idea was that it was
simply a cache, and we didn't want to bother the user with
temporary failures (the worst case is that we would simply
ask their password again).

However, if you have a configuration failure or other
problem, it is helpful for the daemon to report those
problems. Git will happily ignore the failed error code, but
the extra information to stderr can help the user diagnose
the problem.

This actually has a minor regression, fixed below.

-- >8 --
Subject: [PATCH] credential-cache: ignore "connection refused" errors

The credential-cache helper will try to connect to its
daemon over a unix socket. Originally, a failure to do so
was silently ignored, and we would either give up (if
performing a "get" or "erase" operation), or spawn a new
daemon (for a "store" operation).

But since 8ec6c8d, we try to report more errors. We detect a
missing daemon by checking for ENOENT on our connection
attempt.  If the daemon is missing, we continue as before
(giving up or spawning a new daemon). For any other error,
we die and report the problem.

However, checking for ENOENT is not sufficient for a missing
daemon. We might also get ECONNREFUSED if a dead daemon
process left a stale socket. This generally shouldn't
happen, as the daemon cleans up after itself, but the daemon
may not always be given a chance to do so (e.g., power loss,
"kill -9").

The resulting state is annoying not just because the helper
outputs an extra useless message, but because it actually
blocks the helper from spawning a new daemon to replace the
stale socket.

Fix it by checking for ECONNREFUSED.

Signed-off-by: Jeff King <redacted>
---
If we really want to go belt-and-suspenders, the logic should perhaps be
changed to:

  if (send_request(socket, &buf < 0) {
          /* if we're starting a new one, who cares why it didn't work */
          if (flags & FLAG_SPAWN) {
                  spawn_daemon(socket);
                  if (send_request(socket, &buf) < 0)
                          die_errno("unable to connect to spawned daemon");
          }
          /* otherwise, report any non-minor errors */
          else if(errno != ENOENT && errno != ECONNREFUSED)
                  die_errno("unable to connect to cache daemon");
          /* otherwise we are just missing the daemon, and we can ignore */
  }

but that implies there is some condition besides ENOENT and ECONNREFUSED
where actually starting a new daemon (which will try to unlink whatever
is there now!) would be a good idea. I'd rather be conservative and
see if anybody reports a real-world case.

 credential-cache.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/credential-cache.c b/credential-cache.c
index 1933018..9a03792 100644
--- a/credential-cache.c
+++ b/credential-cache.c

@@ -72,7 +72,7 @@ static void do_cache(const char *socket, const char *action, int timeout,
 	}
 
 	if (send_request(socket, &buf) < 0) {
-		if (errno != ENOENT)
+		if (errno != ENOENT && errno != ECONNREFUSED)
 			die_errno("unable to connect to cache daemon");
 		if (flags & FLAG_SPAWN) {
 			spawn_daemon(socket);

-- 
1.7.9.rc0.33.gd3c17