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

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 18, 2008, 05:43 UTC
Message-ID: <7v7ih2u8e1.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <1203286456-26033-2-git-send-email-hjemli@gmail.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."

Show 14 quoted lines

> +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.

Show 9 quoted lines

> +	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.