Bug 244536

Hi Devrim,

Some initial comments:
- How about postgresql-tablelog for the package name instead of postgresql-table_log?
- BuildRoot should be one of 
   %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
   %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
   %{_tmppath}/%{name}-%{version}-%{release}-root
- Maybe rename README.table_log to just README

Hi Ruben,

(In reply to comment #1)

> - How about postgresql-tablelog for the package name instead of
postgresql-table_log?

The name of the software is table_log, so I did not want to change it...

> - BuildRoot should be one of 
>    %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
>    %(mktemp -ud %{_tmppath}/%{name}-%{version}-%{release}-XXXXXX)
>    %{_tmppath}/%{name}-%{version}-%{release}-root

Oops, fixed.

> - Maybe rename README.table_log to just README

Since it will go to contrib directory of PostgreSQL, we want to stick the naming
guidelines there. It is in the form README.nameofthesoftware.

Thanks for the review. I did not bump up the spec file version; but updated spec
and srpm at the website:

Spec URL:
http://developer.postgresql.org/~devrim/rpms/other/table_log/postgresql-table_log.spec
SRPM URL:
http://developer.postgresql.org/~devrim/rpms/other/table_log/postgresql-table_log-0.4.4-1.fc7.src.rpm

Regards, Devrim

> Since it will go to contrib directory of PostgreSQL, we want to stick the naming
> guidelines there. It is in the form README.nameofthesoftware

But in Fedora it is installed in /usr/share/doc/postgresql-table_log-0.4.4/
You can move it in the specfile. Not a big issue though.

On another note, I tried to get the package working. The functions in table_log_init.sql are written in 
plpgsql, which wasn't installed on my system. Can you add postgresql-plpgsql to the Requires?

Hello,

(In reply to comment #3)
 
> But in Fedora it is installed in /usr/share/doc/postgresql-table_log-0.4.4/
> You can move it in the specfile. Not a big issue though.

Ok done :) I also moved .sql files to _datadir/name from contrib. Yeah, it must
not be installed under contrib directory.
 
> On another note, I tried to get the package working. The functions in 
> table_log_init.sql are written in plpgsql, which wasn't installed on my
> system. Can you add postgresql-plpgsql to the Requires?

plpgsql is in core, so I added postgresql-server to Requires. (You will still
need to create plpgsql in your test database with createlang utility. createlang
is provided by postgresql package, but I did not add it to requires, because it
is required by postgresql-server).

Here is the new spec and SRPM:

Spec URL:
http://developer.postgresql.org/~devrim/rpms/other/table_log/postgresql-table_log.spec
SRPM URL:
http://developer.postgresql.org/~devrim/rpms/other/table_log/postgresql-table_log-0.4.4-2.fc7.src.rpm

Regards, Devrim

Hi again,

> plpgsql is in core

Nope, it's in postgresql-plperl

[ruben@odin review]$ rpm -qi postgresql-plperl
Name        : postgresql-plperl            Relocations: (not relocatable)
Version     : 8.2.4                             Vendor: Red Hat, Inc.
Release     : 1.fc7                         Build Date: Tue 24 Apr 2007 09:27:06 PM CEST
Install Date: Sun 17 Jun 2007 01:08:32 PM CEST      Build Host: ls20-bc1-14.build.redhat.com
Group       : Applications/Databases        Source RPM: postgresql-8.2.4-1.fc7.src.rpm
Size        : 67960                            License: BSD
Signature   : DSA/SHA1, Mon 04 Jun 2007 04:45:23 AM CEST, Key ID b44269d04f2a6fd2
Packager    : Red Hat, Inc. <http://bugzilla.redhat.com/bugzilla>
URL         : http://www.postgresql.org/
Summary     : The Perl procedural language for PostgreSQL.
Description :
PostgreSQL is an advanced Object-Relational database management
system.  The postgresql-plperl package contains the PL/Perl
procedural language for the backend.

[ruben@odin review]$ rpm -ql postgresql-plperl
/usr/lib/pgsql/plperl.so

So you have to require postgresql-plperl (which wil pull in postgresql and postgresql-server)

Hello Ruben,

(In reply to comment #5)

> > plpgsql is in core
> 
> Nope, it's in postgresql-plperl

No, it is in postgresql-server:

$ rpm -ql postgresql-server|grep plpgsql
/usr/lib/pgsql/plpgsql.so

and table_log requires plpgsql, not plperl.

Regards, Devrim

I'm so sorry, you're absolutely right. There's a subtle difference between pl and perl ;-)

Review for release 2:
* RPM name is OK
* Source table_log-0.4.4.tar.gz is the same as upstream
* This is the latest version
* Builds fine in mock
* rpmlint looks OK
* File list looks OK
* Runs ok

This package is approved.

New Package CVS Request
=======================
Package Name: postgresql-table_log
Short Description:  Log data changes in a PostgreSQL table
Owners: devrim
Branches: EL-4 EL-5 FC-6 F-7

cvs done.

Devrim, don't forget to close this ticket when you're done.

Oh, packages are already built and submitted. Closing this. Thanks for the reminder.