Обсуждение: [PATCH] Add `headerscheck` run_target to meson

Поиск
Список
Период
Сортировка

[PATCH] Add `headerscheck` run_target to meson

От
Miłosz Bieniek
Дата:
Hi,
While reviewing a patch I noticed that we have a `make -s headerscheck` but there is no equivalent in meson.
I prepared a small patch that adds `headerscheck` and `cpluspluscheck` targets.

Best Regards,
Miłosz Bieniek 
Вложения

Re: [PATCH] Add `headerscheck` run_target to meson

От
Peter Eisentraut
Дата:
On 27.11.25 10:28, Miłosz Bieniek wrote:
> While reviewing a patch I noticed that we have a `make -s headerscheck` 
> but there is no equivalent in meson.
> I prepared a small patch that adds `headerscheck` and `cpluspluscheck` 
> targets.

This would be good to have, but I don't think your patch works.  It 
seems you need to add the srcdir and builddir command-line arguments to 
the invocations.




Re: [PATCH] Add `headerscheck` run_target to meson

От
Miłosz Bieniek
Дата:
pt., 28 lis 2025 o 12:53 Peter Eisentraut <peter@eisentraut.org> napisał(a):
>
> On 27.11.25 10:28, Miłosz Bieniek wrote:
> > While reviewing a patch I noticed that we have a `make -s headerscheck`
> > but there is no equivalent in meson.
> > I prepared a small patch that adds `headerscheck` and `cpluspluscheck`
> > targets.
>
> This would be good to have, but I don't think your patch works.  It
> seems you need to add the srcdir and builddir command-line arguments to
> the invocations.
>

I think you are right. I added srcdir and builddir arguments.

Вложения

Re: [PATCH] Add `headerscheck` run_target to meson

От
Nazir Bilal Yavuz
Дата:
Hi,

Thank you for working on this!

On Fri, 28 Nov 2025 at 17:03, Miłosz Bieniek <bieniek.milosz0@gmail.com> wrote:
>
> pt., 28 lis 2025 o 12:53 Peter Eisentraut <peter@eisentraut.org> napisał(a):
> >
> > On 27.11.25 10:28, Miłosz Bieniek wrote:
> > > While reviewing a patch I noticed that we have a `make -s headerscheck`
> > > but there is no equivalent in meson.
> > > I prepared a small patch that adds `headerscheck` and `cpluspluscheck`
> > > targets.
> >
> > This would be good to have, but I don't think your patch works.  It
> > seems you need to add the srcdir and builddir command-line arguments to
> > the invocations.
> >
>
> I think you are right. I added srcdir and builddir arguments.

The headerscheck script pulls some information from Makefile.global
after the configure [1] but meson does not generate a full version of
Makefile.global [2], so it does not have the required information to
check perl and python headers. If you run 'meson compile
headerscheck', you get errors like:

In file included from
/home/nbyavuz/Desktop/projects/postgres/src/pl/plperl/plperl.h:25,
                 from /tmp/headerscheck.YxsZhn/test.c:2:
/home/nbyavuz/Desktop/projects/postgres/src/pl/plperl/plperl_system.h:85:10:
fatal error: EXTERN.h: No such file or directory
   85 | #include <EXTERN.h>
      |          ^~~~~~~~~~
compilation terminated.

In file included from
/home/nbyavuz/Desktop/projects/postgres/src/pl/plpython/plpython.h:39,
                 from
/home/nbyavuz/Desktop/projects/postgres/src/pl/plpython/plpy_plpymodule.h:8,
                 from /tmp/headerscheck.YxsZhn/test.c:2:
/home/nbyavuz/Desktop/projects/postgres/src/pl/plpython/plpython_system.h:50:10:
fatal error: Python.h: No such file or directory
   50 | #include <Python.h>
      |          ^~~~~~~~~~
compilation terminated.

-----

[1.1] src/tools/pginclude/headerscheck
# Needs to be run after configuring and creating all generated headers.
# It's advisable to configure --with-perl --with-python, else you're
# likely to get errors from associated headers.

[1.2] src/tools/pginclude/headerscheck
# Pull some info from configure's results.
MGLOB="$builddir/src/Makefile.global"
CPPFLAGS=`sed -n 's/^CPPFLAGS[     ]*=[     ]*//p' "$MGLOB"`
CFLAGS=`sed -n 's/^CFLAGS[     ]*=[     ]*//p' "$MGLOB"`
ICU_CFLAGS=`sed -n 's/^ICU_CFLAGS[     ]*=[     ]*//p' "$MGLOB"`
CC=`sed -n 's/^CC[     ]*=[     ]*//p' "$MGLOB"`
CXX=`sed -n 's/^CXX[     ]*=[     ]*//p' "$MGLOB"`
PG_SYSROOT=`sed -n 's/^PG_SYSROOT[     ]*=[     ]*//p' "$MGLOB"`
perl_includespec=`sed -n 's/^perl_includespec[     ]*=[     ]*//p' "$MGLOB"`
python_includespec=`sed -n 's/^python_includespec[     ]*=[     ]*//p' "$MGLOB"`

[2] src/meson.build
makefile_global = configure_file(
  input: 'Makefile.global.in',
  output: 'Makefile.global',
  configuration: pgxs_cdata,
  install: true,
  install_dir: dir_pgxs / 'src',
)
configure_files += makefile_global

--
Regards,
Nazir Bilal Yavuz
Microsoft



Re: [PATCH] Add `headerscheck` run_target to meson

От
Nazir Bilal Yavuz
Дата:
Hi,

On Fri, 28 Nov 2025 at 18:05, Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:
>
> On Fri, 28 Nov 2025 at 17:03, Miłosz Bieniek <bieniek.milosz0@gmail.com> wrote:
> >
> > pt., 28 lis 2025 o 12:53 Peter Eisentraut <peter@eisentraut.org> napisał(a):
> > >
> > > On 27.11.25 10:28, Miłosz Bieniek wrote:
> > > > While reviewing a patch I noticed that we have a `make -s headerscheck`
> > > > but there is no equivalent in meson.
> > > > I prepared a small patch that adds `headerscheck` and `cpluspluscheck`
> > > > targets.
> > >
> > > This would be good to have, but I don't think your patch works.  It
> > > seems you need to add the srcdir and builddir command-line arguments to
> > > the invocations.
> > >
> >
> > I think you are right. I added srcdir and builddir arguments.
>
> The headerscheck script pulls some information from Makefile.global
> after the configure [1] but meson does not generate a full version of
> Makefile.global [2], so it does not have the required information to
> check perl and python headers. If you run 'meson compile
> headerscheck', you get errors like:

Sorry, I clicked send early.

Two solutions came to my mind but I am not sure which one is better:

1) We can add missing information to the generated Makefile.global in
the meson.build.

2) We can send required information as arguments to the headerscheck script.

Any thoughts or suggestions?

--
Regards,
Nazir Bilal Yavuz
Microsoft



Re: [PATCH] Add `headerscheck` run_target to meson

От
Miłosz Bieniek
Дата:
pt., 28 lis 2025 o 16:17 Nazir Bilal Yavuz <byavuz81@gmail.com> napisał(a):
>
> Hi,
>
> On Fri, 28 Nov 2025 at 18:05, Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:
> >
> > On Fri, 28 Nov 2025 at 17:03, Miłosz Bieniek <bieniek.milosz0@gmail.com> wrote:
> > >
> > > pt., 28 lis 2025 o 12:53 Peter Eisentraut <peter@eisentraut.org> napisał(a):
> > > >
> > > > On 27.11.25 10:28, Miłosz Bieniek wrote:
> > > > > While reviewing a patch I noticed that we have a `make -s headerscheck`
> > > > > but there is no equivalent in meson.
> > > > > I prepared a small patch that adds `headerscheck` and `cpluspluscheck`
> > > > > targets.
> > > >
> > > > This would be good to have, but I don't think your patch works.  It
> > > > seems you need to add the srcdir and builddir command-line arguments to
> > > > the invocations.
> > > >
> > >
> > > I think you are right. I added srcdir and builddir arguments.
> >
> > The headerscheck script pulls some information from Makefile.global
> > after the configure [1] but meson does not generate a full version of
> > Makefile.global [2], so it does not have the required information to
> > check perl and python headers. If you run 'meson compile
> > headerscheck', you get errors like:
>
> Sorry, I clicked send early.
>
> Two solutions came to my mind but I am not sure which one is better:
>
> 1) We can add missing information to the generated Makefile.global in
> the meson.build.
>
> 2) We can send required information as arguments to the headerscheck script.
>
> Any thoughts or suggestions?

Thank you for the detailed response.
I initially thought the errors with `#include <Python.h>` and
`#include <EXTREN.h>` were only an issue with my local setup.
If I understand correctly, your first proposal would address this
problem without requiring integration with the headerscheck script,
which in my opinion would be a cleaner solution.
However, I would definitely like to hear what others think as well.



Re: [PATCH] Add `headerscheck` run_target to meson

От
Bilal Yavuz
Дата:
Hi,

On Sat, 29 Nov 2025 at 14:07, Miłosz Bieniek <bieniek.milosz0@gmail.com> wrote:
>
> pt., 28 lis 2025 o 16:17 Nazir Bilal Yavuz <byavuz81@gmail.com> napisał(a):
> >
> > Hi,
> >
> > On Fri, 28 Nov 2025 at 18:05, Nazir Bilal Yavuz <byavuz81@gmail.com> wrote:
> > >
> > > On Fri, 28 Nov 2025 at 17:03, Miłosz Bieniek <bieniek.milosz0@gmail.com> wrote:
> > > >
> > > > pt., 28 lis 2025 o 12:53 Peter Eisentraut <peter@eisentraut.org> napisał(a):
> > > > >
> > > > > On 27.11.25 10:28, Miłosz Bieniek wrote:
> > > > > > While reviewing a patch I noticed that we have a `make -s headerscheck`
> > > > > > but there is no equivalent in meson.
> > > > > > I prepared a small patch that adds `headerscheck` and `cpluspluscheck`
> > > > > > targets.
> > > > >
> > > > > This would be good to have, but I don't think your patch works.  It
> > > > > seems you need to add the srcdir and builddir command-line arguments to
> > > > > the invocations.
> > > > >
> > > >
> > > > I think you are right. I added srcdir and builddir arguments.
> > >
> > > The headerscheck script pulls some information from Makefile.global
> > > after the configure [1] but meson does not generate a full version of
> > > Makefile.global [2], so it does not have the required information to
> > > check perl and python headers. If you run 'meson compile
> > > headerscheck', you get errors like:
> >
> > Sorry, I clicked send early.
> >
> > Two solutions came to my mind but I am not sure which one is better:
> >
> > 1) We can add missing information to the generated Makefile.global in
> > the meson.build.
> >
> > 2) We can send required information as arguments to the headerscheck script.
> >
> > Any thoughts or suggestions?
>
> Thank you for the detailed response.
> I initially thought the errors with `#include <Python.h>` and
> `#include <EXTREN.h>` were only an issue with my local setup.
> If I understand correctly, your first proposal would address this
> problem without requiring integration with the headerscheck script,
> which in my opinion would be a cleaner solution.
> However, I would definitely like to hear what others think as well.

I wanted to experiment with the first proposal and it turns out I need
to edit the headerscheck script.

There are 3 patches attached:

0001 adds python_includespec and perl_includespec variables to the
Makefile.global of the meson build.

0002 adds headerscheck target to meson build like you do but with 2
extra changes. First one is that, I moved the headerscheck script to a
variable and used it in the target commands. Second one is that,
headerscheck script could not find the perl_includespec and
python_includespec variables because of the tabs in the sed command, I
changed them with '[:space:]' and it worked. I am not sure if that is
the correct fix but I just wanted to see if the script will work.

0003 adds icu_flags option to the meson build and sets 'ICU_CFLAGS'
Makefile.global variable to that option. This change is not needed for
the headerscheck script to work but I saw that was missing and just
wanted to show it. If we want to add that, this probably needs its own
thread.

--
Regards,
Nazir Bilal Yavuz
Microsoft

Вложения

Re: [PATCH] Add `headerscheck` run_target to meson

От
Peter Eisentraut
Дата:
On 04.12.25 08:09, Bilal Yavuz wrote:
> 0001 adds python_includespec and perl_includespec variables to the
> Makefile.global of the meson build.

Committed that, with some light cosmetic ordering changes.

> 0002 adds headerscheck target to meson build like you do but with 2
> extra changes. First one is that, I moved the headerscheck script to a
> variable and used it in the target commands. Second one is that,
> headerscheck script could not find the perl_includespec and
> python_includespec variables because of the tabs in the sed command, I
> changed them with '[:space:]' and it worked. I am not sure if that is
> the correct fix but I just wanted to see if the script will work.

Committed that, but without the regular expression changes.  I don't 
understand why those would be needed.  Please provide more information 
if necessary.  It worked for me without it.

> 0003 adds icu_flags option to the meson build and sets 'ICU_CFLAGS'
> Makefile.global variable to that option. This change is not needed for
> the headerscheck script to work but I saw that was missing and just
> wanted to show it. If we want to add that, this probably needs its own
> thread.

I don't think we want that.  We want to encourage the use of pkg-config, 
not the previous legacy options.




Re: [PATCH] Add `headerscheck` run_target to meson

От
Peter Eisentraut
Дата:
On 18.03.26 11:56, Peter Eisentraut wrote:
> On 04.12.25 08:09, Bilal Yavuz wrote:
>> 0001 adds python_includespec and perl_includespec variables to the
>> Makefile.global of the meson build.
> 
> Committed that, with some light cosmetic ordering changes.
> 
>> 0002 adds headerscheck target to meson build like you do but with 2
>> extra changes. First one is that, I moved the headerscheck script to a
>> variable and used it in the target commands. Second one is that,
>> headerscheck script could not find the perl_includespec and
>> python_includespec variables because of the tabs in the sed command, I
>> changed them with '[:space:]' and it worked. I am not sure if that is
>> the correct fix but I just wanted to see if the script will work.
> 
> Committed that, but without the regular expression changes.  I don't 
> understand why those would be needed.  Please provide more information 
> if necessary.  It worked for me without it.

I need another tweak for the cpluspluscheck.

In meson, the distribution of the include flags between CPPFLAGS, 
CFLAGS, and CXXFLAGS ends up being a bit different, and we need to get a 
few -I options from CXXFLAGS in my case.

I have attached two different variants for how to do this.  The first 
one just gets the -D and -I flags from CXXFLAGS.  This preserves most of 
the existing behavior.  The second aligns more with how CFLAGS works and 
removes the ability to override CXXFLAGS from the command line.  This is 
probably now better since we have more support for getting correct CXX 
and CXXFLAGS in the build system.  But it might be a change for some users.

Thoughts?

Вложения

Re: [PATCH] Add `headerscheck` run_target to meson

От
Nazir Bilal Yavuz
Дата:
Hi,

On Wed, 18 Mar 2026 at 14:59, Peter Eisentraut <peter@eisentraut.org> wrote:
>
> On 18.03.26 11:56, Peter Eisentraut wrote:
> >
> > Committed that, but without the regular expression changes.  I don't
> > understand why those would be needed.  Please provide more information
> > if necessary.  It worked for me without it.

It is working now. I can't reproduce the problem, it looks like it is
already fixed.


> I need another tweak for the cpluspluscheck.
>
> In meson, the distribution of the include flags between CPPFLAGS,
> CFLAGS, and CXXFLAGS ends up being a bit different, and we need to get a
> few -I options from CXXFLAGS in my case.
>
> I have attached two different variants for how to do this.  The first
> one just gets the -D and -I flags from CXXFLAGS.  This preserves most of
> the existing behavior.  The second aligns more with how CFLAGS works and
> removes the ability to override CXXFLAGS from the command line.  This is
> probably now better since we have more support for getting correct CXX
> and CXXFLAGS in the build system.  But it might be a change for some users.
>
> Thoughts?

I think we can go with the second variant first. If there is a
complaint, we can then consider switching to the first one. Does that
sound good? Also, second patch probably needs to remove lines below
from the src/tools/pginclude/README:

```
If you are using a non-g++-compatible C++ compiler, you may need to
override the script's CXXFLAGS setting by setting a suitable environment
value.
```


--
Regards,
Nazir Bilal Yavuz
Microsoft



Re: [PATCH] Add `headerscheck` run_target to meson

От
Peter Eisentraut
Дата:
On 18.03.26 16:36, Nazir Bilal Yavuz wrote:
> Hi,
> 
> On Wed, 18 Mar 2026 at 14:59, Peter Eisentraut <peter@eisentraut.org> wrote:
>>
>> On 18.03.26 11:56, Peter Eisentraut wrote:
>>>
>>> Committed that, but without the regular expression changes.  I don't
>>> understand why those would be needed.  Please provide more information
>>> if necessary.  It worked for me without it.
> 
> It is working now. I can't reproduce the problem, it looks like it is
> already fixed.
> 
> 
>> I need another tweak for the cpluspluscheck.
>>
>> In meson, the distribution of the include flags between CPPFLAGS,
>> CFLAGS, and CXXFLAGS ends up being a bit different, and we need to get a
>> few -I options from CXXFLAGS in my case.
>>
>> I have attached two different variants for how to do this.  The first
>> one just gets the -D and -I flags from CXXFLAGS.  This preserves most of
>> the existing behavior.  The second aligns more with how CFLAGS works and
>> removes the ability to override CXXFLAGS from the command line.  This is
>> probably now better since we have more support for getting correct CXX
>> and CXXFLAGS in the build system.  But it might be a change for some users.
>>
>> Thoughts?
> 
> I think we can go with the second variant first. If there is a
> complaint, we can then consider switching to the first one. Does that
> sound good? Also, second patch probably needs to remove lines below
> from the src/tools/pginclude/README:
> 
> ```
> If you are using a non-g++-compatible C++ compiler, you may need to
> override the script's CXXFLAGS setting by setting a suitable environment
> value.
> ```

Ok, done that way.