* 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.

(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.

(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? 
 

(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

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

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? 

(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.

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.) 

(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.

(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?

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

> 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. 

> 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? 

(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.

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

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

* File list: some files were listed multiple times
  (wiki: PackageReviewGuidelines)

Seems the *.so files are also listed with the %dir line.

(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

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

Package imported and pushed through the devel build system with no problems.

Thanks for the review!

Package Change Request
======================
Package Name: itcl
New Branches: EL-4 EL-5

cvs done.