[meson] expose buildtype debug/optimization info to pg_config
build system using configure set VAL_CFLAGS with debug and
optimization flags, so pg_config will show these infos. Some
extensions depend on the mechanism.
This patch exposes these flags with a typo fixed together.
--
Regards
Junwang Zhao
Attachments:
0001-meson-expose-buildtype-debug-optimization-info-to-pg.patchapplication/octet-stream; name=0001-meson-expose-buildtype-debug-optimization-info-to-pg.patchDownload
From 08c74ad8191e533415b9ac9e532ebc63e9736fb2 Mon Sep 17 00:00:00 2001
From: Zhao Junwang <zhjwpku@gmail.com>
Date: Tue, 12 Dec 2023 18:02:29 +0800
Subject: [PATCH] [meson] expose buildtype debug/optimization info to pg_config
build system using configure set VAL_CFLAGS with debug and
optimization flags, so pg_config will show these infos. Some
extensions depend on the mechanism.
This patch exposes these flags.
Signed-off-by: Zhao Junwang <zhjwpku@gmail.com>
---
meson.build | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/meson.build b/meson.build
index 52c2a37c41..b44f0e4587 100644
--- a/meson.build
+++ b/meson.build
@@ -115,6 +115,20 @@ cdata = configuration_data()
+###############################################################
+# Build type
+###############################################################
+build_type = get_option('buildtype')
+if build_type == 'debug'
+ cflags += ['-g', '-O0']
+elif build_type == 'debugoptimized'
+ cflags += ['-g', '-O2']
+elif build_type == 'release'
+ cflags += ['-O3']
+endif
+
+
+
###############################################################
# Version and other metadata
###############################################################
@@ -1139,7 +1153,7 @@ if not get_option('readline').disabled()
if not at_least_one_header_found
error('''readline header not found
-If you have @0@ already installed, see meson-log/meson-log.txt for details on the
+If you have @0@ already installed, see meson-logs/meson-log.txt for details on the
failure. It is possible the compiler isn't looking in the proper directory.
Use -Dreadline=disabled to disable readline support.'''.format(readline_dep))
endif
--
2.41.0
On 12.12.23 11:40, Junwang Zhao wrote:
build system using configure set VAL_CFLAGS with debug and
optimization flags, so pg_config will show these infos. Some
extensions depend on the mechanism.This patch exposes these flags with a typo fixed together.
I have committed the typo fix.
But I would like to learn more about the requirements of extensions in
this area. This seems a bit suspicious to me.
Hi Peter,
Thanks for looking into this.
On Thu, Dec 14, 2023 at 4:50 PM Peter Eisentraut <peter@eisentraut.org> wrote:
On 12.12.23 11:40, Junwang Zhao wrote:
build system using configure set VAL_CFLAGS with debug and
optimization flags, so pg_config will show these infos. Some
extensions depend on the mechanism.This patch exposes these flags with a typo fixed together.
I have committed the typo fix.
But I would like to learn more about the requirements of extensions in
this area. This seems a bit suspicious to me.
This is what I found when building citus against an installation
of meson debug build pg instance, since the CFLAGS doesn't
contain -g flag, the binary doesn't include the debug information,
which is different behavior from configure building system.
Another issue I found is that some C++
extensions(ajust/parquet_fdw for example) don't build against
the meson generated pgxs.mk, since it doesn't set the CXX
command. CXX is only set when llvm option is enabled, which
is different from old building system.
I don't insist we make Meson the same behaviour with old building
system, I just think the issues I raised might stop developers try
the fancy new building system. And the fix I post might not be
ideal, you and Andres might have better solutions.
--
Regards
Junwang Zhao
Hi,
On 2023-12-14 17:24:58 +0800, Junwang Zhao wrote:
On Thu, Dec 14, 2023 at 4:50 PM Peter Eisentraut <peter@eisentraut.org> wrote:
On 12.12.23 11:40, Junwang Zhao wrote:
build system using configure set VAL_CFLAGS with debug and
optimization flags, so pg_config will show these infos. Some
extensions depend on the mechanism.This patch exposes these flags with a typo fixed together.
I have committed the typo fix.
But I would like to learn more about the requirements of extensions in
this area. This seems a bit suspicious to me.This is what I found when building citus against an installation
of meson debug build pg instance, since the CFLAGS doesn't
contain -g flag, the binary doesn't include the debug information,
which is different behavior from configure building system.
Hm. I'm not sure it's the right call to make extensions build the same way as
the main postgres install with regard to optimization and debug info. So I
feel a bit hesitant around generating -g and particularly -Ox. But it's
historically what we've done...
If we want to do so, I think this should not check buildtype, but debug.
Another issue I found is that some C++
extensions(ajust/parquet_fdw for example) don't build against
the meson generated pgxs.mk, since it doesn't set the CXX
command. CXX is only set when llvm option is enabled, which
is different from old building system.
I wanted to skip the C++ tests when we don't need C++, because it makes
configure take longer. But I could be convinced that we should always at least
determine the C++ compiler for Makefile.global.
Greetings,
Andres Freund
Hi,
On Fri, Dec 15, 2023 at 10:20 PM Andres Freund <andres@anarazel.de> wrote:
Hi,
On 2023-12-14 17:24:58 +0800, Junwang Zhao wrote:
On Thu, Dec 14, 2023 at 4:50 PM Peter Eisentraut <peter@eisentraut.org> wrote:
On 12.12.23 11:40, Junwang Zhao wrote:
build system using configure set VAL_CFLAGS with debug and
optimization flags, so pg_config will show these infos. Some
extensions depend on the mechanism.This patch exposes these flags with a typo fixed together.
I have committed the typo fix.
But I would like to learn more about the requirements of extensions in
this area. This seems a bit suspicious to me.This is what I found when building citus against an installation
of meson debug build pg instance, since the CFLAGS doesn't
contain -g flag, the binary doesn't include the debug information,
which is different behavior from configure building system.Hm. I'm not sure it's the right call to make extensions build the same way as
the main postgres install with regard to optimization and debug info. So I
feel a bit hesitant around generating -g and particularly -Ox. But it's
historically what we've done...If we want to do so, I think this should not check buildtype, but debug.
I'm confused which *debug* do you mean, can you be more specific?
Another issue I found is that some C++
extensions(ajust/parquet_fdw for example) don't build against
the meson generated pgxs.mk, since it doesn't set the CXX
command. CXX is only set when llvm option is enabled, which
is different from old building system.I wanted to skip the C++ tests when we don't need C++, because it makes
configure take longer. But I could be convinced that we should always at least
determine the C++ compiler for Makefile.global.
The first idea that came to my mind is using the *project* command
to set [`c`, `cpp`], but this might be a little bit confusing for somebody.
Then I tried another way by adding a 'pgxscpp' option to let the user
choose whether he will set the C++ compiler for Makefile.global.
It works but may not be an ideal way, see the attached.
Greetings,
Andres Freund
--
Regards
Junwang Zhao
Attachments:
0001-PGXS-determine-C-compiler-for-Makefile.global.patchapplication/octet-stream; name=0001-PGXS-determine-C-compiler-for-Makefile.global.patchDownload
From 6078ec2961f6099d05995b22d3397572834b3465 Mon Sep 17 00:00:00 2001
From: Zhao Junwang <zhjwpku@gmail.com>
Date: Fri, 22 Dec 2023 17:16:04 +0800
Subject: [PATCH] PGXS determine C++ compiler for Makefile.global
Signed-off-by: Zhao Junwang <zhjwpku@gmail.com>
---
meson_options.txt | 3 +++
src/makefiles/meson.build | 12 +++++++++++-
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/meson_options.txt b/meson_options.txt
index be1b327f54..02c0e48a46 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -112,6 +112,9 @@ option('libxslt', type: 'feature', value: 'auto',
option('llvm', type: 'feature', value: 'disabled',
description: 'LLVM support')
+option('pgxscpp', type: 'feature', value: 'disabled',
+ description: 'PGXS determine C++ compiler for Makefile.global')
+
option('lz4', type: 'feature', value: 'auto',
description: 'LZ4 support')
diff --git a/src/makefiles/meson.build b/src/makefiles/meson.build
index be946f7b38..979a0dd8c4 100644
--- a/src/makefiles/meson.build
+++ b/src/makefiles/meson.build
@@ -127,9 +127,19 @@ if llvm.found()
else
pgxs_kv += {
'CLANG': '',
- 'CXX': '',
'LLVM_BINPATH': '',
}
+ pgxscppopt = get_option('pgxscpp')
+ if add_languages('cpp', required: pgxscppopt, native: false)
+ cpp = meson.get_compiler('cpp')
+ pgxs_kv += {
+ 'CXX': ' '.join(cpp.cmd_array()),
+ }
+ else
+ pgxs_kv += {
+ 'CXX': '',
+ }
+ endif
endif
pgxs_bins = {
--
2.41.0
On 14.12.23 10:24, Junwang Zhao wrote:
On Thu, Dec 14, 2023 at 4:50 PM Peter Eisentraut <peter@eisentraut.org> wrote:
On 12.12.23 11:40, Junwang Zhao wrote:
build system using configure set VAL_CFLAGS with debug and
optimization flags, so pg_config will show these infos. Some
extensions depend on the mechanism.This patch exposes these flags with a typo fixed together.
I have committed the typo fix.
But I would like to learn more about the requirements of extensions in
this area. This seems a bit suspicious to me.This is what I found when building citus against an installation
of meson debug build pg instance, since the CFLAGS doesn't
contain -g flag, the binary doesn't include the debug information,
which is different behavior from configure building system.
Ok, that makes sense.
I think a better place to add those options would the variable
var_cflags, which are the combined C flags that we export to
Makefile.global and pg_config. The cflags variable that you used is
more for internal use, for passing to the actual compilation commands,
so adding more options there would be duplicative.
And then set var_cxxflags as well.
Maybe you should also check whether the compiler takes unix-style
arguments, perhaps using cc.get_argument_syntax().