Re: Adding CI to our tree

Hi,

> Subject: [PATCH 2/7] cirrus: upload changed html docs as artifacts

I still think the determination of the base branch needs to be resolved before
this can be considered.

> Always run doc build; to allow them to be shown in cfbot, they should not be
> skipped if the linux build fails.
>
> This could be done on the client side (cfbot). One advantage of doing it here
> is that fewer docs are uploaded - many patches won't upload docs at all.

Imo this stuff is largely independent from the commit subject....

> XXX: if this is run in the same task, the configure flags should probably be
> consistent ?

What do you mean?

> Subject: [PATCH 3/7] s!build docs as a separate task..

Could you reorder this to earlier, then we can merge it before resolving the
branch issues. And we don't waffle on the CompilerWarnings dependency.

> I believe this'll automatically show up as a separate "column" on the cfbot
> page.

Yup.

> +# Verify docs can be built, and upload changed docs as artifacts
> +task:
> + name: HTML docs
> +
> + env:
> + CPUS: 1
> +
> + only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*(docs|html).*'
> +
> + container:
> + image: $CONTAINER_REPO/linux_debian_bullseye_ci:latest
> + cpu: $CPUS
> +

how about using something like (the syntax might be slightly off)
skip: !changesInclude('doc/**')
to avoid running it for the many pushes where no docs are changed?

> + sysinfo_script: |
> + id
> + uname -a
> + cat /proc/cmdline
> + ulimit -a -H && ulimit -a -S
> + export
> +
> + git remote -v
> + git branch -a
> + git remote add postgres https://github.com/postgres/postgres
> + time git fetch -v postgres master
> + git log -1 postgres/master
> + git diff --name-only postgres/master..

Hardly "sysinfo"?

> Subject: [PATCH 4/7] wip: cirrus: code coverage
>
> XXX: lcov should be installed in the OS image

FWIW, you can open a PR in https://github.com/anarazel/pg-vm-images/
both the debian docker and VM have their packages installed
via scripts/linux_debian_install_deps.sh

> From 226699150e3e224198fc297689add21bece51c4b Mon Sep 17 00:00:00 2001
> From: Justin Pryzby <pryzbyj(at)telsasoft(dot)com>
> Date: Sun, 9 Jan 2022 18:25:02 -0600
> Subject: [PATCH 5/7] cirrus/vcregress: test modules/contrib with
> NO_INSTALLCHECK=1

I don't want to commit the vcregress.pl part myself. But if you split off I'm
happy to push the --temp-config bits.

> From 08933bcd93d4f57ad73ab6df2f1627b93e61b459 Mon Sep 17 00:00:00 2001
> From: Justin Pryzby <pryzbyj(at)telsasoft(dot)com>
> Date: Sun, 16 Jan 2022 12:51:13 -0600
> Subject: [PATCH 6/7] wip: cirrus/windows: save tmp_install as an artifact..
>
> ..to allow users to easily download compiled binaries to try a patch.
> If they don't have a development environment handy or not familiar with
> compilation.
>
> XXX: maybe this should be conditional or commented out ?

Yea, I don't want to do this by default, that's a fair bit of data that very
likely nobody will ever access. One can make entire tasks triggered manually,
but that'd then require building again :/.

> From a7d2bba6f51d816412fb645b0d4821c36ee5c400 Mon Sep 17 00:00:00 2001
> From: Justin Pryzby <pryzbyj(at)telsasoft(dot)com>
> Date: Sun, 20 Feb 2022 15:01:59 -0600
> Subject: [PATCH 7/7] wip: cirrus/windows: add compiler_warnings_script
>
> I'm not sure how to write this test in windows shell; it's also not easy to
> write it in posix sh, since windows shell is somehow interpretting && and ||...

You could put the script in src/tools/ci and call it from the script to avoid
the quoting issues.

Would be good to add a comment explaining the fileLoggerParameters1 thing and
a warning that compiler_warnings_script should be the last script.

Greetings,

Andres Freund