[cmake-developers] FindImageMagick.cmake backwards compatibility

[David Cole]

On Sun, Jan 29, 2012 at 5:39 PM, Miguel A. Figueroa-Villanueva
<miguelf at ieee.org> wrote:
> On Sun, Jan 29, 2012 at 5:58 PM, Rolf Eike Beer wrote:
>> While hacking on various Find*.cmake modules I also made some changes to the
>> ImageMagick module, and one of them is not really backwards compatible. I
>> would consider it a bugfix, but you may judge yourself.
>>
>> ImageMagick_FOUND was set to true if all requested components were found. Not
>> surprising on the first look. But that also means: if you requested no
>> components and no components were found that means none is missing:
>> ImageMagick_FOUND was TRUE. It is not anymore as you may have guessed.
>>
>> So when ImageMagick is installed everything behaves as always. But if no
>> ImageMagick is installed behaviour was this:
>>
>> find_package(ImageMagick) -> ImageMagick_FOUND = TRUE
>> find_package(ImageMagick mogrigy) -> FALSE
>>
>> and now it is:
>>
>> find_package(ImageMagick) -> ImageMagick_FOUND = FALSE
>> find_package(ImageMagick mogrigy) -> FALSE
>>
>> Any objections?
>>
>> Eike
>
> Hello All,
>
> I am the current maintainer of this module, but I haven't been able to
> keep up with CMake's development in the past couple of years. So, I am
> not sure how to handle this case.
>
> To add a bit to this discussion, the basic problem is that code
> similar to the following would break:
>
> include(FindImageMagick)
> if(NOT IMAGEMAGICK_FOUND)
>  message(FATAL_ERROR "don't have the FindImageMagick module")
> endif()
> ...
> if(IMAGEMAGICK_MOGRIFY_EXECUTABLE)
>  # do something
> else()
>  # do something else
> endif()
>
> However, the old module that this is being derived from:
>
> http://cmake.org/gitweb?p=cmake.git;a=blob;f=Modules/FindImageMagick.cmake;h=66e54bcfee27508c8124c5cd033de779473b3b4b;hb=d2689c95a1591fd7bc5bc8a2f1515a2db1d88b72
>
> doesn't have an IMAGEMAGICK_FOUND variable. So, this seems to be an
> oversight on my part when implementing the code to use components and
> extending to libraries, etc.
>
> In summary, I support the change that Eike is presenting, but I would
> like to have the go ahead from CMake developers.
>
> Thanks,
> --Miguel
> --
>
> Powered by www.kitware.com
>
> Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html
>
> Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ
>
> Follow this link to subscribe/unsubscribe:
> http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers


>From your original post:

  "So when ImageMagick is installed everything behaves as always. But if no
  ImageMagick is installed behaviour was this:

  find_package(ImageMagick) -> ImageMagick_FOUND = TRUE"

This is simply clearly incorrect, and is one of those changes that
deserves to have its backwards compatibility broken.

If I were a client of this module, and I had logic that really
depended on ImageMagick being found, then it would be broken already
in some other way anyhow.

I would rather have ImageMagick_FOUND be set correctly than have it be
set "incorrectly but the same as the last version of CMake."


David