[CMake] FindPerlLibs.cmake and FindSwig.cmake outdated!

[Tristan Carel]

On 12/5/06, Axel Roebel <Axel.Roebel at ircam.fr> wrote:
> Hi,
>
> I just started to add swig support to our project
> http://sourceforge.net/projects/sdif. The project supports
> swig bindings to perl, python and java.
>
> I instantly tried the available
> cmake modules: notably FindSWIG.cmake
> and FindPerlLibs.cmake
>
> I was pretty astonished to find that these
> two  macros do not work at all, because most of the
> important information that is necessary
> to find the components is hardcoded
> using fixed version numbers (that are already out of date)
>
> I added patched version of these macros to  bug reports 4145
>  4146 in the cmake bug database. These patched versions
> are completely backwards compatible (they use the wrong and fixed path
> locations first before they fall back to the new scheme and use
> the same variables to communicate results)
>
> The idea of the new procedure is to base everything on the FIND_PROGRAM
> macros that searches for the perl/swig binaries
> and (if found) asks the binary via inplace scripts (perl)/command line options
> (swig) all the directory locations that are needed.
>
> This approach is very flexible because by simply setting PERL_EXECUTABLE or
> SWIG_EXECUTABLE on the command line (or the cmake gui)
> one can select an arbitrary perl/swig version
> that is installed on  the system.
>

The following concerns `UseSWIG.cmake':

Thanks for what you did, it's a long time this module had to be
rewritten. I guess you're absolutely right to use features provided by
a tool to find its own config directories. I didn't notice that swig
had this option.

Here are some comments (without testing your module):
I guess 2 calls to `FIND_PATH' on the same file `swig.swg' is weird, why don't
you make the EXECUTE_PROCESS before to retrieve the directory given by
`swig -swiglib' and finally make the FIND_PATH of `swig.swg'?

1. Look for swig executable
2. use `swig -swiglib' to get the swiglib path
3. FIND_PATH(SWIG_DIR swig.swg ${SWIG_outlib} /usr/share/swig1.3 ...)

> # - Find SWIG
> # This module finds an installed SWIG.  It sets the following variables:
> #  SWIG_FOUND - set to true if SWIG is found
> #  SWIG_DIR - the directory where swig is installed
> #  SWIG_EXECUTABLE - the path to the swig executable
> SET(SWIG_FOUND FOOBAR)

I guess `FALSE' would be cleaner.

> FIND_PATH(SWIG_DIR
>   SWIGConfig.cmake
>   /usr/share/swig1.3
>   /usr/lib/swig1.3
>   /usr/local/share/swig1.3)

On Linux,  mine are installed  in /usr/local/swig/1.3.{29,30,31} without
specifying anything  strange during the `./configure'.  Maybe you should
add this paths to.

> FIND_PATH(SWIG_DIR
>   swig.swg
>   /usr/share/swig1.3
>   /usr/lib/swig1.3
>   /usr/local/share/swig1.3)
> IF(EXISTS ${SWIG_DIR})
>   IF("x${SWIG_DIR}x" STREQUAL "x${CMAKE_ROOT}/Modulesx")
>     MESSAGE("SWIG_DIR should not be modules subdirectory of CMake")
>   ENDIF("x${SWIG_DIR}x" STREQUAL "x${CMAKE_ROOT}/Modulesx")
>
>   IF(EXISTS ${SWIG_DIR}/SWIGConfig.cmake)
>     INCLUDE(${SWIG_DIR}/SWIGConfig.cmake)
>     SET(SWIG_FOUND 1)
>   ELSE(EXISTS ${SWIG_DIR}/SWIGConfig.cmake)
>     FIND_PROGRAM(SWIG_EXECUTABLE
>       NAMES swig-1.3 swig
>       PATHS ${SWIG_DIR} ${SWIG_DIR}/.. ${SWIG_DIR}/../../bin /usr/bin /usr/local/bin )
>     SET(SWIG_USE_FILE ${CMAKE_ROOT}/Modules/UseSWIG.cmake)
>   ENDIF(EXISTS ${SWIG_DIR}/SWIGConfig.cmake)
> ELSE(EXISTS ${SWIG_DIR})
>   FIND_PROGRAM(SWIG_EXECUTABLE swig )
>   MESSAGE(STATUS "SWIG_EXECUTABLE -- ${SWIG_EXECUTABLE}")

Personnaly I don't like talktative modules. I expect:
IF(SWIG_FIND_QUIETLY)
  MESSAGE(STATUS)
ENDIF(SWIG_FIND_QUIETLY)

Whatever you  can put  the MESSAGE commands  at the  end of the  file to
summary what happened.

>   IF(NOT "${SWIG_EXECUTABLE}" STREQUAL "SWIG_EXECUTABLE-NOTFOUND")

IF(NOT SWIG_EXECUTABLE)
is enough, isn't it?

>     SET(SWIG ${SWIG_EXECUTABLE})
>     EXECUTE_PROCESS(COMMAND     ${SWIG}    -swiglib    OUTPUT_VARIABLE
>     SWIG_DIR_TMP)

The command can fail, however it's common to use lower-case for internal
variables:

EXECUTE_PROCESS(COMMAND ${SWIG_EXECUTABLE} -swiglib
  OUTPUT_VARIABLE SWIG_swiglib_output
  ERROR_VARIABLE SWIG_swiglib_error
  RESULT_VARIABLE SWIG_swiglib_result
  )
IF(NOT ${SWIG_swiglib_result} EQUAL 0)
  MESSAGE(SEND_ERROR "Command \"${SWIG_EXECUTABLE} -swiglib\" failed
with output:\n${SWIG_swiglib_error}")
ENDIF(NOT ${SWIG_swiglib_result} EQUAL 0)


I've got a strange behavior on Windows:
$ which swig
/cygdrive/c/Program Files/swigwin-1.3.31/swig
$ swig -swiglib
c:\Program Files\swigwin-1.3.31\Lib
C:/msys/1.0/local/share/swig/1.3.31
$

... so you have to consider that the command can write several paths.
The best thing would be to only keep the path that "match" the
SWIG_EXECUTABLE path.

I guess a
STRING(REGEX REPLACE "[\n]" ";" SWIG_swiglib_output ${SWIG_swiglib_output})
should be enough  to separate the string in  several arguments. As there
can be spaces in the paths, you can't use the SEPARATE_ARGUMENT command.

>     STRING(REGEX  REPLACE "[\n\r]" ""  SWIG_DIR_TMP_NOCR  ${SWIG_DIR_TMP})
>     FIND_PATH(SWIG_DIR  swig.swg PATHS "${SWIG_DIR_TMP_NOCR}")
>     IF(EXISTS ${SWIG_DIR})
>       MESSAGE(STATUS "swig install dir -- ${SWIG_DIR}")
>       SET(SWIG_FOUND 1)
>       SET(SWIG_USE_FILE ${CMAKE_ROOT}/Modules/UseSWIG.cmake)
>     ELSE (EXISTS ${SWIG_DIR})
>       MESSAGE(STATUS "swig install dir -- ${SWIG_DIR}")
>     ENDIF(EXISTS ${SWIG_DIR})
>   ENDIF(NOT "${SWIG_EXECUTABLE}" STREQUAL "SWIG_EXECUTABLE-NOTFOUND")
> ENDIF(EXISTS ${SWIG_DIR})
>
> IF("x${SWIG_FOUND}x" STREQUAL "xFOOBARx")

>   SET(SWIG_FOUND 0)
>   IF(EXISTS ${SWIG_DIR})
>     IF(EXISTS ${SWIG_USE_FILE})
>       IF(EXISTS ${SWIG_EXECUTABLE})
>         SET(SWIG_FOUND 1)
>       ENDIF(EXISTS ${SWIG_EXECUTABLE})
>     ENDIF(EXISTS ${SWIG_USE_FILE})
>   ENDIF(EXISTS ${SWIG_DIR})
>   IF(NOT ${SWIG_FOUND})
>     IF(${SWIG_FIND_REQUIRED})
>       MESSAGE(FATAL_ERROR "Swig was not found on the system. Please specify the location of Swig.")
>     ENDIF(${SWIG_FIND_REQUIRED})
>   ENDIF(NOT ${SWIG_FOUND})
> ENDIF("x${SWIG_FOUND}x" STREQUAL "xFOOBARx")
>

IF(NOT SWIG_FOUND)
  IF(NOT SWIG_FIND_QUIETLY)
    MESSAGE(STATUS "SWIG was not found.")
  ELSE(NOT SWIG_FIND_QUIETLY)
    IF(SWIG_FIND_REQUIRED)
      MESSAGE(FATAL_ERROR "SWIG was not found.")
    ENDIF(SWIG_FIND_REQUIRED)
  ENDIF(NOT SWIG_FIND_QUIETLY)
ELSE(NOT SWIG_FOUND)
  IF(NOT SWIG_FIND_QUIETLY) # put the verbose commands
    MESSAGE(STATUS ...)
    MESSAGE(STATUS ...)
    MESSAGE(STATUS ...)
  ENDIF(NOT SWIG_FIND_QUIETLY)
ENDIF(NOT SWIG_FOUND)



I would be glad to discuss with you about how the module could be in
the next release. For example it would be fancy that the module
defines the variable SWIG_VERSION (by using another EXECUTE_PROCESS)


I will test the module tomorrow.
Could you please put a RC2 of the `FindSWIG.cmake' on the bug tracker?

Thanks!

CU
-- 
Tristan Carel
Music with dinner is an insult both to the cook and the violinist.