Bug 193161 - Review Request: ruby-postgres
Summary: Review Request: ruby-postgres
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT
TreeView+ depends on / blocked
 
Reported: 2006-05-25 18:54 UTC by David Lutterkort
Modified: 2013-04-30 23:40 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-08-11 23:41:26 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)

Description David Lutterkort 2006-05-25 18:54:15 UTC
Spec URL: http://people.redhat.com/dlutter/yum/spec/ruby-postgres.spec
SRPM URL: http://people.redhat.com/dlutter/yum/SRPMS/ruby-postgres-0.7.1-1.src.rpm
Description: Database driver to access PostgreSQL databases from Ruby.

Comment 1 Parag AN(पराग) 2006-06-01 08:48:06 UTC
Need Some SPEC Changes:-
* There should be a rm -rf $RPM_BUILD_ROOT at the beginning of %install
* Consider using make %{?_smp_mflags} in %build.

Comment 2 David Lutterkort 2006-06-06 00:22:51 UTC
Both issues are addressed in the latest update to the RPM.

Spec URL: http://people.redhat.com/dlutter/yum/spec/ruby-postgres.spec
SRPM URL: http://people.redhat.com/dlutter/yum/SRPMS/ruby-postgres-0.7.1-2.src.rpm

Comment 3 Jason Tibbitts 2006-07-11 03:16:34 UTC
Would you care to fix up this spec given the discussion from the ruby-sqlite3
package?  (ruby(abi) require, strip "dir" from the first two macros, consistency
of macros, etc.)

As before, rpm will find the postgres.so dependency so you can probably drop the
manual postrgresql require.  Also, I note you list the license as Distributable,
but "Ruby License" should be fine.

Comment 4 David Lutterkort 2006-07-11 21:54:56 UTC
Oops .. I've been meaning to make those changes, but didn't get around to it
yetserday. Made them today.

New files:
Spec URL: http://people.redhat.com/dlutter/yum/spec/ruby-postgres.spec
SRPM URL: http://people.redhat.com/dlutter/yum/SRPMS/ruby-postgres-0.7.1-3.src.rpm

Comment 5 Jason Tibbitts 2006-07-31 00:51:58 UTC
I note you use a buildroot of %{_tmppath}/%{name}-%{version}-root instead of the
usual %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n).  This
isn't a blocker and in fact is under some discussion but I did want to bring
attention to it in case it was unintentional.

I note that you export CFLAGS, but the generated makefile doesn't seem to make
use of it and the compiler is called with a different set of flags that, as a
side effect, lead to a busted debuginfo package.

Nothing this package depends on seems to own
/usr/lib64/ruby/site_ruby/1.8/x86_64-linux/ (or whatever that is for each arch).
 Other owners are ruby-sqlite4 and kdebindings.  I think this package needs to
own it as well.  (Or that the base ruby package should, which is I suppose is a
matter for another bugzilla ticket.)

* source files match upstream:
   8ef67b3f4b089248f0420baeb0e3b3c8  ruby-postgres-0.7.1.tar.gz
* package meets naming and packaging guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* dist tag is present.
? build root is correct.
* license field matches the actual license.
* license is open source-compatible.  License text not included upstream.
* BuildRequires are proper.
X compiler flags are appropriate.
* %clean is present.
* package builds in mock (development, x86_64).
X debuginfo package looks complete.
* rpmlint is silent.
* final provides and requires are sane:
   postgres.so()(64bit)
   ruby(postgres) = 0.7.1
   ruby-postgres = 0.7.1-3.fc6
  =
   libpq.so.4()(64bit)
   libruby.so.1.8()(64bit)
   ruby >= 1.3
   ruby(abi) = 1.8
* %check is not present; no callable test suite upstream.
* shared libraries are present, but internal to ruby.
* package is not relocatable.
X owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no scriptlets present.
* code, not content.
* documentation is small, so no -docs subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.
* no headers.
* no pkgconfig files.
* no libtool .la droppings.

Comment 6 David Lutterkort 2006-08-03 00:37:09 UTC
Addressed these issues. New files:
Spec URL: http://people.redhat.com/dlutter/yum/spec/ruby-postgres.spec
SRPM URL: http://people.redhat.com/dlutter/yum/SRPMS/ruby-postgres-0.7.1-3.src.rpm

Specifically:
(1) Fixed the buildroot (was an oversight)
(2) Fixed the way CFLAGS are handled, debuginfo package now looks sane (to me)
(3) Didn't do anything with the issue of
/usr/lib64/ruby/site_ruby/1.8/x86_64-linux/ - this directory should be owned by
ruby-libs (and it is on my i386 box with ruby-libs-1.8.4-8.fc5; it's
%ruby_sitearch) - ruby-libs is a dependency (a) because it provides ruby(abi)
and (b) since the package ruby requires it

Comment 7 Jason Tibbitts 2006-08-03 04:54:35 UTC
Hmmm.  It still builds OK, but here's how the compiler is called:

 gcc -fPIC   -I. -I/usr/lib64/ruby/1.8/x86_64-linux
-I/usr/lib64/ruby/1.8/x86_64-linux -I. -DHAVE_SYS_UN_H -DHAVE_SOCKET
-DHAVE_GETHOSTNAME -DHAVE_PQSETCLIENTENCODING -D
HAVE_PG_ENCODING_TO_CHAR -DHAVE_PQESCAPESTRING  -c postgres.c

Wait, did you actually update the package?  That looks to be the same package as
before.  The spec looks unchanged as well.

BTW, I looked at the ruby-libs package on i386 and x86_64 and neither of them
owns the sitearch directory.  I see these directories:

/usr/lib64/ruby/site_ruby/1.8
/usr/lib/ruby/site_ruby/1.8
but definitely not anything under those.  This is backed up by repoquery:

> repoquery --whatprovides /usr/lib/ruby/site_ruby/1.8/i386-linux
kdebindings-0:3.5.3-0.1.fc5.i386

> repoquery --whatprovides /usr/lib64/ruby/site_ruby/1.8/x86_64-linux
kdebindings-0:3.5.3-0.1.fc5.x86_64

I do think this is a bug in base ruby package, but currently it looks like an
arch-specific Ruby package needs to own that directory.

Comment 8 David Lutterkort 2006-08-03 16:14:14 UTC
Rats, I am really sorry about that - yesterday wasn't my day. I forgot the
crucial step of uploading the rpm's from my local machine to p.r.c. Now I did
(and double-checked):

Spec URL: http://people.redhat.com/dlutter/yum/spec/ruby-postgres.spec
SRPM URL: http://people.redhat.com/dlutter/yum/SRPMS/ruby-postgres-0.7.1-4.src.rpm

You are absolutely right about sitearchdir; for some reason, I kept checking
archdir (which is owned by ruby-libs). I filed a bug against the ruby package
for that (bug 201208)

Comment 9 Jason Tibbitts 2006-08-08 22:57:35 UTC
That looks significantly better; the compiler is called properly and the
debuginfo package includes the source.

It looks like the directory ownership problem is being fixed in the core
ruby-libs package as well, so I think things are ready to go.  The only issue is
whether ruby-libs will be fixed for FC5 and if not how you deal with that.  I'll
leave it to you since everything is fine on FC6.

APPROVED

Comment 10 David Lutterkort 2006-08-11 23:41:26 UTC
Imported and built successfully into devel.


Note You need to log in before you can comment on or make changes to this bug.