Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1sJZ1F-005aJK-9p for pgsql-hackers@arkaria.postgresql.org; Tue, 18 Jun 2024 13:37:05 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.94.2) (envelope-from ) id 1sJZ1D-00DLJj-3s for pgsql-hackers@arkaria.postgresql.org; Tue, 18 Jun 2024 13:37:04 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1sJZ1C-00DLIl-Ht for pgsql-hackers@lists.postgresql.org; Tue, 18 Jun 2024 13:37:03 +0000 Received: from charmander.telsasoft.com ([50.244.222.1] helo=pryzbyj2023.telsasoft) by makus.postgresql.org with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1sJZ19-001utc-Ma for pgsql-hackers@lists.postgresql.org; Tue, 18 Jun 2024 13:37:02 +0000 Received: by pryzbyj2023.telsasoft (Postfix, from userid 1000) id A2B843430; Tue, 18 Jun 2024 08:36:57 -0500 (CDT) Date: Tue, 18 Jun 2024 08:36:57 -0500 From: Justin Pryzby To: Andres Freund Cc: Nazir Bilal Yavuz , Alvaro Herrera , vignesh C , Michael Paquier , Andrew Dunstan , Thomas Munro , pgsql-hackers@lists.postgresql.org, Noah Misch , Anastasia Lubennikova , Tom Lane , Robert Haas , Melanie Plageman , Peter Eisentraut , Daniel Gustafsson , samay sharma Subject: Re: CI and test improvements Message-ID: References: <202401311059.7w5bl2ftqxaj@alvherre.pgsql> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="jviKHM0HgsS9J3Sv" Content-Disposition: inline In-Reply-To: List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --jviKHM0HgsS9J3Sv Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Fri, Jun 14, 2024 at 08:34:37AM -0700, Andres Freund wrote: > Hm. There actually already is the mingw ccache installed. The images for mingw and msvc used to be separate (for startup performance when using containers), but we just merged them. So it might be easiest to just explicitly use the ccache from there > (also an explicit path might be faster). I don't think the path search is significant; it's fast so long as there's no choco stub involved. > Could you check if that's fast? Yes, it is. > 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. -- Justin --jviKHM0HgsS9J3Sv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0001-cirrus-windows-add-compiler_warnings_script.patch" From ad03da2cfa3ecf23334f8b31f2e4529ba3094512 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Wed, 25 May 2022 21:53:22 -0500 Subject: [PATCH 1/7] 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.GB16921@telsasoft.com 20221031223744.GT16921@telsasoft.com 20221104161934.GB16921@telsasoft.com 20221123054329.GG11463@telsasoft.com 20230224002029.GQ1653@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 33646faeadf..5a2b64f64c2 100644 --- a/.cirrus.tasks.yml +++ b/.cirrus.tasks.yml @@ -566,12 +566,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 --jviKHM0HgsS9J3Sv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0002-cirrus-windows-ccache.patch" From 78031c45afad402397e4efa1389e6396557cd405 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Wed, 25 May 2022 22:05:13 -0500 Subject: [PATCH 2/7] 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 | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml index 5a2b64f64c2..6ebbc1ccd81 100644 --- a/.cirrus.tasks.yml +++ b/.cirrus.tasks.yml @@ -536,6 +536,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, @@ -553,6 +559,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 @@ -562,7 +571,7 @@ task: # Use /DEBUG:FASTLINK to avoid high memory usage during linking 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 @@ -571,6 +580,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 --jviKHM0HgsS9J3Sv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0003-WIP-ci-meson-allow-showing-only-failed-tests.patch" From cc09e8363d56933b8d62ebafb8319c91fed3b03f Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Fri, 25 Nov 2022 13:57:17 -0600 Subject: [PATCH 3/7] 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.lxdj3puenfhirhqm@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 6ebbc1ccd81..56061e61a3e 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 @@ -136,6 +137,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 @@ -194,10 +196,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 @@ -400,7 +402,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 --jviKHM0HgsS9J3Sv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0004-WIP-cirrus-upload-changed-html-docs-as-artifacts.patch" From 3a399c6350ed2f341425431f184e382c3f46d981 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Sat, 26 Feb 2022 19:39:10 -0600 Subject: [PATCH 4/7] 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.GP24419@telsasoft.com https://www.postgresql.org/message-id/CAB8KJ=i4qmEuopQ+PCSMBzGd4O-Xv0FCnC+q1x7hN9hsdvkBug@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 56061e61a3e..0ce5b172331 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 @@ -769,7 +778,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: @@ -781,6 +790,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 --jviKHM0HgsS9J3Sv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0005-html-index-file.patch" From c0dd484060e18c4221e7ec883e621720eec230ed Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Mon, 28 Feb 2022 23:18:19 -0600 Subject: [PATCH 5/7] +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" < +Index of docs changed since: $BASE_COMMIT + + +

Index of docs changed since: $BASE_COMMIT

+
    +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 "
  • $fn" +done >>"$index" + +github=https://github.com/$CIRRUS_REPO_FULL_NAME/commit +cirrus=https://cirrus-ci.com/build + +cat >>"$index" < +
    + +
    This file was written on: `date --rfc-822 --utc` +
    CIRRUS_CHANGE_TITLE: $CIRRUS_CHANGE_TITLE +
    CIRRUS_CHANGE_IN_REPO: $CIRRUS_CHANGE_IN_REPO +
    CIRRUS_BUILD_ID: $CIRRUS_BUILD_ID +
    CIRRUS_LAST_GREEN_CHANGE: $CIRRUS_LAST_GREEN_CHANGE +
    CIRRUS_LAST_GREEN_BUILD_ID: $CIRRUS_LAST_GREEN_BUILD_ID +     + +EOF exit 0 -- 2.42.0 --jviKHM0HgsS9J3Sv Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="0006-WIP-show-changed-docs-with-meson.patch" From c68c8e6923328a9a2b0fb461a5ecc089e9d47450 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Wed, 19 Oct 2022 20:33:28 -0500 Subject: [PATCH 6/7] 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 0ce5b172331..c53408197d4 100644 --- a/.cirrus.tasks.yml +++ b/.cirrus.tasks.yml @@ -783,20 +783,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 --jviKHM0HgsS9J3Sv--