Re: [PATCH 1/2] credential-cache--daemon: allow a concurrently created socket dir

From: Patrick Steinhardt <ps@pks.im>
Date: Nov 3, 2026, 08:01 UTC
Message-ID: <aumUxF1uVtVC4qYj@pks.im>
In-Reply-To: <20261101-fz-credential-cache-race-v1-1-28f0bac51e11@gmail.com>

On Sun, Nov 01, 2026 at 03:29:25PM -0800, Farid Zakaria wrote:

Show 8 quoted lines

> diff --git a/builtin/credential-cache--daemon.c b/builtin/credential-cache--daemon.c
> index 65cc619bec..0dfa11e2ec 100644
> --- a/builtin/credential-cache--daemon.c
> +++ b/builtin/credential-cache--daemon.c
> @@ -273,8 +273,18 @@ static void init_socket_directory(const char *path)
>  		 */
>  		if (safe_create_leading_directories_const(the_repository, dir) < 0)
>  			die_errno("unable to create directories for '%s'", dir);

It's quite curious that we use `safe_create_leading_directories()` here. As documented in "path.h", the `safe_...()` functions should only ever be used for creating directories in the ".git" directory itself. In particular, applying "core.sharedRepostiory" to the leading directories of our credential daemon is never going to result in anything good at all.

So we should call `safe_create_leadingdirectories_no_share_const()` instead, which does not apply "core.sharedRepository". I know that's not introduced by your patch, but would you mind maybe fixing that in a preparatory commit?

Show 5 quoted lines

> -		if (mkdir(dir, 0700) < 0)
> -			die_errno("unable to mkdir '%s'", dir);
> +		if (mkdir(dir, 0700) < 0) {
> +			if (errno != EEXIST)
> +				die_errno("unable to mkdir '%s'", dir);

Makes sense.

Show 9 quoted lines

> +			/*
> +			 * Another daemon created it first; check it like one
> +			 * that already existed.
> +			 */
> +			if (stat(dir, &st))
> +				die_errno("unable to stat '%s'", dir);
> +			if (st.st_mode & 077)
> +				die(_(permissions_advice), dir);
> +		}

A bit of a shame that we duplicate this logic, but I don't see an obvious way for how to avoid that.

Patrick