From: | Justin Pryzby <pryzby(at)telsasoft(dot)com> |
---|---|
To: | Andres Freund <andres(at)anarazel(dot)de> |
Cc: | 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>, Michael Paquier <michael(at)paquier(dot)xyz>, 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: | 2022-11-22 22:57:44 |
Message-ID: | 20221122225744.GF11463@telsasoft.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Nov 21, 2022 at 02:45:42PM -0800, Andres Freund wrote:
> > > > + ninja -C build |tee build/meson-logs/build.txt
> > > > + REM Since pipes lose exit status of the preceding command, rerun compilation,
> > > > + REM without the pipe exiting now if it fails, rather than trying to run checks
> > > > + ninja -C build > nul
> > >
> > > This seems mighty grotty :(. but I guess it's quick enough not worry about,
> > > and I can't come up with a better plan.
> > >
> > > It doesn't seem quite right to redirect into meson-logs/ to me, my
> > > interpretation is that that's "meson's namespace". Why not just store it in
> > > build/?
> >
> > I put it there so it'd be included with the build artifacts.
>
> Wouldn't just naming it build-warnings.log suffice? I don't think we
> want to actually upload build.txt - it already is captured.
Originally, I wanted the input and the output to be available as files
and not just in cirrus' web GUI, but maybe that's not important anymore.
I rewrote it again.
> > I'm not sure if this ought to be combined with/before/after your "move
> > compilerwarnings task to meson" patch? (Regarding that patch: I
> > mentioned that it shouldn't use ccache -C, and it should use
> > meson_log_artifacts.)
>
> TBH, I'm not quite sure a separate docs task does really still make
> sense after the SanityCheck task. It's worth building the docs even if
> some flappy test fails, but I don't think we should try to build the
> docs if the code doesn't even compile, in all likelihood a lot more is
> wrong in that case.
It'd be okay either way. I had split it out to 1) isolate the changes
in the "upload changed docs as artifacts" patch; and, 2) so the docs
artifacts are visible in a cfbot link called "Documentation"; and, 3) so
the docs task runs without a dependency on "Linux", since (as you said)
docs/errors are worth showing/reviewing/reporting/addressing separately
from test errors (perhaps similar to compiler warnings...).
I shuffled my branch around and sending now the current "docs" patches,
but I suppose this is waiting on the "convert CompilerWarnings task to
meson" patch.
--
Justin
Attachment | Content-Type | Size |
---|---|---|
0001-cirrus-windows-add-compiler_warnings_script.patch | text/x-diff | 1.6 KB |
0002-cirrus-macos-switch-to-macos_instance-M1.patch | text/x-diff | 2.4 KB |
0003-cirrus-macos-update-to-macos-ventura.patch | text/x-diff | 939 bytes |
0004-cirrus-freebsd-run-with-more-CPUs-RAM-and-do-not-rep.patch | text/x-diff | 1.8 KB |
0005-cirrus-clean-up-typos.patch | text/x-diff | 1.9 KB |
0006-cirrus-build-docs-as-a-separate-task.patch | text/x-diff | 2.4 KB |
0007-cirrus-warnings-use-a-single-common-always-block.patch | text/x-diff | 3.2 KB |
0008-cirrus-upload-changed-html-docs-as-artifacts.patch | text/x-diff | 3.9 KB |
0009-WIP-show-changed-docs-with-meson.patch | text/x-diff | 2.1 KB |
0010-html-index-file.patch | text/x-diff | 3.2 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Matheus Alcantara | 2022-11-22 23:16:27 | Re: Make ON_ERROR_STOP stop on shell script failure |
Previous Message | Steve Chavez | 2022-11-22 22:53:24 | Re: Allow placeholders in ALTER ROLE w/o superuser |