Hi,
On 2022-09-27 19:27:24 -0700, Andres Freund wrote:
> Stared too long at the screen to figure all of this out. Food next. I'll clean
> the patches up later tonight or tomorrow morning.
Attached:
0001 - meson: windows: Normalize slashes in prefix
0002 - meson: pg_regress: Define a HOST_TUPLE sufficient to make resultmap work
0003 - meson: mingw: Allow multiple definitions
0004 - meson: Implement getopt logic from autoconf
0005 - mingw: Define PGDLLEXPORT as __declspec (dllexport) as done for msvc
0006 - meson: mingw: Add -Wl,--disable-auto-import, enable when linking with
readline
0005 is the one that I'd most like review for. The rest just affect meson, so
I'm planning to push them fairly soon - review would nevertheless be nice.
Greetings,
Andres Freund
>From 3bd663d454c05d38b629a9807b89bcb332ccae1a Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Tue, 27 Sep 2022 11:55:00 -0700
Subject: [PATCH v1 1/6] meson: windows: Normalize slashes in prefix
This fixes a build issue on windows, when the prefix is set to a path with
forward slashes. Windows python defaults to a path with a backslash, but mingw
ucrt python defaults to a forward slash. This in turn lead to a wrong PATH set
during tests, causing tests to fail.
Reported-by: Melih Mutlu <m.melihmu...@gmail.com>
Discussion: http://postgr.es/m/20220928022724.erzuk5v4ai4b5...@awork3.anarazel.de
---
meson.build | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/meson.build b/meson.build
index 38b2c3aae2e..02c086c04e7 100644
--- a/meson.build
+++ b/meson.build
@@ -2733,15 +2733,15 @@ endif
prefix = get_option('prefix')
-test_prefix = prefix
+test_prefix = fs.as_posix(prefix)
if fs.is_absolute(get_option('prefix'))
if host_system == 'windows'
- if prefix.split(':\\').length() == 1
+ if prefix.split(':/').length() == 1
# just a drive
test_prefix = ''
else
- test_prefix = prefix.split(':\\')[1]
+ test_prefix = prefix.split(':/')[1]
endif
else
assert(prefix.startswith('/'))
--
2.37.3.542.gdd3f6c4cae
>From 386bf996d7e0e19a3aa3ee2239b7baf35b2ea3a2 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Tue, 27 Sep 2022 12:01:35 -0700
Subject: [PATCH v1 2/6] meson: pg_regress: Define a HOST_TUPLE sufficient to
make resultmap work
This doesn't end up with a triple that's exactly the same as config.guess -
it'd be hard to achieve that and it doesn't seem required. We can't rely on
config.guess as we don't necessarily have a /bin/sh on windows, e.g., when
building on windows with msvc.
This isn't perfect, e.g., clang works on windows as well. But I suspect we'd
need a bunch of other changes to make clang on windows work, and we haven't
supported it historically.
Discussion: http://postgr.es/m/20220928022724.erzuk5v4ai4b5...@awork3.anarazel.de
---
src/test/regress/meson.build | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/src/test/regress/meson.build b/src/test/regress/meson.build
index fd8ee995b79..03de591b0c7 100644
--- a/src/test/regress/meson.build
+++ b/src/test/regress/meson.build
@@ -6,7 +6,16 @@ regress_sources = pg_regress_c + files(
'pg_regress_main.c'
)
-pg_regress_cflags = ['-DHOST_TUPLE="frak"', '-DSHELLPROG="/bin/sh"']
+# Need make up something roughly like x86_64-pc-mingw64. resultmap matches on
+# patterns like ".*-.*-mingw.*". We probably can do better, but for now just
+# replace 'gcc' with 'mingw' on windows.
+host_tuple_cc = cc.get_id()
+if host_system == 'windows' and host_tuple_cc == 'gcc'
+ host_tuple_cc = 'mingw'
+endif
+host_tuple = '@0@-@1@-@2@'.format(host_cpu, host_system, host_tuple_cc)
+
+pg_regress_cflags = ['-DHOST_TUPLE="@0@"'.format(host_tuple), '-DSHELLPROG="/bin/sh"']
pg_regress = executable('pg_regress',
regress_sources,
--
2.37.3.542.gdd3f6c4cae
>From 69ffcb9017bceab1a431a0053b517e0a55d47e00 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Wed, 28 Sep 2022 09:48:09 -0700
Subject: [PATCH v1 3/6] meson: mingw: Allow multiple definitions
I didn't carry this forward from the win32 template. It's not needed anymore
for the reason stated therein, but it turns out to be required to
e.g. override getopt. Possibly a better solution exists, but that's for later.
Discussion: http://postgr.es/m/20220928022724.erzuk5v4ai4b5...@awork3.anarazel.de
---
meson.build | 2 ++
1 file changed, 2 insertions(+)
diff --git a/meson.build b/meson.build
index 02c086c04e7..cd410319f3f 100644
--- a/meson.build
+++ b/meson.build
@@ -277,6 +277,8 @@ elif host_system == 'windows'
# ldflags += '/nxcompat' # generated by msbuild, should have it for ninja?
else
ldflags += '-Wl,--stack,@0@'.format(cdata.get('WIN32_STACK_RLIMIT'))
+ # Need to allow multiple definitions, we e.g. want to override getopt.
+ ldflags += '-Wl,--allow-multiple-definition'
endif
os_deps += cc.find_library('ws2_32', required: true)
--
2.37.3.542.gdd3f6c4cae
>From 5babfb6dfe9189ce89145d6e19f9144894c77794 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Wed, 28 Sep 2022 10:06:32 -0700
Subject: [PATCH v1 4/6] meson: Implement getopt logic from autoconf
Not replacing getopt/getopt_long definitely causes issues on mingw. It's not
as clear whether the solaris & openbsd aspect is still needed, but if not, we
should remove it from both autoconf and meson.
---
meson.build | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/meson.build b/meson.build
index cd410319f3f..48b0b4cbed8 100644
--- a/meson.build
+++ b/meson.build
@@ -2265,6 +2265,15 @@ posix4_dep = cc.find_library('posix4', required: false)
getopt_dep = cc.find_library('getopt', required: false)
gnugetopt_dep = cc.find_library('gnugetopt', required: false)
+# Check if we want to replace getopt/getopt_long even if provided by the system
+# - We want to use system's getopt_long() only if system provides struct
+# option
+# - Mingw has adopted a GNU-centric interpretation of optind/optreset,
+# so always use our version on Windows
+# - On OpenBSD and Solaris, getopt() doesn't do what we want for long options
+# (i.e., allow '-' as a flag character), so use our version on those platforms
+always_replace_getopt = host_system in ['windows', 'openbsd', 'solaris']
+always_replace_getopt_long = cdata.get('HAVE_STRUCT_OPTION', 1) != 1 or host_system == 'windows'
# Required on BSDs
execinfo_dep = cc.find_library('execinfo', required: false)
@@ -2295,8 +2304,8 @@ func_checks = [
['explicit_bzero'],
['fdatasync', {'dependencies': [rt_dep, posix4_dep], 'define': false}], # Solaris
['getifaddrs'],
- ['getopt', {'dependencies': [getopt_dep, gnugetopt_dep]}],
- ['getopt_long', {'dependencies': [getopt_dep, gnugetopt_dep]}],
+ ['getopt', {'dependencies': [getopt_dep, gnugetopt_dep], 'skip': always_replace_getopt}],
+ ['getopt_long', {'dependencies': [getopt_dep, gnugetopt_dep], 'skip': always_replace_getopt_long}],
['getpeereid'],
['getpeerucred'],
['inet_aton'],
--
2.37.3.542.gdd3f6c4cae
>From 8607281424d49727813ec902394fe4454ffa2755 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Wed, 28 Sep 2022 10:16:49 -0700
Subject: [PATCH v1 5/6] mingw: Define PGDLLEXPORT as __declspec (dllexport) as
done for msvc
While mingw would otherwise fall back to
__attribute__((visibility("default"))), that appears to only work as long as
no symbols are declared with __declspec(dllexport). But we can end up with
some, e.g. plpython's Py_Init.
It's quite possible we should do the same for cygwin, but I don't have a test
environment for that...
Discussion: http://postgr.es/m/20220928022724.erzuk5v4ai4b5...@awork3.anarazel.de
Discussion: http://postgr.es/m/20220928025242.ugf7t5ugxxgmk...@awork3.anarazel.de
---
src/include/port/win32.h | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/src/include/port/win32.h b/src/include/port/win32.h
index 67755aadc40..d6c13d0bb8f 100644
--- a/src/include/port/win32.h
+++ b/src/include/port/win32.h
@@ -49,9 +49,11 @@
#endif
/*
- * Under MSVC, functions exported by a loadable module must be marked
- * "dllexport". Other compilers don't need that.
+ * Functions exported by a loadable module must be marked "dllexport".
+ *
+ * While mingw would otherwise fall back to
+ * __attribute__((visibility("default"))), that appears to only work as long
+ * as no symbols are declared with __declspec(dllexport). But we can end up
+ * with some, e.g. plpython's Py_Init.
*/
-#ifdef _MSC_VER
#define PGDLLEXPORT __declspec (dllexport)
-#endif
--
2.37.3.542.gdd3f6c4cae
>From f1dbfedf76ad191212b664f558cbadbfd9805763 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Wed, 28 Sep 2022 10:19:00 -0700
Subject: [PATCH v1 6/6] meson: mingw: Add -Wl,--disable-auto-import, enable
when linking with readline
I hadn't ported -Wl,--disable-auto-import over from the win32 template as I
had focused on msvc for windows. The flag is desirable as it makes it easier
to find problems one would have with msvc, particularly useful during cross
compilation.
This turned out to be a somewhat happy accident, as it allowed me to realize
that readline actually works on windows these days, as long as auto imports to
enable. Therefore enable auto-import again as part of linking to readline.
We perhaps can come up with a better solution for the readline issue, but this
seems good enough for now.
Discussion: http://postgr.es/m/20220928022724.erzuk5v4ai4b5...@awork3.anarazel.de
---
meson.build | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/meson.build b/meson.build
index 48b0b4cbed8..b3f543761f1 100644
--- a/meson.build
+++ b/meson.build
@@ -279,6 +279,7 @@ elif host_system == 'windows'
ldflags += '-Wl,--stack,@0@'.format(cdata.get('WIN32_STACK_RLIMIT'))
# Need to allow multiple definitions, we e.g. want to override getopt.
ldflags += '-Wl,--allow-multiple-definition'
+ ldflags += '-Wl,--disable-auto-import'
endif
os_deps += cc.find_library('ws2_32', required: true)
@@ -1080,6 +1081,13 @@ Use -Dreadline=false to disable readline support.'''.format(readline_dep))
readline = declare_dependency(dependencies: readline,
include_directories: postgres_inc)
endif
+
+ # On windows with mingw readline requires auto-import to successfully
+ # link, as the headers don't use declspec(dllimport)
+ if host_system == 'windows' and cc.get_id() != 'msvc'
+ readline = declare_dependency(dependencies: readline,
+ link_args: '-Wl,--enable-auto-import')
+ endif
endif
# XXX: Figure out whether to implement mingw warning equivalent
--
2.37.3.542.gdd3f6c4cae
