Re: [PATCH 1/5] Add platform-independent .git "symlink"

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

Lars Hjemli <hjemli@gmail.com> writes:

> This patch allows .git to be a regular textfile containing the path of
> the real git directory (formatted like "gitdir: <path>\n"), which is
> useful on platforms lacking support for real symlinks.

Hmmmm.  I have suspected all along that these "platforms lacking
support for real symlinks" are the ones whose native line
termination convention is CRLF.  How do you envision this file
is initialized?  By users editing the file by hand?  Or some git
specific tool?  If the former, it might make sense to define the
format as "a single line, terminated with platform line
terminator" and open and read it in text mode.  If the latter,
we do not have to care, and "terminated with LF" is good enough.

I think a sane simplification is to allow the file to have
any number of optional \r or \n at the end.  We may care about
allowing arbitrary and possibly crazy filenames in the tracked
contents, but we can say "Sorry, you cannot create a directory
'ab\nc\r' and use it as the .git directory substitute."

> +const char *read_gitfile_gently(const char *path)
> +{
> +	static char buf[PATH_MAX + 9];  /* "gitdir: " + "\n" */
> +	struct stat st;
> +	int fd;
> +	size_t len;
> +
> +	if (stat(path, &st))
> +		return NULL;
> +	if (!S_ISREG(st.st_mode) || st.st_size >= sizeof(buf))
> +		return NULL;
> +	fd = open(path, O_RDONLY);
> +	if (fd < 0)
> +		return NULL;

Up to this point it is fine "gently" semantics.

> +	len = read_in_full(fd, buf, sizeof(buf));
> +	close(fd);
> +	if (len != st.st_size)
> +		return NULL;
> +	if (!len || buf[len - 1] != '\n')
> +		return NULL;
> +	buf[len - 1] = '\0';
> +	if (prefixcmp(buf, "gitdir: "))
> +		return NULL;

But I am not sure about this part.  We found what claims to be
the ".git" fake symlink but it is ill-formed.  Don't we want to
diagnose the possible breakage for the user?

> +/*
> +	if (!is_git_directory(buf + 8))
> +		return NULL;
> +*/

Likewise.