Bug 174265 - Review Request: itcl - Object oriented extension to Tcl
Review Request: itcl - Object oriented extension to Tcl
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: John Mahowald
David Lawrence
http://incrtcl.sourceforge.net/
:
Depends On:
Blocks: FE-ACCEPT 177359
  Show dependency treegraph
 
Reported: 2005-11-26 17:33 EST by Wart
Modified: 2007-11-30 17:11 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-02-04 12:07:25 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Wart 2005-11-26 17:33:13 EST
Spec Name or Url: http://www.kobold.org/~wart/fedora/itcl.spec
SRPM Name or Url: http://www.kobold.org/~wart/fedora/itcl-3.2.1-3.src.rpm
MD5 sums:  http://www.kobold.org/~wart/fedora/MD5SUM.asc
Description: [incr Tcl] is Tcl extension that provides object-oriented features that are missing from the Tcl language.  This package contains both itcl and the itk extensions to the Tk toolkit.
Comment 1 John Mahowald 2005-12-26 16:10:56 EST
* Why not package the itk extensions seperately? tcl and tk are seperate.
* Source 0 is not available
(http://sourceforge.net/project/incrtcl/itcl3.2.1_src.tgz)
  (wiki: QAChecklist item 2)
It seems the only way to enable automated sourceforge downloads is to hard code
a mirror. 
* 3.3 seems to have been released.
* Some compiler warnings, mostly about pointers, that you may want to inform
upstream about.
* To clarify, the large patch included is to update configure, respect DESTDIR,
look in lib64, etc.? May go upstream, possibly saving 100KB from the srpm!

Good:
* Builds fine in mock
* rpmlint happy, except for no docs for -devel.
Comment 2 Wart 2005-12-28 19:57:07 EST
(In reply to comment #1)
> * Why not package the itk extensions seperately? tcl and tk are seperate.

Tcl and Tk are built from separate source archives, while itcl/itk use a single
source tarball for both packages.  This gives me two choices if they are to be
packaged separately:

- Create two spec files that point to the same source tarball so that the
package names (itcl, itk) can be preserved
- Create itk as a subpackage so that only one copy of the source tarball is
needed:  itcl, itcl-itk

I'm leaning towards the second approach, even if it doesn't preserve the
upstream package name.  Using the same source tarball in two packages seems
awkward and error-prone.

> * Source 0 is not available
> (http://sourceforge.net/project/incrtcl/itcl3.2.1_src.tgz)
>   (wiki: QAChecklist item 2)
> It seems the only way to enable automated sourceforge downloads is to hard code
> a mirror.

I'll fix this in the next spec file.

> * 3.3 seems to have been released.

3.3 is still a release candidate, and has build problems on Linux. 
Additionally, the 3.3 release breaks the iwidgets package, which is the main
reason that I'm packaging itcl to begin with.  Once itcl 3.3 works with iwidgets
then I'll upgrade.

> * Some compiler warnings, mostly about pointers, that you may want to inform
> upstream about.

Will do.

> * To clarify, the large patch included is to update configure, respect DESTDIR,
> look in lib64, etc.? May go upstream, possibly saving 100KB from the srpm!

Correct.  I'll notifiy upstream so that this makes it into the 3.3 final release.
Comment 3 José Matos 2005-12-29 02:54:16 EST
(In reply to comment #2) 
>  
> - Create two spec files that point to the same source tarball so that the 
> package names (itcl, itk) can be preserved 
> - Create itk as a subpackage so that only one copy of the source tarball is 
> needed:  itcl, itcl-itk 
>  
> I'm leaning towards the second approach, even if it doesn't preserve the 
> upstream package name.  Using the same source tarball in two packages seems 
> awkward and error-prone. 
 
  Why not the second hypothesis calling the subpackage itk instead of 
itcl-itk? 
 
Comment 4 Wart 2005-12-29 13:33:50 EST
(In reply to comment #3)
> (In reply to comment #2) 
> >  
> > - Create two spec files that point to the same source tarball so that the 
> > package names (itcl, itk) can be preserved 
> > - Create itk as a subpackage so that only one copy of the source tarball is 
> > needed:  itcl, itcl-itk 
> >  
> > I'm leaning towards the second approach, even if it doesn't preserve the 
> > upstream package name.  Using the same source tarball in two packages seems 
> > awkward and error-prone. 
>  
>   Why not the second hypothesis calling the subpackage itk instead of 
> itcl-itk? 

Because I didn't know about "%package -n" until 10 minutes ago.  :)

I've now split it up into 4 packages:  itcl, itk, itcl-devel, itk-devel

The new spec file and srpm addressing the issues raised in comment #1 can be
found at:

http://www.kobold.org/~wart/fedora/itcl-3.2.1-4.src.rpm
http://www.kobold.org/~wart/fedora/itcl.spec
Comment 5 Wart 2005-12-29 21:59:08 EST
I added a missing Requires: for itk-devel in this update:

http://www.kobold.org/~wart/fedora/itcl-3.2.1-5.src.rpm
http://www.kobold.org/~wart/fedora/itcl.spec
Comment 6 Kevin Kofler 2006-01-08 07:10:33 EST
Shouldn't the .so files be in the default library search path (i.e. 
in /usr/lib) rather than in /usr/lib/itcl3.2 or /usr/lib/itk3.2, respectively? 
Comment 7 Wart 2006-01-08 12:03:35 EST
(In reply to comment #6)
> Shouldn't the .so files be in the default library search path (i.e. 
> in /usr/lib) rather than in /usr/lib/itcl3.2 or /usr/lib/itk3.2, respectively? 

No, they shouldn't.  These are libraries that are loaded by Tcl directly through
dlopen(), not by using the dynamic linker.
Comment 8 Kevin Kofler 2006-01-08 16:28:20 EST
Insight (the GDB GUI) at least wants to link them directly. (They also include 
their own copies of Tcl/Tk/[incr Tcl]/[incr Tk]/[incr widgets] because they use 
internal Tcl headers, but with some packaging tricks you can get the binary RPM 
to use the installed ones instead.) 
Comment 9 Wart 2006-01-08 17:59:15 EST
(In reply to comment #8)
> Insight (the GDB GUI) at least wants to link them directly. (They also include 
> their own copies of Tcl/Tk/[incr Tcl]/[incr Tk]/[incr widgets] because they use 
> internal Tcl headers, but with some packaging tricks you can get the binary RPM 
> to use the installed ones instead.) 

The recommended way of linking against these libraries is to use the
itclConfig.sh and itkConfig.sh files provided by the -devel packages. 
However...  itcl 3.2 does not generate usable itclConfig.sh/itkConfig.sh files,
so they are not included.  itcl 3.3, which is still under development, will have
usable itclConfig.sh/itkConfig.sh files for linking directly against the extensions.

I'm working on packaging itcl 3.3, even though it's still a release candidate. 
Until it works with iwidgets, though, I don't plan to upgrade the packages for FE.
Comment 10 Wart 2006-01-08 19:06:26 EST
(In reply to comment #8)
> Insight (the GDB GUI) at least wants to link them directly. (They also include 
> their own copies of Tcl/Tk/[incr Tcl]/[incr Tk]/[incr widgets] because they use 
> internal Tcl headers, but with some packaging tricks you can get the binary RPM 
> to use the installed ones instead.) 

Additionally, why does Insight need to link against the itcl/itk .so's instead
of using Tcl's 'package require' mechanism to locate and load the extensions?
Comment 11 Wart 2006-01-08 19:26:44 EST
itcl now owns its own directories /usr/lib/itcl3.2 and /usr/lib/itk3.2:

http://www.kobold.org/~wart/fedora/itcl-3.2.1-6.src.rpm
http://www.kobold.org/~wart/fedora/itcl.spec
Comment 12 Kevin Kofler 2006-01-08 19:36:13 EST
> Additionally, why does Insight need to link against the itcl/itk .so's 
instead 
> of using Tcl's 'package require' mechanism to locate and load the extensions? 
 
Probably because they're hacking internal data structures (the same reason 
they're bundling their own copy of the source to get access to private 
headers). 
 
Anyway, for my Insight-derived package (http://lpg.ticalc.org/prj_tiemu/), I 
can just pass -Wl,-rpath arguments, and those building Insight (or TiEmu) from 
source will get the bundled copy of itcl anyway (and thus not require the 
system itcl package at all), so this is probably a non-issue. 
Comment 13 Kevin Kofler 2006-01-09 16:26:50 EST
> I'm working on packaging itcl 3.3, even though it's still a release 
candidate.  
> Until it works with iwidgets, though, I don't plan to upgrade the packages 
for FE. 
I need iwidgets too, so I can only agree. 
 
But why does the upgrade break iwidgets? If it's incompatible language changes, 
then won't that break user programs too? 
Comment 14 Wart 2006-01-09 16:48:49 EST
(In reply to comment #13)
> > I'm working on packaging itcl 3.3, even though it's still a release 
> candidate.  
> > Until it works with iwidgets, though, I don't plan to upgrade the packages 
> for FE. 
> I need iwidgets too, so I can only agree. 
>  
> But why does the upgrade break iwidgets? If it's incompatible language changes, 
> then won't that break user programs too? 

Quite likely, yes.  I'll try to dig up the original post that mentioned the
incompatibility and verify it myself on some of my own code.  It's possible that
I misread a build incompatibility (which I fixed in the spec files) as a runtime
incompatibility.  If so then I'll update these packages from 3.2 to 3.3.
Comment 15 Wart 2006-01-09 19:36:45 EST
It appears that the incompatibility between itcl3.3 and iwidgets was a build
problem, not a runtime problem.  I was able to test an iwidgets application with
itcl3.3 with no problems.

Starting with version 3.3, itk has been split of into a separate source archive.
 I'll submit itk 3.3 as a separate package shortly.

Without further ado, here is the itcl 3.3 release candidate package:

http://www.kobold.org/~wart/fedora/itcl-3.3-0.1.RC1.src.rpm
http://www.kobold.org/~wart/fedora/itcl.spec
Comment 16 Wart 2006-01-12 00:00:07 EST
New package that fixes the build problems on the devel branch.

http://www.kobold.org/~wart/fedora/itcl-3.3-0.2.RC1.src.rpm
http://www.kobold.org/~wart/fedora/itcl.spec
Comment 17 John Mahowald 2006-01-27 11:42:50 EST
* File list: some files were listed multiple times
  (wiki: PackageReviewGuidelines)

Seems the *.so files are also listed with the %dir line.
Comment 18 Wart 2006-01-27 15:13:56 EST
(In reply to comment #17)
> * File list: some files were listed multiple times
>   (wiki: PackageReviewGuidelines)
> 
> Seems the *.so files are also listed with the %dir line.

Fixed:

http://www.kobold.org/~wart/fedora/itcl.spec
http://www.kobold.org/~wart/fedora/itcl-3.3-0.3.RC1.src.rpm
Comment 19 John Mahowald 2006-02-04 02:09:42 EST
Good:

- rpmlint checks:
W: itcl-devel no-documentation
can ignore
- package meets naming guidelines
- package meets packaging guidelines
- license (BSD) OK, text in %doc, matches source
- spec file legible, in am. english
- source matches upstream
- package compiles on FC4 i386
- no missing BR
- no unnecessary BR
- no locales
- not relocatable
- owns all directories that it creates
- no duplicate files
- permissions ok
- %clean ok
- macro use consistent
- code, not content
- no need for -docs
- nothing in %doc affects runtime
- no need for .desktop file 

APPROVED
Comment 20 Wart 2006-02-04 12:07:25 EST
Package imported and pushed through the devel build system with no problems.

Thanks for the review!
Comment 21 Wart 2007-06-27 11:13:09 EDT
Package Change Request
======================
Package Name: itcl
New Branches: EL-4 EL-5
Comment 22 Kevin Fenzi 2007-06-27 15:46:42 EDT
cvs done.

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