Re: Adding CI to our tree

On Tue, Dec 14, 2021 at 10:12 AM Andres Freund <andres(at)anarazel(dot)de> wrote:
> Attached is an updated version of the CI patches. An example of a test run
> with the attached version of this
> https://cirrus-ci.com/build/6501998521483264

I've been pushing various versions of these patches into my own
development branches for a while now; they're working very nicely and
helping me a lot. This is vastly better than anything I was doing
before, especially on Windows which is a blind spot for most of us.
It'll be great to see this committed, and continue improving it
in-tree. I'd better go and figure out how to fix cfbot when this
lands...

> I think this may be OK for now, but I also could see arguments for wanting to
> transfer the image specifications and the google account to PG properties.

No clue on the GCP account side of it (does pginfra already have
one?), but for the repo I guess it would seem natural to have one on
git.postgresql.org infra, mirrored (just like the main repo) to a repo
on project-owned github.com/postgres, from which image building is
triggered. Then it could be maintained by the whole PostgreSQL
project, patches discussed on -hackers, a bit like pg_bsd_indent.
Perhaps with some way to trigger test image builds, so that people
working on it don't need their own GCP account to do a trial run.

+ # Test that code can be built with gcc/clang without warnings

As a TODO note, I think we should eventually run a warnings check for
MSVC too. IIUC we only aim to be warning free in assertion builds on
that platform, because it has no PG_USED_FOR_ASSERTS_ONLY (I think it
has it in C++ but not C?), but that's something.

+ # XXX: Only do this if there have been changes in doc/ since last build
+ always:
+ docs_build_script: |
+ time ./configure \
+ --cache gcc.cache CC="ccache gcc"
+ time make -s -j4 -C doc

Another TODO note: perhaps we could also make the documentation
results a browsable artefact with a short retention time, if that's a
thing. (I've been confused about how to spell "artefact" for some
time now, and finally I know why: in the US it has an i; I blame Noah
Websta, whose name I have decided to improve.)

I feel like I should apologise in advance for this level of
nit-picking about English grammar, but:

+2) For not yet merged development work, CI can be enabled for some git hosting
+ providers. This allows to test patches on a number of platforms before they
+ are merged (or even submitted).

You can "allow testing" (gerund verb), you can "allow
developers/us/one/... to test" (infinitive, but with a noun phrase to
say who's allowed to do the thing), you can "allow verification of
..." (noun phrase), you can "be allowed to test" (passive), but you
can't "allow to test": it's not allowed![1] :-)

+# It might be nicer to switch to the openssl built as part of curl-for-win,
+# but recent releases only build openssl 3, and that still seems troublesome
+# on windows,

s/windows,/Windows./

+ name: FreeBSD

FreeBSD is missing --with-llvm. If you add package "llvm" to your
image builder you'll currently get LLVM 9, then
LLVM_CONFIG="llvm-config" CXX="ccache c++" CLANG="ccache clang". Or
we could opt for something more modern with package llvm13 and program
names llvm-config13 and clang13.

> The second attention-worthy point is the choice of a new toplevel ci/
> directory as the location for resources referencenced by CI. A few other
> projects also use ci/, but I can also see arguments for moving the contents to
> e.g. src/tools/ci or such?

I'd be +0.75 for moving it to src/tools/ci.

> [1] Some ideas for what could make sense to extend CI to in the future:

To your list, I'd add:

* 32 bit
* test coverage report
* ability to capture complete Window install directory as an artefact
so a Windows user without a dev environment could try out a proposed
change/CF entry/...

I hope we can get ccache working on Windows.

[1] https://english.stackexchange.com/questions/60271/grammatical-complements-for-allow/60285#60285