An ImageMagick review (partial) [Was Re: [ITP - Ready for review] ImageMagick]

[Charles Wilson]

Harold L Hunt II wrote:
> [The package now has plenty of votes and I have installed the package 
> and tested the 'convert' command.  I would like someone else to give the 
> package at least a brief review before Daniel or I upload it.]
> 
> I would like to contribute and maintain ImageMagick:
> 
>     http://www.imagemagick.org/
> 
> ImageMagick 5.5.7 is a robust collection of tools and libraries offered 
> under a usage license to read, write, and manipulate an image in many 
> image formats (over 89 major formats) including popular formats like 
> TIFF, JPEG, PNG, PDF, PhotoCD, and GIF.
> 
> 
> Changes since initial ITP
> =========================
> 1) Patched configure.ac to actually stick LIBS onto the end of the libs 
> passed to the linker, as the ImageMagick documentation says can be done.
> 
> 2) Pass LIBS=-lcygipc to configure.
> 
> 3) Pass --enable-shared to configure.  Builds shared cygMagick-6.dll and 
> cygMagick++-6.dll... drops each executable from 2+ MiB to roughly 10 KiB.
> 
> 4) Create three packages instead of one: ImageMagick, libMagick6, and 
> libMagick-devel.
> 
> 5) Change my PATH before building so that 'convert' is not found in the 
> Windows/System32/ directory.
> 

I think that /usr/bin/Magick-config and /usr/bin/Magick++-config should 
be in the -devel package, instead of the main one. (ditto the man pages 
for them)

I think the entire /usr/lib/ImageMagick-5.5.7/ tree should be in its own 
versioned package: And the dll package and the main package should 
depend on it.  But it definitely shouldn't be in the -devel package 
because the runtime queries it!  Possible names:

   ImageMagick6-5.5.7-1
   ImageMagick5_5_7-5.5.7-1
   ImageMagick5_5_7-libMagick6-5.5.7-1
   <or just stick 'em into the main ImageMagick-5.5.7-1 package>

The problem with 'ImageMagick6' is
   (a) when 5.5.8 is released, IF the API hasn't changed and the DLLNUM 
stays '6', then no harm no foul.  The new libMagick6 (from 5.5.8) will 
still require the matching ImageMagick6 (also from 5.5.8, using the 
/usr/lib/ImageMagick-5.5.8/ directory).

   (b) Suppose for whatever reason, the API changes but the IM version 
does not -- perhaps due to a new choice of compilation options, or 
cygwin-specific patches that modify the API.  Now you've got libMagick7 
-- and ImageMagick7, presumably.  But ImageMagick6 and ImageMagick7 
can't coexist -- because they are both from the same 'upstream' version 
of ImageMagick and use the same dir in /usr/lib/.

   (b1) "Don't do that".  Fine -- but sometimes it has actually happened 
that I needed to bump the DLL version in the same upstream release. Say, 
for instance, when cygwin kernel 1.5 was released.  So it DOES, in fact, 
occur.

   (b2) "Don't version it at all" -- or keep it in the main package. 
Well, if you do that, then you can only have one "copy" at a time 
installed on your system.  But if you have cygMagick-6.dll from 5.5.7 
and, say, cygMagick-7.dll from 5.5.10 -- then cygMagick-6.dll is going 
to want /usr/lib/ImageMagick-5.5.7/* and cygMagick-7.dll is going to 
want /usr/lib/ImageMagick-5.5.10/*

Okay, I get that.  So what about 'ImageMagick5_5_7'
   (c) "What's the harm? Keep a separate sequence number for this set of 
files.  Since it's the dirname "ImageMagick5.5.7" that causes the 
conflict, why don't we just use "5_5_7" for the vernum for this 
subpackage?"  Well, that way all of these /usr/lib/ImageMagick-*/ 
directories can coexist.  But

   (d) Suppose you have to disable some codec in a build, where in 
5.5.7-1 it was enabled, but in 5.5.7-2 is disabled.  This does not 
actually change the API of the library itself (because codecs are 
on-demand loaded, via dlopen() if compiled externally, or via libtool's 
fake static dlopen() if compiled into the main dll).  So there's no 
reason to bump the DLL version number.  However, now the libMagick6 
package depends on the ImageMagick5_5_7 package -- but the -1 version 
(ImageMagick5_5_7-5.5.7-1) advertises support for the newly disabled 
codec, while the -2 version (correctly) does not.  So, you really need 
to ensure that the libMagick6 package and the ImageMagick5_5_7 package 
are upgraded together -- but there's no support, in setup.exe, for doing 
that -- unless you add ANOTHER level of versioning.

And in fact, that's often the way RPMs containing kernel modules are 
distributed:
   kernel-module-alsa-0.9.6-1csw_2.4.21_0.25mdk.athlon.rpm
with the version of the module AND the version of the kernel for which 
it is built.

But wait, there's more!

Now suppose that you succeed in building the codecs and filters as 
separate DLL's, instead of monolithically in the cygMagick-6.dll.  NOW, 
the contents of /usr/lib/ImageMagick-5.5.7/modules-Q16/codecs/* really 
DO depend on cygMagick-6.dll -- not just vice versa.  So, you really 
need BOTH the '6' AND the '5_5_7' as part of the package name for this 
group of files.  Double double blech.

So, if you have to keep the stuff in /usr/lib/ImageMagick-*/ and the DLL 
tightly coupled as far as versioning goes, then it seems that what you 
really ought to do is just use 5_5_7 for the DLL version -- and issue a 
new DLL with each updated release.  You can do this by using
   "libXXXX_LDFLAGS = -release 5.5.7"
instead of
   "libXXXX_LDFLAGS = -version-info 6:0:0

And then you'll get cygmagick-5-5-7.dll and stick it into 
libmagick5.5.7-5.5.7-1.tar.bz2; put the /usr/lib/ImageMagick-5.5.7/ 
stuff into ImageMagick5.5.7-5.5.7-1.tar.bz2, and all is well.

Except that now, if case (b) occurs, you're out of luck.  Because you 
*can't* bump the DLL version number now that it is equivalent to the 
release number.  (And another "problem" -- users will accumulate lots 
and lots of 'libmagickX.Y.Z' and 'ImageMagickX.Y.Z' packages, unless 
they manually remove them every now and then.  All this is to support 
third party apps that may be compiled against cygMagick-5-5-7.dll etc.

And here's the final rub: back in the 20021102 timeframe, 
<src>/magick/Makefile.am DID specify _LDFLAGS = -release $MAJ.$MIN.$MIC. 
  Sometime between then and now, somebody deliberately decided to 
suddenly start following libtool guidelines, and use -version-info 
$LIBCUR:$LIBREV:$LIBAGE !  What a great time to start following the 
rules!  (Unfortunately, I can't figure out when/who did this; when IM 
moved from simplesystems.org to imagemagick.org, they lost all CVS 
history older than about 5.5.5 it seems)

So, maybe you *shouldn't* revert back to -release versioning.  I'd ask 
Bob F. what he was thinking...because 'strings cygMagick-6.dll' (or 
'strings libMagick-5.5.x.so' on linux) shows that the 
/usr/lib/ImageMagick-X.Y.Z/modules-Q16/coders/ path is compiled into the 
DLL, which means that the interface really really DOES change with every 
release, so you might as well use -${MAJ}-${MIN}-${MIC} to name the 
shared library!

----------------------------

Now, I've taken a look at my original build of ImageMagick, and I've got 
a few questions based on that. Note that my build was from CVS 
ImageMagick between the 5.5.2 release and the 5.5.3 release, on 20021102 
(slow release cycles, I guess: over one year to go from micro version .2 
to .7)  At that time, I was able to
   1) build the codecs as separate DLLs
   2) the 'main' DLL was named cygmagick-5-5-2.dll, as described above.
   3) cygwin's libtool-1.5 is not actually stock libtool-1.5.  There is 
a fix from Max Bowser for library search order (but that should have no 
effect here), plus a fix for the binary wrappers -- but that should only 
affect built-in testing and running the executables from the build 
dir[*]; it should have no effect on a fully installed ImageMagick.

[*] IIRC, this fix was actually occasioned by a failure to build 
PerlMagick -- because the PerlMagick configury needs to be able to run a 
proggie from the .build dir, but it invokes it via setting the $PATH and 
calling the proggie indirectly (which doesn't work on cygwin with stock 
libtool-1.5), instead of explicitly using /the/full/path/proggie (which 
DOES work on cygwin with stock libtool-1.5).  So, it's just as well 
you're not building PerlMagick right now. :-)

---------------------------------

I'm about to test the build procedure (but adding --with-modules) for 
your -src package, and install your binary for testing.  Unfortunately, 
this involves some risk for me, as I can no longer find my binary 
tarball for IM-5.5.2...which is working fine.  But I need to uninstall 
it before installing and testing yours.  Perhaps I'll recreate my 
tarball using the /etc/setup/*.lst.gz file first.  So patience, plz. :-)

---------------------------------
Chuck