← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] expat dependency for cmake

 

On Oct 15, 2010, at 16:32 PM, Wayne Stambaugh wrote:

> On 10/15/2010 6:37 AM, Brian Sidebotham wrote:
>>> I already had a suspicion my patch was not complete.
>>> I was hoping to achieve that cmake does all the dependency checks and then decides if there is enough to build it or to ask for other dependencies, instead of finding out at linking-time.
>>> 
>>> 
>>> Probably we need to do the wxWidgets check before the expat check, right?
>>> 
>>> So the patch should to the following (?):
>>> # only gl and mono can be detected
>>> find_package(wxWidgets COMPONENTS mono gl REQUIRED)
> 
> Why is the wxWidgets monolithic library being used?  I can build separate
> wxWidgets libraries on Windows using both dynamic link and static libraries on
> both Mingw/MSYS and Visual Studio 2005 with no problems.  I usually use static
> because it make my life easier but I have successfully built Kicad both ways.
> If you are going use the monolithic library, use a command line switch as shown
> below.
> 
> option(KICAD_USE_WX_MONO "Enable/disable building with wxWidgets monolithic
> library (default OFF)")
> 
> if(KICAD_USE_WX_MONO)
>    find_package( wxWidgets COMPONENTS mono gl REQUIRED )
> else(KICAD_USE_WX_MONO)
>    find_package( wxWidgets COMPONENTS gl adv html core net base xml aui REQUIRED )
> endif(KICAD_USE_WX_MONO)
> 
You don't need expat here, like
> find_package( wxWidgets COMPONENTS gl adv html core net base xml aui expat REQUIRED )



> Use the -DKICAD_USE_WX_MONO=ON switch when running CMake.
> 
>>> if(NOT WIN32)
>>>       find_package(wxWidgets COMPONENTS gl adv html core net base xml REQUIRED)
>>> endif(NOT WIN32)
> 
> There is no reason to use if(NOT WIN32) here.  I know I've said this before and
> I will say it again.  Build tools should test for features not platforms.
> Whenever you see WIN32, APPLE, POSIX, etc., try to replace it with a feature
> test solution.  See the "Recommendations for Writing Autoconf Macros" page of
> http://www.lrde.epita.fr/~adl/dl/autotools.pdf for an excellent explanation of
> why to test for features.  Unfortunately the CMake folks accept and create
> these kinds of tests.
I wonder why we changed to cmake if autoconf does things the-right-way, but either way is fine with me.


> I know you thinking, how does that solve our expat library problem.  There are
> several ways to approach this problem but fortunately for us, the wxWidgets
> folks have a clue and provided a wxUSE_EXPAT definition when wxWidgets is built
> using its own internal version of expat.  You should be able to use the CMake
> CheckSymbolExists macro to check for wxUSE_EXPAT in the setup.h file for the
> version of wxWidgets that was found.  Now you know which expat library to link
> against.  
Neat, learned something new.

> Assuming that the linker paths are defined correctly, you can link to
> the correct library.  This is actually not the optimal way to do it.  On
> platforms that support wx-config, you should use it instead.  So we end up with
> something as following.
OSX has wx-config, it just does not show expat flags when built as a sys library with wxwidgets.
I am willing to send a bug report to the wx-folks, but in the mean time we could easily work around it, add a comment and rip it out when it is supported upstream. 

> 
> find_program( wx-config, HAS_WX_CONFIG )
> 
> if( NOT HAS_WX_CONFIG OR CROSS_COMPILING )
>    CheckSymbolExists( wxUSE_EXPAT path_to_correct_wx_version/setup.h
> USE_WX_EXPAT )
>    if( USE_WX_EXPAT )
> 	add_library( name_of_wx_widgets_expat_library )
>    else( USE_WX_EXPAT )
>        add_library( expat )
>    endif( USE_WX_EXPAT )
> endif( NOT HAS_WX_CONFIG OR CROSS_COMPILING )
> 
> Assuming your development environment is setup correctly, this should work no
> matter what platform you are building against.
> 
Windows has no wx-config? Ouch.

/Martijn

> Wayne
> 
>>> 
>>> but how can we check for (wx)expat and if that fails look for (sys)expat? if wxWidgets cannot find wxexpat it should look for the system one with find_package...and add the linker flag -lexpat
>>> 
>>> These changes are tricky, since cmake behaves different on linux/win/osx, but with a little help I am sure we can work it out. Also, if building a deb-package you probably do not want the static linked version, but one with dependencies on the other packages, but I have not looked into that yet.
>> 
>> Hi Martijn,
>> 
>> I think the type of check you need is something like: (psuedo code)
>> 
>> check (wxWidgets COMPONENTS expat QUIET)
>> if (NOT wxWidgets_FOUND)
>>    check (expat QUIET)
>>    if(expat_FOUND)
>>        add_link_library(expat)
>>    else(expat_FOUND)
>>        print "Neither wxWidgets expat, or sys expat found"
>>        check_find_package_result(expat)
>>    endif(expat_FOUND)
>> endif(NOT wxWidgets_FOUND)
>> 
>> ...
>> (carry on with std wxWidgets checks)
>> ...
>> 
>> Something along those lines anyway I guess. If you struggle and just
>> want a patch made, let me know. I could have a look at it when I get
>> home for work tonight.
>> 
>> Best Regards,
>> 
>> Brian.
>> 
>> _______________________________________________
>> Mailing list: https://launchpad.net/~kicad-developers
>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>> Unsubscribe : https://launchpad.net/~kicad-developers
>> More help   : https://help.launchpad.net/ListHelp
>> 
> 
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp




Follow ups

References