On Tue, Jun 18, 2024 at 02:25:46PM -0700, Andres Freund wrote:
> > > If not, we can just install the binaries distributed by the project, it's
> > > just more work to keep up2date that way.
> >
> > I guess you mean a separate line to copy choco's ccache.exe somewhere.
>
> I was thinking of alternatively just using the binaries upstream
> distributes. But the mingw way seems easier.
>
> Perhaps we should add an environment variable pointing to ccache to the image,
> or such?
I guess you mean changing the OS image so there's a $CCACHE.
That sounds fine...
> > + CCACHE_MAXSIZE: "500M"
>
> Does it have to be this big to work?
It's using 150MB for an initial compilation, and maxsize will need to be
20% larger for it to not evict its cache before it can be used.
The other ccaches (except for mingw) seem to be several times larger
than what's needed for a single compilation, which makes sense to cache
across multiple branches (or even commits in a single branch), and for
cfbot.
> A comment explaining why /Z7 is necessary would be good.
Sure
> > 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
>
> Perhaps we could do something like
> (ninja -C build && touch success) | tee build.txt
> cat success
> ?
I don't know -- a pipe alone seems more direct than a
subshell+conditional+pipe written in cmd.exe, whose syntax is not well
known here.
Maybe you're suggesting to write
sh -c "(ninja -C build && touch success) | tee build.txt ; cat ./success"
But that's another layer of complexity .. for what ?
> > Subject: [PATCH 4/7] WIP: cirrus: upload changed html docs as artifacts
>
> I still think that this runs the risk of increasing space utilization and thus
> increase frequency of caches/artifacts being purged.
Maybe it should run on the local macs where I think you can control
that.
> > + # The commit that this branch is rebased on. There's no easy way to
> > find this.
>
> I don't think that's true, IIRC I've complained about it before. We can do
> something like:
cfbot now exposes it, so it'd be trivial for that case (although there
was no response here to my inquiries about that). I'll revisit this in
the future, once other patches have progressed.
> major_num=$(grep PG_MAJORVERSION_NUM build/src/include/pg_config.h|awk
> '{print $3}');
> echo major: $major_num;
> if git rev-parse --quiet --verify origin/REL_${major_num}_STABLE > /dev/null
> ; then
> basebranch=origin/REL_${major_num}_STABLE;
> else
> basebranch=origin/master;
> fi;
> echo base branch: $basebranch
> base_commit=$(git merge-base HEAD $basebranch)
>From 0cf4fa490b19aea8e536506e975f1e9b61be6574 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
20221104161934.gb16...@telsasoft.com
20221031223744.gt16...@telsasoft.com
20221104161934.gb16...@telsasoft.com
20221123054329.gg11...@telsasoft.com
20230224002029.gq1...@telsasoft.com
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 1ce6c443a8c..5dd37e07aae 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -576,12 +576,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 764987ff4093783e967c13f55dee60fb6d89b4a0 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Wed, 25 May 2022 22:05:13 -0500
Subject: [PATCH 2/6] cirrus/windows: ccache
ccache 4.7 added support for depend mode with MSVC, and ccache v4.10
supports PCH, so this is now viable.
https://www.postgresql.org/message-id/flat/20220522232606.GZ19626%40telsasoft.com
https://cirrus-ci.com/task/5213936495099904 build 29sec
---
.cirrus.tasks.yml | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml
index 5dd37e07aae..fb4ea3d666a 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -546,6 +546,12 @@ task:
env:
TEST_JOBS: 8 # wild guess, data based value welcome
+ CC: c:\msys64\ucrt64\bin\ccache.exe cl.exe
+ CCACHE_DIR: $CIRRUS_WORKING_DIR/.ccache
+ CCACHE_MAXSIZE: "500M"
+ CCACHE_SLOPPINESS: pch_defines,time_macros
+ CCACHE_DEPEND: 1
+
# Cirrus defaults to SetErrorMode(SEM_NOGPFAULTERRORBOX | ...). That
# prevents crash reporting from working unless binaries do SetErrorMode()
# themselves. Furthermore, it appears that either python or, more likely,
@@ -563,6 +569,9 @@ task:
setup_additional_packages_script: |
REM choco install -y --no-progress ...
+ ccache_cache:
+ folder: $CCACHE_DIR
+
setup_hosts_file_script: |
echo 127.0.0.1 pg-loadbalancetest >> c:\Windows\System32\Drivers\etc\hosts
echo 127.0.0.2 pg-loadbalancetest >> c:\Windows\System32\Drivers\etc\hosts
@@ -570,9 +579,10 @@ task:
type c:\Windows\System32\Drivers\etc\hosts
# Use /DEBUG:FASTLINK to avoid high memory usage during linking
+ # Use /Z7 to write separate debug files, to allow ccache to work
configure_script: |
vcvarsall x64
- meson setup --backend ninja --buildtype debug -Dc_link_args=/DEBUG:FASTLINK -Dcassert=true -Dinjection_points=true -Db_pch=true -Dextra_lib_dirs=c:\openssl\1.1\lib -Dextra_include_dirs=c:\openssl\1.1\include -DTAR=%TAR% -DPG_TEST_EXTRA="%PG_TEST_EXTRA%" build
+ meson setup build --backend ninja --buildtype debug -Dc_link_args=/DEBUG:FASTLINK -Dcassert=true -Dinjection_points=true -Db_pch=true -Dextra_lib_dirs=c:\openssl\1.1\lib -Dextra_include_dirs=c:\openssl\1.1\include -DTAR=%TAR% -DPG_TEST_EXTRA="%PG_TEST_EXTRA%" -Dc_args="/Z7"
build_script: |
vcvarsall x64
@@ -581,6 +591,9 @@ task:
REM without the pipe, exiting now if it fails, to avoid trying to run checks
ninja -C build > nul
+ always:
+ upload_caches: [ccache]
+
check_world_script: |
vcvarsall x64
meson test %MTEST_ARGS% --num-processes %TEST_JOBS%
--
2.42.0
>From 75c304b2282d4252c7ad41c95bf8ee77b08f53e4 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 | 14 ++++++++------
src/tools/testwrap | 6 ++++++
2 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml
index fb4ea3d666a..8ed7f7e5dd7 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -21,6 +21,7 @@ env:
PGCTLTIMEOUT: 120 # avoids spurious failures during parallel tests
TEMP_CONFIG: ${CIRRUS_WORKING_DIR}/src/tools/ci/pg_ci_base.conf
PG_TEST_EXTRA: kerberos ldap ssl libpq_encryption load_balance
+ PG_FAILED_TESTDIR: ${CIRRUS_WORKING_DIR}/failed.build
# What files to preserve in case tests fail
@@ -35,9 +36,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 +139,7 @@ task:
PG_TEST_INITDB_EXTRA_OPTS: -c debug_copy_parse_plan_trees=on -c debug_write_read_parse_plan_trees=on -c debug_raw_expression_coverage_test=on
PG_TEST_PG_UPGRADE_MODE: --link
+ PG_FAILED_TESTDIR: ${CIRRUS_WORKING_DIR}/failed.build
<<: *freebsd_task_template
@@ -195,10 +197,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
@@ -403,7 +405,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 9a270beb72d..0d0121e62da 100755
--- a/src/tools/testwrap
+++ b/src/tools/testwrap
@@ -59,4 +59,10 @@ if 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(returncode)
--
2.42.0
>From e028901375221cc6bcdc102132fcf884fdf51fae 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 | 25 ++++++++++++++++++++++++-
src/tools/ci/copy-changed-docs | 29 +++++++++++++++++++++++++++++
2 files changed, 53 insertions(+), 1 deletion(-)
create mode 100755 src/tools/ci/copy-changed-docs
diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml
index 8ed7f7e5dd7..8b07fb2e538 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -23,6 +23,15 @@ env:
PG_TEST_EXTRA: kerberos ldap ssl libpq_encryption 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~55
+
# What files to preserve in case tests fail
on_failure_ac: &on_failure_ac
@@ -783,7 +792,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:
@@ -793,6 +802,20 @@ task:
CC="ccache gcc" CXX="ccache g++" CLANG="ccache clang-16"
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 c5466195f82ad000abb68fa12b7269f5e53c4465 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 7112295112dff33e541ea680084ca97aa01ec7f4 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 8b07fb2e538..aea79f29d7e 100644
--- a/.cirrus.tasks.yml
+++ b/.cirrus.tasks.yml
@@ -797,18 +797,36 @@ 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 \
CC="ccache gcc" CXX="ccache g++" CLANG="ccache clang-16"
+ --without-icu \
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
