← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] Re: SHAPE_LINE_CHAIN in swig?

 

Removing or renaming operator<< does not work because it is used by boost
test suite in qa/geometry/test_fillet.cpp

But I found an easier solution. There is no need to have friend declaration
in VECTOR2 class at all because it's fields are public anyway.
I removed that declaration but kept operator<< implementation and that
compiles just fine. Tested on debian8 and msys2.

If this solution is acceptable to you, see my amended patch attached.

Andrew


On Tue, Jul 31, 2018 at 1:01 PM Wayne Stambaugh <stambaughw@xxxxxxxxx>
wrote:

> If option 2 is the only option that works, please make sure to set the
> minimum swig version in the cmake file that finds swig.  I would rather
> the config fail then the build fail because an unusable version of swig
> is found.
>
> On 7/31/2018 2:57 PM, Andrew Lutsenko wrote:
> > I will test later today both options
> > 1. Removing VECTOR2::operator<< or renaming it to str() if it's used.
> > 2. Upgrading to swig 3.0.10 from backports.
> >
> > Hopefully first is doable and would be transparent for users.
> > Second one should definitely solve the issue and I feel like compared to
> > other hoops a user has to jump through to make KiCad compile on debian8
> > this would not be the worst.
> >
> > Regards,
> > Andrew
> >
> > On Tue, Jul 31, 2018 at 11:32 AM Wayne Stambaugh <stambaughw@xxxxxxxxx
> > <mailto:stambaughw@xxxxxxxxx>> wrote:
> >
> >     On 7/31/2018 1:13 PM, Seth Hillbrand wrote:
> >     >
> >     >
> >     > Am Di., 31. Juli 2018 um 07:31 Uhr schrieb Wayne Stambaugh
> >     > <stambaughw@xxxxxxxxx <mailto:stambaughw@xxxxxxxxx>
> >     <mailto:stambaughw@xxxxxxxxx <mailto:stambaughw@xxxxxxxxx>>>:
> >     >
> >     >     On 7/31/2018 8:33 AM, Carsten Schoenert wrote:
> >     >     > Am 31.07.18 um 17:50 schrieb Andrew Lutsenko:
> >     >     > ...
> >     >     >> Can swig on the qa machine be updated? Or better yet can you
> >     >     upgrade to
> >     >     >> debian 9? Debian 9 has swig 3.0.10 and compiles this just
> fine.
> >     >     >> Aside from this debian 8 is very old and should be done
> >     away with
> >     >     anyway
> >     >     >> because of security, old compilers, etc.
> >     >     >
> >     >     > Argumentation by missing security isn't a valid choice, even
> >     now the
> >     >     > ELTS team is taking care of security updates, old versions
> >     can be
> >     >     solved
> >     >     > by using backports, even swig has 3.0.10 in
> >     jessie-backports. I agree
> >     >     > that GCC wont become any version updates for Jessie.
> >     >     >
> >     >     > But there are still users out there which use Jessie based
> >     desktops.
> >     >     >
> >     >
> >     >     I'm siding with Carsten on this.  There are people who prefer
> >     stable
> >     >     computing platforms and I want to avoid making kicad only
> >     build on the
> >     >     latest distros.  I prefer that we keep as large of a target
> >     audience as
> >     >     possible.  How difficult would it be to change the
> >     SHAPE_LINE_CHAIN
> >     >     object (actually its the VECTOR2 object that causes the swig
> >     issue) so
> >     >     that older versions of swig don't choke on it?  I would be
> >     open to that
> >     >     solution.
> >     >
> >     >     Cheers,
> >     >
> >     >     Wayne
> >     >
> >     >
> >     > ​I'm not sure I follow the discussion.  I thought Carsten was
> saying
> >     > that jessie-backports does have SWIG 3.0.10 and so we can upgrade
> swig
> >     > on the kicad-qa​ without changing to a newer debian.
> >
> >     I was operating under the assumption that not every user will track
> or
> >     want to track Debian backports so in this case the user would still
> only
> >     have the older version of swig.  The line of code that is causing
> swig
> >     to choke is the VECTOR2 << operator which I'm almost sure is being
> used
> >     for debugging output and therefore could easily be removed without
> >     issue.  I'm not sure that there are not other swig related issues in
> the
> >     SHAPE_LINE_CHAIN implementation this change may not be enough.  If we
> >     are going to use a version of swig that works with the current code,
> we
> >     should set the cmake find package minimum version of swig to the
> correct
> >     version.  I'm fine either way.  Others may not be fine with this.
> >
> >     >
> >     > @Andrew - can you compile your changes on debian 8 using the swig
> from
> >     > backports as Carsten described?  If not, then this is moot and
> >     we'd need
> >     > to look at a SWIG-specific VECTOR2, an outcome that might be
> long-term
> >     > problematic.
> >     >
> >     > -S
> >
> >     _______________________________________________
> >     Mailing list: https://launchpad.net/~kicad-developers
> >     <https://launchpad.net/%7Ekicad-developers>
> >     Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> >     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
> >     Unsubscribe : https://launchpad.net/~kicad-developers
> >     <https://launchpad.net/%7Ekicad-developers>
> >     More help   : https://help.launchpad.net/ListHelp
> >
>
From e2ddd1f87f43562ec944a89a6ae827b25a986cd0 Mon Sep 17 00:00:00 2001
From: qu1ck <anlutsenko@gmail.com>
Date: Sun, 22 Jul 2018 17:45:15 -0700
Subject: [PATCH] Extend swig definitions to contain SHAPE_LINE_CHAIN and
 VECTOR2I

---
 common/swig/kicad.i     |  6 ++++++
 common/swig/math.i      | 34 ++++++++++++++++++++++++++++++++++
 include/math/vector2d.h |  2 --
 3 files changed, 40 insertions(+), 2 deletions(-)
 create mode 100644 common/swig/math.i

diff --git a/common/swig/kicad.i b/common/swig/kicad.i
index d41e1b1..be76cec 100644
--- a/common/swig/kicad.i
+++ b/common/swig/kicad.i
@@ -135,6 +135,12 @@ typedef long time_t;
 #include <geometry/shape.h>
 %include <geometry/shape.h>
 
+// Contains VECTOR2I
+%include math.i
+
+#include <geometry/shape_line_chain.h>
+%include <geometry/shape_line_chain.h>
+
 #include <geometry/shape_poly_set.h>
 %include <geometry/shape_poly_set.h>
 
diff --git a/common/swig/math.i b/common/swig/math.i
new file mode 100644
index 0000000..a8b9ad3
--- /dev/null
+++ b/common/swig/math.i
@@ -0,0 +1,34 @@
+/*
+ * This program source code file is part of KiCad, a free EDA CAD application.
+ *
+ * Copyright (C) 1992-2017 KiCad Developers, see AUTHORS.txt for contributors.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, you may find one here:
+ * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html
+ * or you may search the http://www.gnu.org website for the version 2 license,
+ * or you may write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA
+ */
+
+/**
+ * @file math.i
+ * @brief wrappers for math helper classes
+ */
+
+%ignore VECTOR2<int>::ECOORD_MAX;
+%ignore VECTOR2<int>::ECOORD_MIN;
+
+#include <math/vector2d.h>
+%include <math/vector2d.h>
+%template(VECTOR2I) VECTOR2<int>;
diff --git a/include/math/vector2d.h b/include/math/vector2d.h
index d883b97..421d9a9 100644
--- a/include/math/vector2d.h
+++ b/include/math/vector2d.h
@@ -249,8 +249,6 @@ public:
     /// Greater than operator
     bool operator>( const VECTOR2<T>& aVector ) const;
     bool operator>=( const VECTOR2<T>& aVector ) const;
-
-    friend std::ostream & operator<< <T> ( std::ostream & stream, const VECTOR2<T> &vector );
 };
 
 
-- 
2.7.4


Follow ups

References