From: | Justin Pryzby <pryzby(at)telsasoft(dot)com> |
---|---|
To: | Andres Freund <andres(at)anarazel(dot)de> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Robert Haas <robertmhaas(at)gmail(dot)com>, Andrew Dunstan <andrew(at)dunslane(dot)net>, pgsql-hackers(at)postgresql(dot)org, Thomas Munro <thomas(dot)munro(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> |
Subject: | Re: Adding CI to our tree |
Date: | 2022-02-28 20:58:02 |
Message-ID: | 20220228205802.GE25269@telsasoft.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Sat, Feb 26, 2022 at 08:08:38PM -0800, Andres Freund wrote:
> On 2022-02-26 21:10:57 -0600, Justin Pryzby wrote:
> > If someone renames or removes an xref target, shouldn't CI fail on its next
> > run for a patch which tries to reference it ?
>
> Why wouldn't it?
I suppose you're right - I was thinking that cirrus was checking whether the
*patch* had changed any matching files, but it probably checks (as it should)
whether "the sources" have changed.
Hmm, it's behaving strangely...if there's a single argument ('docs/**'), then
it will skip the docs task if I resubmit it after git commit --amend --no-edit.
But with multiple args ('.cirrus.yaml', 'docs/**') it reruns it...
I tried it as skip: !changesInclude() and by adding it to the existing only_if:
(.. || ..) && changesInclude().
> > Are you sure about cfbot ? AIUI cirrus would see that docs didn't change
> > relative to the previous run for branch: commitfest/NN/MMMM.
>
> Not entirely sure, but it's what I remember observing when ammending commits
> in a repo using changesInclues. If I were to build a feature like it I'd look
> at the list of files of
> git diff $(git merge-base last_green new_commit)..new_commit
>
> or such. cfbot doesn't commit incrementally but replaces the prior commit, so
> I suspect it'll always be viewn as new. But who knows, shouldn't be hard to
> figure out.
Anyway...
I still think that if "Build Docs" is a separate cirrus task, it should rebuild
docs on every CI run, even if they haven't changed, for any patch that touches
docs/. It'll be confusing if cfbot shows 5 green circles and 4 of them were
built 1 day ago, and 1 was built 3 weeks ago. Docs are the task that runs
quickest, so I don't think it's worth doing anything special there (especially
without understanding the behavior of changesInclude()).
Also, to allow users to view the built HTML docs, cfbot would need to 1) keep
track of previous CI runs; and 2) logic to handle "skipped" CI runs, to allow
showing artifacts from the previous run. If it's not already done, I think the
first half is a good idea on its own. But the 2nd part doesn't seem desirable.
However, I realized that we can filter on cfbot with either of these:
| $CIRRUS_CHANGE_TITLE =~ '^\[CF...'
| git log -1 |grep '^Author: Commitfest Bot <cfbot(at)cputube(dot)org>'
If we can assume that cfbot will continue submitting branches as a single
patch, this resolves the question of a "base branch", for cfbot.
(Actually, I'd prefer if it preserved the original patches as separate commits,
but that isn't what it does).
These patches implement that idea, and make "code coverage" and "HTML diffs"
stuff only run for cfbot commits. This still needs another round of testing,
though.
--
Justin
PS. I've just done this. I'm unsure whether to say that it's wonderful or
terrible. This would certainly be better if each branch preserved the original
set of patches.
$ git remote add cfbot https://github.com/postgresql-cfbot/postgresql
$ git fetch cfbot
$ git branch -a |grep -c cfbot
5417
Attachment | Content-Type | Size |
---|---|---|
0001-cirrus-include-hints-how-to-install-OS-packages.patch | text/x-diff | 2.2 KB |
0002-cirrus-build-docs-as-a-separate-task.patch | text/x-diff | 1.9 KB |
0003-cirrus-upload-changed-html-docs-as-artifacts.patch | text/x-diff | 2.4 KB |
0004-wip-cirrus-code-coverage.patch | text/x-diff | 2.7 KB |
0005-wip-cirrus-windows-add-compiler_warnings_script.patch | text/x-diff | 2.4 KB |
0006-cirrus-compile-with-Og.patch | text/x-diff | 2.7 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Stephen Frost | 2022-02-28 21:00:36 | Re: [PATCH] Expose port->authn_id to extensions and triggers |
Previous Message | Stephen Frost | 2022-02-28 20:46:34 | Re: Proposal: Support custom authentication methods using hooks |