← Back to team overview

kicad-developers team mailing list archive

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

 

Hi,

On 25.04.2017 17:11, Jan Mrázek wrote:

> I am a little confused. What exactly is wrong with the patch? According
> to the IRC discussion a while ago, I think it was with nickoe, the best
> approach is probably to adopt the changes in FindwxWidgets and when
> KiCAD upgrades to a reasonably new CMake, get rid of the KiCAD copy and
> use the system one. Is there something I miss?

As said, the patch should be fine. Windows often behaves in curious and
interesting ways because dynamic linking works completely differently
there, and testing a patch is easy with Jenkins -- I should have done
this in parallel with the first reply already.

My point is that the KiCad copy and the upstream copy should be mostly
identical anyway -- if they aren't, then we'd might want to
resynchronize. Indeed, when we ever require a recent enough version of
CMake, then we can drop our own copy, but I'm not holding my breath for
that -- right now, we use 3.0 as a minimum IIRC, and there are still
Debian and Ubuntu releases that ship with 3.0.

The "mostly" in the previous paragraph concerns the include path and the
licence statement.

For CMake scripts shipped inside the CMake source archive, the licence
statement is a reference to the licence file in the toplevel directory,
with a requirement to paste the full licence text when the script is
copied into a separate project. I did that back then, and you haven't
touched it, so that is fine.

The other difference between scripts in the CMake package and in other
packages is that CMake scripts are required to refer to each other using
the full path, to make sure that overrides only go in one direction --
package scripts can call package scripts or core scripts, but core
scripts never use package scripts. You haven't changed that either, so
all is well.

My point is that we should always try to stay as close as possible to
the upstream script and possibly also pull in improvements that don't
concern us directly -- but simply copying that script does not work.

   Simon

Attachment: signature.asc
Description: OpenPGP digital signature


References