Hi, On 2022-08-28 12:10:29 -0500, Justin Pryzby wrote: > On Sun, Aug 28, 2022 at 09:07:52AM -0700, Andres Freund wrote: > > > --- /dev/null > > > +++ b/src/tools/ci/windows-compiler-warnings > > > > Wouldn't that be doable as something like > > sh -c 'if test -s file; then cat file;exit 1; fi" > > inside .cirrus.yml? > > I had written it inline in a couple ways, like > - sh -exc 'f=msbuild.warn.log; if [ -s "$f" ]; then cat "$f"; exit 1; else > exit 0; fi' > > but then separated it out as you suggested in > 20220227010908.vz2a7dmfzgwg7...@alap3.anarazel.de > > after I complained about cmd.exe requiring escaping for && and || > That makes writing any shell script a bit perilous and a separate script > seems better.
I remember that I suggested it - but note that the way I wrote above doesn't have anything needing escaping. Anyway, what do you think of the multiline split I suggested? > > > Subject: [PATCH 03/25] cirrus/ccache: disable compression and show stats > > > > > > ccache since 4.0 enables zstd compression by default. > > > > > > With default compression enabled > > > (https://cirrus-ci.com/task/6692342840164352): > > > linux has 4.2; 99MB cirrus cache; cache_size_kibibyte 109616 > > > macos has 4.5.1: 47MB cirrus cache; cache_size_kibibyte 52500 > > > freebsd has 3.7.12: 42MB cirrus cache; cache_size_kibibyte 134064 > > > windows has 4.6.1; 180MB cirrus cache; cache_size_kibibyte 51179 > > > todo: compiler warnings > > > > > > With compression disabled (https://cirrus-ci.com/task/4614182514458624): > > > linux: 91MB cirrus cache; cache_size_kibibyte 316136 > > > macos: 41MB cirrus cache; cache_size_kibibyte 118068 > > > windows: 42MB cirrus cache; cache_size_kibibyte 134064 > > > freebsd is the same > > > > > > The stats should either be shown or logged (or maybe run with > > > CCACHE_NOSTATS, > > > to avoid re-uploading cache tarball in a 100% cached build, due only to > > > updating ./**/stats). > > > > > > Note that ccache 4.4 supports CCACHE_STATSLOG, which seems ideal. > > > > I stared at this commit message for a while, trying to make sense of it, and > > couldn't really. I assume you're saying that the cirrus compression is > > better > > with ccache compression disabled, but it's extremely hard to parse out of > > it. > > Yes, because ccache uses zstd-1, and cirrus uses gzip, which it's going > to use no matter what ccache does, and gzip's default -6 is better than > ccache's zstd-1. > > > This does too much at once. Show stats, change cache sizes, disable > > compression. > > The cache size change is related to the compression level change; ccache > prunes based on the local size, which was compressed with zstd-1 and, > with this patch, not compressed (so ~2x larger). Also, it's more > interesting to control the size uploaded to cirrus (after compression > ith gzip-6). That's what should have been in the commit message. > > > From 6a6a97fc869fd1fd8b7ab5da5147f145581634f9 Mon Sep 17 00:00:00 2001 > > > From: Justin Pryzby <pryz...@telsasoft.com> > > > Date: Fri, 24 Jun 2022 00:09:12 -0500 > > > Subject: [PATCH 08/25] cirrus/freebsd: run with more CPUs+RAM and do not > > > repartitiion > > > > > > There was some historic problem where tests under freebsd took 8+ minutes > > > (and > > > before 4a288a37f took 15 minutes). > > > > > > This reduces test time from 10min to 3min. > > > 4 CPUs 4 tests https://cirrus-ci.com/task/4880240739614720 > > > 4 CPUs 6 tests https://cirrus-ci.com/task/4664440120410112 > > > https://cirrus-ci.com/task/4586784884523008 > > > 4 CPUs 8 tests https://cirrus-ci.com/task/5001995491737600 > > > > > > 6 CPUs https://cirrus-ci.com/task/6678321684545536 > > > 8 CPUs https://cirrus-ci.com/task/6264854121021440 > > > > > > See also: > > > https://www.postgresql.org/message-id/flat/20220310033347.hgxk4pyarzq4h...@alap3.anarazel.de#f36c0b17e33e31e7925e7e5812998686 > > > 8 jobs 7min https://cirrus-ci.com/task/6186376667332608 > > > > > > xi-os-only: freebsd > > > > Typo. > > No - it's deliberate so I can switch to and from "everything" to "this > only". I don't see the point in posting patches to be applied if they contain lots of such things that a potential committer would need to catch and include a lot of of fixup patches. > > > @@ -71,8 +69,6 @@ task: > > > fingerprint_key: ccache/freebsd > > > reupload_on_changes: true > > > > > > - # Workaround around performance issues due to 32KB block size > > > - repartition_script: src/tools/ci/gcp_freebsd_repartition.sh > > > create_user_script: | > > > pw useradd postgres > > > chown -R postgres:postgres . > > > -- > > > > What's the story there - at some point that was important for performance > > because of the native block size triggering significant read-modify-write > > cycles with postres' writes. You didn't comment on it in the commit message. > > Well, I don't know the history, but it seems to be unneeded now. It's possible it was mainly needed for testing with aio + dio. But also possible that an upgrade improved the situation since. > > > From fd1c36a0bd8fa608ccdff5be3735dac5e3e48bf3 Mon Sep 17 00:00:00 2001 > > > From: Justin Pryzby <pryz...@telsasoft.com> > > > Date: Wed, 27 Jul 2022 16:54:47 -0500 > > > Subject: [PATCH 09/25] cirrus/freebsd: run build+check in a make vpath > > > > > From 7052a32a21752b59632225684fc9426bb94e46e0 Mon Sep 17 00:00:00 2001 > > > From: Justin Pryzby <pryz...@telsasoft.com> > > > Date: Sun, 13 Feb 2022 17:56:40 -0600 > > > Subject: [PATCH 10/25] cirrus/windows: increase timeout to 25min > > > > No explanation? > > Because of the immediately following commit which makes it run all the > tests. Mention that in the commit message then. Especially when dealing with 25 commits I don't think you can expect others to infer such things. > > > From 602983b2cf37fc43465c62330b2e15e9d6d2035d Mon Sep 17 00:00:00 2001 > > > From: Justin Pryzby <pryz...@telsasoft.com> > > > Date: Fri, 26 Aug 2022 12:00:10 -0500 > > > Subject: [PATCH 15/25] f!and chdir > > > > I don't see the point of pointing fixup commits to the list. > > It's a separate commit to make it easy to see the changes, separately, > since I imagine maybe the "chdir" part won't be desirable, or maybe the > PATH part won't. But I'm not sure, so I'm here soliciting feedback. Shrug, I doubt you'll get much if asked that way. Greetings, Andres Freund
