pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedmeson vs mingw, plpython, readline and other fun
3+ messages / 1 participants
[nested] [flat]
Commits from this thread:
2022-10-01 24ef127b6d meson: mingw: Add -Wl,--disable-auto-import, enable when linking with readline
2022-09-30 0e497eadb1 mingw: Define PGDLLEXPORT as __declspec (dllexport) as done for msvc
2022-09-29 a1b3bca1c8 meson: Implement getopt logic from autoconf
2022-09-29 caeeabd110 meson: mingw: Allow multiple definitions
2022-09-29 dfefa0e464 meson: pg_regress: Define a HOST_TUPLE sufficient to make resultmap work
2022-09-29 e3a892539b meson: windows: Normalize slashes in prefix
* meson vs mingw, plpython, readline and other fun
@ 2022-09-28 02:27 Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 3+ messages in thread
From: Andres Freund @ 2022-09-28 02:27 UTC (permalink / raw)
To: pgsql-hackers@postgresql.org, Melih Mutlu <m.melihmutlu@gmail.com>; Tom Lane <tgl@sss.pgh.pa.us>; Andrew Dunstan <andrew@dunslane.net>
Hi,
Melih mentioned on IM that while he could build postgres with meson on windows
w/ mingw, the tests didn't run.
Issues:
- The bit computing PATH to the temporary installation for running tests only
dealt with backward slashes in paths on windows, because that's what
python.org python uses by default. But a msys ucrt python returns forward
slashes. Trivial fix.
I didn't encounter this because I'd used a meson from git, which thus didn't
have msys's patch to return a different prefix.
This made pg_regress/isolationtester tests other than the main regression
tests pass.
- I'd only passed in a fake HOST_TUPLE when building pg_regress, oops.
I don't think it makes sense to come up with a config.guess compatible name
- they're quite random. And we can't rely on shell to work when targetting
msvc. The attached patch does:
# 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)
which I don't perfectly like (e.g. clang also kind of works on windows), but
it seems ok enough for now. I suspect we'd need a bunch of other changes to
make clang on windows work.
This made the main pg_regress tests pass.
- I had not added the logic to not use existing getopt on mingw, causing tap
tests to fail. Fixing that didn't immediately work because of duplicate
symbols - because I hadn't copied over -Wl,--allow-multiple-definition.
"Contrary" to the comment in src/template/win32 it doesn't appear to be
needed for libpq and pgport - likely because for the meson build an export
file is generated (needed for msvc anyway, I didn't think to disable it for
mingw). But since we need to be able to override getopt(), we obviously
need --allow-multiple-definition anyway.
- This lead me to try to also add -Wl,--disable-auto-import. However, that
caused two problems.
1) plpython tests started to fail, due to not finding Pg_magic_func in
plpython3.dll. This confounded me for quite a while. It worked for every
other extension .dll? A lot of looking around lead me to define
#define PGDLLEXPORT __declspec (dllexport)
for mingw as well. For mingw we otherwise end up with
#define PGDLLEXPORT __attribute__((visibility("default")))
which works.
As far as I can tell __attribute__((visibility("default"))) works as long as
as there's no declspec(dllexport) symbol in the same dll. If there is, it
stops working. Ugh.
I don't see a reason not to define PGDLLEXPORT as __declspec(dllexport)
for mingw as well?
I suspect this is an issue for autoconf mingw build as well, but that
fails to even configure - there's no coverage on the BF for it I think.
This made plpython's test pass (again).
2) psql failed to build due to readline. I hadn't implemented disabling it
automatically. Somewhat luckily - turns out it actually works (as long as
--disable-auto-import isn't used), including autocomplete!
The issue afaict is that while readline has an import library, functions
aren't "annotated" with __declspec(dllimport), thus without
--enable-auto-import the references are assumed to be local, and thus
linking fails.
It's possible we could "fix" this by declaring the relevant symbols
ourselves or such. But for now just adding --enable-auto-import to the
flags used to link to readline seems saner?
I think it'd be very cool to finally have a working readline on windows.
Unfortunately IO::Pty isn't installable on windows, it'd have been
interesting to see how well that readline works.
- Before I updated mingw, interactive psql didn't show a prompt, making me
think something was broken. That turned out to be the same in an autoconf
build. When inside the msys terminal (mintty) isatty returned 0, because of
some detail of how it emulates ttys. After updating mingw that problem is
gone.
I included this partially so I have something to find in email next time I
search for mintty and isatty :)
With these things fixed, postgres built and ran tests successfully! With
nearly all "relevant" dependencies:
icu, libxml, libslt, lz4, nls, plperl, plpython, pltcl, readline, ssl, zlib,
zstd
Missing are gss and uuid. Both apparently work on windows, but they're not in
the msys repository, and I don't feel like compiling them myself.
Only 5 tests skipped:
- recovery/017_shm - probably not applicable
- recovery/022_crash_temp_files - I don't think it's ok that this test skips,
but that's for another thread
- psql/010_tab_completion - IO::Pty can't be installed
- psql/010_cancel - IO::Pty can't be installed
- ldap/001_auth - doesn't know how to find slapd on windows
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.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: meson vs mingw, plpython, readline and other fun
@ 2022-09-28 19:33 Andres Freund <andres@anarazel.de>
parent: Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 3+ messages in thread
From: Andres Freund @ 2022-09-28 19:33 UTC (permalink / raw)
To: pgsql-hackers@postgresql.org, Melih Mutlu <m.melihmutlu@gmail.com>; Tom Lane <tgl@sss.pgh.pa.us>; Andrew Dunstan <andrew@dunslane.net>
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
Attachments: download all (6 files, .tar.gz)
v1-0001-meson-windows-Normalize-slashes-in-prefix.patch (text/x-diff, 1.3K) download
From 3bd663d454c05d38b629a9807b89bcb332ccae1a Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@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.melihmutlu@gmail.com>
Discussion: http://postgr.es/m/20220928022724.erzuk5v4ai4b53do@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
v1-0002-meson-pg_regress-Define-a-HOST_TUPLE-sufficient-t.patch (text/x-diff, 1.7K) download
From 386bf996d7e0e19a3aa3ee2239b7baf35b2ea3a2 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@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.erzuk5v4ai4b53do@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
v1-0003-meson-mingw-Allow-multiple-definitions.patch (text/x-diff, 1.1K) download
From 69ffcb9017bceab1a431a0053b517e0a55d47e00 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@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.erzuk5v4ai4b53do@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
v1-0004-meson-Implement-getopt-logic-from-autoconf.patch (text/x-diff, 2.0K) download
From 5babfb6dfe9189ce89145d6e19f9144894c77794 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@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
v1-0005-mingw-Define-PGDLLEXPORT-as-__declspec-dllexport-.patch (text/x-diff, 1.5K) download
From 8607281424d49727813ec902394fe4454ffa2755 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@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.erzuk5v4ai4b53do@awork3.anarazel.de
Discussion: http://postgr.es/m/20220928025242.ugf7t5ugxxgmkraa@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
v1-0006-meson-mingw-Add-Wl-disable-auto-import-enable-whe.patch (text/x-diff, 2.0K) download
From f1dbfedf76ad191212b664f558cbadbfd9805763 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@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.erzuk5v4ai4b53do@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
* Re: meson vs mingw, plpython, readline and other fun
@ 2022-09-30 04:21 Andres Freund <andres@anarazel.de>
parent: Andres Freund <andres@anarazel.de>
0 siblings, 0 replies; 3+ messages in thread
From: Andres Freund @ 2022-09-30 04:21 UTC (permalink / raw)
To: pgsql-hackers@postgresql.org, Melih Mutlu <m.melihmutlu@gmail.com>; Tom Lane <tgl@sss.pgh.pa.us>; Andrew Dunstan <andrew@dunslane.net>
Hi,
On 2022-09-28 12:33:17 -0700, Andres Freund wrote:
> 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.
I have pushed 1-4, was holding out for opinions on 5.
I'm planning to push 0005 (and 0006) soon, to allow the mingw CI patch to
progress. So if somebody has concerns defining PGDLLEXPORT to __declspec
(dllexport) for mingw (already the case for msvc)...
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2022-09-30 04:21 UTC | newest]
Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2022-09-28 02:27 meson vs mingw, plpython, readline and other fun Andres Freund <andres@anarazel.de>
2022-09-28 19:33 ` Andres Freund <andres@anarazel.de>
2022-09-30 04:21 ` Andres Freund <andres@anarazel.de>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox