Re: [PATCH 1/1] Makefile: use `git ls-files` to list header files, if possible

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 5, 2019, 22:26 UTC
Message-ID: <xmqqlg1tt0uf.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <nycvar.QRO.7.76.6.1903051627050.41@tvgsbejvaqbjf.bet>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Show 11 quoted lines

>> After a normal build, with dynamic dependency checking on, we would
>> have sufficient information to figure it out.  Would that help?
>
> Yes, *if* the dynamic dependency checking is in effect (read: if we are
> compiling with GCC).
>
> However, I think that one of the benefits of the current approach is that
> hdr-check finds issues also in headers that are *not* part of the current
> build. Read: once hdr-check is run as part of the CI build, it is harder
> for some random contributor to break hdr-check for somebody else with
> different compile time options.

Sure. That is exactly why we have multiple configurations at the CI, hopefully more than each individual contributor has at hand so that CI can give more coverage than any single individual can test.

What I was assuming was that at least one of these multiple configurations would cover the case where sha256/gcrypt.h truly is used, because that particular build is configured to use the system header and library. And it would be a far better approach to make that build check also for the header, if we can cover all the problematic (in the same sense as the sha256/gcrypt.h) header files, than unconditionally "checking" without knowing the prerequisite and causing a false positive.

Having said that, the optional use of ls-files introduced by the patch in question is a valid incremental improvement over the current Makefile, so it is a pretty much independent/orthogonal issue.