Re: [GSoC][PATCH v4 2/4] dir: add directory_exists

[Junio C Hamano <gitster@pobox.com>]

Devin Lehmacher <lehmacdj@gmail.com> writes:

> +int directory_exists(const char *path)
> +{
> +	struct stat sb;
> +	int ret = lstat(path, &sb);
> +	return ret == 0 && S_ISDIR(sb.st_mode);
> +}

I am not a great fan of using file_exists() [*1*] on anything other
than paths in the working tree (i.e. in preparation for checking
things out of or into the index), as paths inside .git/ and paths
outside have different requirements.  One of the difference is if it
makes sense to use stat(2) or lstat(2) for a check like this
function does.  For working tree entities, which file_exists() and
friends are designed for, we absolutely should use lstat(2).  The
"RelNotes" symbolic link at the top level of the project must be
known as a symbolic link and a question "is this a directory?" on it
must be answered "no", for example.

But there is no fundamental reason ~/.git-credential-cache (or
anything outside the working tree) must be S_ISDIR().  If the user
wanted to have the real directory elsewhere and point at it with
a symbolic link ~/.git-credential-cache, we should allow it.

If we need a helper function to see if a path in the working tree is
a directory, adding this new helper function to dir.c (which is
about the paths in the working tree) and use lstat(2) in it is the
right thing to do.  But I do not think it should be used to check if
~/.git-credential-cache directory, which is not a filesystem entity
in a working tree, exists.

IOW, I do not think this patch helps the topic of this series.  Drop
this patch from the series and have a similar code (but use stat(2)
instead of lstat(2)) directly inside get_socket_path()'s in the next
patch, perhaps?


[Footnote]

*1* ... and friends, like safe_create_leading_directories().