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

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Apr 10, 2025, 14:50 UTC
Message-ID: <dc0d6acb-bcf1-4a7e-bfc8-7c2e7793c72b@gmail.com>
In-Reply-To: <20250410-505-wire-up-sparse-via-meson-v2-3-acb45cc8a2e5@gmail.com>

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

Show 14 quoted lines

> 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.

Show 63 quoted lines

> +  'command-list.h',
> +  'config-list.h',
> +  'hook-list.h'
> +]
> +
>   if not get_option('breaking_changes')
>     builtin_sources += 'builtin/pack-redundant.c'
>   endif
> @@ -1995,6 +2001,107 @@ endif
>   
>   subdir('contrib')
>   
> +headers_check_exclude = headers_generated
> +headers_check_exclude += [
> +  'compat/apple-common-crypto.h',
> +  'compat/bswap.h',
> +  'compat/compiler.h',
> +  'compat/disk.h',
> +  'compat/fsmonitor/fsm-darwin-gcc.h',
> +  'compat/fsmonitor/fsm-health.h',
> +  'compat/fsmonitor/fsm-listen.h',
> +  'compat/mingw.h',
> +  'compat/msvc.h',
> +  'compat/nedmalloc/malloc.c.h',
> +  'compat/nedmalloc/nedmalloc.h',
> +  'compat/nonblock.h',
> +  'compat/obstack.h',
> +  'compat/poll/poll.h',
> +  'compat/precompose_utf8.h',
> +  'compat/regex/regex.h',
> +  'compat/regex/regex_internal.h',
> +  'compat/sha1-chunked.h',
> +  'compat/terminal.h',
> +  'compat/vcbuild/include/sys/param.h',
> +  'compat/vcbuild/include/sys/time.h',
> +  'compat/vcbuild/include/sys/utime.h',
> +  'compat/vcbuild/include/unistd.h',
> +  'compat/vcbuild/include/utime.h',
> +  'compat/win32.h',
> +  'compat/win32/alloca.h',
> +  'compat/win32/dirent.h',
> +  'compat/win32/lazyload.h',
> +  'compat/win32/path-utils.h',
> +  'compat/win32/pthread.h',
> +  'compat/win32/syslog.h',
> +  'compat/zlib-compat.h',
> +  't/unit-tests/clar/clar.h',
> +  't/unit-tests/clar/clar/fixtures.h',
> +  't/unit-tests/clar/clar/fs.h',
> +  't/unit-tests/clar/clar/print.h',
> +  't/unit-tests/clar/clar/sandbox.h',
> +  't/unit-tests/clar/clar/summary.h',
> +  't/unit-tests/clar/test/clar_test.h',
> +  'unicode-width.h',
> +  'xdiff/xdiff.h',
> +  'xdiff/xdiffi.h',
> +  'xdiff/xemit.h',
> +  'xdiff/xinclude.h',
> +  'xdiff/xmacros.h',
> +  'xdiff/xprepare.h',
> +  '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?

Show 27 quoted lines

> +if sha1_backend != 'openssl'
> +  headers_check_exclude += 'sha1/openssl.h'
> +endif
> +if sha256_backend != 'openssl'
> +  headers_check_exclude += 'sha256/openssl.h'
> +endif
> +if sha256_backend != 'nettle'
> +  headers_check_exclude += 'sha256/nettle.h'
> +endif
> +if sha256_backend != 'gcrpyt'
> +  headers_check_exclude += 'sha256/gcrypt.h'
> +endif
> +
> +if headers.length() != 0 and compiler.get_argument_syntax() == 'gcc'
> +  hco_targets = []
> +  foreach h : headers
> +    if headers_check_exclude.contains(h)
> +      continue
> +    endif
> +
> +    hcc = custom_target(
> +      input: h,
> +      output: h.underscorify() + 'cc',
> +      command: [
> +        shell,
> +        '-c',
> +        'echo \'#include "git-compat-util.h"\' > @OUTPUT@ && echo -n \'#include "' + h + '"\' >> @OUTPUT@'

This line is rather long. Also do we really need "echo -n" here, the Makefile does not use it.

Show 11 quoted lines

> +      ]
> +    )
> +
> +    hco = custom_target(
> +      input: hcc,
> +      output: h.underscorify().replace('.h', '.hco'),
> +      command: [
> +        compiler.cmd_array(),
> +        libgit_c_args,
> +        '-I', meson.project_source_root(),
> +        '-I', meson.project_source_root() / 't/unit-tests',

Do you know why we don't need to add these include paths for the equivalent rule in the Makefile?

Thanks

Phillip

Show 15 quoted lines

> +        '-o', '/dev/null',
> +        '-c', '-xc',
> +        '@INPUT@'
> +      ]
> +    )
> +    hco_targets += hco
> +  endforeach
> +
> +  alias_target('hdr-check', hco_targets)
> +endif
> +
>   foreach key, value : {
>     'DIFF': diff.full_path(),
>     'GIT_SOURCE_DIR': meson.project_source_root(),
>