On Wed, Jan 31, 2024 at 11:59:21AM +0100, Alvaro Herrera wrote:
> On 2024-Jan-31, vignesh C wrote:
>
> > Are we planning to do anything more on this? I was not sure if we
> > should move this to next commitfest or return it.
>
> Well, the patches don't apply anymore since .cirrus.tasks.yml has been
> created. However, I'm sure we still want [some of] the improvements
> to the tests in [1]. I can volunteer to rebase the patches in time for the
> March commitfest, if Justin is not available to do so. If you can
> please move it forward to the March cf and set it WoA, I'd appreciate
> that.
The patches are rebased. A couple were merged since I last rebased them
~10 months ago. The freebsd patch will probably be obsoleted by a patch
of Thomas.
On Mon, Mar 13, 2023 at 07:39:52AM +0100, Peter Eisentraut wrote:
> On 03.02.23 15:26, Justin Pryzby wrote:
> > 9baf41674ad pg_upgrade: tap test: exercise --link and --clone
>
> This seems like a good idea.
On Wed, Mar 15, 2023 at 10:58:41AM +0100, Peter Eisentraut wrote:
> > [PATCH 4/8] pg_upgrade: tap test: exercise --link and --clone
>
> I haven't been able to get any changes to the test run times outside
> of noise from this. But some more coverage is sensible in any case.
>
> I'm concerned that with this change, the only platform that tests
> --copy is Windows, but Windows has a separate code path for copy. So
> we should leave one Unix platform to test --copy. Maybe have FreeBSD
> test --link and macOS test --clone and leave the others with --copy?
I addressed Peter's comments, but haven't heard further.
The patch to show HTML docs artifacts may be waiting for Andres' patch
to convert CompilerWarnings to meson.
It may also be waiting on cfbot to avoid squishing all the patches
together.
I sent various patches to cfbot but haven't heard back.
https://www.postgresql.org/message-id/flat/20220409021853.gp24...@telsasoft.com
https://www.postgresql.org/message-id/flat/20220623193125.gb22...@telsasoft.com
https://github.com/justinpryzby/cfbot/commits/master
https://github.com/macdice/cfbot/pulls
--
Justin
>From 37a697bf2ff2e9e24c7b8cb2df289f21e1fca924 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Wed, 25 May 2022 21:53:22 -0500
Subject: [PATCH 1/6] cirrus/windows: add compiler_warnings_script
The goal is to fail due to warnings only after running tests.
(At least historically, it's too slow to run a separate windows VM to
compile with -Werror.)
https://www.postgresql.org/message-id/20220212212310.f645c6vw3njkgxka%40alap3.anarazel.de
I'm not sure how to write this test in windows shell; it's also easy to
write something that doesn't work in posix sh, since windows shell is
interpretting && and ||...
See also:
8a1ce5e54f6d144e4f8e19af7c767b026ee0c956
https://cirrus-ci.com/task/6241060062494720
https://cirrus-ci.com/task/6496366607204352
ci-os-only: windows
---
.cirrus.tasks.yml | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml
index e4e1bcfeb99..2d397448851 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -561,12 +561,21 @@ task:
build_script: |
vcvarsall x64
- ninja -C build
+ ninja -C build |tee build.txt
+ REM Since pipes lose the exit status of the preceding command, rerun the compilation
+ REM without the pipe, exiting now if it fails, to avoid trying to run checks
+ ninja -C build > nul
check_world_script: |
vcvarsall x64
meson test %MTEST_ARGS% --num-processes %TEST_JOBS%
+ # This should be last, so check_world is run even if there are warnings
+ always:
+ compiler_warnings_script:
+ # this avoids using metachars which would be interpretted by the windows shell
+ - sh -c 'if grep ": warning " build.txt; then exit 1; fi; exit 0'
+
on_failure:
<<: *on_failure_meson
crashlog_artifacts:
--
2.42.0
>From 71d3bdad00bd0a4967db5a394247a54eb0f8a1fa Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Sat, 30 Jul 2022 19:25:51 -0500
Subject: [PATCH 2/6] cirrus/002_pg_upgrade: exercise --link and --clone
This increases code coverage (and maybe accelerates the test).
See also: b059a2409faf5833b3ba7792e247d6466c9e8090
linux,
macos,
//-os-only: freebsd
---
.cirrus.tasks.yml | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml
index 2d397448851..53be0ce15e4 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -138,6 +138,8 @@ task:
CPPFLAGS: -DRELCACHE_FORCE_RELEASE -DCOPY_PARSE_PLAN_TREES -DWRITE_READ_PARSE_PLAN_TREES -DRAW_EXPRESSION_COVERAGE_TEST -DENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS
CFLAGS: -Og -ggdb
+ PG_TEST_PG_UPGRADE_MODE: --link
+
<<: *freebsd_task_template
depends_on: SanityCheck
@@ -431,6 +433,8 @@ task:
CFLAGS: -Og -ggdb
CXXFLAGS: -Og -ggdb
+ PG_TEST_PG_UPGRADE_MODE: --clone
+
<<: *macos_task_template
depends_on: SanityCheck
--
2.42.0
>From 78dd05a15f1b0f476eaa29c8f8ceb70964e009a0 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Fri, 25 Nov 2022 13:57:17 -0600
Subject: [PATCH 3/6] WIP: ci/meson: allow showing only failed tests ..
It's simpler and seems to make more sense to integrate this with
testwrap, rather than to run it after check-world, but only if it
failed, and finding a way to preserve the exit code.
https://www.postgresql.org/message-id/20221114235328.lxdj3puenfhir...@awork3.anarazel.de
> It is wasteful to upload thousdands of logfiles to show a single
> failure. That would make our cirrus tasks faster - compressing and
> uploading the logs takes over a minute.
>
> It's also a lot friendlier to show fewer than 8 pages of test folders to
> search through to find the one that failed.
macos
linux-meson
ci-os-only: freebsd
---
.cirrus.tasks.yml | 24 ++++++++++++++++++------
src/tools/testwrap | 6 ++++++
2 files changed, 24 insertions(+), 6 deletions(-)
diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml
index 53be0ce15e4..c0d72998199 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -22,6 +22,17 @@ env:
TEMP_CONFIG: ${CIRRUS_WORKING_DIR}/src/tools/ci/pg_ci_base.conf
PG_TEST_EXTRA: kerberos ldap ssl load_balance
+ PG_FAILED_TESTDIR: ${CIRRUS_WORKING_DIR}/failed.build
+
+ # 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.
+ # Patch series manually submitted to cirrus would 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
+ # For demo purposes:
+ BASE_COMMIT: HEAD~11
+
# What files to preserve in case tests fail
on_failure_ac: &on_failure_ac
@@ -35,9 +46,9 @@ on_failure_ac: &on_failure_ac
on_failure_meson: &on_failure_meson
testrun_artifacts:
paths:
- - "build*/testrun/**/*.log"
- - "build*/testrun/**/*.diffs"
- - "build*/testrun/**/regress_log_*"
+ - failed.build*/**/*.log
+ - failed.build*/**/*.diffs
+ - failed.build*/**/regress_log_*
type: text/plain
# In theory it'd be nice to upload the junit files meson generates, so that
@@ -138,6 +149,7 @@ task:
CPPFLAGS: -DRELCACHE_FORCE_RELEASE -DCOPY_PARSE_PLAN_TREES -DWRITE_READ_PARSE_PLAN_TREES -DRAW_EXPRESSION_COVERAGE_TEST -DENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS
CFLAGS: -Og -ggdb
+ PG_FAILED_TESTDIR: ${CIRRUS_WORKING_DIR}/failed.build
PG_TEST_PG_UPGRADE_MODE: --link
<<: *freebsd_task_template
@@ -196,10 +208,10 @@ task:
ulimit -c unlimited
meson test $MTEST_ARGS --quiet --suite setup
export LD_LIBRARY_PATH="$(pwd)/build/tmp_install/usr/local/pgsql/lib/:$LD_LIBRARY_PATH"
- mkdir -p build/testrun
+ mkdir -p build/testrun ${PG_FAILED_TESTDIR}
build/tmp_install/usr/local/pgsql/bin/initdb -N build/runningcheck --no-instructions -A trust
echo "include '$(pwd)/src/tools/ci/pg_ci_base.conf'" >> build/runningcheck/postgresql.conf
- build/tmp_install/usr/local/pgsql/bin/pg_ctl -c -o '-c fsync=off' -D build/runningcheck -l build/testrun/runningcheck.log start
+ build/tmp_install/usr/local/pgsql/bin/pg_ctl -c -o '-c fsync=off' -D build/runningcheck -l ${PG_FAILED_TESTDIR}/runningcheck.log start
meson test $MTEST_ARGS --num-processes ${TEST_JOBS} --setup running
build/tmp_install/usr/local/pgsql/bin/pg_ctl -D build/runningcheck stop
EOF
@@ -402,7 +414,7 @@ task:
test_world_32_script: |
su postgres <<-EOF
ulimit -c unlimited
- PYTHONCOERCECLOCALE=0 LANG=C meson test $MTEST_ARGS -C build-32 --num-processes ${TEST_JOBS}
+ PYTHONCOERCECLOCALE=0 LANG=C PG_FAILED_TESTDIR=`pwd`/failed.build-32 meson test $MTEST_ARGS -C build-32 --num-processes ${TEST_JOBS}
EOF
on_failure:
diff --git a/src/tools/testwrap b/src/tools/testwrap
index d01e61051cb..7e5a17dbf94 100755
--- a/src/tools/testwrap
+++ b/src/tools/testwrap
@@ -49,4 +49,10 @@ if sp.returncode == 0:
else:
print('# test failed')
open(os.path.join(testdir, 'test.fail'), 'x')
+ faileddir = os.getenv('PG_FAILED_TESTDIR')
+ if faileddir:
+ parentdir = os.path.dirname(testdir)
+ newdest = os.path.join(faileddir, os.path.basename(parentdir), os.path.basename(testdir))
+ shutil.copytree(testdir, newdest)
+
sys.exit(sp.returncode)
--
2.42.0
>From 896a72c9e193a3ad0bdb5d1d8872156c18c38d73 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Sat, 26 Feb 2022 19:39:10 -0600
Subject: [PATCH 4/6] WIP: cirrus: upload changed html docs as artifacts
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://www.postgresql.org/message-id/flat/20220409021853.gp24...@telsasoft.com
https://www.postgresql.org/message-id/CAB8KJ=i4qmeuopq+pcsmbzgd4o-xv0fcnc+q1x7hn9hsdvk...@mail.gmail.com
https://cirrus-ci.com/task/5396696388599808
//-os-only:
---
.cirrus.tasks.yml | 18 ++++++++++++++++--
src/tools/ci/copy-changed-docs | 29 +++++++++++++++++++++++++++++
2 files changed, 45 insertions(+), 2 deletions(-)
create mode 100755 src/tools/ci/copy-changed-docs
diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml
index c0d72998199..8774db82291 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -31,7 +31,7 @@ env:
# number of patches in the series (or directly to the commit the series was rebased on).
#BASE_COMMIT: HEAD~1
# For demo purposes:
- BASE_COMMIT: HEAD~11
+ BASE_COMMIT: HEAD~55
# What files to preserve in case tests fail
@@ -767,7 +767,7 @@ task:
time make -s -j${BUILD_JOBS} world-bin
###
- # Verify docs can be built
+ # Verify docs can be built, and upload changed docs as artifacts
###
# XXX: Only do this if there have been changes in doc/ since last build
always:
@@ -779,6 +779,20 @@ task:
CLANG="ccache clang"
make -s -j${BUILD_JOBS} clean
time make -s -j${BUILD_JOBS} -C doc
+ cp -r doc new-docs
+
+ # Re-build HTML docs from the base commit.
+ git checkout "$BASE_COMMIT" -- doc
+ make -s -C doc clean
+ time make -s -C doc html
+ cp -r doc old-docs
+
+ copy_changed_docs_script:
+ - src/tools/ci/copy-changed-docs "old-docs" "new-docs" "html_docs"
+
+ html_docs_artifacts:
+ paths: ['html_docs/*.html', 'html_docs/*.png', 'html_docs/*.css']
+
###
# Verify headerscheck / cpluspluscheck succeed
diff --git a/src/tools/ci/copy-changed-docs b/src/tools/ci/copy-changed-docs
new file mode 100755
index 00000000000..1c921a8df6f
--- /dev/null
+++ b/src/tools/ci/copy-changed-docs
@@ -0,0 +1,29 @@
+#! /bin/sh
+# Copy HTML which differ between $old and $new into $outdir
+set -e
+
+old=$1
+new=$2
+outdir=$3
+
+# The index is large and changes often
+skippages="bookindex.html"
+
+mkdir "$outdir"
+cp "$new"/src/sgml/html/*.css "$new"/src/sgml/html/*.svg "$outdir"
+
+changed=`git diff --no-index --name-only "$old"/src/sgml/html "$new"/src/sgml/html` ||
+ [ $? -eq 1 ]
+
+for f in $changed
+do
+ # Avoid removed files
+ [ -f "$f" ] || continue
+
+ echo "$f" |grep -Ew "$skippages" >/dev/null &&
+ continue
+
+ cp -v "$f" "$outdir"
+done
+
+exit 0
--
2.42.0
>From 0c7778bd46f295b01fedd83daf1b57c73ead1b26 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Mon, 28 Feb 2022 23:18:19 -0600
Subject: [PATCH 5/6] +html index file
This allows linking to the artifacts from the last successful build.
//freebsd
ci-os-only: warnings
---
src/tools/ci/copy-changed-docs | 39 ++++++++++++++++++++++++++++++++--
1 file changed, 37 insertions(+), 2 deletions(-)
diff --git a/src/tools/ci/copy-changed-docs b/src/tools/ci/copy-changed-docs
index 1c921a8df6f..0efad386cca 100755
--- a/src/tools/ci/copy-changed-docs
+++ b/src/tools/ci/copy-changed-docs
@@ -5,6 +5,7 @@ set -e
old=$1
new=$2
outdir=$3
+branch=$CIRRUS_BRANCH
# The index is large and changes often
skippages="bookindex.html"
@@ -12,6 +13,19 @@ skippages="bookindex.html"
mkdir "$outdir"
cp "$new"/src/sgml/html/*.css "$new"/src/sgml/html/*.svg "$outdir"
+# The index is useful to allow a static link (not specific to a cirrus run) to the artifacts for the most-recent, successful CI run for a branch
+branchurl=https://api.cirrus-ci.com/v1/artifact/github/$CIRRUS_REPO_FULL_NAME/$CIRRUS_TASK_NAME/html_docs/html_docs/00-index.html?branch=$branch
+
+index="$outdir/00-index.html"
+cat >"$index" <<EOF
+<html>
+<head><title>Index of docs changed since: $BASE_COMMIT</title></head>
+<body>
+<!-- A link to documentation for the most recent successful build: $branchurl -->
+<h1>Index of docs changed since: $BASE_COMMIT</h1>
+<ul>
+EOF
+
changed=`git diff --no-index --name-only "$old"/src/sgml/html "$new"/src/sgml/html` ||
[ $? -eq 1 ]
@@ -23,7 +37,28 @@ do
echo "$f" |grep -Ew "$skippages" >/dev/null &&
continue
- cp -v "$f" "$outdir"
-done
+ cp -v "$f" "$outdir" >&2
+ fn=${f##*/}
+ # ?branch=... is needed when accessing the artifacts for the static link for the branch
+ # It's not needed and ignored if accessing artifacts for *this* CI run
+ echo "<li><a href='$fn?branch=$branch'>$fn</a>"
+done >>"$index"
+
+github=https://github.com/$CIRRUS_REPO_FULL_NAME/commit
+cirrus=https://cirrus-ci.com/build
+
+cat >>"$index" <<EOF
+</ul>
+<hr>
+<code>
+<br>This file was written on: `date --rfc-822 --utc`
+<br>CIRRUS_CHANGE_TITLE: $CIRRUS_CHANGE_TITLE
+<br>CIRRUS_CHANGE_IN_REPO: <a href="$github/$CIRRUS_CHANGE_IN_REPO">$CIRRUS_CHANGE_IN_REPO</a>
+<br>CIRRUS_BUILD_ID: <a href="$cirrus/$CIRRUS_BUILD_ID">$CIRRUS_BUILD_ID</a>
+<br>CIRRUS_LAST_GREEN_CHANGE: <a href="$github/$CIRRUS_LAST_GREEN_CHANGE">$CIRRUS_LAST_GREEN_CHANGE</a>
+<br>CIRRUS_LAST_GREEN_BUILD_ID: <a href="$cirrus/$CIRRUS_LAST_GREEN_BUILD_ID">$CIRRUS_LAST_GREEN_BUILD_ID</a>
+</code>
+</body></html>
+EOF
exit 0
--
2.42.0
>From d1aa01727dc4e0caee74d54c1c4b5061c627ffa0 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Wed, 19 Oct 2022 20:33:28 -0500
Subject: [PATCH 6/6] WIP: show changed docs with meson
//-os-only:
---
.cirrus.tasks.yml | 26 ++++++++++++++++++++++----
src/tools/ci/copy-changed-docs | 3 ++-
2 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml
index 8774db82291..342761c146e 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -772,20 +772,38 @@ task:
# XXX: Only do this if there have been changes in doc/ since last build
always:
docs_build_script: |
- time ./configure \
+ mkdir build-autoconf
+ cd build-autoconf
+ time ../configure \
--cache gcc.cache \
+ --without-icu \
CC="ccache gcc" \
CXX="ccache g++" \
CLANG="ccache clang"
make -s -j${BUILD_JOBS} clean
time make -s -j${BUILD_JOBS} -C doc
- cp -r doc new-docs
+ cp -r doc ../new-docs
# Re-build HTML docs from the base commit.
- git checkout "$BASE_COMMIT" -- doc
+ git checkout "$BASE_COMMIT" -- ../doc
make -s -C doc clean
time make -s -C doc html
- cp -r doc old-docs
+ cp -r doc ../old-docs
+
+ # Exercise HTML and other docs:
+ ninja_docs_build_script: |
+ make maintainer-clean # XXX not needed once compiler-warnings is switched meson
+ mkdir build-ninja
+ cd build-ninja
+ time meson setup
+ time ninja docs
+ cp -r doc ../new-docs
+
+ # Re-build HTML docs from the base commit.
+ git checkout "$BASE_COMMIT" -- ../doc
+ ninja clean
+ time ninja doc/src/sgml/html
+ cp -r doc ../old-docs
copy_changed_docs_script:
- src/tools/ci/copy-changed-docs "old-docs" "new-docs" "html_docs"
diff --git a/src/tools/ci/copy-changed-docs b/src/tools/ci/copy-changed-docs
index 0efad386cca..ee3135c298b 100755
--- a/src/tools/ci/copy-changed-docs
+++ b/src/tools/ci/copy-changed-docs
@@ -11,7 +11,8 @@ branch=$CIRRUS_BRANCH
skippages="bookindex.html"
mkdir "$outdir"
-cp "$new"/src/sgml/html/*.css "$new"/src/sgml/html/*.svg "$outdir"
+cp doc/src/sgml/*.css "$outdir"
+cp doc/src/sgml/images/*.svg "$outdir"
# The index is useful to allow a static link (not specific to a cirrus run) to the artifacts for the most-recent, successful CI run for a branch
branchurl=https://api.cirrus-ci.com/v1/artifact/github/$CIRRUS_REPO_FULL_NAME/$CIRRUS_TASK_NAME/html_docs/html_docs/00-index.html?branch=$branch
--
2.42.0
