Subject: Re: git on afs

From: "Shawn O. Pearce" <[email-masked]>
Date: Sat, 20 Oct 2007 01:29:06 -0400

"Todd T. Fries" <[email-masked]> 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.
 
> --- 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.