← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] expat dependency for cmake

 

Hi Martijn,

I don't think this patch is correct. You have made the build alter if
expat is found as a system library. In this case you add the system
expat library to the linker. The trouble is, although I have a system
expat, I compile wxWidgets to use its own implementation as I'm sure
this is the more thoroughly controlled code path (less susceptible to
bugs).

Instead, you could check for just the component expat for wxWidgets,
and only if you cannot locate expat shoudl you then do the sys expat
check and conditionally add the expat library link option, and then
continue on to check for the full list of other wxWidgets components.

QUIET does not mean not required - it just means do not fail at this
point, and do not print messages to the screeen. This is important,
although most checks in the KiCad cmake files are straight forward. A
patch I put together the other day would require this operation. It
was along the lines of:

---

find_package(wxWidgets COMPONENTS gl adv html core net base xml QUIET)

# On a windows console (i.e. not MSYS), a monolithic build cannot
detect other components,
# only gl and mono can be detected
if(WIN32)
    if(NOT wxWidgets_FOUND)
        find_package(wxWidgets COMPONENTS mono gl QUIET)
    endif(NOT wxWidgets_FOUND)
endif(WIN32)

---

If REQUIRED was used, it would have failed before doing the
exceptional check for the win32 monolithic build. KiCad uses this
method, and then fails through the check_find_package_result() method
if needed after all possible checks have been run. This is a neat way
of handling multiple checks. You can find the
check_find_package_result() cmake macro under the CMakeModules
directory of the kicad source.

This is only my opinion of course, others might expand on this or
approve it and tell me to shut up! ;-)

Best Regards,

Brian.

On 14 October 2010 12:10, Martijn Kuipers <martijn.kuipers@xxxxxxxxx> wrote:
> Dear Devs,
>
> Kicad has a dependency on expat. It can either be provided by expat or by wxWidgets. However, cmake does not look for expat and as such if not found, a linker error occurs (which is bad, since it happens near the end of the building process).
>
> This patch adds a non-required dependency on expat (since it can be provided by wxWidgets) and if it finds the library it adds it to the linker flag (-lexpat). If the library is not found, it is added as a component to wxWidgets library.
>
> This patch allows me to build kicad, with the system provided library for expat. I have tested this on OSX, but I think it is needed on all platforms.
>
>
> Note, all dependencies in the CMake files are of type QUIET, which means not required.  It also means, the compiler will try to build kicad, whether it finds the libraries or not. From a quick look it might make sense to make wxWidgets REQUIRED and to make expat REQUIRED if it is not provided by wxWidgets. This is not added to this patch, because I would like to hear your opinion on this first.
>
> /Martijn
>
>
>
>
>
>
> _______________________________________________
> 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