Обсуждение: Add PG CI to older PG releases

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

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.

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

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].

            regards, tom lane

[1] https://www.postgresql.org/message-id/flat/20230808021541.7lbzdefvma7qmn3w%40awork3.anarazel.de

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

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):
https://postgr.es/m/20220928192226.4c6zeenujaoqq4bq%40alvherre.pgsql

Greetings,

Andres Freund

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].

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

Regards,
Nazir Bilal Yavuz
Microsoft