On Sat, Feb 12, 2022 at 05:20:08PM -0800, Andres Freund wrote:
> On 2022-02-03 23:04:04 -0600, Justin Pryzby wrote:
> > > I'm a bit worried about the increased storage and runtime overhead due to
> > > the
> > > docs changes. We probably can make it a good bit cheaper though.
> >
> > If you mean overhead of additional disk operations, it shouldn't be an
> > issue,
> > since the git clone uses shared references (not even hardlinks).
>
> I was concerned about those and just the increased runtime of the script. Our
> sources are 130MB, leaving the shared .git aside. But maybe it's just fine.
>
> We probably can just get rid of the separate clone and configure run though?
> Build the docs, copy the output, do a git co parent docs/, build again?
Yes - works great, thanks.
> What was the reason behind moving the docs stuff from the compiler warnings
> task to linux?
I wanted to build docs even if the linux task fails. To allow CFBOT to link to
them, so somoene can always review the docs, in HTML (rather than reading SGML
with lines prefixed with +).
> Not that either fits very well... I think it might be worth
> moving the docs stuff into its own task, using a 1 CPU container (docs build
> isn't parallel anyway). Think that'll be easier to see in the cfbot page. I
Yeah. The only drawback is the duplication (including the "git parent" stuff).
BTW, docs can be built in parallel, and CI is using BUILD_JOBS: 4.
/usr/bin/xmllint --path . --noout --valid postgres.sgml
/usr/bin/xmllint --path . --noout --valid postgres.sgml
/usr/bin/xsltproc --path . --stringparam pg.version '15devel' stylesheet.xsl
postgres.sgml
/usr/bin/xsltproc --path . --stringparam pg.version '15devel'
stylesheet-man.xsl postgres.sgml
> 1) iterate over release branches and see which has the smallest diff
Maybe for each branch: do echo `git revlist or merge base |wc -l` $branch; done
|sort -n |head -1
> > > Is looking at the .c files in the change really a reliable predictor of
> > > where
> > > code coverage changes? I'm doubtful. Consider stuff like removing the last
> > > user of some infrastructure or such. Or adding the first.
> >
> > You're right that it isn't particularly accurate, but it's a step forward if
> > lots of patches use it to check/improve coverge of new code.
>
> Maybe it's good enough... The overhead in test runtime is noticeable (~5.30m
> -> ~7.15m), but probably acceptable. Although I also would like to enable
> -fsanitize=alignment and -fsanitize=alignment, which add about 2 minutes as
> well (-fsanitize=address is a lot more expensive), they both work best on
> linux.
There's other things that it'd be nice to enable, but maybe these don't need to
be on by default. Maybe just have a list of optional compiler flags (and hints
when they're useful). Like WRITE_READ_PARSE_PLAN_TREES.
> > In addition to the HTML generated for changed .c files, it's possible to
> > create
> > a coverage.gcov output for everything, which could be retrieved to generate
> > full HTML locally. It's ~8MB (or 2MB after gzip).
>
> Note sure that doing doing it locally adds much over just running tests
> locally.
You're right, since one needs to have the patched source files to generate the
HTML.
On Thu, Feb 03, 2022 at 11:57:18AM -0800, Andres Freund wrote:
> I think it may be a bit cleaner to have the "install additional packages"
> "template step" be just that, and not merge in other contents into it?
I renamed the "cores" task since it consistently makes me think you're doing
with CPU cores. It took it as an opportunity to generalize the task.
These patches are ready for review. I'll plan to mail about TAP stuff
tomorrow.
>From 05c24a1e679e5ae0dd0cb6504d397f77c8b1fc5c 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/3] cirrus: include hints how to install OS packages..
This is useful for patches during development, but once a features 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 | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index dd96a97efe5..eda8ac9596c 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
--
2.17.1
>From 6ac56adae8daea5461b03a2b279aacf73f23769a Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Sat, 12 Feb 2022 18:59:50 -0600
Subject: [PATCH 2/3] cirrus/windows: add compiler_warnings_script
ci-os-only: windows
https://cirrus-ci.com/task/6316260295180288
---
.cirrus.yml | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index eda8ac9596c..ea87c890cc3 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -397,7 +397,7 @@ task:
- perl src/tools/msvc/mkvcbuild.pl
build_script:
- vcvarsall x64
- - msbuild %MSBFLAGS% pgsql.sln
+ - msbuild %MSBFLAGS% pgsql.sln >build.log
tempinstall_script:
# Installation on windows currently only completely works from src/tools/msvc
- cd src/tools/msvc && perl install.pl %CIRRUS_WORKING_DIR%/tmp_install
@@ -440,6 +440,11 @@ task:
cd src/tools/msvc
%T_C% perl vcregress.pl ecpgcheck
+ always:
+ compiler_warnings_script: |
+ findstr "warning " build.log && exit /b 1
+ exit /b 0
+
on_failure:
<<: *on_failure
crashlog_artifacts:
--
2.17.1
>From d0d46260a6e2af5b9cd160d9362c31549e930d2d Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Sat, 15 Jan 2022 11:27:28 -0600
Subject: [PATCH 3/3] cirrus: upload changed html docs as artifacts
Build docs during separate task; to all 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 | 69 ++++++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 55 insertions(+), 14 deletions(-)
diff --git a/.cirrus.yml b/.cirrus.yml
index ea87c890cc3..7ff8f72937a 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -191,6 +191,7 @@ task:
configure_script: |
su postgres <<-EOF
./configure \
+ --cache gcc.cache \
--enable-cassert --enable-debug --enable-tap-tests \
--enable-nls \
\
@@ -555,19 +556,59 @@ 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, and upload changed docs as artifacts
+task:
+ name: HTML docs
+
+ env:
+ CPUS: 1
+
+ only_if: $CIRRUS_CHANGE_MESSAGE !=~ '.*\nci-os-only:.*' || $CIRRUS_CHANGE_MESSAGE =~ '.*\nci-os-only:[^\n]*(docs|html).*'
+
+ container:
+ image: $CONTAINER_REPO/linux_debian_bullseye_ci:latest
+ cpu: $CPUS
+
+ sysinfo_script: |
+ id
+ uname -a
+ cat /proc/cmdline
+ ulimit -a -H && ulimit -a -S
+ export
+
+ git remote -v
+ git branch -a
+ git remote add postgres https://github.com/postgres/postgres
+ time git fetch -v postgres master
+ git log -1 postgres/master
+ git diff --name-only postgres/master..
+
+ docs_build_script: |
+ # Do nothing if the patch doesn't update doc:
+ git diff --name-only --cherry-pick --exit-code postgres/master... doc && exit
+
+ # Exercise HTML and other docs:
+ ./configure >/dev/null
+ make -s -C doc
+ cp -a doc new-docs
+
+ # Build HTML docs from the parent commit
+ git checkout postgres/master -- doc
+ make -s -C doc clean
+ make -s -C doc html
+ cp -a doc old-docs
+
+ # Copy HTML which differ into html_docs
+ mkdir html_docs
+ cp new-docs/src/sgml/html/*.css new-docs/src/sgml/html/*.svg html_docs/
+ for f in `git diff --no-index --name-only old-docs/src/sgml/html new-docs/src/sgml/html`
+ do
+ cp $f html_docs/
+ done
+
+ html_docs_artifacts:
+ paths: ['html_docs/**/*.html', 'html_docs/**/*.png', 'html_docs/**/*.css']
--
2.17.1
