Add PG CI to older PG releases

Started by Nazir Bilal Yavuzabout 3 years ago7 messageshackers
Beta feature

Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.

never appliedCI history
Jump to latest
#1Nazir Bilal Yavuz
byavuz81@gmail.com

Hi,

PG CI is added starting from PG 15, adding PG CI to PG 14 and below
could be beneficial. So, firstly I tried adding it to the
REL_14_STABLE branch. If that makes sense, I will try to add PG CI to
other old PG releases.

'Add PG CI to PG 14' patch is attached. I merged both CI commits and
the least amount of commits to pass the CI on all OSes.

Addition to CI commits, other changes are:

1_ 76e38b37a5f179d4c9d2865ff31b79130407530b is added for debugging
Windows. Also a couple of SSL tests were failing without this because
the log file is empty. Example failures on 001_ssltests.pl:

# Failed test 'certificate authorization succeeds with DN mapping:
log matches'
# at c:/cirrus/src/test/perl/PostgresNode.pm line 2195.
# ''
# doesn't match '(?^:connection authenticated:
identity="CN=ssltestuser-dn,OU=Testing,OU=Engineering,O=PGDG"
method=cert)'

# Failed test 'certificate authorization succeeds with CN mapping:
log matches'
# at c:/cirrus/src/test/perl/PostgresNode.pm line 2195.
# ''
# doesn't match '(?^:connection authenticated:
identity="CN=ssltestuser-dn,OU=Testing,OU=Engineering,O=PGDG"
method=cert)'

# Failed test 'certificate authorization fails with client cert
belonging to another user: log matches'
# at c:/cirrus/src/test/perl/PostgresNode.pm line 2243.
# ''
# doesn't match '(?^:connection authenticated:
identity="CN=ssltestuser" method=cert)'
# Looks like you failed 3 tests of 110.

2_ 45f52709d86ceaaf282a440f6311c51fc526340b is added for fixing socket
directories on Windows.

3_ I removed '--with-zstd' since it was not supported yet.

Any kind of feedback would be appreciated.

Regards,
Nazir Bilal Yavuz
Microsoft

Attachments:

v1-0001-ci-Add-PG-CI-to-PG-14.patchtext/x-diff; charset=US-ASCII; name=v1-0001-ci-Add-PG-CI-to-PG-14.patchDownload+841-4
#2Peter Eisentraut
peter_e@gmx.net
In reply to: Nazir Bilal Yavuz (#1)
Re: Add PG CI to older PG releases

On 10.08.23 16:43, Nazir Bilal Yavuz wrote:

1_ 76e38b37a5f179d4c9d2865ff31b79130407530b is added for debugging
Windows. Also a couple of SSL tests were failing without this because
the log file is empty. Example failures on 001_ssltests.pl:

I see only one attachment, so it's not clear what these commit hashes
refer to.

#3Nazir Bilal Yavuz
byavuz81@gmail.com
In reply to: Peter Eisentraut (#2)
Re: Add PG CI to older PG releases

Hi,

On Thu, 10 Aug 2023 at 18:05, Peter Eisentraut <peter@eisentraut.org> wrote:

I see only one attachment, so it's not clear what these commit hashes
refer to.

I split the commit into 3 parts.

v2-0001-windows-Only-consider-us-to-be-running-as-service.patch is an
older commit (59751ae47fd43add30350a4258773537e98d4063). A couple of
tests were failing without this because the log file was empty and the
tests were comparing strings with the log files (like in the first
mail).

v2-0002-Make-PG_TEST_USE_UNIX_SOCKETS-work-for-tap-tests-.patch is
slightly modified version of 45f52709d86ceaaf282a440f6311c51fc526340b
to backpatch it to PG 14. Without this, Windows tests were failing
when they tried to create sockets with 'unix_socket_directories' path.

v2-0003-ci-Add-PG-CI-to-PG-14.patch adds files that are needed for CI
to work. I copied these files from the REL_15_STABLE branch. Then I
removed '--with-zstd' from .cirrus.yml since it was not supported yet.

Regards,
Nazir Bilal Yavuz
Microsoft

Attachments:

v2-0002-Make-PG_TEST_USE_UNIX_SOCKETS-work-for-tap-tests-.patchtext/x-diff; charset=US-ASCII; name=v2-0002-Make-PG_TEST_USE_UNIX_SOCKETS-work-for-tap-tests-.patchDownload+13-2
v2-0003-ci-Add-PG-CI-to-PG-14.patchtext/x-diff; charset=US-ASCII; name=v2-0003-ci-Add-PG-CI-to-PG-14.patchDownload+788-1
v2-0001-windows-Only-consider-us-to-be-running-as-service.patchtext/x-diff; charset=US-ASCII; name=v2-0001-windows-Only-consider-us-to-be-running-as-service.patchDownload+48-4
#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Nazir Bilal Yavuz (#1)
Re: Add PG CI to older PG releases

Nazir Bilal Yavuz <byavuz81@gmail.com> writes:

PG CI is added starting from PG 15, adding PG CI to PG 14 and below
could be beneficial. So, firstly I tried adding it to the
REL_14_STABLE branch. If that makes sense, I will try to add PG CI to
other old PG releases.

I'm not actually sure this is worth spending time on. We don't
do new development in three-release-back branches, nor do we have
so many spare CI cycles that we can routinely run CI testing in
those branches [1]/messages/by-id/20230808021541.7lbzdefvma7qmn3w@awork3.anarazel.de.

regards, tom lane

[1]: /messages/by-id/20230808021541.7lbzdefvma7qmn3w@awork3.anarazel.de

#5Andres Freund
andres@anarazel.de
In reply to: Nazir Bilal Yavuz (#3)
Re: Add PG CI to older PG releases

Hi,

On 2023-08-10 19:55:15 +0300, Nazir Bilal Yavuz wrote:

v2-0001-windows-Only-consider-us-to-be-running-as-service.patch is an
older commit (59751ae47fd43add30350a4258773537e98d4063). A couple of
tests were failing without this because the log file was empty and the
tests were comparing strings with the log files (like in the first
mail).

Hm. I'm a bit worried about backpatching this one, it's not a small
behavioural change. But the prior behaviour is pretty awful and could very
justifiably be viewed as a bug.

At the very least this would need to be combined with

commit 950e64fa46b164df87b5eb7c6e15213ab9880f87
Author: Andres Freund <andres@anarazel.de>
Date: 2022-07-18 17:06:34 -0700

Use STDOUT/STDERR_FILENO in most of syslogger.

Greetings,

Andres Freund

#6Andres Freund
andres@anarazel.de
In reply to: Tom Lane (#4)
Re: Add PG CI to older PG releases

Hi,

On 2023-08-10 18:09:03 -0400, Tom Lane wrote:

Nazir Bilal Yavuz <byavuz81@gmail.com> writes:

PG CI is added starting from PG 15, adding PG CI to PG 14 and below
could be beneficial. So, firstly I tried adding it to the
REL_14_STABLE branch. If that makes sense, I will try to add PG CI to
other old PG releases.

I'm not actually sure this is worth spending time on. We don't
do new development in three-release-back branches, nor do we have
so many spare CI cycles that we can routinely run CI testing in
those branches [1].

I find it much less stressful to backpatch if I can test the patches via CI
first. I don't think I'm alone in that - Alvaro for example has previously
done this in a more limited fashion (no windows):
/messages/by-id/20220928192226.4c6zeenujaoqq4bq@alvherre.pgsql

Greetings,

Andres Freund

#7Nazir Bilal Yavuz
byavuz81@gmail.com
In reply to: Andres Freund (#5)
Re: Add PG CI to older PG releases

Hi,

On Fri, 11 Aug 2023 at 02:00, Andres Freund <andres@anarazel.de> wrote:

At the very least this would need to be combined with

commit 950e64fa46b164df87b5eb7c6e15213ab9880f87
Author: Andres Freund <andres@anarazel.de>
Date: 2022-07-18 17:06:34 -0700

Use STDOUT/STDERR_FILENO in most of syslogger.

950e64fa46b164df87b5eb7c6e15213ab9880f87 needs to be combined with
92f478657c5544eba560047c39eba8a030ddb83e. So, I combined
59751ae47fd43add30350a4258773537e98d4063,
950e64fa46b164df87b5eb7c6e15213ab9880f87 and
92f478657c5544eba560047c39eba8a030ddb83e in a single commit [1]and the rest are the same..

v2-0001-windows-Fix-windows-logging-problems.patch is a combination of
[1]: and the rest are the same.

Regards,
Nazir Bilal Yavuz
Microsoft

Attachments:

v2-0002-Make-PG_TEST_USE_UNIX_SOCKETS-work-for-tap-tests-.patchtext/x-diff; charset=US-ASCII; name=v2-0002-Make-PG_TEST_USE_UNIX_SOCKETS-work-for-tap-tests-.patchDownload+13-2
v2-0001-windows-Fix-windows-logging-problems.patchtext/x-diff; charset=US-ASCII; name=v2-0001-windows-Fix-windows-logging-problems.patchDownload+65-13
v2-0003-ci-Add-PG-CI-to-PG-14.patchtext/x-diff; charset=US-ASCII; name=v2-0003-ci-Add-PG-CI-to-PG-14.patchDownload+788-1