Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
Bug 225784 - Merge Review: gdbm
Summary: Merge Review: gdbm
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Michal Nowak
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On: 223688
Blocks: F9MergeReviewTarget
TreeView+ depends on / blocked
 
Reported: 2007-01-31 18:41 UTC by Nobody's working on this, feel free to take it
Modified: 2013-03-08 02:03 UTC (History)
6 users (show)

Fixed In Version: gdbm-1.8.0-32
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-04-17 11:22:04 UTC
mnowak: fedora-review+


Attachments (Terms of Use)

Description Nobody's working on this, feel free to take it 2007-01-31 18:41:39 UTC
Fedora Merge Review: gdbm

http://cvs.fedora.redhat.com/viewcvs/devel/gdbm/
Initial Owner: jbj@redhat.com

Comment 1 Ville Skyttä 2007-01-31 19:46:05 UTC
Based on changes made in bug 223688, initial owner should be rvokal instead of jbj?

Comment 2 Bill Nottingham 2007-02-01 15:37:52 UTC
For the moment, yeah.

Comment 3 Radek Vokal 2007-02-01 15:51:40 UTC
I'm a temporary owner before I find someone who will take care of it. Volunteers
welcomed. 

Comment 4 Robert Scheck 2007-02-03 16:09:24 UTC
The package should be updated, because the version Fedora ships seems to be 
outdated - or asked the other way round: Is this package really required?

Comment 5 Robert Scheck 2007-02-03 16:13:02 UTC
Ah well, if cyrus-sasl, python and perl must depend to gdbm and gdbm-devel 
further on, I would take care of it - when allowed and possible of course.

Comment 6 Robert Scheck 2007-02-03 22:12:34 UTC
IMHO the main reason for still keeping gdbm (which is compatibility interface 
to dbm and ndbm) were removed from the main library and moved to gdbm_compat in 
gdbm 1.8.1. Or am I wrong?

Comment 7 Matthias Saou 2007-08-31 16:28:17 UTC
If someone decides to give the gdbm package some attention, then I'd really
appreciate if bug #162416 was also taken care of.

Comment 8 Ondrej Dvoracek 2007-09-05 23:00:11 UTC
Hi,
bug #162416 is RFE and doesn't block this merge review. It is a request for some
additional package that would help to port gdbm files between architectures.
Cheers Ondrej.

Comment 9 Sam Steingold 2007-10-19 04:29:14 UTC
gdbm is a small and useful package and I really see no reason not to offer it.
GNU CLISP 2.42 just added an interface to gdbm.
is there a way to vote for gdbm?

Comment 10 Sam Steingold 2008-06-04 13:58:08 UTC
fc9 comes with gdbm 1.8.0 which is 9 years old and has known bugs, 
fixed in 1.8.3 (the latest version, released 5.5 years ago).
please upgrade!

also, please change "component" to "gdbm"
also, please change "version" to "9"

Comment 11 Michal Nowak 2009-04-08 16:02:27 UTC
* rpmlint

gdbm.spec: E: non-utf8-spec-file gdbm.spec

  newman@dhcp-lab-124 SPECS $ file gdbm.spec 
  gdbm.spec: ISO-8859 English text

gdbm.spec:30: E: prereq-use /sbin/install-info

"""
The use of PreReq is deprecated. In the majority of cases, a plain Requires is
enough and the right thing to do. Sometimes Requires(pre), Requires(post),
Requires(preun) and/or Requires(postun) can also be used instead of PreReq.
"""

gdbm.src: W: summary-ended-with-dot A GNU set of database routines which use extensible hashing.

> Summary: A GNU set of database routines which use extensible hashing.

gdbm.x86_64: W: shared-lib-calls-exit /usr/lib64/libgdbm.so.2.0.0 exit@GLIBC_2.2.5
gdbm.x86_64: W: shared-lib-calls-exit /usr/lib64/libgdbm.so.2.0.0 exit@@GLIBC_2.2.5

Should be investigated.

* 1.8.3 was released but not sure whether is it good idea to incorporate it

* 

Patch0: gdbm-1.8.0-jbj.patch
Patch1: gdbm-1.8.0-fhs.patch
Patch3: gdbm-1.8.0-64offset.patch

Could be: 0-1-2

* Source: ftp://ftp.gnu.org/gnu/gdbm-%{version}.tar.gz 

is wrong, correct it to

Source: ftp://ftp.gnu.org/gnu/gdbm/gdbm-%{version}.tar.gz

* > BuildRoot: %{_tmppath}/%{name}-%{version}-root

https://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag

* -devel: Requires: gdbm = %{version}

https://fedoraproject.org/wiki/Packaging/Guidelines#Requiring_Base_Package

* Generally, depend on packages not files with full path:

  Prereq: /sbin/install-info

this might also be the case where you can safely depend on "info"package.

* Consistency with the "-p1" option

%patch1 -p 1 -b .fhs
%patch3 -p1 -b .offset

* 

> # refresh config.sub, the original one does not recognize "redhat"
> # as vendorname:
> for f in /usr/share/automake-1.1?/config.sub; do
>   :
> done
> cp -p $f .
> libtoolize --force --copy
> aclocal
> autoconf

Perhaps autoreconf and patching the build system seems better to me. But not that important.

* discouraged: %makeinstall install-compat

https://fedoraproject.org/wiki/Packaging/Guidelines#Why_the_.25makeinstall_macro_should_not_be_used

* %defattr(-,root,root)
       ->
  %defattr(-,root,root,-)

(twice in spec)

Comment 12 Stepan Kasal 2009-04-17 10:23:58 UTC
Thank you for the review.
I have fixed most of the issues, the remaining ones are explained below.
Tell me if I missed any.

(In reply to comment #11)
> gdbm.x86_64: W: shared-lib-calls-exit /usr/lib64/libgdbm.so.2.0.0
> exit@GLIBC_2.2.5
> gdbm.x86_64: W: shared-lib-calls-exit /usr/lib64/libgdbm.so.2.0.0
> exit@@GLIBC_2.2.5
> 
> Should be investigated.

exit() is called from _gdbm_fatal() which gets called if a malloc or read fails.

The reason why calling exit from a library is a bad idea is that the caller should get a chance to clean up.  With gdbm, you can register a hook function that gets called from _gdbm_fatal before the exit. This may not be the best design decision, but it still somehow works.

Concerning this and the fact that the code has not changed the last five years, I conclude that is would not be a good idea to try to patch this issue.
(At leat until a real life problem related to the issue appears.)
 
> * 1.8.3 was released but not sure whether is it good idea to incorporate it

Might be a good idea, but I believe that can be postponed after finishing this review.

> Patch0: gdbm-1.8.0-jbj.patch
> Patch1: gdbm-1.8.0-fhs.patch
> Patch3: gdbm-1.8.0-64offset.patch
> 
> Could be: 0-1-2

I don't think it is advisable to renumber each time a patch is dropped.
Consequently, the sequence of the numbers can hardly be maintained.

> * discouraged: %makeinstall install-compat
> 
> https://fedoraproject.org/wiki/Packaging/Guidelines#Why_the_.25makeinstall_macro_should_not_be_used

Quote: "%makeinstall macro [...] must NOT be used when make install DESTDIR=%{buildroot} works"

grep -r DESTDIR gdbm-1.8.0 returns nothig.  Most projects get DESTDIR support for free from Automake but this one does not use Automake, as mentioned above.

> > # refresh config.sub, the original one does not recognize "redhat"
> > # as vendorname:
> > for f in /usr/share/automake-1.1?/config.sub; do
> >   :
> > done
> > cp -p $f .
> > libtoolize --force --copy
> > aclocal
> > autoconf
> 
> Perhaps autoreconf and patching the build system seems better to me. But not
> that important.

config.{sub,guess} scripts are totally autonomous shell script.
They can get copied to the project by "automake --add-missing --force-missing"  but, unfortunately, this project does not use Automake.

Otherwise, autoreconf is just another way to call the appropriate tools.
Calling them directly is not a bad idea either, it is more transparent.

Perhaps you meant that we could patch the build system to use Automake.
That's a valid idea, I might decide to do that in future.
But keeping the original build system should also be acceptible for the review.

Comment 13 Michal Nowak 2009-04-17 11:22:04 UTC
I guess it's good idea to patch GDBM in version 1.8.3 to be auto*-aware.

Thanks for changes in GDBM, this package is APPROVED & REVIEWED by me. Closing.


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