Re: [PATCH v7 0/3] Conditional config include

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 3, 2017, 18:24 UTC
Message-ID: <xmqqlgsm9q2i.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170303063329.ji6do6eqjbpuwmxz@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 7 quoted lines

> For those following on the mailing list, there is some discussion at:
>
>   https://github.com/git/git/commit/484f78e46d00c6d35f20058671a8c76bb924fb33
>
> I think that is mostly focused around another failing in the
> error-handling of the config code, and that does not need to be
> addressed by this series (though of course I'd welcome any fixes).

Thanks. Without a message like this, the list may have never known about the discussion taken elsewhere. I'd appreciate such a report to appear on list the next time much earlier ;-)

When built with FREAD_READS_DIRECTORIES=Yes on Linux, the error in the test can easily reproduce.

In early days of UNIX it was sometimes handy to be able to read the bytes off of directory to "investigate", but we are not a filesystem application, and I do not offhand see any reason why we should be relying on being able to successfully fopen() a directory for reading. A FILE * successfully opened that just returns EOF when read is totally useless for any purpose anyway.

When the path to be opened from the end user (either from the command line or in a configuration file) is a directory, it is better to diagnose it as a user error, and if the path was computed by our code, it may be a bug.

I am wondering if we should enable this on Linux, at least in DEVELOPER builds but possibly even on the release builds, to catch these problems more easily.