Bug 225784 - Merge Review: gdbm
Merge Review: gdbm
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Michal Nowak
Fedora Package Reviews List
:
Depends On: 223688
Blocks: F9MergeReviewTarget
  Show dependency treegraph
 
Reported: 2007-01-31 13:41 EST by Nobody's working on this, feel free to take it
Modified: 2013-03-07 21:03 EST (History)
6 users (show)

See Also:
Fixed In Version: gdbm-1.8.0-32
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-04-17 07:22:04 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
mnowak: fedora‑review+


Attachments (Terms of Use)

  None (edit)
Description Nobody's working on this, feel free to take it 2007-01-31 13:41:39 EST
Fedora Merge Review: gdbm

http://cvs.fedora.redhat.com/viewcvs/devel/gdbm/
Initial Owner: jbj@redhat.com
Comment 1 Ville Skyttä 2007-01-31 14:46:05 EST
Based on changes made in bug 223688, initial owner should be rvokal instead of jbj?
Comment 2 Bill Nottingham 2007-02-01 10:37:52 EST
For the moment, yeah.
Comment 3 Radek Vokal 2007-02-01 10:51:40 EST
I'm a temporary owner before I find someone who will take care of it. Volunteers
welcomed. 
Comment 4 Robert Scheck 2007-02-03 11:09:24 EST
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 11:13:02 EST
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 17:12:34 EST
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 12:28:17 EDT
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 19:00:11 EDT
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 00:29:14 EDT
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 09:58:08 EDT
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 12:02:27 EDT
* 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 06:23:58 EDT
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 07:22:04 EDT
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.