Bug 213180 - Review Request: tcl-thread - Thread extension for Tcl
Review Request: tcl-thread - Thread extension for Tcl
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Wart
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-10-30 21:24 EST by Josh Boyer
Modified: 2007-11-30 17:11 EST (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-11-06 09:41:01 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
Look for gdbm library in %{_libdir} (838 bytes, patch)
2006-11-02 17:47 EST, Wart
no flags Details | Diff

  None (edit)
Description Josh Boyer 2006-10-30 21:24:39 EST
Spec URL: http://jdub.homelinux.org/pub/tcl-thread.spec
SRPM URL: http://jdub.homelinux.org/pub/tcl-thread-2.6.5-1.src.rpm
Description:

Thread extention for Tcl
Comment 1 Wart 2006-11-02 17:25:20 EST
This does not build properly on x86_64 in mock.  It seems that the configure
script is written to only look for libgdbm.so in /usr/lib, not /usr/lib64.  You
might try adding '--with-gdbm=%{_libdir}' to %configure, or modifying the
configure script to look for the library in ${libdir}.
Comment 2 Wart 2006-11-02 17:47:03 EST
Created attachment 140195 [details]
Look for gdbm library in %{_libdir}

This patch modifies the configure script to properly look for the gdbm library
in $libdir, instead of only looking in /usr/lib.
Comment 3 Wart 2006-11-02 18:04:56 EST
GOOD
====
* Package and spec named appropriately:  The upstream name is the simply
  'thread', which is far too generic.  Following the examples for python
  and perl modules, the name tcl-thread is acceptable.
* Spec file is legible and in Am. English
* Source matches upstream:
  3c69b4a891590f23bb79a1fa98d879f7  thread2.6.5.tar.gz
* No unnecessary BuildRequires
* No locales
* No shared libraries in the default linker path; the shared library
  that is produced is loaded by Tcl via dlopen.
* RPM_BUILD_ROOT cleaned where appropriate
* Not relocatable
* No duplicate %files
* File permissions look ok
* No need for a -devel subpackage
* Not a gui program; no need for a .desktop file
* Package loads into Tcl as expected and passes its own test suite.
* Consistent use of macros
* Does not own any directories that it should not own.

MUSTFIX
=======
 * License does not match upstream.  Should be BSD.
 * License file 'license.terms' not included.
 * Add the README and ChangeLog files to %doc

 * Does not own all directories that it creates.  In %files, change
%{_libdir}/thread%{version}/*
to
%{_libdir}/thread%{version}

 * Does not build properly on x86_64 in mock.  The attached patch
   fixes the problem.

 * The dependency on gdbm is picked up automatically.  You can drop
   Requires: gdbm.

SHOULDFIX
=========
* Missing a %check section for running the unit tests.
Comment 4 Josh Boyer 2006-11-02 23:20:49 EST
Thanks much for the review.  I'll get those items fixed up, including the x86_64
issue, tomorrow.  I should have fixed the MUSTFIXes before submitting, sorry
about that.  That's what I get for copying the initial spec from a different
package.
Comment 5 Wart 2006-11-02 23:39:01 EST
np.  I'm a sucker for Tcl package reviews, and I'm interested to see how well
this works with tclhttpd.  Is there another review on the way that depends on
this package?
Comment 6 Josh Boyer 2006-11-03 06:38:22 EST
(In reply to comment #5)
> np.  I'm a sucker for Tcl package reviews, and I'm interested to see how well
> this works with tclhttpd.  Is there another review on the way that depends on
> this package?

Not as of yet.  However the gitk author is probably going to use the Thread
extention soon so that the main window is still responsive while another thread
does the refresh/lookups.

Comment 7 Josh Boyer 2006-11-03 22:03:18 EST
Ok, all comments should be fixed.

Spec URL: http://jdub.homelinux.org/pub/tcl-thread.spec
SRPM URL: http://jdub.homelinux.org/pub/tcl-thread-2.6.5-2.src.rpm
Comment 8 Wart 2006-11-04 01:21:34 EST
Looks good.

APPROVED

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