Date: Thu, 10 Mar 2022 16:06:12 -0600
From: Justin Pryzby <pryzby@telsasoft.com>
To: Andres Freund <andres@anarazel.de>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Robert Haas <robertmhaas@gmail.com>,
	Andrew Dunstan <andrew@dunslane.net>, pgsql-hackers@postgresql.org,
	Thomas Munro <thomas.munro@gmail.com>,
	Melanie Plageman <melanieplageman@gmail.com>,
	Peter Eisentraut <peter.eisentraut@enterprisedb.com>,
	Daniel Gustafsson <daniel@yesql.se>
Subject: Re: Adding CI to our tree
Message-ID: <20220310220611.GH28503@telsasoft.com>
References: <20220213220709.vjz5rziuhfdpqxrg@alap3.anarazel.de>
 <20220226025116.GJ9008@telsasoft.com>
 <20220227010908.vz2a7dmfzgwg742w@alap3.anarazel.de>
 <20220227024352.GC25269@telsasoft.com>
 <20220227025000.mjmfimls5gnz7em7@alap3.anarazel.de>
 <20220227031057.GD25269@telsasoft.com>
 <20220227040838.3z4gqihimyerxe3h@alap3.anarazel.de>
 <20220228205802.GE25269@telsasoft.com>
 <20220302205058.GJ15744@telsasoft.com>
 <20220310205015.7k5yr4bhmwutjp6t@alap3.anarazel.de>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="wLAMOaPNJ0fu1fTG"
Content-Disposition: inline
In-Reply-To: <20220310205015.7k5yr4bhmwutjp6t@alap3.anarazel.de>
User-Agent: Mutt/1.9.4 (2018-02-28)
Archived-At: 
 <https://www.postgresql.org/message-id/20220310220611.GH28503%40telsasoft.com>
Precedence: bulk


--wLAMOaPNJ0fu1fTG
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

See attached, or at
https://github.com/justinpryzby/postgres/runs/5503079878

--wLAMOaPNJ0fu1fTG
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="0001-cirrus-include-hints-how-to-install-OS-packages.patch"

From c631c3d9bdb8325aaaecc5dcdfac46eca7bd907a Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Mon, 17 Jan 2022 00:53:04 -0600
Subject: [PATCH 1/7] cirrus: include hints how to install OS packages..

This is useful for patches during development, but once a feature is merged,
new libraries should be added to the OS image files, rather than installed
during every CI run forever into the future.
---
 .cirrus.yml | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/.cirrus.yml b/.cirrus.yml
index 40854046d66..6026a973dbb 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -73,10 +73,12 @@ task:
     chown -R postgres:postgres .
     mkdir -p ${CCACHE_DIR}
     chown -R postgres:postgres ${CCACHE_DIR}
-  setup_cores_script: |
+  setup_core_files_script: |
     mkdir -m 770 /tmp/cores
     chown root:postgres /tmp/cores
     sysctl kern.corefile='/tmp/cores/%N.%P.core'
+  setup_additional_packages_script: |
+    #pkg install -y ...
 
   # NB: Intentionally build without --with-llvm. The freebsd image size is
   # already large enough to make VM startup slow, and even without llvm
@@ -180,10 +182,13 @@ task:
     chown -R postgres:postgres ${CCACHE_DIR}
     echo '* - memlock 134217728' > /etc/security/limits.d/postgres.conf
     su postgres -c "ulimit -l -H && ulimit -l -S"
-  setup_cores_script: |
+  setup_core_files_script: |
     mkdir -m 770 /tmp/cores
     chown root:postgres /tmp/cores
     sysctl kernel.core_pattern='/tmp/cores/%e-%s-%p.core'
+  setup_additional_packages_script: |
+    #apt-get update
+    #apt-get -y install ...
 
   configure_script: |
     su postgres <<-EOF
@@ -237,7 +242,7 @@ task:
     ulimit -a -H && ulimit -a -S
     export
 
-  setup_cores_script:
+  setup_core_files_script:
     - mkdir ${HOME}/cores
     - sudo sysctl kern.corefile="${HOME}/cores/core.%P"
 
@@ -254,7 +259,7 @@ task:
   # packages do not need to be downloaded.
   homebrew_cache:
     folder: $HOMEBREW_CACHE
-  homebrew_install_script: |
+  setup_additional_packages_script: |
     brew install \
       ccache \
       icu4c \
@@ -389,6 +394,9 @@ task:
     powershell -Command get-psdrive -psprovider filesystem
     set
 
+  setup_additional_packages_script: |
+    REM choco install -y --no-progress ...
+
   configure_script:
     # copy errors out when using forward slashes
     - copy src\tools\ci\windows_build_config.pl src\tools\msvc\config.pl
@@ -484,6 +492,10 @@ task:
   ccache_cache:
     folder: $CCACHE_DIR
 
+  setup_additional_packages_script: |
+    #apt-get update
+    #apt-get -y install ...
+
   ###
   # Test that code can be built with gcc/clang without warnings
   ###
-- 
2.17.1


--wLAMOaPNJ0fu1fTG
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="0002-cirrus-compile-with-Og.patch"

From 9e5df76468c4ecc8f411f106c86d54c1f06f70f3 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sun, 27 Feb 2022 15:17:50 -0600
Subject: [PATCH 2/7] cirrus: compile with -Og..

To improve performance of check-world, and improve debugging, without
significantly slower builds (they're cached anyway).

This makes freebsd check-world run in 8.5 minutes rather than 15 minutes.
And on linux mitigates the run-time performance effect of --coverage.
---
 .cirrus.yml | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/.cirrus.yml b/.cirrus.yml
index 6026a973dbb..5179353dc9d 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -107,7 +107,7 @@ task:
         \
         CC="ccache cc" \
         CXX="ccache c++" \
-        CFLAGS="-O0 -ggdb"
+        CFLAGS="-Og -ggdb"
     EOF
   build_script: su postgres -c "gmake -s -j${BUILD_JOBS} world-bin"
   upload_caches: ccache
@@ -201,8 +201,8 @@ task:
         CC="ccache gcc" \
         CXX="ccache g++" \
         CLANG="ccache clang" \
-        CFLAGS="-O0 -ggdb" \
-        CXXFLAGS="-O0 -ggdb"
+        CFLAGS="-Og -ggdb" \
+        CXXFLAGS="-Og -ggdb"
     EOF
   build_script: su postgres -c "make -s -j${BUILD_JOBS} world-bin"
   upload_caches: ccache
@@ -315,8 +315,8 @@ task:
       CC="ccache cc" \
       CXX="ccache c++" \
       CLANG="ccache ${brewpath}/llvm/bin/ccache" \
-      CFLAGS="-O0 -ggdb" \
-      CXXFLAGS="-O0 -ggdb" \
+      CFLAGS="-Og -ggdb" \
+      CXXFLAGS="-Og -ggdb" \
       \
       LLVM_CONFIG=${brewpath}/llvm/bin/llvm-config \
       PYTHON=python3
-- 
2.17.1


--wLAMOaPNJ0fu1fTG
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="0003-cirrus-windows-add-compiler_warnings_script.patch"

From 92f64c297628ebb7026b6b61795444ea078720d2 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sun, 20 Feb 2022 15:01:59 -0600
Subject: [PATCH 3/7] 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 ||...

ci-os-only: windows

https://cirrus-ci.com/task/6183879907213312
https://cirrus-ci.com/task/4876271443247104
---
 .cirrus.yml                            |  9 ++++++++-
 src/tools/ci/windows-compiler-warnings | 16 ++++++++++++++++
 2 files changed, 24 insertions(+), 1 deletion(-)
 create mode 100755 src/tools/ci/windows-compiler-warnings

diff --git a/.cirrus.yml b/.cirrus.yml
index 5179353dc9d..a3af4a0a808 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -366,7 +366,8 @@ task:
     # ForceNoAlign prevents msbuild from introducing line-breaks for long lines
     # disable file tracker, we're never going to rebuild, and it slows down the
     #   build
-    MSBFLAGS: -m -verbosity:minimal "-consoleLoggerParameters:Summary;ForceNoAlign" /p:TrackFileAccess=false -nologo
+    # -fileLoggerParameters1: write warnings to msbuild.warn.log.
+    MSBFLAGS: -m -verbosity:minimal "-consoleLoggerParameters:Summary;ForceNoAlign" /p:TrackFileAccess=false -nologo -fileLoggerParameters1:warningsonly;logfile=msbuild.warn.log
 
     # If tests hang forever, cirrus eventually times out. In that case log
     # output etc is not uploaded, making the problem hard to debug. Of course
@@ -447,6 +448,12 @@ task:
     cd src/tools/msvc
     %T_C% perl vcregress.pl ecpgcheck
 
+  # These should be last, so all the important checks are always run
+  always:
+    # Success if the file doesn't exist or is empty, else fail
+    compiler_warnings_script:
+      - sh src\tools\ci\windows-compiler-warnings msbuild.warn.log
+
   on_failure:
     <<: *on_failure
     crashlog_artifacts:
diff --git a/src/tools/ci/windows-compiler-warnings b/src/tools/ci/windows-compiler-warnings
new file mode 100755
index 00000000000..d6f9a1fc569
--- /dev/null
+++ b/src/tools/ci/windows-compiler-warnings
@@ -0,0 +1,16 @@
+#! /bin/sh
+# Success if the given file doesn't exist or is empty, else fail
+# This is a separate file only to avoid dealing with windows shell quoting and escaping.
+set -e
+
+fn=$1
+
+if [ -s "$fn" ]
+then
+	# Display the file's content, then exit indicating failure
+	cat "$fn"
+	exit 1
+else
+	# Success
+	exit 0
+fi
-- 
2.17.1


--wLAMOaPNJ0fu1fTG
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="0004-cirrus-code-coverage.patch"

From c6e80a3058010652241294fd2e76da028bbf395e Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Mon, 17 Jan 2022 00:54:28 -0600
Subject: [PATCH 4/7] cirrus: code coverage

ci-os-only: linux
---
 .cirrus.yml                       | 20 ++++++++++++++++++--
 src/tools/ci/code-coverage-report | 25 +++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 2 deletions(-)
 create mode 100755 src/tools/ci/code-coverage-report

diff --git a/.cirrus.yml b/.cirrus.yml
index a3af4a0a808..a5c3b97925f 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -26,6 +26,13 @@ env:
   TEMP_CONFIG: ${CIRRUS_WORKING_DIR}/src/tools/ci/pg_ci_base.conf
   PG_TEST_EXTRA: kerberos ldap ssl
 
+  # The commit that this branch is rebased on.  There's no easy way to find this.
+  # This does the right thing for cfbot, which always squishes all patches into a single commit.
+  # And does the right thing for any 1-patch commits.
+  # Patches series manually submitted to cirrus may benefit from setting this to the
+  # number of patches in the series (or directly to the commit the series was rebased on).
+  BASE_COMMIT: HEAD~1
+
 
 # What files to preserve in case tests fail
 on_failure: &on_failure
@@ -175,6 +182,7 @@ task:
     cat /proc/cmdline
     ulimit -a -H && ulimit -a -S
     export
+    git diff --name-only "$BASE_COMMIT"
   create_user_script: |
     useradd -m postgres
     chown -R postgres:postgres .
@@ -187,13 +195,14 @@ task:
     chown root:postgres /tmp/cores
     sysctl kernel.core_pattern='/tmp/cores/%e-%s-%p.core'
   setup_additional_packages_script: |
-    #apt-get update
-    #apt-get -y install ...
+    apt-get update
+    apt-get -y install lcov
 
   configure_script: |
     su postgres <<-EOF
       ./configure \
         --enable-cassert --enable-debug --enable-tap-tests \
+        --enable-coverage \
         --enable-nls \
         \
         ${LINUX_CONFIGURE_FEATURES} \
@@ -213,6 +222,13 @@ task:
       make -s ${CHECK} ${CHECKFLAGS} -j${TEST_JOBS}
     EOF
 
+  # Build coverage report for files changed since the base commit.
+  generate_coverage_report_script: |
+    src/tools/ci/code-coverage-report "$BASE_COMMIT"
+
+  coverage_artifacts:
+    paths: ['coverage/**/*.html', 'coverage/**/*.png', 'coverage/**/*.gcov', 'coverage/**/*.css' ]
+
   on_failure:
     <<: *on_failure
     cores_script: src/tools/ci/cores_backtrace.sh linux /tmp/cores
diff --git a/src/tools/ci/code-coverage-report b/src/tools/ci/code-coverage-report
new file mode 100755
index 00000000000..57126247ea0
--- /dev/null
+++ b/src/tools/ci/code-coverage-report
@@ -0,0 +1,25 @@
+#! /bin/sh
+# Called during the linux CI task to generate a code coverage report.
+set -e
+
+base_branch=$1
+changed=`git diff --name-only "$base_branch" '*.c'`
+
+outdir=coverage
+mkdir "$outdir"
+
+# Coverage is shown only for changed files
+# This is useful to see coverage of newly-added code, but won't
+# show added/lost coverage in files which this patch doesn't modify.
+
+gcov=$outdir/coverage.gcov
+for f in $changed
+do
+	lcov --quiet --capture --directory "$f"
+done >"$gcov"
+
+# Exit successfully if no relevant files were changed
+[ -s "$gcov" ] || exit 0
+
+genhtml "$gcov" --show-details --legend --quiet --num-spaces=4 --output-directory "$outdir" --title="Coverage report of files changed since: $base_branch"
+cp "$outdir"/index.html "$outdir"/00-index.html
-- 
2.17.1


--wLAMOaPNJ0fu1fTG
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="0005-cirrus-build-docs-as-a-separate-task.patch"

From eaa5b000adea73e8355d2506c56cbfad04b980fb Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sat, 26 Feb 2022 19:34:35 -0600
Subject: [PATCH 5/7] cirrus: build docs as a separate task..

Because a failure in documentation should be shown even if Linux also fails
check-world.

This'll automatically show up as a separate "column" on cfbot.

Also, in the future, this will hopefully upload each patch's changed HTML docs
as an artifact, for easy review.

ci-os-only: html
---
 .cirrus.yml | 46 ++++++++++++++++++++++++++++++++--------------
 1 file changed, 32 insertions(+), 14 deletions(-)

diff --git a/.cirrus.yml b/.cirrus.yml
index a5c3b97925f..54ee1de4371 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -584,19 +584,37 @@ task:
       make -s -j${BUILD_JOBS} clean
       time make -s -j${BUILD_JOBS} world-bin
 
-  ###
-  # Verify docs can be built
-  ###
-  # 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" \
-        CXX="ccache g++" \
-        CLANG="ccache clang"
-      make -s -j${BUILD_JOBS} clean
-      time make -s -j${BUILD_JOBS} -C doc
-
   always:
     upload_caches: ccache
+
+
+###
+# Verify docs can be built
+###
+
+task:
+  name: Documentation
+
+  env:
+    CPUS: 1
+    BUILD_JOBS: 1
+
+  only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*(docs|html).*'
+  #skip: "!changesInclude('.cirrus.yml', 'doc/**')"
+
+  container:
+    image: $CONTAINER_REPO/linux_debian_bullseye_ci:latest
+    cpu: $CPUS
+    memory: 2G
+
+  sysinfo_script: |
+    id
+    uname -a
+    cat /proc/cmdline
+    ulimit -a -H && ulimit -a -S
+    export
+
+  # Exercise HTML and other docs:
+  docs_build_script: |
+    time ./configure
+    make -s -j${BUILD_JOBS} -C doc
-- 
2.17.1


--wLAMOaPNJ0fu1fTG
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="0006-cirrus-upload-changed-html-docs-as-artifacts.patch"

From 960c02a317296d45e62d8a97d512a6bb34ac3c21 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Sat, 26 Feb 2022 19:39:10 -0600
Subject: [PATCH 6/7] cirrus: upload changed html docs as artifacts

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.

https://cirrus-ci.com/task/5396696388599808

ci-os-only: html
---
 .cirrus.yml                    | 18 ++++++++++++++++--
 src/tools/ci/copy-changed-docs | 16 ++++++++++++++++
 2 files changed, 32 insertions(+), 2 deletions(-)
 create mode 100755 src/tools/ci/copy-changed-docs

diff --git a/.cirrus.yml b/.cirrus.yml
index 54ee1de4371..48e85b73649 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -589,7 +589,7 @@ task:
 
 
 ###
-# Verify docs can be built
+# Verify docs can be built, and (only on cfbot) upload changed docs as artifacts
 ###
 
 task:
@@ -600,7 +600,7 @@ task:
     BUILD_JOBS: 1
 
   only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*(docs|html).*'
-  #skip: "!changesInclude('.cirrus.yml', 'doc/**')"
+  #skip: "!changesInclude('.cirrus.yml', 'doc/**', 'src/tools/ci/copy-changed-docs')"
 
   container:
     image: $CONTAINER_REPO/linux_debian_bullseye_ci:latest
@@ -613,8 +613,22 @@ task:
     cat /proc/cmdline
     ulimit -a -H && ulimit -a -S
     export
+    git diff --name-only "$BASE_COMMIT"
 
   # Exercise HTML and other docs:
   docs_build_script: |
     time ./configure
     make -s -j${BUILD_JOBS} -C doc
+    cp -r doc new-docs
+
+    # Build HTML docs from the base commit.
+    git checkout "$BASE_COMMIT" -- doc
+    make -s -C doc clean
+    make -s -C doc html
+    cp -r doc old-docs
+
+  copy_changed_docs_script: |
+    src/tools/ci/copy-changed-docs
+
+  html_docs_artifacts:
+    paths: ['html_docs/*.html', 'html_docs/*.png', 'html_docs/*.css']
diff --git a/src/tools/ci/copy-changed-docs b/src/tools/ci/copy-changed-docs
new file mode 100755
index 00000000000..0a942838f5c
--- /dev/null
+++ b/src/tools/ci/copy-changed-docs
@@ -0,0 +1,16 @@
+#! /bin/sh
+# Copy HTML which differ into html_docs
+set -e
+
+outdir=html_docs
+
+mkdir "$outdir"
+cp new-docs/src/sgml/html/*.css new-docs/src/sgml/html/*.svg "$outdir/"
+
+changed=`git diff --no-index --name-only old-docs/src/sgml/html new-docs/src/sgml/html` ||
+	[ $? -eq 1 ]
+
+for f in $changed
+do
+	cp "$f" "$outdir"
+done
-- 
2.17.1


--wLAMOaPNJ0fu1fTG
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="0007-f-html-index-file.patch"

From c0c823f393924680880bbbedc16256c6533ff1a8 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Mon, 28 Feb 2022 23:18:19 -0600
Subject: [PATCH 7/7] f!html: index file

This allows linking to the artifacts from the last successful build, which
itself allows *not* rebuilding when sources haven't changed.

ci/os/only: html
---
 .cirrus.yml                    |  2 +-
 src/tools/ci/copy-changed-docs | 16 ++++++++++++++--
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/.cirrus.yml b/.cirrus.yml
index 48e85b73649..60f1528a9f1 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -628,7 +628,7 @@ task:
     cp -r doc old-docs
 
   copy_changed_docs_script: |
-    src/tools/ci/copy-changed-docs
+    src/tools/ci/copy-changed-docs "$CIRRUS_BRANCH"
 
   html_docs_artifacts:
     paths: ['html_docs/*.html', 'html_docs/*.png', 'html_docs/*.css']
diff --git a/src/tools/ci/copy-changed-docs b/src/tools/ci/copy-changed-docs
index 0a942838f5c..a23f4dca9b5 100755
--- a/src/tools/ci/copy-changed-docs
+++ b/src/tools/ci/copy-changed-docs
@@ -3,9 +3,15 @@
 set -e
 
 outdir=html_docs
+base_branch=$1
 
 mkdir "$outdir"
-cp new-docs/src/sgml/html/*.css new-docs/src/sgml/html/*.svg "$outdir/"
+cp new-docs/src/sgml/html/*.css new-docs/src/sgml/html/*.svg "$outdir"
+
+# The index is useful to allow a static link to the artifacts for the most-recent, successful CI run for a branch
+# https://api.cirrus-ci.com/v1/artifact/github/USERNAME/postgres/Documentation/html_docs/html_docs/00-doc.html?branch=BRANCH
+index="$outdir/00-doc.html"
+echo "<html><head><title>Index of docs changed since: $base_branch</title></head><body><ul>" >"$index"
 
 changed=`git diff --no-index --name-only old-docs/src/sgml/html new-docs/src/sgml/html` ||
 	[ $? -eq 1 ]
@@ -13,4 +19,10 @@ changed=`git diff --no-index --name-only old-docs/src/sgml/html new-docs/src/sgm
 for f in $changed
 do
 	cp "$f" "$outdir"
-done
+	fn=${f##*/}
+	# ?branch=... is needed for the static link for the branch
+	# It's not used if accessing artifacts for *this* CI run
+	echo "<li><a href='$fn?branch=$base_branch'>$fn</a>"
+done >>"$index"
+
+echo "</ul></body></html>" >>"$index"
-- 
2.17.1


--wLAMOaPNJ0fu1fTG--