On Mon, Feb 28, 2022 at 02:58:02PM -0600, Justin Pryzby wrote:
> I still think that if "Build Docs" is a separate cirrus task, it should
> rebuild
> docs on every CI run, even if they haven't changed, for any patch that touches
> docs/. It'll be confusing if cfbot shows 5 green circles and 4 of them were
> built 1 day ago, and 1 was built 3 weeks ago. Docs are the task that runs
> quickest, so I don't think it's worth doing anything special there (especially
> without understanding the behavior of changesInclude()).
>
> Also, to allow users to view the built HTML docs, cfbot would need to 1) keep
> track of previous CI runs; and 2) logic to handle "skipped" CI runs, to allow
> showing artifacts from the previous run. If it's not already done, I think
> the
> first half is a good idea on its own. But the 2nd part doesn't seem
> desirable.
Maybe changesInclude() could work if we use this URL (from cirrus'
documentation), which uses the artifacts from the last successful build.
https://api.cirrus-ci.com/v1/artifact/github/justinpryzby/postgres/Documentation/html_docs/html_docs/00-doc.html?branch=citest-cirrus2
That requires knowing the file being modified, so we'd have to generate an
index of changed files - which I've started doing here.
> However, I realized that we can filter on cfbot with either of these:
> | $CIRRUS_CHANGE_TITLE =~ '^\[CF...'
> | git log -1 |grep '^Author: Commitfest Bot <cf...@cputube.org>'
> If we can assume that cfbot will continue submitting branches as a single
> patch, this resolves the question of a "base branch", for cfbot.
I don't know what you think of that idea, but I think I want to amend my
proposal: show HTML and coverage artifacts for HEAD~1, unless set otherwise by
an environment var. Today, that'd do the right thing for cfbot, and also for
any 1-patch commits.
> These patches implement that idea, and make "code coverage" and "HTML diffs"
> stuff only run for cfbot commits. This still needs another round of testing,
> though.
The patch was missing a file due to an issue while rebasing - oops.
BTW (regarding the last patch), I just noticed that -Og optimization can cause
warnings with gcc-4.8.5-39.el7.x86_64.
be-fsstubs.c: In function 'be_lo_export':
be-fsstubs.c:522:24: warning: 'fd' may be used uninitialized in this function
[-Wmaybe-uninitialized]
if (CloseTransientFile(fd) != 0)
^
trigger.c: In function 'ExecCallTriggerFunc':
trigger.c:2400:2: warning: 'result' may be used uninitialized in this function
[-Wmaybe-uninitialized]
return (HeapTuple) DatumGetPointer(result);
^
xml.c: In function 'xml_pstrdup_and_free':
xml.c:1205:2: warning: 'result' may be used uninitialized in this function
[-Wmaybe-uninitialized]
return result;
--
Justin
>From 883edaa653bcf7f1a2369d8edf46eaaac1ba0ba2 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@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 | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index d10b0a82f9f..1b7c36283e9 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -73,10 +73,11 @@ task:
chown -R postgres:postgres .
mkdir -p ${CCACHE_DIR}
chown -R postgres:postgres ${CCACHE_DIR}
- setup_cores_script: |
+ setup_os_script: |
mkdir -m 770 /tmp/cores
chown root:postgres /tmp/cores
sysctl kern.corefile='/tmp/cores/%N.%P.core'
+ #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 +181,12 @@ 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_os_script: |
mkdir -m 770 /tmp/cores
chown root:postgres /tmp/cores
sysctl kernel.core_pattern='/tmp/cores/%e-%s-%p.core'
+ #apt-get update
+ #apt-get -y install ...
configure_script: |
su postgres <<-EOF
@@ -237,7 +240,7 @@ task:
ulimit -a -H && ulimit -a -S
export
- setup_cores_script:
+ setup_os_script:
- mkdir ${HOME}/cores
- sudo sysctl kern.corefile="${HOME}/cores/core.%P"
@@ -384,6 +387,9 @@ task:
powershell -Command get-psdrive -psprovider filesystem
set
+ setup_os_script: |
+ REM choco install -y ...
+
configure_script:
# copy errors out when using forward slashes
- copy src\tools\ci\windows_build_config.pl src\tools\msvc\config.pl
@@ -479,6 +485,10 @@ task:
ccache_cache:
folder: $CCACHE_DIR
+ setup_os_script: |
+ #apt-get update
+ #apt-get -y install ...
+
###
# Test that code can be built with gcc/clang without warnings
###
--
2.17.1
>From 12127d5e4fe9dfccafa12d70311e58f91c34abec Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Sat, 26 Feb 2022 19:34:35 -0600
Subject: [PATCH 2/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 1b7c36283e9..72671a66eda 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -554,19 +554,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
>From 4a64b4306d5d6157a0ee8c3795005068541cdda4 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Sat, 26 Feb 2022 19:39:10 -0600
Subject: [PATCH 3/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 | 25 +++++++++++++++++++++++--
src/tools/ci/copy-changed-docs | 16 ++++++++++++++++
2 files changed, 39 insertions(+), 2 deletions(-)
create mode 100755 src/tools/ci/copy-changed-docs
diff --git a/.cirrus.yml b/.cirrus.yml
index 72671a66eda..e168a52a85a 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
@@ -559,7 +566,7 @@ task:
###
-# Verify docs can be built
+# Verify docs can be built, and (only on cfbot) upload changed docs as artifacts
###
task:
@@ -570,7 +577,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
@@ -583,8 +590,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
>From 3eb756f669aadfe065d96561817aae39a8b24a75 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Mon, 28 Feb 2022 23:18:19 -0600
Subject: [PATCH 4/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
---
src/tools/ci/copy-changed-docs | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/src/tools/ci/copy-changed-docs b/src/tools/ci/copy-changed-docs
index 0a942838f5c..ca6214d40a6 100755
--- a/src/tools/ci/copy-changed-docs
+++ b/src/tools/ci/copy-changed-docs
@@ -7,10 +7,19 @@ outdir=html_docs
mkdir "$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 all changed docs
+# 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 changed docs</title></head><body><ul>" >"$index"
+
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
+ fn=${f##*/}
+ echo "<li><a href='$fn'>$fn</a>"
+done >>"$index"
+
+echo "</ul></body></html>" >>"$index"
--
2.17.1
>From b8733e960daa678ef4d568ffaa7aae0610166fb4 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Mon, 17 Jan 2022 00:54:28 -0600
Subject: [PATCH 5/7] wip: cirrus: code coverage
XXX: lcov should be installed in the OS image
XXX: Use --num-spaces=4 like in src/Makefile.global.in ?
ci-os-only: linux
---
.cirrus.yml | 13 +++++++++++--
src/tools/ci/code-coverage-report | 24 ++++++++++++++++++++++++
2 files changed, 35 insertions(+), 2 deletions(-)
create mode 100755 src/tools/ci/code-coverage-report
diff --git a/.cirrus.yml b/.cirrus.yml
index e168a52a85a..d9136365724 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -181,6 +181,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 .
@@ -192,13 +193,14 @@ task:
mkdir -m 770 /tmp/cores
chown root:postgres /tmp/cores
sysctl kernel.core_pattern='/tmp/cores/%e-%s-%p.core'
- #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} \
@@ -218,6 +220,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..c09a159704b
--- /dev/null
+++ b/src/tools/ci/code-coverage-report
@@ -0,0 +1,24 @@
+#! /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'`
+
+mkdir coverage
+
+# Coverage 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=coverage/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" --output-directory coverage --show-details --legend --quiet
+cp coverage/index.html coverage/00-index.html
--
2.17.1
>From feceea4413b84f478e6a0888cdfab4be1c80767a Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Sun, 20 Feb 2022 15:01:59 -0600
Subject: [PATCH 6/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 ||...
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 d9136365724..6f05d420c85 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -380,7 +380,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 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
@@ -456,6 +457,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
>From d180953d273c221a30c5e9ad8d74b1b4dfc60bd1 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Sun, 27 Feb 2022 15:17:50 -0600
Subject: [PATCH 7/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.
---
.cirrus.yml | 12 +++++++-----
src/tools/msvc/MSBuildProject.pm | 4 ++--
2 files changed, 9 insertions(+), 7 deletions(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index 6f05d420c85..8b673bf58cf 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -113,7 +113,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
@@ -208,8 +208,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
@@ -329,8 +329,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
@@ -383,6 +383,8 @@ task:
# -fileLoggerParameters1: write to msbuild.warn.log.
MSBFLAGS: -m -verbosity:minimal "-consoleLoggerParameters:Summary;ForceNoAlign" /p:TrackFileAccess=false -nologo -fileLoggerParameters1:warningsonly;logfile=msbuild.warn.log
+ MSBUILD_OPTIMIZE: MaxSpeed
+
# 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
# tests internally should have shorter timeouts, but that's proven to not
diff --git a/src/tools/msvc/MSBuildProject.pm b/src/tools/msvc/MSBuildProject.pm
index 5e312d232e9..05e0c41eb5c 100644
--- a/src/tools/msvc/MSBuildProject.pm
+++ b/src/tools/msvc/MSBuildProject.pm
@@ -85,7 +85,7 @@ EOF
$f, 'Debug',
{
defs => "_DEBUG;DEBUG=1",
- opt => 'Disabled',
+ opt => $ENV{MSBUILD_OPTIMIZE} || 'Disabled',
strpool => 'false',
runtime => 'MultiThreadedDebugDLL'
});
@@ -94,7 +94,7 @@ EOF
'Release',
{
defs => "",
- opt => 'Full',
+ opt => $ENV{MSBUILD_OPTIMIZE} || 'Full',
strpool => 'true',
runtime => 'MultiThreadedDLL'
});
--
2.17.1
