Обсуждение: proposal: rpm package for plpgsql_check

On 08/25/2014 03:57 PM, Pavel Stehule wrote:
> Hi
>
> I prepared a spec file for plpgsql_check
>
> https://github.com/okbob/plpgsql_check/blob/master/plpgsql_check.spec
>
> please, can you integrate it?

It doesn't look ready.


%global pgmajorversion 9

should be

%global pgmajorversion 94

i.e. the full PostgreSQL major version in short form.


It should make sure to install the extension into the correct
/usr/pgsql-9.4/lib, /usr/pgsql-9.4/share/extension, etc, probably with
appropriate %files macros .


The %description should be about what the package does, not why it
exists. That material can go in a /usr/share/doc/%{name}/README.rpm-dist
if it's RPM-specific, or just in the regular README.


The package makes no attempt to set PATH or an explicit pg_config
location, so it might not build against the correct PostgreSQL if more
than one is installed.


Ideally the requires should be arch-qualified, e.g.

BuildRequires:  postgresql%{pgmajorversion}-devel%{_?isa}
Requires:       postgresql%{pgmajorversion}%{_?isa}



--
 Craig Ringer                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

Hi Pavel,

Apart from what Craig wrote, can you please add %changelog entry?

Regards, Devrim

On Mon, 2014-08-25 at 16:22 +0800, Craig Ringer wrote:
> On 08/25/2014 03:57 PM, Pavel Stehule wrote:
> > Hi
> >
> > I prepared a spec file for plpgsql_check
> >
> > https://github.com/okbob/plpgsql_check/blob/master/plpgsql_check.spec
> >
> > please, can you integrate it?
>
> It doesn't look ready.
>
>
> %global pgmajorversion 9
>
> should be
>
> %global pgmajorversion 94
>
> i.e. the full PostgreSQL major version in short form.
>
>
> It should make sure to install the extension into the correct
> /usr/pgsql-9.4/lib, /usr/pgsql-9.4/share/extension, etc, probably with
> appropriate %files macros .
>
>
> The %description should be about what the package does, not why it
> exists. That material can go in a /usr/share/doc/%{name}/README.rpm-dist
> if it's RPM-specific, or just in the regular README.
>
>
> The package makes no attempt to set PATH or an explicit pg_config
> location, so it might not build against the correct PostgreSQL if more
> than one is installed.
>
>
> Ideally the requires should be arch-qualified, e.g.
>
> BuildRequires:  postgresql%{pgmajorversion}-devel%{_?isa}
> Requires:       postgresql%{pgmajorversion}%{_?isa}
>
>
>
> --
>  Craig Ringer                   http://www.2ndQuadrant.com/
>  PostgreSQL Development, 24x7 Support, Training & Services
>
>


--
Devrim GÜNDÜZ
Principal Systems Engineer @ EnterpriseDB: http://www.enterprisedb.com
PostgreSQL Danışmanı/Consultant, Red Hat Certified Engineer
Twitter: @DevrimGunduz , @DevrimGunduzTR

On 08/26/2014 02:43 AM, Pavel Stehule wrote:
> Hi
>
> I prepared package for 9.3 - what I tested - for 9.2 and 9.4 it is
> mechanical work - change in preambule

I still don't see any check to make sure the PostgreSQL being built
against is the right one for the %pgmajorversion specified.

You could test

   pg_config --version

but it'd probably be better to just ensure the correct pg_config gets
used from the target postgresql%{pgmajorversion}-devel by placing

    %{pginstdir}/bin

first on the PATH.

--
 Craig Ringer                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

On 08/28/2014 08:27 PM, Pavel Stehule wrote:
>
> is it better? I explicitly set a PATH

Looks sane to me. Devrim?

--
 Craig Ringer                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

Hi,

On Thu, 2014-08-28 at 14:27 +0200, Pavel Stehule wrote:
> is it better? I explicitly set a PATH

Committed with minor changes:

http://svn.pgrpms.org/changeset/719

I added a patch to Makefile (this is how we change the path for
pg_config), updated version to 0.9.2, and changed plpgsql to PL/pgSQL. I
also used more macros for plpgsql_check in the spec file.

Package will appear in a few days.

Thank you for submitting this to the repo. I was looking forward it.

Regards,
--
Devrim GÜNDÜZ
Principal Systems Engineer @ EnterpriseDB: http://www.enterprisedb.com
PostgreSQL Danışmanı/Consultant, Red Hat Certified Engineer
Twitter: @DevrimGunduz , @DevrimGunduzTR

Hi,

On Wed, 2014-08-27 at 12:14 +0800, Craig Ringer wrote:
> but it'd probably be better to just ensure the correct pg_config gets
> used from the target postgresql%{pgmajorversion}-devel by placing
>
>     %{pginstdir}/bin
>
> first on the PATH.

FWIW, pg_config is installed with the core rpm, not -devel package.
Also, I don't remember why, but the packaging guidelines asked us to
avoid setting $PATH inside the spec files.

Regards,
--
Devrim GÜNDÜZ
Principal Systems Engineer @ EnterpriseDB: http://www.enterprisedb.com
PostgreSQL Danışmanı/Consultant, Red Hat Certified Engineer
Twitter: @DevrimGunduz , @DevrimGunduzTR