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

[Harold L Hunt II]

Chuck,

Sigh... the cat is already out of the bag.  In the absence of a review I 
uploaded the package since it didn't seem that anyone was interested and 
this was the general pattern when I requested a review.

If Chris, Daniel, et al want to delete release/ImageMagick and clean it 
from setup.ini that is fine... otherwise we are going to have to provide 
compatibility packages if we change the layout significantly.

Thanks for the review anyway.  I am going to bed though.  It looks like 
anything other than the way I have done it now is going to take a lot of 
time to think through.  Maybe it is best to just have a version that 
isn't perfect released so that people start using it and we have an 
incentive to actually get it right.  :)

Harold

Charles Wilson wrote:

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