Re: [GSoC][PATCH v4 2/7] clone: better handle symlinked files at .git/objects/

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Mar 29, 2019, 15:40 UTC
Message-ID: <nycvar.QRO.7.76.6.1903291635050.41@tvgsbejvaqbjf.bet>
In-Reply-To: <20190328221049.GK32487@hank.intra.tgummerer.com>

Hi Thomas,

On Thu, 28 Mar 2019, Thomas Gummerer wrote:

Show 19 quoted lines

> On 03/22, Matheus Tavares wrote:
> >
> > diff --git a/builtin/clone.c b/builtin/clone.c
> > index 50bde99618..b76f33c635 100644
> > --- a/builtin/clone.c
> > +++ b/builtin/clone.c
> > @@ -443,7 +443,7 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,
> >  		if (unlink(dest->buf) && errno != ENOENT)
> >  			die_errno(_("failed to unlink '%s'"), dest->buf);
> >  		if (!option_no_hardlinks) {
> > -			if (!link(src->buf, dest->buf))
> > +			if (!linkat(AT_FDCWD, src->buf, AT_FDCWD, dest->buf, AT_SYMLINK_FOLLOW))
>
> [...]
>
> I notice that we are currently not using 'linkat()' anywhere else in
> our codebase.  It looks like it has been introduced in POSIX.1-2008,
> which sounds fairly recent by git's standards.  So I wonder if this is
> really supported on all platforms that git is being built on.

I bet you it isn't.

Show 5 quoted lines

> I also wonder what would need to be done on Windows if we were to
> introduce this.  I see we define the 'link()' function in
> 'compat/mingw.c' for that currently, so I guess something similar
> would be needed for 'linkat()'.  I added Dscho to Cc for Windows
> expertise.

Indeed, `linkat()` would have to be implemented in `compat/mingw.c`. It would be a bit involved because the last parameter of that function changes behavior noticeably, but the main difficulty (to determine the path from a file descriptor) should be overcome using `HANDLE olddirhandle = _get_osfhandle(olddirfd);` and the calling `GetFinalPathNameByHandleW(olddirhandle, wbuf, sizeof(wbuf));`.

So yes, this is *not* something I'd do lightly.

The bigger problem will be to continue to support older Unices such as SunOS and AIX. I highly doubt that they have that function. You should find out, Matheus.

Ciao, Johannes