Re: [GSoC][PATCH v4 3/4] credential-cache: use XDG_CACHE_HOME for socket

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 16, 2017, 16:20 UTC
Message-ID: <xmqqd1dhchwy.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170316051827.97198-4-lehmacdj@gmail.com>

Devin Lehmacher <lehmacdj@gmail.com> writes:

> +	If your home directory is on a network-mounted filesystem, you
> +	may need to change this to a local filesystem. You must specify
> +	an absolute path.

Nicely explained.

If a socket cannot be created in "~/.git-credential-cache", existing users would have created a symbolic link that points at a local directory. With XDG_CACHE_HOME mechanism, they would just set the variable to directly point at a local directory and there is no need for symlink trick, on the other hand.

Which points us back to the observation I made in my review on the previous step.

Show 11 quoted lines

> +static char *get_socket_path(void)
> +{
> +	char *old_credential_dir, *socket;
> +	old_credential_dir = expand_user_path("~/.git-credential-cache");
> +	if (old_credential_dir && directory_exists(old_credential_dir))
> +		socket = expand_user_path("~/.git-credential-cache/socket");
> +	else
> +		socket = xdg_cache_home("credential/socket");
> +	free(old_credential_dir);
> +	return socket;
> +}

As we do not want to use the dir.c::directory_exists(), which is meant to be used for working tree files, we can do something like this instead:

static char *get_socket_path(void)
{
	struct stat st;
	char *path;

	path = expand_user_path("~/.git-credential-cache");
	if (path && !stat(path, &st) && S_ISDIR(st.st_mode))) {
		free(path);
		path = expand_user_path("~/.git-credential-cache/socket");
	} else {
		path = xdg_cache_home("credential/socket");
	}                
	return path;
}

The duplication of "~/.git-credential-cache" bothers me somewhat and perhaps people can suggest better ways to get rid of the dup.

Other than that, makes sense to me.

Thanks.