On Sat, Feb 26, 2022 at 08:08:38PM -0800, Andres Freund wrote:
> On 2022-02-26 21:10:57 -0600, Justin Pryzby wrote:
> > If someone renames or removes an xref target, shouldn't CI fail on its next
> > run for a patch which tries to reference it ?
>
> Why wouldn't it?
I suppose you're right - I was thinking that cirrus was checking whether the
*patch* had changed any matching files, but it probably checks (as it should)
whether "the sources" have changed.
Hmm, it's behaving strangely...if there's a single argument ('docs/**'), then
it will skip the docs task if I resubmit it after git commit --amend --no-edit.
But with multiple args ('.cirrus.yaml', 'docs/**') it reruns it...
I tried it as skip: !changesInclude() and by adding it to the existing only_if:
(.. || ..) && changesInclude().
> > Are you sure about cfbot ? AIUI cirrus would see that docs didn't change
> > relative to the previous run for branch: commitfest/NN/MMMM.
>
> Not entirely sure, but it's what I remember observing when ammending commits
> in a repo using changesInclues. If I were to build a feature like it I'd look
> at the list of files of
> git diff $(git merge-base last_green new_commit)..new_commit
>
> or such. cfbot doesn't commit incrementally but replaces the prior commit, so
> I suspect it'll always be viewn as new. But who knows, shouldn't be hard to
> figure out.
Anyway...
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.
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.
(Actually, I'd prefer if it preserved the original patches as separate commits,
but that isn't what it does).
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.
--
Justin
PS. I've just done this. I'm unsure whether to say that it's wonderful or
terrible. This would certainly be better if each branch preserved the original
set of patches.
$ git remote add cfbot https://github.com/postgresql-cfbot/postgresql
$ git fetch cfbot
$ git branch -a |grep -c cfbot
5417
>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/6] 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/6] 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 83f21964508638bdd623a7d5f7efe54f682ef4de 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/6] 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: linux
---
.cirrus.yml | 23 +++++++++++++++++++++--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index 72671a66eda..1bb6463cb84 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -559,7 +559,7 @@ task:
###
-# Verify docs can be built
+# Verify docs can be built, and (only on cfbot) upload changed docs as artifacts
###
task:
@@ -570,7 +570,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 +583,27 @@ task:
cat /proc/cmdline
ulimit -a -H && ulimit -a -S
export
+ git diff --name-only HEAD~
# Exercise HTML and other docs:
docs_build_script: |
+ #git diff --name-only --cherry-pick --exit-code postgres/master... doc && exit
time ./configure
make -s -j${BUILD_JOBS} -C doc
+ cp -rl doc new-docs
+
+ # Build HTML docs from the previous commit. This logic is sufficient for cfbot, but not otherwise.
+ #echo "$CIRRUS_CHANGE_TITLE" |grep -Ev '^\[CF [0-9]+/[0-9]+\]' >/dev/null && echo skipping && exit
+ git log -1 --format='%an' |grep -Fxv 'Commitfest Bot' >/dev/null && echo skipping && exit
+ git checkout HEAD~ -- doc
+ make -s -C doc clean
+ make -s -C doc html
+ cp -rl doc old-docs
+
+ copy_changed_docs_script: |
+ #echo "$CIRRUS_CHANGE_TITLE" |grep -Ev '^\[CF [0-9]+/[0-9]+\]' >/dev/null && echo skipping && exit
+ git log -1 --format='%an' |grep -Fxv 'Commitfest Bot' >/dev/null && echo skipping && exit
+ src/tools/ci/copy-changed-docs
+
+ html_docs_artifacts:
+ paths: ['html_docs/**/*.html', 'html_docs/**/*.png', 'html_docs/**/*.css']
--
2.17.1
>From 3af717e80f48fe7e15c50863c0eaf9f88e2899f0 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Mon, 17 Jan 2022 00:54:28 -0600
Subject: [PATCH 4/6] 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 | 15 +++++++++++++--
src/tools/ci/coverage | 18 ++++++++++++++++++
2 files changed, 31 insertions(+), 2 deletions(-)
create mode 100755 src/tools/ci/coverage
diff --git a/.cirrus.yml b/.cirrus.yml
index 1bb6463cb84..8d8c2203e01 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -174,6 +174,7 @@ task:
cat /proc/cmdline
ulimit -a -H && ulimit -a -S
export
+ git diff --name-only HEAD~
create_user_script: |
useradd -m postgres
chown -R postgres:postgres .
@@ -185,13 +186,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} \
@@ -211,6 +213,15 @@ task:
make -s ${CHECK} ${CHECKFLAGS} -j${TEST_JOBS}
EOF
+ # Build coverage report for files changed in the previous commit.
+ # This logic is sufficient for cfbot, but not otherwise.
+ generate_coverage_report_script: |
+ git log -1 --format='%an' |grep -Fxv 'Commitfest Bot' >/dev/null && echo skipping && exit
+ src/tools/ci/coverage
+
+ 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/coverage b/src/tools/ci/coverage
new file mode 100755
index 00000000000..fe4adfe5648
--- /dev/null
+++ b/src/tools/ci/coverage
@@ -0,0 +1,18 @@
+#! /bin/sh
+# Called during the linux CI task to generate a code coverage report.
+set -e
+
+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.
+
+# This assumes a cfbot branch, which has all changes squished into a single patch.
+for f in `git diff --name-only HEAD~ '*.c'`
+do
+ lcov --quiet --capture --directory "$f"
+done >coverage/coverage.gcov
+
+genhtml coverage/coverage.gcov --output-directory coverage --show-details --legend --quiet
+cp coverage/index.html coverage/00-index.html
--
2.17.1
>From d676a404e3d3a789193aaa970c592a699fa40733 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Sun, 20 Feb 2022 15:01:59 -0600
Subject: [PATCH 5/6] 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 8d8c2203e01..1965d0fe685 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -375,7 +375,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
@@ -451,6 +452,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 201f2b974bd04ef9d7f590756a1d208d2e234364 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Sun, 27 Feb 2022 15:17:50 -0600
Subject: [PATCH 6/6] 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 1965d0fe685..adeef2d346e 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -106,7 +106,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
@@ -324,8 +324,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
@@ -378,6 +378,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
