Re: [PATCH] config.mak.uname: Define FREAD_READS_DIRECTORIES for GNU/Hurd

From: Brandon Casey <drafnel@gmail.com>
Date: Apr 22, 2020, 18:48 UTC
Message-ID: <CA+sFfMfre6W5GcPh1pWcroFD9S9OPj_uLp5CK11yh-UhqgDs2w@mail.gmail.com>
In-Reply-To: <20200422164150.GA140314@google.com>

On Wed, Apr 22, 2020 at 9:41 AM Jonathan Nieder <jrnieder@gmail.com> wrote:

Show 33 quoted lines

>
> Jessica Clarke wrote:
>
> > GNU/Hurd is another platform that behaves like this. Set it to
> > UnfortunatelyYes so that config directory files are correctly processed.
> > This fixes the corresponding 'proper error on directory "files"' test in
> > t1308-config-set.sh.
> >
> > Thanks-to: Jeff King <peff@peff.net>
> > Signed-off-by: Jessica Clarke <jrtc27@jrtc27.com>
> > ---
> >  config.mak.uname | 1 +
> >  1 file changed, 1 insertion(+)
>
> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
> Thanks.
>
> > diff --git a/config.mak.uname b/config.mak.uname
> > index 0ab8e00938..3e526f6b9f 100644
> > --- a/config.mak.uname
> > +++ b/config.mak.uname
> > @@ -308,6 +308,7 @@ ifeq ($(uname_S),GNU)
> >       NO_STRLCPY = YesPlease
> >       HAVE_PATHS_H = YesPlease
> >       LIBC_CONTAINS_LIBINTL = YesPlease
> > +     FREAD_READS_DIRECTORIES = UnfortunatelyYes
> >  endif
>
> I wonder why we set up this knob this way.  A lot of operating systems
> support fopen(..., "r") of a directory --- wouldn't it make sense for
> FREAD_READS_DIRECTORIES to be the default and for users on stricter
> platforms to be able to set FREAD_DOES_NOT_READ_DIRECTORIES if they
> want to speed Git up by taking advantage of their saner fread?

At the time it was written, the assumption in the code was that an fread() on a directory would produce a failure, and that that was the sane and common behavior. fopen(..., "r") on a directory was expected to be successful on most platforms, but does fail on some. I don't recall if it failed on any of the platforms I had access to at the time (Solaris, IRIX), but it does fail on Windows. So I introduced this feature that would make fopen() fail when opening a directory for use on the platforms where fread() of a directory did not fail, instead of trying to wrap fread().

I just looked in config.mak.uname, and I'm surprised to see FREAD_READS_DIRECTORIES set for so many platforms. And it's set for Linux and Darwin?!?!? Junio added it for Darwin (8e178ec4d072da4cd8f4449e17aef3aff5b57f6a) and Nguyễn Thái Ngọc Duy added it for Linux (e2d90fd1c33ae57e4a6da5729ae53876107b3463), but also seemed to mistake the intention of FREAD_FREADS_DIRECTORIES as being about the fopen(..., "r") of a directory rather than about an fread() of a directory.

I just wrote a test program and tested on Linux, Darwin, and Windows. Linux and Darwin both succeed to fopen() a directory and fail to fread() it, as expected. Windows fails to fopen() a directory.

I notice this earlier commit mentions a failure of t1308 (4e3ecbd43958b1400d6cb85fe5529beda1630e3a). I wonder if this is the reason FREAD_READS_DIRECTORIES was added to so many platforms?

-Brandon