Bug 225784

Summary: Merge Review: gdbm
Product: [Fedora] Fedora Reporter: Nobody's working on this, feel free to take it <nobody>
Component: Package ReviewAssignee: Michal Nowak <mnowak>
Status: CLOSED RAWHIDE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: kasal, matthias, mnowak, ohudlick, redhat-bugzilla, rvokal
Target Milestone: ---Flags: mnowak: fedora-review+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: gdbm-1.8.0-32 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2009-04-17 11:22:04 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 223688    
Bug Blocks: 426387    

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

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 Vokál 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.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.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.