[cmake-developers] [PATCH] Improve FindGIF version detection (fix for issue #16196)

[Ben Campbell]

On 14/07/16 01:52, Brad King wrote:
> On 07/12/2016 11:16 PM, Ben Campbell wrote:
>> A fix for https://gitlab.kitware.com/cmake/cmake/issues/16196
>
> Thanks!  Here are some comments.
[snip]
Cool - very helpful!
I've revised my patch to:
  - only scan the headerfile once
  - update the docs in the comment block
  - unset the scratch variables

> If the version is now expected to work please also update
> `Tests/CMakeOnly/AllFindModules/CMakeLists.txt` to check that
> a version number is extracted.  In a CMake build tree run
> `ctest -R CMakeOnly.AllFindModules -V` to run the test.

The existing GIF test looks like it'll keep working fine, so I've not 
fiddled with the tests at all.

Actually, the tests didn't complete on my machine anyway. Looking at the 
output, I'm pretty sure the GIF part worked.
Here's a very cut-down log of the test output (running the latest cmake 
from git, in situ):

$ bin/ctest -R CMakeOnly.AllFindModules -V | grep -i fail
Guessing configuration NoConfig
268: -- Trying to find DCMTK expecting DCMTKConfig.cmake - failed
268: -- Failed to find all ICU components (missing:  ICU_LIBRARY) (found 
version "55.1")
268: -- Failed to find all Ice components (missing: 
Ice_SLICE2CPP_EXECUTABLE Ice_INCLUDE_DIR Ice_SLICE_DIR Ice_LIBRARY)
268: -- Failed to find XercesC (missing:  XercesC_LIBRARY 
XercesC_INCLUDE_DIR XercesC_VERSION)
268: -- Failed to find XalanC (missing:  XalanC_LIBRARY 
XalanC_INCLUDE_DIR XalanC_VERSION XercesC_FOUND)
268: -- Failed to find XercesC (missing:  XercesC_LIBRARY 
XercesC_INCLUDE_DIR XercesC_VERSION)
268:   CMake failed to configure AllFindModules
1/1 Test #268: CMakeOnly.AllFindModules .........***Failed    8.07 sec
0% tests passed, 1 tests failed out of 1
The following tests FAILED:
	268 - CMakeOnly.AllFindModules (Failed)
Errors while running CTest

Not quite sure what the environment the tests expect - is it just a case 
that I've not got all the expected libraries installed?

Ben.



---
  Modules/FindGIF.cmake | 46 +++++++++++++++++++++++++++++++++++-----------
  1 file changed, 35 insertions(+), 11 deletions(-)

diff --git a/Modules/FindGIF.cmake b/Modules/FindGIF.cmake
index 7bbb8cf..d657d83 100644
--- a/Modules/FindGIF.cmake
+++ b/Modules/FindGIF.cmake
@@ -2,12 +2,18 @@
  # FindGIF
  # -------
  #
+# This finds the GIF library (giflib)
  #
+# The module defines the following variables:
  #
-# This module searches giflib and defines GIF_LIBRARIES - libraries to
-# link to in order to use GIF GIF_FOUND, if false, do not try to link
-# GIF_INCLUDE_DIR, where to find the headers GIF_VERSION, reports either
-# version 4 or 3 (for everything before version 4)
+# ``GIF_FOUND``
+#   True if giflib was found
+# ``GIF_LIBRARIES``
+#   Libraries to link to in order to use giflib
+# ``GIF_INCLUDE_DIR``
+#   where to find the headers
+# ``GIF_VERSION``
+#   3, 4 or a full version string (eg 5.1.4) for versions >= 4.1.6
  #
  # The minimum required version of giflib can be specified using the
  # standard syntax, e.g.  find_package(GIF 4)
@@ -29,7 +35,7 @@
  #  License text for the above reference.)

  # Created by Eric Wing.
-# Modifications by Alexander Neundorf
+# Modifications by Alexander Neundorf, Ben Campbell

  find_path(GIF_INCLUDE_DIR gif_lib.h
    HINTS
@@ -61,22 +67,40 @@ set(GIF_LIBRARIES ${GIF_LIBRARY})
  # to be always " Version 2.0, " in versions 3.x of giflib.
  # In version 4 the member UserData was added to GifFileType, so we 
check for this
  # one.
-# http://giflib.sourcearchive.com/documentation/4.1.4/files.html
+# Versions after 4.1.6 define GIFLIB_MAJOR, GIFLIB_MINOR, and 
GIFLIB_RELEASE
+# see http://giflib.sourceforge.net/gif_lib.html#compatibility
  if(GIF_INCLUDE_DIR)
    include(${CMAKE_CURRENT_LIST_DIR}/CMakePushCheckState.cmake)
    include(${CMAKE_CURRENT_LIST_DIR}/CheckStructHasMember.cmake)
    CMAKE_PUSH_CHECK_STATE()
    set(CMAKE_REQUIRED_QUIET ${GIF_FIND_QUIETLY})
-  set(GIF_VERSION 3)
    set(CMAKE_REQUIRED_INCLUDES "${GIF_INCLUDE_DIR}")
-  CHECK_STRUCT_HAS_MEMBER(GifFileType UserData gif_lib.h 
GIF_GifFileType_UserData )
-  if(GIF_GifFileType_UserData)
-    set(GIF_VERSION 4)
+
+  # Check for the specific version defines (>=4.1.6 only)
+  file(STRINGS ${GIF_INCLUDE_DIR}/gif_lib.h _GIF_DEFS REGEX "^[ 
\t]*#define[ \t]+GIFLIB_(MAJOR|MINOR|RELEASE)")
+  if(_GIF_DEFS)
+    # yay - got exact version info
+    string(REGEX REPLACE ".*GIFLIB_MAJOR ([0-9]+).*" "\\1" _GIF_MAJ 
${_GIF_DEFS})
+    string(REGEX REPLACE ".*GIFLIB_MINOR ([0-9]+).*" "\\1" _GIF_MIN 
${_GIF_DEFS})
+    string(REGEX REPLACE ".*GIFLIB_RELEASE ([0-9]+).*" "\\1" _GIF_REL 
${_GIF_DEFS})
+    set(GIF_VERSION ${_GIF_MAJ}.${_GIF_MIN}.${_GIF_REL})
+  else()
+    # use UserData field to sniff version instead
+    CHECK_STRUCT_HAS_MEMBER(GifFileType UserData gif_lib.h 
GIF_GifFileType_UserData )
+    if(GIF_GifFileType_UserData)
+      set(GIF_VERSION 4)
+    else()
+      set(GIF_VERSION 3)
+    endif()
    endif()
+
+  unset(_GIF_MAJ)
+  unset(_GIF_MIN)
+  unset(_GIF_REL)
+  unset(_GIF_DEFS)
    CMAKE_POP_CHECK_STATE()
  endif()

-
  # handle the QUIETLY and REQUIRED arguments and set GIF_FOUND to TRUE if
  # all listed variables are TRUE
  include(${CMAKE_CURRENT_LIST_DIR}/FindPackageHandleStandardArgs.cmake)
-- 
2.7.4