kicad-developers team mailing list archive

[Dick Hollenbeck <dick@xxxxxxxxxxx>]

On 10/07/2013 05:21 AM, Brian Sidebotham wrote:
> 
> On 7 October 2013 05:32, Dick Hollenbeck <dick@xxxxxxxxxxx <mailto:dick@xxxxxxxxxxx>> wrote:
> 
>     Hi Brian,
> 
>     You have a total comprehension of what is going on, and your solution has left the linux
>     build working.  So I would say thank you for an excellent job.  I would hope windows users
>     would say the same.
> 
>     Comments below.
> 
> 
> Excellent, glad that's the case - I don't mean to waste your time asking you to look at
> everything. It's good to get the feedback before the commit at the moment though. Thanks
> for taking the time to review the changes.
>  
> 
> 
>     Remove this if() block:
> 
>     > +if( MINGW )
>     > +    set( GITHUB_ADDITIONAL_LIBS ws2_32 )
>     > +endif()
>     > +
>     >  # No, you don't get github without boost and openssl
>     >  target_link_libraries( github_plugin
>     >      ${Boost_LIBRARIES}
>     >      ${OPENSSL_LIBRARIES}
>     > +    ${wxWidgets_LIBRARIES}
> 
>     remove this:
>     > +    ${GITHUB_ADDITIONAL_LIBS}
>     >      )


Maybe also move wxWidgets_LIBRARIES down into the lower optional appendage also.

> 
>     and replace it with this trailing additive statement:
> 
>     if( MINGW )
>     target_link_libraries( github_plugin
>         ws2_32
>         )
>     endif()
> 
>     >
>     >  add_dependencies( github_plugin boost )
> 
> 
> Perfect, that looks more obvious regarding what's going on. Thanks.
> 
> As an aside, I noticed that some targets such as libpcbcommon and libpolygon have a
> header-only dependency on boost. CMake won't know about that dependency. Although CMake
> doesn't encounter the problem at the moment I think with many parallel build processes it
> may be possible that one of those targets gets built before boost has been installed.
> Should I add the add_dependencies( target boost ) command for those targets? I think it
> appears best to do it.

We did have those parallel build problems, they went away moons ago when I added the lines
you talk about near line 403 of the main topmost CMakeLists.txt file.

I thought it best to aggregate them in one place because it serves as a reminder to those
adding new libraries.



> 
> Best Regards,
> 
> Brian.