Re: Adding NetBSD and OpenBSD to Postgres CI

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?

> + 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?

> + # 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?

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.

> + <<: *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-89806e7420797025@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.

> + -Dcassert=true -Dssl=openssl ${UUID} \
> + -DPG_TEST_EXTRA="$PG_TEST_EXTRA" \
> + ${INCLUDE_DIRS} \
> + build
> + EOF

Should probably enable injection points.

> + 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.

Greetings,

Andres Freund