Re: git on afs

From: Shawn O. Pearce <spearce@spearce.org>
Date: Oct 19, 2007, 05:48 UTC
Message-ID: <20071019054814.GJ14735@spearce.org>
In-Reply-To: <20071018203106.GA13518@fries.net>

There's two different issues here so I'm going to split the thread into two to simplify the discussion. Well, for me anyway. ;-)

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

> 1) git presumes that link() works fine across subdirs; in afs land,
>    hardlinks do not succeed ever

...

Show 22 quoted lines

> diff --git a/sha1_file.c b/sha1_file.c
> index 83a06a7..1b93322 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -1961,7 +1961,7 @@ static int link_temp_to_file(const char *tmpfile, const char *filename)
>  	int ret;
>  	char *dir;
>  
> -	if (!link(tmpfile, filename))
> +	if (!rename(tmpfile, filename))
>  		return 0;
>  
>  	/*
> @@ -1980,7 +1980,7 @@ static int link_temp_to_file(const char *tmpfile, const char *filename)
>  			return -2;
>  		}
>  		*dir = '/';
> -		if (!link(tmpfile, filename))
> +		if (!rename(tmpfile, filename))
>  			return 0;
>  		ret = errno;
>  	}

These cases should already be handled lower, see l.1997-2012 of sha1_file.c:

    /*
     * Coda hack - coda doesn't like cross-directory links,
     * so we fall back to a rename, which will mean that it
     * won't be able to check collisions, but that's not a
     * big deal.
     *
     * The same holds for FAT formatted media.
     *
     * When this succeeds, we just return 0. We have nothing
     * left to unlink.
     */
    if (ret && ret != EEXIST) {
        if (!rename(tmpfile, filename))

Show 26 quoted lines

> Brandon Casey <casey@nrlssc.navy.mil> wrote:
> On Thu, 18 Oct 2007, Todd T. Fries wrote:
> 
> > link() returns -1 errno 17 File exists on afs.
> >
> > To further muddy the waters, linking within the same dir is ok,
> > linking outside the same dir is not:
> >
> > todd@ispdesk/p6 ~/tmp=A661$ mkdir dir
> > todd@ispdesk/p6 ~/tmp=A662$ touch a
> > todd@ispdesk/p6 ~/tmp=A663$ ln a b
> > todd@ispdesk/p6 ~/tmp=A664$ ls -l a b
> > -rw-r--r--  2 4  wheel  0 Oct 18 17:09 a
> > -rw-r--r--  2 4  wheel  0 Oct 18 17:09 b
> > todd@ispdesk/p6 ~/tmp=A665$ ls -li a b
> > 2068032 -rw-r--r--  2 4  wheel  0 Oct 18 17:09 a
> > 2068032 -rw-r--r--  2 4  wheel  0 Oct 18 17:09 b
> > todd@ispdesk/p6 ~/tmp=A666$ ln a dir/b
> > ln: dir/b: File exists
> > todd@ispdesk/p6 ~/tmp=A667$ echo $?
> 
> That error is just bogus on afs. If it returned a sane
> error, things would just work.
> 
> But, looks like afs only supports linking within the same
> directory: http://www.angelfire.com/hi/plutonic/afs-faq.html

So according to that error message from "ln" we really should have fallen into that Coda hack I just mentioned. Did we instead get a different errno here and not use that hack?

We probably could just use rename as you do above but then we would want to remove the unlink(tmpfile) call on l.2013 in sha1_file.c. Otherwise we're always incurring a syscall for no reason. I'm not against a change here, I just want to make sure we make the right change for AFS. :-)

-- 
Shawn.