Re: Git 2.14.1: t6500: error during test on musl libc

From: Jeff King <peff@peff.net>
Date: Sep 15, 2017, 11:30 UTC
Message-ID: <20170915113011.emko6q5utb7x4bvu@sigill.intra.peff.net>
In-Reply-To: <20170915063740.GB21499@alpha.vpn.ikke.info>

On Fri, Sep 15, 2017 at 08:37:40AM +0200, Kevin Daudt wrote:

Show 16 quoted lines

> On Thu, Sep 14, 2017 at 09:43:12PM -0500, A. Wilcox wrote:
> > -----BEGIN PGP SIGNED MESSAGE-----
> > Hash: SHA256
> > 
> > Hi there,
> > 
> > While bumping Git's version for our Linux distribution to 2.14.1, I've
> > run in to a new test failure in t6500-gc.sh.  This is the output of
> > the failing test with debug=t verbose=t:
> 
> This is a new test introduced by c45af94db 
> (gc: run pre-detach operations under lock, 2017-07-11) which was
> included in v2.14.0.
> 
> So it might be that this was already a problem for a longer time, only
> just recently uncovered.

The code change there is not all that big. Mostly we're just checking that the lock is actually respected. The lock code doesn't exercise libc all that much. It does use fscanf, which I guess is a little exotic for us. It's also possible that hostname() doesn't behave quite as we expect.

If you instrument gc like the patch below, what does it report when you run:

  GIT_TRACE=1 ./t6500-gc.sh --verbose-only=8

I get:

  [...]
  trace: built-in: git 'gc' '--auto'
  Auto packing the repository in background for optimum performance.
  See "git help gc" for manual housekeeping.
  debug: gc lock already held by $my_hostname
  [...]

If you get "acquired gc lock", then the problem is in lock_repo_for_gc(), and I'd suspect some problem with fscanf or hostname.

-Peff

---

diff --git a/builtin/gc.c b/builtin/gc.c
index c22787ac72..a7450a0058 100644
--- a/builtin/gc.c
+++ b/builtin/gc.c
@@ -242,9 +242,11 @@ static const char *lock_repo_for_gc(int force, pid_t* ret_pid)
 	int fd;
 	char *pidfile_path;
 
-	if (is_tempfile_active(pidfile))
+	if (is_tempfile_active(pidfile)) {
 		/* already locked */
+		trace_printf("debug: we already hold the gc lock");
 		return NULL;
+	}
 
 	if (xgethostname(my_host, sizeof(my_host)))
 		xsnprintf(my_host, sizeof(my_host), "unknown");
@@ -284,6 +286,7 @@ static const char *lock_repo_for_gc(int force, pid_t* ret_pid)
 				rollback_lock_file(&lock);
 			*ret_pid = pid;
 			free(pidfile_path);
+			trace_printf("debug: gc lock already held by %s", locking_host);
 			return locking_host;
 		}
 	}
@@ -295,6 +298,7 @@ static const char *lock_repo_for_gc(int force, pid_t* ret_pid)
 	commit_lock_file(&lock);
 	pidfile = register_tempfile(pidfile_path);
 	free(pidfile_path);
+	trace_printf("debug: we have acquired the gc lock");
 	return NULL;
 }