Re: git on afs

From: Shawn O. Pearce <spearce@spearce.org>
Date: Oct 20, 2007, 05:29 UTC
Message-ID: <20071020052906.GT14735@spearce.org>
In-Reply-To: <200710190742.08174.todd@fries.net>

"Todd T. Fries" <todd@fries.net> wrote:

> It should not matter, but I'm using arla's afs client on OpenBSD; the errno
> is 17 (EEXIST), the very errno that bypasses the coda hack's rename():

...

> I can assure you that the 2nd argument to link does not exist ;-)

...

> The only downside is that either on coda or if the file already exists, it
> will try a spurrous rename(), in which case it will fail with EEXIST again,
> so I hope this is not a big negative.

Actually there is a really big downside. rename() is defined to unlink the destination if it already exists, so you'll never get EEXIST from a rename() call.

This means that an existing (known to be good) object can be
overwritten as a result of a rename with another copy of the object.
We've never really had that behavior as part of our security model
has been to always trust what is already in the repository and
refuse to replace something we already have.

Show 17 quoted lines

> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -2004,8 +2004,13 @@ int move_temp_to_file(const char *tmpfile, const char *filename)
>  	 *
>  	 * When this succeeds, we just return 0. We have nothing
>  	 * left to unlink.
> +	 *
> +	 * AFS hack - afs is similar to coda, but inconveniently
> +	 * set errno to EEXIST, so call rename() if the link()
> +	 * above fails unconditionally.  Small bit of extra work
> +	 * so afs functions properly.
>  	 */
> -	if (ret && ret != EEXIST) {
> +	if (ret) {
>  		if (!rename(tmpfile, filename))
>  			return 0;
>  		ret = errno;

This is very unfortunate. There's no way to tell that the file already exists. This whole AFS link() returning EEXIST is sort of like the unlink() call on Solaris UFS working on directories as root and leaving corrupted filesystems. At some point the application just cannot be reasonably expected to work on a system that acts this insane.

I think I would rather change sha1_file.c to write temporary files into the destination directory, rather than one level up and try to hardlink them into position. At least there we can rely on hardlinks within Coda and AFS, and only need this rename special case for FAT.

-- 
Shawn.