Обсуждение: missing perl test modules in postgresqlXX-devel ?

Hi,

I noticed the perl test modules from PostgreSQL source tree, eg.
PostgresNode, are not installed with package "postgresqlXX-devel". Only
pg_regress is installed.

Looking at the web, I can find old memories [1] where postgresql12-devel
installed them in /usr/pgsql-12/lib/pgxs/src/test/perl/

Comparing with Debian packages, they install both pg_regress and perl modules
with the client package under:

  $(pg_config --pkglibdir)/pgxs/src/test/perl/
  $(pg_config --pkglibdir)/pgxs/src/test/regress

Is it on purpose?

Regards,


[1] eg.:
https://fedora.pkgs.org/31/postgresql-12-x86_64/postgresql12-devel-12.3-1PGDG.f31.x86_64.rpm.html

Hello Craig,

On Mon, 9 Nov 2020 10:29:16 +0800
Craig Ringer <craig.ringer@enterprisedb.com> wrote:

> On Fri, Nov 6, 2020 at 2:02 AM Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
> wrote:
> > I noticed the perl test modules from PostgreSQL source tree, eg.
> > PostgresNode, are not installed with package "postgresqlXX-devel". Only
> > pg_regress is installed.
> >
> > Looking at the web, I can find old memories [1] where postgresql12-devel
> > installed them in /usr/pgsql-12/lib/pgxs/src/test/perl/
> >  
> 
> We really need some consistency and cross checks between the RPMs and debs.
> But it seems fine to me in the -12 and -13 packages for Fedora 32. So
> please provide some more specifics about exactly what OS, yum repo
> configuration, metadata version, etc you are using.

Oh, sorry, I didn't thought about looking for other packages from other distro
and versions before writing my email. I should I thought about that as, as I
wrote in my email, I found some old fedora packages providing the files...

So it looks like at least EL7 packages are broken/different from fedora 32. Here
are some more informations from a CentOS Linux 7 (vagrant box "centos/7"):

  # repoquery -f /usr/pgsql-12/bin/postgres
  postgresql12-server-0:12.2-1PGDG.rhel7.x86_64
  postgresql12-server-0:12.1-2PGDG.rhel7.x86_64
  postgresql12-server-0:12.2-2PGDG.rhel7.x86_64
  postgresql12-server-0:12.3-1PGDG.rhel7.x86_64
  postgresql12-server-0:12.3-5PGDG.rhel7.x86_64
  postgresql12-server-0:12.4-1PGDG.rhel7.x86_64

  # repoquery -f /usr/pgsql-13/bin/postgres
  postgresql13-server-0:13.0-1PGDG.rhel7.x86_64

  # repoquery -f /usr/pgsql-12/lib/pgxs/src/test/perl/PostgresNode.pm
  (nothing)

  # repoquery -f /usr/pgsql-13/lib/pgxs/src/test/perl/PostgresNode.pm
  (nothing)

  # head -2 /etc/os-release
  NAME="CentOS Linux"
  VERSION="7 (Core)"

  # yum repolist enabled pgdg* 2>&1|colrm 22 72
  repo id                status
  pgdg-common/7/x86_64   350
  pgdg10/7/x86_64        770
  pgdg11/7/x86_64        813
  pgdg12/7/x86_64        395
  pgdg13/7/x86_64        127
  pgdg95/7/x86_64        679
  pgdg96/7/x86_64        740
  repolist: 3874


Thank you,

/Jehan-Guillaume de Rorthais

On Mon, 9 Nov 2020 10:33:07 +0800
Craig Ringer <craig.ringer@enterprisedb.com> wrote:

> Looking at the web, I can find old memories [1] where postgresql12-devel
> >> installed them in /usr/pgsql-12/lib/pgxs/src/test/perl/
> >>  
> >
> > We really need some consistency and cross checks between the RPMs and debs.
> >  
> 
> Ah. I just noticed you said they're installed with the *client* packages.

Yes, under Debian. This was just for comparison and I was surprised as well.

Under EL7, I installed -devel packages as written in $subject

> So that's an example of how we actually need some postgres packaging
> guidance that applies to rpms, debs, EDB's binary installers, and anything
> else, separate to any particular packaging method or team.
> 
> In this case I think it's a bit silly to put the test perl modules along
> with the client.

This is a bit out of the scope of $subject, but I'll answer here anyway, as far
as it could be interesting.

> They can't really be used without the server installed, and they're not
> useful for much unless you're building extensions. They should be in a -dev
> package IMO. They should preferably not be in the test package along with the
> actual test input files, though, since they're used by extensions to run
> their own tests and extensions only have to depend on server-dev not on a
> -test package.

I agree they should live in -dev packages. But I should admit I have no idea
why Debian put them in -client ones. Maybe they have some other constraints
bound to the postgresql-common debian wrapper?

I am currently writing TAP tests for check_pgactivity and rely on PostgresNode
and friends to write and run them. They are very valuable and saves a lot of
time.

> Anyway, I hope the above answers your questions about where they live.

I had no doubt about where they live, sorry for the confusion.

Thank you,

/Jehan-Guillaume de Rorthais

On Tue, 10 Nov 2020 08:53:40 +0800
Craig Ringer <craig.ringer@enterprisedb.com> wrote:

> On Mon, Nov 9, 2020 at 6:50 PM Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
> wrote:
> 
> >
> > I am currently writing TAP tests for check_pgactivity and rely on
> > PostgresNode
> > and friends to write and run them. They are very valuable and saves a lot
> > of
> > time.
> >  
> 
> I'm really glad. I have put a fair bit of effort into getting the test
> infra installed as part of "make install" so it gets packaged.

Then, I have to thank you very much :)

> Next I need to push for consistent -test packages for rpms and debs that
> bundle the actual test .sql, .out, .spec, and t/*.pl files needed to
> actually run the tests on the installed postgres. This is exceedingly
> useful when validating the behaviour of extensions.

Indeed.

Thank you for this effort!

On Tue, 10 Nov 2020 13:57:22 +0800
Craig Ringer <craig.ringer@enterprisedb.com> wrote:

> [...]
> 
> so apparently the TAP tests aren't built/enabled for any RHEL/CentOS
> versions in -13. The same is true for 10 and up, at least.
> 
> That's ... surprising. There's no comment to indicate why.
> 
> Git logs suggest the cause is commit 511834297 "Disable tap tests  on RHEL
> 7" . I think the package in question is perl-Test-Simple a.k.a
> 'perl(Test::Simple)'.

That's surprising. Grep'ing postgresql source doesn't catch anything about
Test::Simple. TAP tests in PostgreSQL sources rely on Test::More which is part
of the official Perl core modules at least since 5.8.0... Do I miss something?

> It looks like the test should be updated to enable them for RHEL 8 at least.
> 
> I don't really feel like wrestling builds for an obsolete OS version to get
> these packaged for EL-7 though.

EL7 is still very popular. A lot of new upgrade/packages are still going to
EL7, at least for maintenance.

Regards,

On Wed, 11 Nov 2020 11:29:49 +0800
Craig Ringer <craig.ringer@enterprisedb.com> wrote:

> On Tue, Nov 10, 2020 at 9:56 PM Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
> wrote:
> 
> > On Tue, 10 Nov 2020 13:57:22 +0800
> > Craig Ringer <craig.ringer@enterprisedb.com> wrote:
> >  
> > > [...]
> > >
> > > so apparently the TAP tests aren't built/enabled for any RHEL/CentOS
> > > versions in -13. The same is true for 10 and up, at least.
> > >
> > > That's ... surprising. There's no comment to indicate why.
> > >
> > > Git logs suggest the cause is commit 511834297 "Disable tap tests  on  
> > RHEL  
> > > 7" . I think the package in question is perl-Test-Simple a.k.a
> > > 'perl(Test::Simple)'.  
> >
> > That's surprising. Grep'ing postgresql source doesn't catch anything about
> > Test::Simple. TAP tests in PostgreSQL sources rely on Test::More which is
> > part
> > of the official Perl core modules at least since 5.8.0... Do I miss
> > something?
> >  
> 
> Yep.
> 
> Perl test frameworks are an insane messy tangle of WTFery, so it's not
> surprising.
> 
> Test::More became part of Test::Simple at some point.

Oh, I didn't noticed that.


> [...] Anyway, we should probably be declaring
> 
>     Requires 'perl(Test::More)'
> 
> instead of
> 
>     Requires perl-Test-Simple

I believe you are right.


> [...}
> Would you mind firing up a RHEL7 AWS EC2 instance and checking if
> 
>     yum -y install 'perl(Test::More)'
> 
> runs?

I fired the vagrant generic/rhel7 box and registered it using my dev account:

  $ head -2 /etc/os-release 
  NAME="Red Hat Enterprise Linux Server"
  VERSION="7.8 (Maipo)"

  $ sudo -i yum -y install 'perl(Test::More)'"
  [...]
  ---> Package perl-Test-Simple.noarch 0:0.98-243.el7 will be installed
  [...]
  Installed:
    perl-Test-Simple.noarch 0:0.98-243.el7

  $ source /etc/os-release
  $ export VERSION NAME
  $ perl -MTest::More -le '
    like $ENV{VERSION}, qr/7\./, "version is $ENV{VERSION}"; 
    like $ENV{NAME}, qr/Red Hat Enterprise/, "OS is $ENV{NAME}"; 
    done_testing'

  ok 1 - version is 7.8 (Maipo)
  ok 2 - OS is Red Hat Enterprise Linux Server
  1..2
  
Note that the package came from rhel-7-server-rpms:

  $ sudo yum history packages-info perl-Test-Simple|grep ^From
  From repo      : rhel-7-server-rpms

RHEL official documentation confirm the package is available in base repository:


https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/7/html/package_manifest/package_lists_base_repository

I checked in RHEL 7 changelog and couldn't find anything about
removing/adding/deprecating this package.

I couldn't find the original discussion deciding to remove --enable-tap-tests
for RHEL packages after a quick look at archives, but it seems the package IS
actually available for RHEL 7. Is it really because of this package the perl
modules are not build RHEL?

> If so, we can probably restore the dependency, and then restore
> --enable-tap-tests support.
> 
> Alternatively we might be able to build with --enable-tap-tests and make
> 'perl(Test::More)' a build-time-only dependency when built on EL7. This
> means that TAP test running would have an undeclared dependency that won't
> actually work on EL7 without a separate package install step, which kind of
> sucks. But so long as it doesn't break other functionality in
> Makefile.global it's probably actually OK.
> 
> If EL7's yum and rpm aren't too stupid to cope, we can possibly use a
> Recommends: dependency for 'perl(Test::More)' there.

Since Test::More is available in EL7, it doesn't look necessary in my opinion.

> I'd need you to run some tests on a real RHEL 7 VM or host if you want to
> pursue this. I can help but can't take this on right now. I'll be able to
> send you a toy specfile to use rpmbuilds commands with to check things.

Sure, I'm available.

Regards,

On Fri, 13 Nov 2020 13:38:05 +0800
Craig Ringer <craig.ringer@enterprisedb.com> wrote:

> On Thu, Nov 12, 2020 at 6:36 PM Jehan-Guillaume de Rorthais <jgdr@dalibo.com>
> wrote:
> > Note that the package came from rhel-7-server-rpms:
> >
> >   $ sudo yum history packages-info perl-Test-Simple|grep ^From
> >   From repo      : rhel-7-server-rpms
> >
> > RHEL official documentation confirm the package is available in base
> > repository:
> >
> >
> >
https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/7/html/package_manifest/package_lists_base_repository
> >
> >
> Based on this, I think it's appropriate to modify the EL-7 spec file for
> all Pg versions (example patch is for -10) something like the following.
> 
> Note that I removed a duplicate guard for "&& 0%{?suse_version} >= 1315"
> where the test for "enabletaptests" already excluded that. And I made sure
> that both fedora and rhel require both IPC::Run and Test::More ; I don't
> see why that was ever not be the case.

All these changes seem fine to me.

Thank you,

Hi,

On Fri, 2020-11-13 at 13:38 +0800, Craig Ringer wrote:
> Based on this, I think it's appropriate to modify the EL-7 spec file
> fo all Pg versions (example patch is for -10) something like the
> following.

We have unified spec file, so can you please send a patch that will
work on all platforms?

Thanks!

--
Devrim Gündüz
Open Source Solution Architect, Red Hat Certified Engineer
Twitter: @DevrimGunduz , @DevrimGunduzTR