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

[Brandon Casey <drafnel@gmail.com>]

On Wed, Apr 22, 2020 at 9:41 AM Jonathan Nieder <jrnieder@gmail.com> wrote:
>
> 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