Hi,

On Fri, 1 Nov 2024 at 21:44, Andres Freund <and...@anarazel.de> wrote:
>
> Hi,
>
> Thanks for the patch!
>
>
> On 2024-11-01 12:17:00 +0300, Nazir Bilal Yavuz wrote:
> > I made these tasks triggered manually like MinGW task to save CI credits
> > but a related line is commented out for now to trigger CFBot.
>
> Oh, I need to pick my patch which allows repo-level config of which tasks run
> back up. Then we can enable these by default for cfbot, but not for individual
> repos...
>
>
> > +task:
> > +  depends_on: SanityCheck
> > +  # trigger_type: manual
> > +
> > +  env:
> > +    # Below are experimentally derived to be a decent choice.
> > +    CPUS: 2
> > +    BUILD_JOBS: 8
> > +    TEST_JOBS: 8
> > +
> > +    CIRRUS_WORKING_DIR: /home/postgres/postgres
>
> Why do you need to set that?

Otherwise, it is set to /tmp path and the size of /tmp mount point is
1.2GB, which is not enough.

> > +    CCACHE_DIR: /tmp/ccache_dir
> > +
> > +    PATH: /usr/sbin:$PATH
> > +
> > +    # Postgres interprets LANG as a 'en_US.UTF-8' but it is 'C', then
>
> What does "Postgres interprets LANG as a 'en_US.UTF-8'" mean?

It was because initdb was failing on NetBSD when the LANG and LC_ALL
is not set to C. I rephrased the comment and moved this under NetBSD
task.

> > +    # Postgres tries to set 'LC_COLLATE' to 'en_US.UTF-8' but it is not
> > +    # changeable. Initdb fails because of that. So, LANG is forced to be 
> > 'C'.
> > +    LANG: "C"
> > +    LC_ALL: "C"
>
> > +  matrix:
> > +    - name: NetBSD - 10 - Meson
> > +      only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || 
> > $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*netbsd.*'
> > +      env:
> > +        IMAGE_FAMILY: pg-ci-netbsd-postgres
> > +        INCLUDE_DIRS: -Dextra_lib_dirs=/usr/pkg/lib 
> > -Dextra_include_dirs=/usr/pkg/include
> > +      <<: *netbsd_task_template
> > +
> > +    - name: OpenBSD - 7 - Meson
> > +      only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || 
> > $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*openbsd.*'
> > +      env:
> > +        IMAGE_FAMILY: pg-ci-openbsd-postgres
> > +        INCLUDE_DIRS: -Dextra_include_dirs=/usr/local/include 
> > -Dextra_lib_dirs=/usr/local/lib
> > +        UUID: -Duuid=e2fs
>
> Shouldn't something be added to PKG_CONFIG_PATH / --pkg-config-path?

I don't think so. Both OSes are able to find pkgconfig at
'/usr/pkg/bin/pkg-config'. Am I missing something?

> For other OSs we have stanzas like
>
>   setup_additional_packages_script: |
>     #apt-get update
>     #DEBIAN_FRONTEND=noninteractive apt-get -y install ...
>
> it'd be good to have something similar for openbsd/netbsd, given that most
> won't be as familiar with that.

Done.

> > +      <<: *openbsd_task_template
> > +  sysinfo_script: |
> > +    locale
> > +    id
> > +    uname -a
> > +    ulimit -a -H && ulimit -a -S
> > +    env
> > +
> > +  ccache_cache:
> > +    folder: $CCACHE_DIR
> > +
> > +  create_user_script: |
> > +    useradd postgres
> > +    chown -R postgres:users /home/postgres
> > +    mkdir -p ${CCACHE_DIR}
> > +    chown -R postgres:users ${CCACHE_DIR}
> > +
> > +  # -Duuid=bsd is not set since 'bsd' uuid option
> > +  # is not working on NetBSD & OpenBSD. See
> > +  # 
> > https://www.postgresql.org/message-id/17358-89806e7420797...@postgresql.org
> > +  # And other uuid options are not available on NetBSD.
> > +  configure_script: |
> > +    su postgres <<-EOF
> > +      meson setup \
> > +        --buildtype debug \
>
> I suspect it'd be good to either add -Og to cflags (as done in a bunch of
> other tasks) or to use debugoptimized, given that the tests on these machines
> are fairly slow.

Done.

> > +        -Dcassert=true -Dssl=openssl ${UUID} \
> > +        -DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
> > +        ${INCLUDE_DIRS} \
> > +        build
> > +    EOF
>
> Should probably enable injection points.

Done.

> > +  build_script: su postgres -c 'ninja -C build -j${BUILD_JOBS}'
> > +  upload_caches: ccache
> > +
> > +  test_world_script: |
> > +    su postgres <<-EOF
> > +      ulimit -c unlimited
> > +      # Otherwise tests will fail on OpenBSD, due to the lack of enough 
> > processes.
> > +      ulimit -p 256
> > +      meson test $MTEST_ARGS --num-processes ${TEST_JOBS}
> > +    EOF
> > +
> > +  on_failure:
> > +    <<: *on_failure_meson
> > +
> > +
>
> Right now you don't seem to be collecting core files - but you're still
> enabling them via ulimit -c unlimited.  At least we shouldn't use ulimit -c
> unlimited without collecting core files, but it'd probably be better to add
> support for collecting core files. Shouldn't be too hard.

Done. I separated this patch to make review easier.

--
Regards,
Nazir Bilal Yavuz
Microsoft
From cbea598b11e85b5c7090ca8e9cc05c35f0359f54 Mon Sep 17 00:00:00 2001
From: Nazir Bilal Yavuz <byavu...@gmail.com>
Date: Thu, 7 Nov 2024 15:48:31 +0300
Subject: [PATCH v2 1/3] Fix meson could not find bsd_auth.h

bsd_auth.h file needs to be compiled together with the 'sys/types.h' as
it has missing type definitions.

See synopsis at https://man.openbsd.org/authenticate.3
---
 meson.build | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index 5b0510cef78..84107955d5d 100644
--- a/meson.build
+++ b/meson.build
@@ -551,7 +551,8 @@ test_c_args = cppflags + cflags
 bsd_authopt = get_option('bsd_auth')
 bsd_auth = not_found_dep
 if cc.check_header('bsd_auth.h', required: bsd_authopt,
-    args: test_c_args, include_directories: postgres_inc)
+    args: test_c_args, prefix: '#include <sys/types.h>',
+    include_directories: postgres_inc)
   cdata.set('USE_BSD_AUTH', 1)
   bsd_auth = declare_dependency()
 endif
-- 
2.45.2

From cd5bb66a55e5226b543f5b7db9128cfa48e338e3 Mon Sep 17 00:00:00 2001
From: Nazir Bilal Yavuz <byavu...@gmail.com>
Date: Mon, 11 Nov 2024 13:23:22 +0300
Subject: [PATCH v2 2/3] Add NetBSD and OpenBSD tasks to the Postgres CI

NetBSD and OpenBSD Postgres CI images are generated [1] but their tasks
are not added to the upstream Postgres yet. This patch adds them.

Note: These tasks will be triggered manually to save CI credits but a
related line is commented out for now to trigger CFBot.

Author: Nazir Bilal Yavuz <byavu...@gmail.com>

[1] https://github.com/anarazel/pg-vm-images
---
 .cirrus.tasks.yml | 84 +++++++++++++++++++++++++++++++++++++++++++++++
 .cirrus.yml       | 10 ++++++
 2 files changed, 94 insertions(+)

diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml
index fc413eb11ef..f338af902aa 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -213,6 +213,90 @@ task:
     cores_script: src/tools/ci/cores_backtrace.sh freebsd /tmp/cores
 
 
+task:
+  depends_on: SanityCheck
+  # trigger_type: manual
+
+  env:
+    # Below are experimentally derived to be a decent choice.
+    CPUS: 2
+    BUILD_JOBS: 8
+    TEST_JOBS: 8
+
+    CIRRUS_WORKING_DIR: /home/postgres/postgres
+    CCACHE_DIR: /tmp/ccache_dir
+
+    PATH: /usr/sbin:$PATH
+
+  matrix:
+    - name: NetBSD - 10 - Meson
+      only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*netbsd.*'
+      env:
+        IMAGE_FAMILY: pg-ci-netbsd-postgres
+        INCLUDE_DIRS: -Dextra_include_dirs=/usr/pkg/include -Dextra_lib_dirs=/usr/pkg/lib
+        # initdb fails with: 'invalid locale settings' error on NetBSD.
+        # Force 'LANG' and 'LC_*' variables to be 'C'.
+        LANG: "C"
+        LC_ALL: "C"
+      setup_additional_packages_script: |
+        #pkgin -y install ...
+      <<: *netbsd_task_template
+
+    - name: OpenBSD - 7 - Meson
+      only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*openbsd.*'
+      env:
+        IMAGE_FAMILY: pg-ci-openbsd-postgres
+        INCLUDE_DIRS: -Dextra_include_dirs=/usr/local/include -Dextra_lib_dirs=/usr/local/lib
+        UUID: -Duuid=e2fs
+      setup_additional_packages_script: |
+        #pkg_add -I ...
+      <<: *openbsd_task_template
+
+  sysinfo_script: |
+    locale
+    id
+    uname -a
+    ulimit -a -H && ulimit -a -S
+    env
+
+  ccache_cache:
+    folder: $CCACHE_DIR
+
+  create_user_script: |
+    useradd postgres
+    chown -R postgres:users /home/postgres
+    mkdir -p ${CCACHE_DIR}
+    chown -R postgres:users ${CCACHE_DIR}
+
+  # -Duuid=bsd is not set since 'bsd' uuid option
+  # is not working on NetBSD & OpenBSD. See
+  # https://www.postgresql.org/message-id/17358-89806e7420797...@postgresql.org
+  # And other uuid options are not available on NetBSD.
+  configure_script: |
+    su postgres <<-EOF
+      meson setup \
+        --buildtype=debugoptimized \
+        -Dcassert=true -Dinjection_points=true \
+        -Dssl=openssl ${UUID} \
+        -DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
+        ${INCLUDE_DIRS} \
+        build
+    EOF
+
+  build_script: su postgres -c 'ninja -C build -j${BUILD_JOBS}'
+  upload_caches: ccache
+
+  test_world_script: |
+    su postgres <<-EOF
+      # Otherwise tests will fail on OpenBSD, due to the lack of enough processes.
+      ulimit -p 256
+      meson test $MTEST_ARGS --num-processes ${TEST_JOBS}
+    EOF
+
+  on_failure:
+    <<: *on_failure_meson
+
+
 # configure feature flags, shared between the task running the linux tests and
 # the CompilerWarnings task
 LINUX_CONFIGURE_FEATURES: &LINUX_CONFIGURE_FEATURES >-
diff --git a/.cirrus.yml b/.cirrus.yml
index a83129ae46d..33c6e481d74 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -52,6 +52,16 @@ default_freebsd_task_template: &freebsd_task_template
     PLATFORM: freebsd
   <<: *cirrus_community_vm_template
 
+default_netbsd_task_template: &netbsd_task_template
+  env:
+    PLATFORM: netbsd
+  <<: *cirrus_community_vm_template
+
+default_openbsd_task_template: &openbsd_task_template
+  env:
+    PLATFORM: openbsd
+  <<: *cirrus_community_vm_template
+
 
 default_windows_task_template: &windows_task_template
   env:
-- 
2.45.2

From 6f43797df839c189545d87b5c9ccbf45403cf631 Mon Sep 17 00:00:00 2001
From: Nazir Bilal Yavuz <byavu...@gmail.com>
Date: Mon, 11 Nov 2024 17:27:08 +0300
Subject: [PATCH v2 3/3] Collect core files on NetBSD and OpenBSD

---
 .cirrus.tasks.yml               | 17 ++++++++++++++++-
 src/tools/ci/cores_backtrace.sh |  6 ++++--
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml
index f338af902aa..a1500b42e4c 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -210,7 +210,7 @@ task:
         build/tmp_install/usr/local/pgsql/bin/pg_ctl -D build/runningcheck stop || true
       EOF
     <<: *on_failure_meson
-    cores_script: src/tools/ci/cores_backtrace.sh freebsd /tmp/cores
+    cores_script: src/tools/ci/cores_backtrace.sh bsd /tmp/cores
 
 
 task:
@@ -227,11 +227,13 @@ task:
     CCACHE_DIR: /tmp/ccache_dir
 
     PATH: /usr/sbin:$PATH
+    CORE_DUMP_DIR: /var/crash
 
   matrix:
     - name: NetBSD - 10 - Meson
       only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*netbsd.*'
       env:
+        OS_NAME: netbsd
         IMAGE_FAMILY: pg-ci-netbsd-postgres
         INCLUDE_DIRS: -Dextra_include_dirs=/usr/pkg/include -Dextra_lib_dirs=/usr/pkg/lib
         # initdb fails with: 'invalid locale settings' error on NetBSD.
@@ -245,11 +247,14 @@ task:
     - name: OpenBSD - 7 - Meson
       only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*openbsd.*'
       env:
+        OS_NAME: openbsd
         IMAGE_FAMILY: pg-ci-openbsd-postgres
         INCLUDE_DIRS: -Dextra_include_dirs=/usr/local/include -Dextra_lib_dirs=/usr/local/lib
         UUID: -Duuid=e2fs
       setup_additional_packages_script: |
         #pkg_add -I ...
+      # Always core dump to ${CORE_DUMP_DIR}
+      set_core_dump_script: sysctl -w kern.nosuidcoredump=2
       <<: *openbsd_task_template
 
   sysinfo_script: |
@@ -267,6 +272,10 @@ task:
     chown -R postgres:users /home/postgres
     mkdir -p ${CCACHE_DIR}
     chown -R postgres:users ${CCACHE_DIR}
+  setup_core_files_script: |
+    mkdir -p ${CORE_DUMP_DIR}
+    chmod -R 770 ${CORE_DUMP_DIR}
+    chown -R postgres:users ${CORE_DUMP_DIR}
 
   # -Duuid=bsd is not set since 'bsd' uuid option
   # is not working on NetBSD & OpenBSD. See
@@ -288,6 +297,7 @@ task:
 
   test_world_script: |
     su postgres <<-EOF
+      ulimit -c unlimited
       # Otherwise tests will fail on OpenBSD, due to the lack of enough processes.
       ulimit -p 256
       meson test $MTEST_ARGS --num-processes ${TEST_JOBS}
@@ -295,6 +305,11 @@ task:
 
   on_failure:
     <<: *on_failure_meson
+    cores_script: |
+      # Although OSes are forced to core dump inside ${CORE_DUMP_DIR}, they may
+      # not obey this. So, move core files to the ${CORE_DUMP_DIR} directory.
+      find build/ -maxdepth 1 -type f -name '*.core' -exec mv '{}' ${CORE_DUMP_DIR} \;
+      src/tools/ci/cores_backtrace.sh ${OS_NAME} ${CORE_DUMP_DIR}
 
 
 # configure feature flags, shared between the task running the linux tests and
diff --git a/src/tools/ci/cores_backtrace.sh b/src/tools/ci/cores_backtrace.sh
index 28d3cecfc67..4b174a5cf56 100755
--- a/src/tools/ci/cores_backtrace.sh
+++ b/src/tools/ci/cores_backtrace.sh
@@ -9,7 +9,7 @@ os=$1
 directory=$2
 
 case $os in
-    freebsd|linux|macos)
+    freebsd|netbsd|openbsd|linux|macos)
     ;;
     *)
         echo "unsupported operating system ${os}"
@@ -26,7 +26,7 @@ for corefile in $(find "$directory" -type f) ; do
         echo -e '\n\n'
     fi
 
-    if [ "$os" = 'macos' ]; then
+    if [ "$os" = 'macos' ] || [ "$os" = 'openbsd' ]; then
         lldb -c $corefile --batch -o 'thread backtrace all' -o 'quit'
     else
         auxv=$(gdb --quiet --core ${corefile} --batch -ex 'info auxv' 2>/dev/null)
@@ -37,6 +37,8 @@ for corefile in $(find "$directory" -type f) ; do
 
         if [ "$os" = 'freebsd' ]; then
             binary=$(echo "$auxv" | grep AT_EXECPATH | perl -pe "s/^.*\"(.*)\"\$/\$1/g")
+        elif [ "$os" = 'netbsd' ]; then
+            binary=$(echo "$auxv" | grep AT_SUN_EXECNAME | perl -pe "s/^.*\"(.*)\"\$/\$1/g")
         elif [ "$os" = 'linux' ]; then
             binary=$(echo "$auxv" | grep AT_EXECFN | perl -pe "s/^.*\"(.*)\"\$/\$1/g")
         else
-- 
2.45.2

Reply via email to