← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] Check required libraries in FindwxWidgets.cmake

 

I'm confused as to why you are checking for just the libraries rather
than the missing headers which cause the build error.  The libraries can
be present and the headers still missing since libwxbase3.0-dev is not a
dependency of libwxbase3.0 so kicad will fail to build in this case.
The opposite cannot happen as libwxbase3.0 is a dependency for
libwxbase3.0-dev.  This is the way wxwidgets is packaged in Debain so
I'm guessing Ubuntu is the same.

IMO, the real problem lies in the packaging of wxwidgets.  The wx-config
file is part of the wxcommon package which doesn't make much sense to
me.  Should it not be distributed with the libwxbase3.0-dev package?
AFAICK, wx-config is used to determine the build configuration for
wxwidgets.  What other purpose would it be used for?  I would think that
the only reason you would need to call wx-config is when compiling a
program that depends on wxwidgets which would require both the headers
and libraries.

The other issue I see with this patch is that it depends on the help
output of wx-config.  I would think that depending on this to be
consistent might be wishful thinking leading to maintenance issues.  I
am not thrilled with the idea of patching our build config to fix what
seems to me to be a packaging issue.

On 3/20/2017 10:16 AM, Jan Mrázek wrote:
> Both official one and KiCad one version of FindwxWidgets.cmake do not check if
> all the required components are part of the wxWidgets build. This couses trouble
> on Ubuntu when user misses libwxgtk3.0-dev package - CMake succeeds, build does
> not.
> 
> Probably only posibility to get a list of components is to grep output of
> wx-config.
> 
> Base component is removed from the dependencies as it is not present on GUI
> builds of wxWidgets.
> 
> Fixes: lp:1630020
> * https://bugs.launchpad.net/kicad/+bug/1630020
> ---
>  CMakeLists.txt                   |  2 +-
>  CMakeModules/FindwxWidgets.cmake | 36 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 37 insertions(+), 1 deletion(-)
> 
> diff --git a/CMakeLists.txt b/CMakeLists.txt
> index 18f2052e1..63f5b17a5 100644
> --- a/CMakeLists.txt
> +++ b/CMakeLists.txt
> @@ -511,7 +511,7 @@ add_definitions( -DWX_COMPATIBILITY )
>  # See line 41 of CMakeModules/FindwxWidgets.cmake
>  set( wxWidgets_CONFIG_OPTIONS ${wxWidgets_CONFIG_OPTIONS} --static=no )
>  
> -find_package( wxWidgets 3.0.0 COMPONENTS gl aui adv html core net base xml stc REQUIRED )
> +find_package( wxWidgets 3.0.0 REQUIRED gl aui adv html core net xml stc )
>  
>  # Include wxWidgets macros.
>  include( ${wxWidgets_USE_FILE} )
> diff --git a/CMakeModules/FindwxWidgets.cmake b/CMakeModules/FindwxWidgets.cmake
> index 9a6e56f7b..cb13bf93a 100644
> --- a/CMakeModules/FindwxWidgets.cmake
> +++ b/CMakeModules/FindwxWidgets.cmake
> @@ -141,6 +141,7 @@
>  #
>  # Miguel A. Figueroa-Villanueva (miguelf at ieee dot org).
>  # Jan Woetzel (jw at mip.informatik.uni-kiel.de).
> +# Jan Mrázek (email at honzamrazek.cz)
>  #
>  # Based on previous works of:
>  # Jan Woetzel (FindwxWindows.cmake),
> @@ -728,6 +729,38 @@ else()
>        endforeach()
>      endmacro()
>  
> +    #
> +    # Check if all the required libraries are present in the build. The only way
> +    # to find out the list of libraries is to run wx-config and grep its output
> +    # - libraries are hardcoded in the help text.
> +    #
> +    macro(WX_CONFIG_CHECK_LIBS)
> +      execute_process(
> +        COMMAND "${wxWidgets_CONFIG_EXECUTABLE}"
> +        OUTPUT_VARIABLE _wx_selected_config
> +        ERROR_VARIABLE _wx_selected_config
> +        RESULT_VARIABLE _wx_result
> +        ERROR_QUIET
> +        )
> +      string(REGEX MATCH "Available libraries in this build are:\n(.*)\n"
> +       _wx_available_libs ${_wx_selected_config})
> +      string(REGEX REPLACE "Available libraries in this build are:\n(.*)\n"
> +        "\\1" _wx_available_libs ${_wx_available_libs})
> +      string(STRIP _wx_available_libs ${_wx_available_libs})
> +      string(REPLACE " " ";" _wx_available_libs "${_wx_available_libs}")
> +      foreach(lib ${wxWidgets_FIND_COMPONENTS})
> +        list(FIND _wx_available_libs "${lib}" _index)
> +        if (${_index} LESS 0)
> +          list(APPEND _missing ${lib})
> +          DBG_MSG("Library ${lib} not found")
> +        endif()
> +      endforeach()
> +      if (_missing)
> +        string(REPLACE ";" " " _missing "${_missing}")
> +        message(FATAL_ERROR "Missing following wxWidgets libraris: ${_missing}")
> +      endif()
> +    endmacro()
> +
>      #-----------------------------------------------------------------
>      # UNIX: Start actual work.
>      #-----------------------------------------------------------------
> @@ -741,6 +774,9 @@ else()
>      if(wxWidgets_CONFIG_EXECUTABLE)
>        set(wxWidgets_FOUND TRUE)
>  
> +      # check if current build supports all required libraries
> +      WX_CONFIG_CHECK_LIBS()
> +
>        # get defaults based on "wx-config --selected-config"
>        WX_CONFIG_SELECT_GET_DEFAULT()
>  
> 


Follow ups

References