You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Branko Čibej <br...@wandisco.com> on 2015/07/13 11:07:57 UTC

Re: svn commit: r1690591 - /subversion/trunk/subversion/bindings/swig/include/proxy.swg

On 13.07.2015 11:04, jorton@apache.org wrote:
> Author: jorton
> Date: Mon Jul 13 09:04:13 2015
> New Revision: 1690591
>
> URL: http://svn.apache.org/r1690591
> Log:
> * subversion/bindings/swig/include/proxy.swg:
>   Use %{ %} with %pythoncode so comments avoid the SWIG preprocessor,
>   fixing the bindings with SWIG 3.0.6.
>
> Modified:
>     subversion/trunk/subversion/bindings/swig/include/proxy.swg
>
> Modified: subversion/trunk/subversion/bindings/swig/include/proxy.swg
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/swig/include/proxy.swg?rev=1690591&r1=1690590&r2=1690591&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/bindings/swig/include/proxy.swg (original)
> +++ subversion/trunk/subversion/bindings/swig/include/proxy.swg Mon Jul 13 09:04:13 2015
> @@ -62,7 +62,7 @@
>  
>  /* Default code for all wrapped proxy classes in Python */
>  %define %proxy_pythoncode(TYPE)
> -%pythoncode {
> +%pythoncode %{
>    def set_parent_pool(self, parent_pool=None):
>      """Create a new proxy object for TYPE"""
>      import libsvn.core, weakref
> @@ -104,7 +104,7 @@
>      self.__dict__.setdefault("_members",{})[name] = value
>  
>      return _swig_setattr(self, self.__class__, name, value)
> -}
> +%}
>  %enddef
>  
>  /* Define a proxy for wrapping an existing struct */


I've tried this before and it didn't fix the bindings at all.

-- Brane

Re: svn commit: r1690591 - /subversion/trunk/subversion/bindings/swig/include/proxy.swg

Posted by Branko Čibej <br...@wandisco.com>.
On 13.07.2015 11:20, Joe Orton wrote:
> On Mon, Jul 13, 2015 at 11:07:57AM +0200, Branko Čibej wrote:
>> On 13.07.2015 11:04, jorton@apache.org wrote:
>>> Author: jorton
>>> Date: Mon Jul 13 09:04:13 2015
>>> New Revision: 1690591
>>>
>>> URL: http://svn.apache.org/r1690591
>>> Log:
>>> * subversion/bindings/swig/include/proxy.swg:
>>>   Use %{ %} with %pythoncode so comments avoid the SWIG preprocessor,
>>>   fixing the bindings with SWIG 3.0.6.
> ...
>> I've tried this before and it didn't fix the bindings at all.
> Hi Brane - yes I know, note the comment on the version: the SWIG 
> developers have addressed the issues we were hitting in the new 3.0.6 
> release last week.

Ah, I must've been using a pre-release of 3.0.6. I'll certainly retest.

I agree with using %{ %} fwiw; it just didn't help last time I did that.

-- Brane


Re: Swig 3.x support (was Re: svn commit: r1690591 - /subversion/trunk/subversion/bindings/swig/include/proxy.swg)

Posted by James McCoy <ja...@jamessan.com>.
On Tue, Dec 22, 2015 at 02:45:03PM +0100, Branko Čibej wrote:
> On 22.12.2015 04:26, James McCoy wrote:
> > On Sun, Nov 01, 2015 at 01:17:56AM -0500, James McCoy wrote:
> >> On Mon, Jul 13, 2015 at 11:00:17AM +0100, Joe Orton wrote:
> >>> On Mon, Jul 13, 2015 at 11:41:23AM +0200, Branko Čibej wrote:
> >>>> https://ci.apache.org/builders/svn-x64-ubuntu-gcc/builds/317
> >>>>
> >>>> Apparently this breaks older versions of Swig ...
> >>> Ah, ugh... yes I should have realised this.  Sorry.
> >>>
> >>> I think the choices are:
> >>>
> >>> a) keep comments in that code & use {}, only support SWIG < 3.0
> >>> b) keep comments & use %{/%}, only support SWIG > 3.0.5
> >>> c) drop the comments from that code, support SWIG < 3.0 && > 3.0.5
> >>>
> >>> I'm guessing (c) is preferred?
> > It seems like using %{/%} and adding SWIG >= 3.0.6 as supported versions
> > should work.  I re-applied r1690591 along with Joe's proposed patch for
> > SWIG version checking and was able to build with both swig 2.0.7 & swig
> > 3.0.7.
> >
> > The revert of r1690591 mentioned that older SWIG versions didn't work,
> > but the CI build log isn't available anymore.  What was the problem and
> > what was the SWIG versions?
> 
> IIRC, anything newer than 3.0.2 would fail no matter what I did.

In r1721488 I committed what I described above.  It's worked in my
testing.  If there is fallout from the CI builds, I'll take a look at
it.

Cheers,
-- 
James
GPG Key: 4096R/331BA3DB 2011-12-05 James McCoy <ja...@jamessan.com>

Re: Swig 3.x support (was Re: svn commit: r1690591 - /subversion/trunk/subversion/bindings/swig/include/proxy.swg)

Posted by Branko Čibej <br...@apache.org>.
On 22.12.2015 04:26, James McCoy wrote:
> On Sun, Nov 01, 2015 at 01:17:56AM -0500, James McCoy wrote:
>> On Mon, Jul 13, 2015 at 11:00:17AM +0100, Joe Orton wrote:
>>> On Mon, Jul 13, 2015 at 11:41:23AM +0200, Branko Čibej wrote:
>>>> https://ci.apache.org/builders/svn-x64-ubuntu-gcc/builds/317
>>>>
>>>> Apparently this breaks older versions of Swig ...
>>> Ah, ugh... yes I should have realised this.  Sorry.
>>>
>>> I think the choices are:
>>>
>>> a) keep comments in that code & use {}, only support SWIG < 3.0
>>> b) keep comments & use %{/%}, only support SWIG > 3.0.5
>>> c) drop the comments from that code, support SWIG < 3.0 && > 3.0.5
>>>
>>> I'm guessing (c) is preferred?
> It seems like using %{/%} and adding SWIG >= 3.0.6 as supported versions
> should work.  I re-applied r1690591 along with Joe's proposed patch for
> SWIG version checking and was able to build with both swig 2.0.7 & swig
> 3.0.7.
>
> The revert of r1690591 mentioned that older SWIG versions didn't work,
> but the CI build log isn't available anymore.  What was the problem and
> what was the SWIG versions?

IIRC, anything newer than 3.0.2 would fail no matter what I did.

-- Brane

Swig 3.x support (was Re: svn commit: r1690591 - /subversion/trunk/subversion/bindings/swig/include/proxy.swg)

Posted by James McCoy <ja...@jamessan.com>.
On Sun, Nov 01, 2015 at 01:17:56AM -0500, James McCoy wrote:
> On Mon, Jul 13, 2015 at 11:00:17AM +0100, Joe Orton wrote:
> > On Mon, Jul 13, 2015 at 11:41:23AM +0200, Branko Čibej wrote:
> > > https://ci.apache.org/builders/svn-x64-ubuntu-gcc/builds/317
> > > 
> > > Apparently this breaks older versions of Swig ...
> > 
> > Ah, ugh... yes I should have realised this.  Sorry.
> > 
> > I think the choices are:
> > 
> > a) keep comments in that code & use {}, only support SWIG < 3.0
> > b) keep comments & use %{/%}, only support SWIG > 3.0.5
> > c) drop the comments from that code, support SWIG < 3.0 && > 3.0.5
> > 
> > I'm guessing (c) is preferred?

It seems like using %{/%} and adding SWIG >= 3.0.6 as supported versions
should work.  I re-applied r1690591 along with Joe's proposed patch for
SWIG version checking and was able to build with both swig 2.0.7 & swig
3.0.7.

The revert of r1690591 mentioned that older SWIG versions didn't work,
but the CI build log isn't available anymore.  What was the problem and
what was the SWIG versions?

Cheers,
-- 
James
GPG Key: 4096R/331BA3DB 2011-12-05 James McCoy <ja...@jamessan.com>

Re: svn commit: r1690591 - /subversion/trunk/subversion/bindings/swig/include/proxy.swg

Posted by James McCoy <ja...@debian.org>.
On Mon, Jul 13, 2015 at 11:00:17AM +0100, Joe Orton wrote:
> On Mon, Jul 13, 2015 at 11:41:23AM +0200, Branko Čibej wrote:
> > https://ci.apache.org/builders/svn-x64-ubuntu-gcc/builds/317
> > 
> > Apparently this breaks older versions of Swig ...
> 
> Ah, ugh... yes I should have realised this.  Sorry.
> 
> I think the choices are:
> 
> a) keep comments in that code & use {}, only support SWIG < 3.0
> b) keep comments & use %{/%}, only support SWIG > 3.0.5
> c) drop the comments from that code, support SWIG < 3.0 && > 3.0.5
> 
> I'm guessing (c) is preferred?

Was there a resolution to this?  Debian recently switch to the 3.0
series of swig by default, which broke the subversion build due to the
requirement of < 3.0 swig.

I can work around this for now by explicitly using the 2.0 series of
swig, but that's being considered for removal from Debian before the
next stable release.

Cheers,
-- 
James
GPG Key: 4096R/331BA3DB 2011-12-05 James McCoy <ja...@debian.org>


Re: svn commit: r1690591 - /subversion/trunk/subversion/bindings/swig/include/proxy.swg

Posted by Joe Orton <jo...@redhat.com>.
On Mon, Jul 13, 2015 at 11:41:23AM +0200, Branko Čibej wrote:
> https://ci.apache.org/builders/svn-x64-ubuntu-gcc/builds/317
> 
> Apparently this breaks older versions of Swig ...

Ah, ugh... yes I should have realised this.  Sorry.

I think the choices are:

a) keep comments in that code & use {}, only support SWIG < 3.0
b) keep comments & use %{/%}, only support SWIG > 3.0.5
c) drop the comments from that code, support SWIG < 3.0 && > 3.0.5

I'm guessing (c) is preferred?

Regards, Joe

Re: svn commit: r1690591 - /subversion/trunk/subversion/bindings/swig/include/proxy.swg

Posted by Branko Čibej <br...@wandisco.com>.
On 13.07.2015 11:20, Joe Orton wrote:
> On Mon, Jul 13, 2015 at 11:07:57AM +0200, Branko Čibej wrote:
>> On 13.07.2015 11:04, jorton@apache.org wrote:
>>> Author: jorton
>>> Date: Mon Jul 13 09:04:13 2015
>>> New Revision: 1690591
>>>
>>> URL: http://svn.apache.org/r1690591
>>> Log:
>>> * subversion/bindings/swig/include/proxy.swg:
>>>   Use %{ %} with %pythoncode so comments avoid the SWIG preprocessor,
>>>   fixing the bindings with SWIG 3.0.6.
> ...
>> I've tried this before and it didn't fix the bindings at all.
> Hi Brane - yes I know, note the comment on the version: the SWIG 
> developers have addressed the issues we were hitting in the new 3.0.6 
> release last week.
>
> Can you retest?  It works for me now - including the tests.

https://ci.apache.org/builders/svn-x64-ubuntu-gcc/builds/317

Apparently this breaks older versions of Swig ...

-- Brane


Re: svn commit: r1690591 - /subversion/trunk/subversion/bindings/swig/include/proxy.swg

Posted by Joe Orton <jo...@redhat.com>.
On Mon, Jul 13, 2015 at 11:07:57AM +0200, Branko Čibej wrote:
> On 13.07.2015 11:04, jorton@apache.org wrote:
> > Author: jorton
> > Date: Mon Jul 13 09:04:13 2015
> > New Revision: 1690591
> >
> > URL: http://svn.apache.org/r1690591
> > Log:
> > * subversion/bindings/swig/include/proxy.swg:
> >   Use %{ %} with %pythoncode so comments avoid the SWIG preprocessor,
> >   fixing the bindings with SWIG 3.0.6.
...
> I've tried this before and it didn't fix the bindings at all.

Hi Brane - yes I know, note the comment on the version: the SWIG 
developers have addressed the issues we were hitting in the new 3.0.6 
release last week.

Can you retest?  It works for me now - including the tests.

If you or anybody else can confirm, I can update the version 
requirement.

[[[
Update the SWIG version requirement to reflect fixes in 3.0.6.

* build/ac-macros/swig.m4,
  subversion/bindings/swig/INSTALL:
  Require SWIG version >= 1.3.24 but exclude 3.0.0 through 3.0.6.

]]]


RE: svn commit: r1690591 - /subversion/trunk/subversion/bindings/swig/include/proxy.swg

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: Branko Čibej [mailto:brane@wandisco.com]
> Sent: maandag 13 juli 2015 11:08
> To: dev@subversion.apache.org
> Subject: Re: svn commit: r1690591 -
> /subversion/trunk/subversion/bindings/swig/include/proxy.swg
> 

> http://svn.apache.org/viewvc/subversion/trunk/subversion/bindings/swig/inclu
> de/proxy.swg?rev=1690591&r1=1690590&r2=1690591&view=diff
> >
> ================================================================
> ==============
> > --- subversion/trunk/subversion/bindings/swig/include/proxy.swg (original)
> > +++ subversion/trunk/subversion/bindings/swig/include/proxy.swg Mon Jul 13
> 09:04:13 2015
> > @@ -62,7 +62,7 @@
> >
> >  /* Default code for all wrapped proxy classes in Python */
> >  %define %proxy_pythoncode(TYPE)
> > -%pythoncode {
> > +%pythoncode %{
> >    def set_parent_pool(self, parent_pool=None):
> >      """Create a new proxy object for TYPE"""
> >      import libsvn.core, weakref
> > @@ -104,7 +104,7 @@
> >      self.__dict__.setdefault("_members",{})[name] = value
> >
> >      return _swig_setattr(self, self.__class__, name, value)
> > -}
> > +%}
> >  %enddef
> >
> >  /* Define a proxy for wrapping an existing struct */
> 
> 
> I've tried this before and it didn't fix the bindings at all.

Swig was patched to support this starting with 3.0.6.

The problem is now: Which swig versions do we want to support...

	Bert