Re: [PATCH v2 3/4] meson: add support for 'hdr-check'

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 10, 2025, 19:06 UTC
Message-ID: <xmqqplhjomy6.fsf@gitster.g>
In-Reply-To: <dc0d6acb-bcf1-4a7e-bfc8-7c2e7793c72b@gmail.com>

Phillip Wood <phillip.wood123@gmail.com> writes:

Show 26 quoted lines

> Hi Karthik
>
> On 10/04/2025 12:30, Karthik Nayak wrote:
>> The Makefile supports a target called 'hdr-check', which checks if
>> individual header files can be independently compiled. Let's port this
>> functionality to meson, our new build system too. The implementation
>> resembles that of the Makefile and provides the same check.
>
> Thanks for adding this, I've left a few comments below
>
>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
>> ---
>>   meson.build | 107 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 107 insertions(+)
>> diff --git a/meson.build b/meson.build
>> index 790d178007..6fce1aa618 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -655,6 +655,12 @@ if git.found()
>>     endforeach
>>   endif
>>   +headers_generated = [
>
> To me "generated_headers" would be a more natural name and would match
> the style of "coccinelle_headers" from patch 1 as well as the
> equivalent in the Makefile.

To me too.

Show 9 quoted lines

>> ...
>> +  'xdiff/xtypes.h',
>> +  'xdiff/xutils.h',
>> +]
>
> Having to manually maintain this list is a bit of a shame as every
> time a new file is added to compat we need to add it to meson.build
> twice. The Makefile avoids this by filtering the list of headers used
> when building git based on their path - can we do the same here?

Yup. It cuts both ways, but generally I would prefer that. Your creating a new file, and saying "git add" on it, should be a sign enough that you intend to make it a part of the project, and the file having a name that follows the project convention (e.g., "C header files are in certain directories and ends with .h") should be a sign enough that you want it to be part of the build. Forcing people to list them explicitly is a pain.

Yes, the approach to "glob" the list of files used would not help you catch mistakes like forgetting to "git add" new files. You test locally and you commit (without the new file), and you'll be notified by whoever cloned from you about a "missing file". But forcing to list the files in Makefile or meson.build would not help you catch that kind of mistake. While it may give you one more chance to remind yourself that you need to "git add" by ignoring a new file until you add it to meson.build, if you add it to the file but not to the commit, the build procedure would not complain.

So, yes, I would pretty much agree with you and prefer to see this kind of list go, if possible.

Thanks.