Re: CI and test improvements

From: Justin Pryzby <pryzby(at)telsasoft(dot)com>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Nazir Bilal Yavuz <byavuz81(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, vignesh C <vignesh21(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, Andrew Dunstan <andrew(at)dunslane(dot)net>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Noah Misch <noah(at)leadboat(dot)com>, Anastasia Lubennikova <lubennikovaav(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Robert Haas <robertmhaas(at)gmail(dot)com>, Melanie Plageman <melanieplageman(at)gmail(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, Daniel Gustafsson <daniel(at)yesql(dot)se>, samay sharma <smilingsamay(at)gmail(dot)com>
Subject: Re: CI and test improvements
Date: 2024-08-06 19:10:15
Message-ID: ZrJ1FwZAU_OCmb5N@pryzbyj2023
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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)

Attachment Content-Type Size
0001-cirrus-windows-add-compiler_warnings_script.patch text/x-diff 1.9 KB
0002-cirrus-windows-ccache.patch text/x-diff 2.6 KB
0003-WIP-ci-meson-allow-showing-only-failed-tests.patch text/x-diff 4.0 KB
0004-WIP-cirrus-upload-changed-html-docs-as-artifacts.patch text/x-diff 3.3 KB
0005-html-index-file.patch text/x-diff 2.6 KB
0006-WIP-show-changed-docs-with-meson.patch text/x-diff 2.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Heikki Linnakangas 2024-08-06 19:18:45 Re: Fix comments in instr_time.h and remove an unneeded cast to int64
Previous Message Yugo NAGATA 2024-08-06 19:06:29 Re: Inconsistency with EXPLAIN ANALYZE CREATE MATERIALIZED VIEW