Mingw task for Cirrus CI

Hi,

Andrew, CCIng you both because you might be interested in the CI bit, and
because you might know the answer.

On 2022-02-25 19:44:27 +0300, Melih Mutlu wrote:

I've been working on adding Windows+MinGW environment into cirrus-ci tasks
(discussion about ci is here [1]).
It uses MSYS2 to set the environment. UCRT is chosen as C standard library,
instead of MSVCRT.
The task configures postgres with features that are available in MSYS2 (see
available packages [2]) and tap tests are enabled.
I already added the necessary docker image, you can find the related PR at
[3] and a successful cirruc-ci run with these changes at [4].
Attached patch adds a task runs on Windows with MinGW for cirrus-ci

However, I cannot run configure with --with-python, --with-perl or
--with-tcl.
There are two issues I encountered while trying to enable. e.g. for
--with-python

1- python_ldlibrary is set to "libpython3.9.dll.a". So the related line in
configure cannot retrieve "ldlibrary"

This presumably is due to using mingw's python rather than python.org's
python. Seems like a reasonable thing to support for the mingw build.

Melih, you could try to build against the python.org python (i installed in
the CI container).

2- src/pl/plpython/Makefile looks under "C:/Windows/system32/" for
PYTHONDLL. gendef cannot open that file, give an error like " failed to
open()" when creating a .def file.

On my win10 VM in which I installed python.org python I don't see a python dll
in c:/windows/system32 either. Looks like none of our windows mingw animals
build with python. So it might just be bitrot.

In the end, make check-world still fails, even though I was able to run
configure and make without any obvious error.
I see bunch of errors in tests like:
+ERROR:  language "plpython3u" does not exist
+HINT:  Use CREATE EXTENSION to load the language into the database.

Here is the logs from failed ci run:
https://api.cirrus-ci.com/v1/artifact/task/4645682031099904/log/build/src/pl/plpython/regression.diffs

The URL to the rest of the CI run is https://cirrus-ci.com/task/4645682031099904

The relevant failure is earlier:

+ERROR: could not load library "C:/cirrus/build/tmp_install/ucrt64/lib/postgresql/plpython3.dll": The specified module could not be found.

Clearly plpython is being built, because we see some warnings:

[22:44:24.456] C:/msys64/ucrt64/include/python3.9/pyconfig.h:1474: warning: "SIZEOF_OFF_T" redefined
[22:44:24.456] 1474 | #define SIZEOF_OFF_T 8
[22:44:24.456] |
[22:44:24.456] In file included from c:/cirrus/src/include/c.h:54,
[22:44:24.456] from c:/cirrus/src/include/postgres.h:46,
[22:44:24.456] from c:/cirrus/contrib/jsonb_plpython/jsonb_plpython.c:1:
[22:44:24.456] ../../src/include/pg_config.h:875: note: this is the location of the previous definition
[22:44:24.456] 875 | #define SIZEOF_OFF_T 4

Seems we're doing something wrong and end up with a 4 byte off_t, whereas
python ends up with an 8byte one. We probably need to fix that. But it's not
the cause of this problem.

You could take out -s from the make flags and see whether plpython3.dll is
being built and installed, and where to.

Greetings,

Andres Freund

On 2/25/22 19:27, Andres Freund wrote:

Hi,

Andrew, CCIng you both because you might be interested in the CI bit, and
because you might know the answer.

2- src/pl/plpython/Makefile looks under "C:/Windows/system32/" for
PYTHONDLL. gendef cannot open that file, give an error like " failed to
open()" when creating a .def file.

On my win10 VM in which I installed python.org python I don't see a python dll
in c:/windows/system32 either. Looks like none of our windows mingw animals
build with python. So it might just be bitrot.

There certainly was a time when python from python.org used to install
its DLL in the system32 directory, so I imagine that's why it's there.
I'm very glad to see that's no longer the case.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

Hi Andres,

This presumably is due to using mingw's python rather than python.org's
python. Seems like a reasonable thing to support for the mingw build.

Melih, you could try to build against the python.org python (i installed
in
the CI container).

I tried to use the installed python from python.org in the container.
The problem with this is that "LIBDIR" and "LDLIBRARY" configs of python
for windows from python.org are empty. Therefore python_libdir or other
related variables in configure file are not set correctly.

Seems we're doing something wrong and end up with a 4 byte off_t, whereas

python ends up with an 8byte one. We probably need to fix that. But it's
not
the cause of this problem.

You could take out -s from the make flags and see whether plpython3.dll is
being built and installed, and where to.

Here is a run without -s: https://cirrus-ci.com/task/4569363104661504
I couldn't get what's wrong from these logs to be honest. But I see that
plpython3.dll exists under src/pl/plpython after build when I run these
steps locally.

Best,
Melih

On 3/3/22 05:16, Melih Mutlu wrote:

Hi Andres, 
 

This presumably is due to using mingw's python rather than
python.org <http://python.org&gt;&#39;s
python. Seems like a reasonable thing to support for the mingw build.

Melih, you could try to build against the python.org
<http://python.org&gt; python (i installed in
the CI container).

I tried to use the installed python from python.org
<http://python.org&gt; in the container. 
The problem with this is that "LIBDIR" and "LDLIBRARY" configs of
python for windows from python.org <http://python.org&gt; are empty.
Therefore python_libdir or other related variables in configure file
are not set correctly.

Yeah, here's what it has:

# python -c "import sysconfig; import pprint; pp =
pprint.PrettyPrinter(); pp.pprint(sysconfig.get_config_vars())"
{'BINDIR': 'C:\\prog\\python310',
 'BINLIBDEST': 'C:\\prog\\python310\\Lib',
 'EXE': '.exe',
 'EXT_SUFFIX': '.cp310-win_amd64.pyd',
 'INCLUDEPY': 'C:\\prog\\python310\\Include',
 'LIBDEST': 'C:\\prog\\python310\\Lib',
 'SO': '.cp310-win_amd64.pyd',
 'TZPATH': '',
 'VERSION': '310',
 'abiflags': '',
 'base': 'C:\\prog\\python310',
 'exec_prefix': 'C:\\prog\\python310',
 'installed_base': 'C:\\prog\\python310',
 'installed_platbase': 'C:\\prog\\python310',
 'platbase': 'C:\\prog\\python310',
 'platlibdir': 'lib',
 'prefix': 'C:\\prog\\python310',
 'projectbase': 'C:\\prog\\python310',
 'py_version': '3.10.2',
 'py_version_nodot': '310',
 'py_version_nodot_plat': '310',
 'py_version_short': '3.10',
 'srcdir': 'C:\\prog\\python310',
 'userbase': 'C:\\Users\\Administrator\\AppData\\Roaming\\Python'}

The DLL lives in the BINDIR, so maybe I guess we should search there if
we can't get the other things.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

Hi,

On 2022-02-25 19:44:27 +0300, Melih Mutlu wrote:

I've been working on adding Windows+MinGW environment into cirrus-ci tasks
(discussion about ci is here [1]).

This doesn't apply anymore: http://cfbot.cputube.org/patch_37_3575.log

Could you rebase?

Greetings,

Andres Freund

Hi Andres,

Rebased it.
I also removed the temp installation task and
used NoDefaultCurrentDirectoryInExePath env variable instead.

Best,
Melih

Hi,

On 2022-03-22 19:00:42 +0300, Melih Mutlu wrote:

Rebased it.
I also removed the temp installation task and
used NoDefaultCurrentDirectoryInExePath env variable instead.

Hm. But you're still using a separate build directory, from what I can see?
The NoDefaultCurrentDirectoryInExePath thing should only have an effect when
not using a separate build directory, no?

Does it work to not use the separate build dir? Without it we don't need the
the "preparing build tree" step, and that's quite slow on mingw:
https://cirrus-ci.com/task/4713509253545984?logs=configure#L392

[00:23:44.371] preparing build tree... done
[00:24:25.429] configure: creating ./config.status

Chatting about this patch with Thomas I started to wonder about other reasons
for the slow speed of configure. I briefly experimented locally, and it looks
like using 'dash' as the shell makes configure run a good bit quicker.

---
.cirrus.yml | 79 +++++++++++++++++++++++++++++++++++++++++++----------
1 file changed, 65 insertions(+), 14 deletions(-)

diff --git a/.cirrus.yml b/.cirrus.yml
index e5335fede7..1ed40347cf 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -23,7 +23,6 @@ env:
CHECKFLAGS: -Otarget
PROVE_FLAGS: --timer
PGCTLTIMEOUT: 120 # avoids spurious failures during parallel tests
-  TEMP_CONFIG: ${CIRRUS_WORKING_DIR}/src/tools/ci/pg_ci_base.conf
PG_TEST_EXTRA: kerberos ldap ssl

This removes TEMP_CONFIG from all other tasks. You added it back to the VS
windows task, but not the others? I assume that was accidental?

+  env:
+    CCACHE_DIR: C:/msys64/ccache
+    BUILD_DIR: "%CIRRUS_WORKING_DIR%/build"

I think this should use TEMP_CONFIG too. Is the problem that you need to
change the path?

+  ccache_cache:
+    folder: ${CCACHE_DIR}
+
+  mingw_info_script:
+    - C:\msys64\usr\bin\bash.exe -lc "where gcc"
+    - C:\msys64\usr\bin\bash.exe -lc "gcc --version"
+    - C:\msys64\usr\bin\bash.exe -lc "where perl"
+    - C:\msys64\usr\bin\bash.exe -lc "perl --version"
+
+  configure_script:
+    - C:\msys64\usr\bin\bash.exe -lc "mkdir %BUILD_DIR% &&
+      cd %BUILD_DIR% &&
+      %CIRRUS_WORKING_DIR%/configure

Could you try using dash to invoke configure here, and whether it makes configure faster?

+        --host=x86_64-w64-mingw32
+        --enable-cassert
+        --enable-tap-tests
+        --with-icu
+        --with-libxml
+        --with-libxslt
+        --with-lz4
+        --enable-debug
+        CC='ccache gcc'
+        CXX='ccache g++'"

I think this task should specify CFLAGS="-Og", CXXFLAGS="-Og" similar to other
tasks. We end up with -O2 otherwise, which makes the build measurably slower.

+  tests_script:
+  - set "NoDefaultCurrentDirectoryInExePath=0"

A comment about why NoDefaultCurrentDirectoryInExePath=0 is used would be
good.

Greetings,

Andres Freund

Hi,

On 2022-03-30 17:26:18 -0700, Andres Freund wrote:

Hi,

On 2022-03-22 19:00:42 +0300, Melih Mutlu wrote:

Rebased it.
I also removed the temp installation task and
used NoDefaultCurrentDirectoryInExePath env variable instead.

Hm. But you're still using a separate build directory, from what I can see?
The NoDefaultCurrentDirectoryInExePath thing should only have an effect when
not using a separate build directory, no?

Melih chatted with me about making this work. Turns out it doesn't readily -
pg_ctl still fails.

The reason that NoDefaultCurrentDirectoryInExePath doesn't suffice to get a
working in-tree build, is that we break it ourselves:

int
find_my_exec(const char *argv0, char *retpath)
...
#ifdef WIN32
/* Win32 checks the current directory first for names without slashes */
join_path_components(retpath, cwd, argv0);
if (validate_exec(retpath) == 0)
return resolve_symlinks(retpath);
#endif

So even if windows doesn't actually use the current path, and the current
pg_ctl process isn't the one from the current directory, we *still* return
that.

Gah.

Maybe we should just use GetModuleFileName()?

But even after that the tests don't work. Commands started via IPC::Run do,
but when using system() they don't. Looks like perl parses the path the itself
:(.

- Andres

On 4/4/22 16:41, Andres Freund wrote:

Hi,

On 2022-03-30 17:26:18 -0700, Andres Freund wrote:

Hi,

On 2022-03-22 19:00:42 +0300, Melih Mutlu wrote:

Rebased it.
I also removed the temp installation task and
used NoDefaultCurrentDirectoryInExePath env variable instead.

Hm. But you're still using a separate build directory, from what I can see?
The NoDefaultCurrentDirectoryInExePath thing should only have an effect when
not using a separate build directory, no?

Melih chatted with me about making this work. Turns out it doesn't readily -
pg_ctl still fails.

The reason that NoDefaultCurrentDirectoryInExePath doesn't suffice to get a
working in-tree build, is that we break it ourselves:

int
find_my_exec(const char *argv0, char *retpath)
...
#ifdef WIN32
/* Win32 checks the current directory first for names without slashes */
join_path_components(retpath, cwd, argv0);
if (validate_exec(retpath) == 0)
return resolve_symlinks(retpath);
#endif

So even if windows doesn't actually use the current path, and the current
pg_ctl process isn't the one from the current directory, we *still* return
that.

Gah.

I notice a few things about the latest file in this thread.

You should set MSYSTEM=UCRT64 in the environment section. Given that,
there should be no need to specify a --host= setting for configure.

If it's not done already, the shebang line in
/ucrt64/bin/core_perl/prove needs to be modified to use /ucrt64/bin/perl.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

On 3/30/22 20:26, Andres Freund wrote:

Could you try using dash to invoke configure here, and whether it makes configure faster?

I got weird failures re libxml/parser.h when I tried with dash. See
<https://cirrus-ci.com/task/5963254039052288&gt; (It would be nice if we
could see config.log on failure.)

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

Hi,

On 2022-04-06 11:03:37 -0400, Andrew Dunstan wrote:

On 3/30/22 20:26, Andres Freund wrote:

Could you try using dash to invoke configure here, and whether it makes configure faster?

I got weird failures re libxml/parser.h when I tried with dash.

Hm. Hadn't enabled that when I tried...

See <https://cirrus-ci.com/task/5963254039052288&gt; (It would be nice if we
could see config.log on failure.)

All *.log files are preserved on failure. There's a file directory brower at
the top to navigate around:
https://api.cirrus-ci.com/v1/artifact/task/5963254039052288/log/build/config.log

Greetings,

Andres Freund

Hi,

On 2022-04-06 11:03:37 -0400, Andrew Dunstan wrote:

On 3/30/22 20:26, Andres Freund wrote:

Could you try using dash to invoke configure here, and whether it makes configure faster?

I got weird failures re libxml/parser.h when I tried with dash. See
<https://cirrus-ci.com/task/5963254039052288&gt; (It would be nice if we
could see config.log on failure.)

Since dash won't help us to get the build time down sufficiently, and the
tests don't pass without a separate build tree, I looked at what makes
config/prep_buildtree so slow.

It's largely just bad code. The slowest part are spawning one expr and mkdir
-p for each directory. One 'cmp' for each makefile doesn't help either.

The expr can be replaced with
subdir=${item#$sourcetree}
that's afaics posix syntax ([1]https://pubs.opengroup.org/onlinepubs/009604499/utilities/xcu_chap02.html), not bash.

Spawning one mkdir for each directory can be replaced by a single mkdir
invocation with all the directories. On my linux workstation that gets the
time for the first loop down from 1005ms to 38ms, really.

That has the danger of the commandline getting too long. But since we rely on
the final link of the backend to be done in a single command, I don't think
it's making things worse? We could try to use xargs otherwise, iirc that's in
posix as well.

Using parameter substitution in the second loop takes it down from 775ms to
533ms. Not calling cmp when the file doesn't exist cuts it down to 337ms.

I don't know of a way to batch the call to ln. The time with ln replaced with
: is 151ms, fwiw.

On windows that makes prep_buildtree go from 42.4s to 5.8s for me.

Greetings,

Andres Freund

[1]: https://pubs.opengroup.org/onlinepubs/009604499/utilities/xcu_chap02.html

On 4/6/22 12:34, Andres Freund wrote:

Hi,

On 2022-04-06 11:03:37 -0400, Andrew Dunstan wrote:

On 3/30/22 20:26, Andres Freund wrote:

Could you try using dash to invoke configure here, and whether it makes configure faster?

I got weird failures re libxml/parser.h when I tried with dash.

Hm. Hadn't enabled that when I tried...

See <https://cirrus-ci.com/task/5963254039052288&gt; (It would be nice if we
could see config.log on failure.)

All *.log files are preserved on failure. There's a file directory brower at
the top to navigate around:
https://api.cirrus-ci.com/v1/artifact/task/5963254039052288/log/build/config.log

Thanks.

I got it working with this added to the config settings:

--with-includes='/ucrt64/include/libxml2 /c/cirrus/src/include/port/win32'

I conclude tentatively that while bash translates widows paths to msys
paths, dash does not.

see https://cirrus-ci.com/task/5968968560148480?logs=configure#L1

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

On 4/7/22 13:10, Andres Freund wrote:

Hi,

On 2022-04-06 11:03:37 -0400, Andrew Dunstan wrote:

On 3/30/22 20:26, Andres Freund wrote:

Could you try using dash to invoke configure here, and whether it makes configure faster?

I got weird failures re libxml/parser.h when I tried with dash. See
<https://cirrus-ci.com/task/5963254039052288&gt; (It would be nice if we
could see config.log on failure.)

Since dash won't help us to get the build time down sufficiently, and the
tests don't pass without a separate build tree, I looked at what makes
config/prep_buildtree so slow.

It's largely just bad code. The slowest part are spawning one expr and mkdir
-p for each directory. One 'cmp' for each makefile doesn't help either.

The expr can be replaced with
subdir=${item#$sourcetree}
that's afaics posix syntax ([1]), not bash.

Spawning one mkdir for each directory can be replaced by a single mkdir
invocation with all the directories. On my linux workstation that gets the
time for the first loop down from 1005ms to 38ms, really.

That has the danger of the commandline getting too long. But since we rely on
the final link of the backend to be done in a single command, I don't think
it's making things worse? We could try to use xargs otherwise, iirc that's in
posix as well.

Using parameter substitution in the second loop takes it down from 775ms to
533ms. Not calling cmp when the file doesn't exist cuts it down to 337ms.

I don't know of a way to batch the call to ln. The time with ln replaced with
: is 151ms, fwiw.

AFAIK Msy2s 'ln -s' by default copies a non-directory rather than
actually symlinking it. If we want real symlinks, then we need
MSYS=|winsymlinks:nativestrict set. The is will fail unless the calling
user is an Administrator or has the SeCreateSymbolicLink privilege. See
|

|</messages/by-id/%7Ce05b213c-1257-84d4-f079-5c4d8c79e3ad@dunslane.net&gt;
for more details.

On windows that makes prep_buildtree go from 42.4s to 5.8s for me.

That's pretty good.

I think we can get rid of the CVS pruning, it's only 15 years or so
since we've had that in the tree.

+        if test ! -d "$buildtree/$subdir"; then
+            echo "$buildtree/$subdir"
+        fi

I would probably just write that as

test -d "$buildtree/$subdir' || echo "$buildtree/$subdir"

although it's really just a matter of taste.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

On 4/7/22 16:48, Andrew Dunstan wrote:

On 4/7/22 13:10, Andres Freund wrote:

Hi,

On 2022-04-06 11:03:37 -0400, Andrew Dunstan wrote:

On 3/30/22 20:26, Andres Freund wrote:

Could you try using dash to invoke configure here, and whether it makes configure faster?

I got weird failures re libxml/parser.h when I tried with dash. See
<https://cirrus-ci.com/task/5963254039052288&gt; (It would be nice if we
could see config.log on failure.)

Since dash won't help us to get the build time down sufficiently, and the
tests don't pass without a separate build tree, I looked at what makes
config/prep_buildtree so slow.

It's largely just bad code. The slowest part are spawning one expr and mkdir
-p for each directory. One 'cmp' for each makefile doesn't help either.

The expr can be replaced with
subdir=${item#$sourcetree}
that's afaics posix syntax ([1]), not bash.

Spawning one mkdir for each directory can be replaced by a single mkdir
invocation with all the directories. On my linux workstation that gets the
time for the first loop down from 1005ms to 38ms, really.

That has the danger of the commandline getting too long. But since we rely on
the final link of the backend to be done in a single command, I don't think
it's making things worse? We could try to use xargs otherwise, iirc that's in
posix as well.

Using parameter substitution in the second loop takes it down from 775ms to
533ms. Not calling cmp when the file doesn't exist cuts it down to 337ms.

I don't know of a way to batch the call to ln. The time with ln replaced with
: is 151ms, fwiw.

AFAIK Msy2s 'ln -s' by default copies a non-directory rather than
actually symlinking it. If we want real symlinks, then we need
MSYS=|winsymlinks:nativestrict set. The is will fail unless the calling
user is an Administrator or has the SeCreateSymbolicLink privilege. See
|

Sometimes I hate Thunderbird. Of course the | is spurious above, we
would need

MSYS=winsymlinks:nativestrict

set.

cheers

andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

Hi Andrew,

You should set MSYSTEM=UCRT64 in the environment section. Given that,

there should be no need to specify a --host= setting for configure.

It's set to UCRT64 in the docker image side [1]https://github.com/anarazel/pg-vm-images/blob/main/docker/windows_ci_mingw64#L11 <https://github.com/anarazel/pg-vm-images/blob/main/docker/windows_ci_mingw64#L11&gt;. I didn't know --host isn't
necessary on UCRT64 environment. I'll remove it then.

[1]: https://github.com/anarazel/pg-vm-images/blob/main/docker/windows_ci_mingw64#L11 <https://github.com/anarazel/pg-vm-images/blob/main/docker/windows_ci_mingw64#L11&gt;
https://github.com/anarazel/pg-vm-images/blob/main/docker/windows_ci_mingw64#L11
<https://github.com/anarazel/pg-vm-images/blob/main/docker/windows_ci_mingw64#L11&gt;

Best,
Melih

On 2022-Apr-07, Andres Freund wrote:

Since dash won't help us to get the build time down sufficiently, and the
tests don't pass without a separate build tree, I looked at what makes
config/prep_buildtree so slow.

Maybe we can replace prep_buildtree with a Perl script. Surely that
should be faster.

--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"Sallah, I said NO camels! That's FIVE camels; can't you count?"
(Indiana Jones)

On windows that makes prep_buildtree go from 42.4s to 5.8s for me.

I applied Andres's faster prep build tree changes and triggered some cirrus
runs

Without these changes, preparing build tree was taking around 42.3s
(sometimes even more) [1]https://cirrus-ci.com/task/6562493345562624
It seems like with these changes it drops to around 8s [2]https://cirrus-ci.com/task/4836843802853376

[1]: https://cirrus-ci.com/task/6562493345562624
[2]: https://cirrus-ci.com/task/4836843802853376

Best,
Melih

Hi,

On 2022-04-08 17:04:34 +0200, Alvaro Herrera wrote:

Since dash won't help us to get the build time down sufficiently, and the
tests don't pass without a separate build tree, I looked at what makes
config/prep_buildtree so slow.

Maybe we can replace prep_buildtree with a Perl script. Surely that
should be faster.

Currently building doesn't depend on perl :(

I think the improvements that I suggested are big enough that they're worth
doing on their own, particularly for windows, but also other OSs.

I just realized that the second find is pretty expensive compared to the
first.

time find "$sourcetree" -type d \( \( -name CVS -prune \) -o \( -name .git -prune \) -o -print \) | grep -v "$sourcetree/doc/src/sgml/\+" > /dev/null
real 0m0.019s
user 0m0.008s
sys 0m0.017s

second:
time find "$sourcetree" -name Makefile -print -o -name GNUmakefile -print | grep -v "$sourcetree/doc/src/sgml/images/" > /dev/null

real 0m0.118s
user 0m0.071s
sys 0m0.053s

It think we could just obsolete the second find, by checking for the existence
of Makefile / GNUmakefile in the first loop...

The invocation of ln -s is quite measurable - looks like it's mostly the
process startup overhead (on linux, at least). Doing a ln --version > /dev/null
each iteration takes about the same time as actually creating the symlinks.

Greetings,

Andres Freund