Re: [PATCH v3 2/4] meson: move headers definition from 'contrib/coccinelle'

From: Karthik Nayak <karthik.188@gmail.com>
Date: Apr 20, 2025, 10:09 UTC
Message-ID: <CAOLa=ZRchwJWQfLDCMASEe9RWMOKDQvTfXaKqO=Shy=jhWMNaQ@mail.gmail.com>
In-Reply-To: <23493ec4-fd9f-44ae-a81a-8851e495b427@gmail.com>

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

Show 17 quoted lines

> Hi Karthik
>
> On 14/04/2025 22:16, Karthik Nayak wrote:
>> diff --git a/meson.build b/meson.build
>> index e98cfa4909..3ca5d01071 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -633,6 +633,28 @@ builtin_sources = [
>>     'builtin/write-tree.c',
>>   ]
>>
>> +third_party_sources = [
>
> This is not the fault of this patch but I found this name rather
> confusing as it is not a list of sources but a list of exclude patterns.
> Calling it "third_party_excludes" would be clearer to me at least.
>

This seems to map what the Makefile has, but I would agree, that this should be renamed.

Show 14 quoted lines

>> +  ':!contrib',
>> +  ':!compat/inet_ntop.c',
>> +  ':!compat/inet_pton.c',
>> +  ':!compat/nedmalloc',
>> +  ':!compat/obstack.*',
>> +  ':!compat/poll',
>> +  ':!compat/regex',
>> +  ':!sha1collisiondetection',
>> +  ':!sha1dc',
>> +  ':!t/unit-tests/clar',
>> +  ':!t/unit-tests/clar',
>
> Again not a new problem but this line is a duplicate
>

Fair, considering we're touching these fields, I will add an additional commit to simply cleanup these two issues.

Show 10 quoted lines

>> +  ':!t/t[0-9][0-9][0-9][0-9]*',
>> +]
>> +
>> +if git.found()
>> +  headers = []
>
> This is called "headers" but it is only really the subset of our headers
> that we want to run static analysis on. Maybe we could call it
> "headers_to_check" or something that makes it clearer what the list is for.
>

Agreed, will rename. Thanks!

Show 13 quoted lines

> Best Wishes
>
> Phillip
>
>> +  foreach header : run_command(git, '-C', meson.project_source_root(), 'ls-files', '--deduplicate', '*.h', third_party_sources, check: true).stdout().split()
>> +    headers += header
>> +  endforeach
>> +endif
>> +
>>   if not get_option('breaking_changes')
>>     builtin_sources += 'builtin/pack-redundant.c'
>>   endif
>>