You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Yasuhito FUTATSUKI <fu...@poem.co.jp> on 2019/10/21 22:25:03 UTC

Supported SWIG version on swig-py3 (was: Re: Test failures with Python 3 (Re: PMCs: any Hackathon requests? (deadline 11 October)))

On 2019/10/21 19:55, Jun Omae wrote:
> Hi,
> 
> I'm trying to build and test swig-py3 branch (r1868677) on Ubuntu 16.04 with
> Python 3.7, however I get FAILED(failures=16, errors=22) from the unit tests.
> 
> Investigating the issue with helps of Yasuhito, that is caused by using old
> SWIG version with no SWIG_PYTHON_STRICT_BYTE_CHAR feature. The
> SWIG_PYTHON_STRICT_BYTE_CHAR feature is available since SWIG 3.0.9 but SWIG is
> 3.0.8 in Ubuntu 16.04.
> 
> I consider that we should warn the required SWIG version at least.
> 
> 
> [1] https://github.com/swig/swig/blob/rel-3.0.10/CHANGES#L160

Thank you for your report. I think if that feature or some other changes on
swig-py3 breaks Python 2 (or accidentally Ruby and/or Perl bindings),
we should bump required SWIG verersion or resolve issues with older SWIGs.
However if it affect only with Python 3, we only need to restrict
per Language bindings requirement.

Anyway, I think we need more test with older SWIG (or restrict required
SWIG version if we can't test).

Thanks,
-- 
Yasuhito FUTATSUKI <fu...@poem.co.jp>

Re: Supported SWIG version on swig-py3

Posted by James McCoy <ja...@jamessan.com>.
On Tue, Oct 29, 2019 at 11:40:35AM +0100, Stefan Sperling wrote:
> On Tue, Oct 29, 2019 at 11:37:17AM +0100, Branko Čibej wrote:
> > Our .tar.bz2 and .tar.gz packages contain source files generated by
> > Swig. A packager /can/ decide to regenerate them; I don't know why they
> > would want to, but they can.
> 
> I had to do just that for quite some time in the OpenBSD package because
> swig versions were out of sync with those used by Subversion's release
> manager at the time, which led to problems during the build.
> I don't remember the details but regenerating those files with the
> installed version of swig fixed things.

We also do that in Debian, but it's more to ensure that we _can_
regenerate them if needed.  If we have to patch something in the swig
files, we want to know that we'll be able to successfully rebuild the
resulting output.

Cheers,
-- 
James
GPG Key: 4096R/91BF BF4D 6956 BD5D F7B7  2D23 DFE6 91AE 331B A3DB

Re: [Patch] Supported SWIG version(Re: Supported SWIG version on swig-py3)

Posted by Branko Čibej <br...@apache.org>.
On 12.11.2019 16:19, Nathan Hartman wrote:
> On Tue, Nov 12, 2019 at 7:26 AM Branko Čibej <brane@apache.org
> <ma...@apache.org>> wrote:
>
>     On 12.11.2019 13:08, Julian Foad wrote:
>     > Daniel Shahaf wrote:
>     >> There should be a comma after "e.g.".
>     >
>     > In my experience (in England)[,] a comma can be used[,] but is not
>     > often used.  I appreciate [that] we like to get grammar
>     > {right|"Right"}, but ... :-)
>
>
>     In MY experience, my English teacher (i.e., someone from England, not
>     just someone who taught me English, in a British school) hammered the
>     comma after 'e.g.' and 'i.e.' into my brainstem in no uncertain
>     terms. :)
>
>
> Thank you for that. :-)
>
> What about my "SWIG version 2.0.0 and later" singular / plural question
> and mix of treating it as singular in some places and plural in others?


In the specific example you quote, I believe the singular is correct
since you only use one version at a time, but have a choice of which
version that may be.


> may I suggest:
>
>     * SWIG installation is optional if you are using a Subversion
>       distribution tarball, because it contains the source files generated
>       by SWIG.

Rule number one: active voice, not passive voice.

    You do not have to install SWIG if you are using a Subversion
    distribution tarball, etc.


> may I suggest:
>
>     * Currently, SWIG versions 2.0.0 and later are supported, with the
>       following notes:

See rule number one.

    We currently support SWIG version 2.0.0 and later, etc.


Or:

    We currently support SWIG versions from 2.0.0 onwards, etc.


> I suggest adding "one of the":
>
>     * To verify you have SWIG installed correctly, run "swig -version"
>       from the command line.

English relies on prepositions, not on verb conjugations (since has
none). Also see rule number one.

    To verify that you installed SWIG correctly, etc.



--  Brane


Re: [Patch] Supported SWIG version(Re: Supported SWIG version on swig-py3)

Posted by Nathan Hartman <ha...@gmail.com>.
On Wed, Nov 13, 2019 at 1:20 AM Yasuhito FUTATSUKI <fu...@poem.co.jp>
wrote:

> On 2019/11/13 14:16, Nathan Hartman wrote:
> > Yasuhito, if possible could you please commit your patch?
> > Then, I will work on the description in the INSTALL file and I'll
> > incorporate the good input from Daniel, Julian, and Brane... :-)
>
> Thanks a lot, for your reviews, helps, and advices.
>
> I've committed my patch, with update for messages in  build/ac-macros
> by Nathan's suggestion and URLs update for ViewVC and Trac in INSTALL
> file only, in r1869722.


Thank you!

I have followed up with r1869745, to improve the text in
subversion/bindings/swig/INSTALL.

If any mistakes are found, let me know.

Note: If the patch contributed by Jun Omae a little earlier is
committed, the text regarding SWIG 4.0.0+ in INSTALL will need to be
adjusted accordingly. I have not changed it because the patch has not
been committed yet. See the thread "[Patch] Support building with
SWIG 4 on Python 3.x" --
https://lists.apache.org/thread.html/49766e2f6b78b6a89cbb398526b202d248ea90aa2b7c7e73cb8b22c0@%3Cdev.subversion.apache.org%3E

Cheers,
Nathan

Re: [Patch] Supported SWIG version(Re: Supported SWIG version on swig-py3)

Posted by Yasuhito FUTATSUKI <fu...@poem.co.jp>.
On 2019/11/13 14:16, Nathan Hartman wrote:
> On Tue, Nov 12, 2019 at 10:39 AM Julian Foad <ju...@apache.org> wrote:
> 
>> Nathan Hartman wrote:
>>> What about my "SWIG version 2.0.0 and later" singular / plural question
>>> and mix of treating it as singular in some places and plural in others?
>>
>> "SWIG versions 2 and later are ..."
>> "SWIG version 2 or later is ..."
>>
>> If there are no concerns about the substance of the patch, I suggest a
>> good course of action is to commit it ASAP, then just make your edits in
>> follow-up commits, and optionally mention in email that you did so.
>> Even when some of the language is a bit unclear, most of this sort of
>> thing doesn't need pre-commit review.
> 
> 
> I agree.
> 
> Yasuhito, if possible could you please commit your patch?
> Then, I will work on the description in the INSTALL file and I'll
> incorporate the good input from Daniel, Julian, and Brane... :-)

Thanks a lot, for your reviews, helps, and advices.

I've committed my patch, with update for messages in  build/ac-macros
by Nathan's suggestion and URLs update for ViewVC and Trac in INSTALL
file only, in r1869722.

Thanks,
-- 
Yasuhito FUTATSUKI <fu...@poem.co.jp>

Re: [Patch] Supported SWIG version(Re: Supported SWIG version on swig-py3)

Posted by Nathan Hartman <ha...@gmail.com>.
On Tue, Nov 12, 2019 at 10:39 AM Julian Foad <ju...@apache.org> wrote:

> Nathan Hartman wrote:
> > What about my "SWIG version 2.0.0 and later" singular / plural question
> > and mix of treating it as singular in some places and plural in others?
>
> "SWIG versions 2 and later are ..."
> "SWIG version 2 or later is ..."
>
> If there are no concerns about the substance of the patch, I suggest a
> good course of action is to commit it ASAP, then just make your edits in
> follow-up commits, and optionally mention in email that you did so.
> Even when some of the language is a bit unclear, most of this sort of
> thing doesn't need pre-commit review.


I agree.

Yasuhito, if possible could you please commit your patch?
Then, I will work on the description in the INSTALL file and I'll
incorporate the good input from Daniel, Julian, and Brane... :-)

Thanks!

Nathan

Re: [Patch] Supported SWIG version(Re: Supported SWIG version on swig-py3)

Posted by Julian Foad <ju...@apache.org>.
Nathan Hartman wrote:
> What about my "SWIG version 2.0.0 and later" singular / plural question
> and mix of treating it as singular in some places and plural in others?

"SWIG versions 2 and later are ..."
"SWIG version 2 or later is ..."

If there are no concerns about the substance of the patch, I suggest a 
good course of action is to commit it ASAP, then just make your edits in 
follow-up commits, and optionally mention in email that you did so. 
Even when some of the language is a bit unclear, most of this sort of 
thing doesn't need pre-commit review.

- Julian

Re: [Patch] Supported SWIG version(Re: Supported SWIG version on swig-py3)

Posted by Nathan Hartman <ha...@gmail.com>.
On Tue, Nov 12, 2019 at 7:26 AM Branko Čibej <br...@apache.org> wrote:

> On 12.11.2019 13:08, Julian Foad wrote:
> > Daniel Shahaf wrote:
> >> There should be a comma after "e.g.".
> >
> > In my experience (in England)[,] a comma can be used[,] but is not
> > often used.  I appreciate [that] we like to get grammar
> > {right|"Right"}, but ... :-)
>
>
> In MY experience, my English teacher (i.e., someone from England, not
> just someone who taught me English, in a British school) hammered the
> comma after 'e.g.' and 'i.e.' into my brainstem in no uncertain terms. :)
>

Thank you for that. :-)

What about my "SWIG version 2.0.0 and later" singular / plural question
and mix of treating it as singular in some places and plural in others?

Nathan

Re: [Patch] Supported SWIG version(Re: Supported SWIG version on swig-py3)

Posted by Branko Čibej <br...@apache.org>.
On 12.11.2019 13:08, Julian Foad wrote:
> Daniel Shahaf wrote:
>> There should be a comma after "e.g.".
>
> In my experience (in England)[,] a comma can be used[,] but is not
> often used.  I appreciate [that] we like to get grammar
> {right|"Right"}, but ... :-)


In MY experience, my English teacher (i.e., someone from England, not
just someone who taught me English, in a British school) hammered the
comma after 'e.g.' and 'i.e.' into my brainstem in no uncertain terms. :)

-- Brane


Re: [Patch] Supported SWIG version(Re: Supported SWIG version on swig-py3)

Posted by Julian Foad <ju...@apache.org>.
Daniel Shahaf wrote:
> There should be a comma after "e.g.".

In my experience (in England)[,] a comma can be used[,] but is not often 
used.  I appreciate [that] we like to get grammar {right|"Right"}, but 
... :-)

- Julian

Re: [Patch] Supported SWIG version(Re: Supported SWIG version on swig-py3)

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Nathan Hartman wrote on Mon, Nov 11, 2019 at 21:37:44 -0500:
>     * In the swig-x.y.z, directory, run ./configure (where x.y.z is
>       the SWIG version, e.g. 3.0.12).

There should be a comma after "e.g.".

Cheers,

Daniel

Re: Supported SWIG version on swig-py3

Posted by Stefan Sperling <st...@elego.de>.
On Tue, Oct 29, 2019 at 11:37:17AM +0100, Branko Čibej wrote:
> Our .tar.bz2 and .tar.gz packages contain source files generated by
> Swig. A packager /can/ decide to regenerate them; I don't know why they
> would want to, but they can.

I had to do just that for quite some time in the OpenBSD package because
swig versions were out of sync with those used by Subversion's release
manager at the time, which led to problems during the build.
I don't remember the details but regenerating those files with the
installed version of swig fixed things.

Re: [Patch] Supported SWIG version(Re: Supported SWIG version on swig-py3)

Posted by Nathan Hartman <ha...@gmail.com>.
On Fri, Nov 8, 2019 at 9:59 AM Yasuhito FUTATSUKI <fu...@poem.co.jp>
wrote:

> Hi, I updated the patch to restrict supported SWIG version
> posted before, <4e...@poem.co.jp>.
>
...

> Please review, especially I don't have any confidence in description
> in INSTALL file.


Hello Yasuhito,

I have reviewed your patch and made some edits, which I will describe
in detail here...

I hope it is helpful. :-)

I think that the SWIG bindings should be considered plural for
linguistic purposes (i.e., bindings, rather than binding).

I am not sure whether a phrase like "SWIG version 2.0.0 and later"
should be considered singular or plural. Is it plural because we are
potentially discussing multiple SWIG versions? Or is it singular
because the subject is "SWIG version 2.0.0" with any later version(s)
being incidental? I made some edits based on what "sounds" correct to
me. This results in some cases being singular and others plural.
Please let me know if this does not seem correct.

With the above points in mind, I suggest the following changes. To
save you some work, I have updated your patch with these changes
(attached):

In build/ac-macros/swig.m4:

AC_MSG_WARN([Subversion Python bindings for Python 3 requires 3.0.10 <=
SWIG < 4.0.0])
                                                     ^^^^^^^^
                                                     (require)

AC_MSG_WARN([Subversion Python bindings for Python 2 requires 1.3.24 <=
SWIG < 4.0.0])
                                                     ^^^^^^^^
                                                     (require)

In subversion/bindings/swig/INSTALL:

This bullet point:

+    * SWIG installation is optional if you are using distribution tarball,
+      because it contains source fils generated by SWIG. If you are using
+      a source working copy checked out from Subversion source repository,
+      or you want to regenerate SWIG language bindings source files from
+      SWIG source files for some reason, you need suitable version of
+      SWIG installation.

may I suggest:

    * SWIG installation is optional if you are using a Subversion
      distribution tarball, because it contains the source files generated
      by SWIG.  If you are using a working copy checked out from
      Subversion's source repository, or you want to regenerate the SWIG
      language bindings source files for some reason, you need a suitable
      version of SWIG installed.

This bullet point:

+    * Currently supported SWIG version is 2.0.0 and later, however,
+      - SWIG 1.3.24 and later in 1.3.x may work, but we didn't test on
+        latest our source code.
+      - Python bindings for Python 2 doesn't supported SWIG 4.0.0 and
later.
+      - Python 3 support for Python bindings requires SWIG 3.0.10 and
later,
+        but SWIG 4.0.0 and later is not supported (yet).
+      - Ruby bindings doesn't support SWIG 3.0.8.
+      - Make sure language specific supported version of SWIG itself, e.g.
+        - Perl 5.16 and later requires SWIG 2.0.8 and later.
+        - SWIG 3.0.9 has some trouble with Python support.
+         (see
https://sourceforge.net/p/swig/news/2016/06/swig-3010-released/)

may I suggest:

    * Currently, SWIG versions 2.0.0 and later are supported, with the
      following notes:
      - SWIG 1.3.24 and later 1.3.x may work, but we do not test these
        versions on our latest source code.
      - For Python 2 bindings, SWIG 4.0.0 or later is not supported.
      - For Python 3 bindings, SWIG 3.0.10 or later is required, but
        SWIG 4.0.0 and later is not supported (yet).
      - Note that SWIG 3.0.9 has some trouble with Python support.
        (See https://sourceforge.net/p/swig/news/2016/06/swig-3010-released/
)
      - For Perl 5.16 and later, SWIG 2.0.8 or later is required.
      - For Ruby bindings, SWIG 3.0.8 is not supported.

This was not part of your patch, but:

>    * Perhaps your distribution packages a suitable version - if it does
>      install it, and skip to the last bullet point in this section.

I suggest changing the punctuation:

    * Perhaps your distribution packages a suitable version.  If it does,
      install it and skip to the last bullet point in this section.

This bullet point:

+    * In the swig-x.y.z, directory, run ./configure (where x.y.z is
+      SWIG version, e.g. 3.0.12).

I suggest adding "the":

    * In the swig-x.y.z, directory, run ./configure (where x.y.z is
      the SWIG version, e.g. 3.0.12).

This bullet point:

     * To verify you have SWIG installed correctly, run "swig -version"
+      from the command line. SWIG should report that it is suitable version
+      mentioned above.

I suggest adding "one of the":

    * To verify you have SWIG installed correctly, run "swig -version"
      from the command line.  SWIG should report that it is one of the
      suitable versions mentioned above.

I hope this is helpful. Updated patch attached...

Kind regards,
Nathan

[Patch] Supported SWIG version(Re: Supported SWIG version on swig-py3)

Posted by Yasuhito FUTATSUKI <fu...@poem.co.jp>.
Hi, I updated the patch to restrict supported SWIG version
posted before, <4e...@poem.co.jp>.

In build system:
* Make symbolic link for *.so files in build tree on the copy-swig-py
  target, for the check-swig-py target (for SWIG >= 4.0.0). this symbolic
  links are cleared on the clean-swig-py target
* Stop build with Python 3 + SWIG < 3.0.10 (or SWIG >= 4.0.0, yet)
* Stop build with Python 2 + SWIG >= 4.0.0

In subversion/bindings/swig/INSTALL:
* Add description that SWIG is optional for using distribution tarball.
* Update description of supported SWIG version.

Please review, especially I don't have any confidence in description
in INSTALL file.

Thanks,
-- 
Yasuhito FUTATSUKI <fu...@poem.co.jp>

Re: Supported SWIG version on swig-py3

Posted by Yasuhito FUTATSUKI <fu...@poem.co.jp>.
On 2019/10/29 19:37, Branko Čibej wrote:
> If I understand your patch correctly, those symbolic links would be
> created in the build directory, not the source directory? In any case,
> if you commit that patch, you'll also have to fix the "clean-swig-py"
> target to remove the symlinks.

Yes, the symbolic links are created in $(SWIG_PY_DIR)/libsvn in the
build directory, built by the "$(SWIG_PY_DIR)/libsvn" target which is
also source of the "copy-swig-py" target. $(SWIG_PY_DIR)/libsvn
directory and its contents are removed by the first action
"rm -rf $(SWIG_PY_DIR}/libsvn" in the rule of "clean-swig-py" target.

Cheers,
-- 
Yasuhito FUTATSUKI <fu...@poem.co.jp>

Re: Supported SWIG version on swig-py3

Posted by Branko Čibej <br...@apache.org>.
On 29.10.2019 01:47, Yasuhito FUTATSUKI wrote:
> On 2019/10/29 2:45, Branko Čibej wrote:
>>                                  Because if that's the case, I would
>> prefer
>> making 3.0.10 our required minimum version. Remember that users who
>> build
>> from our tarballs do not need Swig on Unix, it's only needed by
>> developers
>> and the RM.
>
> I see. I missunderstood it because ome downstream packager attempt to use
> their own platform SWIG, at least on FreeBSD and Arch Linux make
> dependecy
> to SWIG for their package building....
>
> https://www.freebsd.org/cgi/ports.cgi?query=py27-subversion&stype=all&sektion=all
>
> https://www.archlinux.org/packages/extra/x86_64/subversion/
>
> Then, I think we should update subversion/bindings/INSTALL to mension
> it is optional even for building swig language bindings.

Our .tar.bz2 and .tar.gz packages contain source files generated by
Swig. A packager /can/ decide to regenerate them; I don't know why they
would want to, but they can.

If I understand your patch correctly, those symbolic links would be
created in the build directory, not the source directory? In any case,
if you commit that patch, you'll also have to fix the "clean-swig-py"
target to remove the symlinks.

-- Brane


Re: Supported SWIG version on swig-py3

Posted by Yasuhito FUTATSUKI <fu...@poem.co.jp>.
On 2019/10/29 2:45, Branko Čibej wrote:
> On Mon 28 Oct 2019, 15:11 Yasuhito FUTATSUKI, <fu...@poem.co.jp> wrote:
> 
>> (I sent this message 40 hours ago, however it is not derivered yet,
>> so I send again ....)
>>
>> On 2019/10/23 6:36, Yasuhito FUTATSUKI wrote:
>>> I ran check-swig-{py|pl|rb} with some SWIG versions on swig-py3 branch.
>>>
>>> Environment:
>>>     OS: FreeBSD 11.2
>>>     Python 2: 2.7.16
>>>     Python 3: 3.7.3
>>>     Perl: 5.28.2
>>>     Ruby: 2.5.5p157
>>>
>>> Results are below.
>>
>> <snip>
>>
>>> SWIG 3.0.9:
>>>     Python 2  ... can't import modules
>>>                   (regression, fixed in SWIG 3.0.10)[2]
>>>     Python 3  ... can't import modules
>>>                   (regression, fixed in SWIG 3.0.10)[2]
>>>     Perl      ... PASS
>>>     Ruby      ... 100% passed
>>
>>
>> With patch against Makefile.in below, which makes install time module
>> hierarchy in  build/test directory by using symbolic link in
>> copy-swig-py target, 'make check-swig-py' passed with SWIG 3.0.9,
>> both with Python 2 and with Python 3.
>>
>> [[[
>> Index: Makefile.in
>> ===================================================================
>> --- Makefile.in (revision 1868723)
>> +++ Makefile.in (working copy)
>> @@ -934,6 +934,7 @@
>>             @for f in $(SWIG_PY_SRC_DIR)/*.py $(SWIG_PY_DIR)/*.py; do \
>>               ! [ -f "$$f" ] || cp -pf $$f $(SWIG_PY_DIR)/libsvn; \
>>             done
>> +       @cd $(SWIG_PY_DIR)/libsvn;ln -sf ../.libs/*.so .
>>             @touch $(SWIG_PY_DIR)/libsvn/__init__.py
>>
>>      swig-py: autogen-swig-py copy-swig-py
>> ]]]
>>
> 
> 
> Do I understand correctly that this is a workaround for a bug in Swig
> 3.0.9, that was fixed in 3.0.10?

Yes, with Python >= 2.7 + SWIG 3.0.9, libsvn.foo try to import C extension
module _foo from its relative path only, doesn't fall back to normal module
search path. This only affects on test. Also, the code of libsvn.foo
produced by SWIG 4.0 attempts relative import only (as far as it is imported
as `libsvn.foo', not as `foo'). So This patch also address to SWIG 4.0
(as there are other issues with SWIG 4.0, our code doesn't work yet, though).

>                                  Because if that's the case, I would prefer
> making 3.0.10 our required minimum version. Remember that users who build
> from our tarballs do not need Swig on Unix, it's only needed by developers
> and the RM.

I see. I missunderstood it because ome downstream packager attempt to use
their own platform SWIG, at least on FreeBSD and Arch Linux make dependecy
to SWIG for their package building....

https://www.freebsd.org/cgi/ports.cgi?query=py27-subversion&stype=all&sektion=all
https://www.archlinux.org/packages/extra/x86_64/subversion/

Then, I think we should update subversion/bindings/INSTALL to mension
it is optional even for building swig language bindings.

> (N.b., we may have to include generated sources for both Python 2 and 3 in
> our tarballs, but that's a separate issue.)
> 
> 
> 
> (On Windows, as far as I read win_tests.py, it copies modules
>>
> for test with same hierarchy as install time, so it doesn't
>> affect, I guess.)
>>
> 
> 
> Yes, Windows shouldn't be affected.

Ah, thanks for clearify it.

Cheers,
-- 
Yasuhito FUTATSUKI <fu...@poem.co.jp>

Re: Supported SWIG version on swig-py3

Posted by Branko Čibej <br...@apache.org>.
On Mon 28 Oct 2019, 15:11 Yasuhito FUTATSUKI, <fu...@poem.co.jp> wrote:

> (I sent this message 40 hours ago, however it is not derivered yet,
> so I send again ....)
>
> On 2019/10/23 6:36, Yasuhito FUTATSUKI wrote:
> > I ran check-swig-{py|pl|rb} with some SWIG versions on swig-py3 branch.
> >
> > Environment:
> >    OS: FreeBSD 11.2
> >    Python 2: 2.7.16
> >    Python 3: 3.7.3
> >    Perl: 5.28.2
> >    Ruby: 2.5.5p157
> >
> > Results are below.
>
> <snip>
>
> > SWIG 3.0.9:
> >    Python 2  ... can't import modules
> >                  (regression, fixed in SWIG 3.0.10)[2]
> >    Python 3  ... can't import modules
> >                  (regression, fixed in SWIG 3.0.10)[2]
> >    Perl      ... PASS
> >    Ruby      ... 100% passed
>
>
> With patch against Makefile.in below, which makes install time module
> hierarchy in  build/test directory by using symbolic link in
> copy-swig-py target, 'make check-swig-py' passed with SWIG 3.0.9,
> both with Python 2 and with Python 3.
>
> [[[
> Index: Makefile.in
> ===================================================================
> --- Makefile.in (revision 1868723)
> +++ Makefile.in (working copy)
> @@ -934,6 +934,7 @@
>            @for f in $(SWIG_PY_SRC_DIR)/*.py $(SWIG_PY_DIR)/*.py; do \
>              ! [ -f "$$f" ] || cp -pf $$f $(SWIG_PY_DIR)/libsvn; \
>            done
> +       @cd $(SWIG_PY_DIR)/libsvn;ln -sf ../.libs/*.so .
>            @touch $(SWIG_PY_DIR)/libsvn/__init__.py
>
>     swig-py: autogen-swig-py copy-swig-py
> ]]]
>


Do I understand correctly that this is a workaround for a bug in Swig
3.0.9, that was fixed in 3.0.10? Because if that's the case, I would prefer
making 3.0.10 our required minimum version. Remember that users who build
from our tarballs do not need Swig on Unix, it's only needed by developers
and the RM.

(N.b., we may have to include generated sources for both Python 2 and 3 in
our tarballs, but that's a separate issue.)



(On Windows, as far as I read win_tests.py, it copies modules
>
for test with same hierarchy as install time, so it doesn't
> affect, I guess.)
>


Yes, Windows shouldn't be affected.

-- Brane



>

Re: Supported SWIG version on swig-py3

Posted by Yasuhito FUTATSUKI <fu...@poem.co.jp>.
(I sent this message 40 hours ago, however it is not derivered yet,
so I send again ....)
  
On 2019/10/23 6:36, Yasuhito FUTATSUKI wrote:
> I ran check-swig-{py|pl|rb} with some SWIG versions on swig-py3 branch.
> 
> Environment:
>    OS: FreeBSD 11.2
>    Python 2: 2.7.16
>    Python 3: 3.7.3
>    Perl: 5.28.2
>    Ruby: 2.5.5p157
> 
> Results are below.

<snip>

> SWIG 3.0.9:
>    Python 2  ... can't import modules
>                  (regression, fixed in SWIG 3.0.10)[2]
>    Python 3  ... can't import modules
>                  (regression, fixed in SWIG 3.0.10)[2]
>    Perl      ... PASS
>    Ruby      ... 100% passed


With patch against Makefile.in below, which makes install time module
hierarchy in  build/test directory by using symbolic link in
copy-swig-py target, 'make check-swig-py' passed with SWIG 3.0.9,
both with Python 2 and with Python 3.

[[[
Index: Makefile.in
===================================================================
--- Makefile.in (revision 1868723)
+++ Makefile.in (working copy)
@@ -934,6 +934,7 @@
           @for f in $(SWIG_PY_SRC_DIR)/*.py $(SWIG_PY_DIR)/*.py; do \
             ! [ -f "$$f" ] || cp -pf $$f $(SWIG_PY_DIR)/libsvn; \
           done
+       @cd $(SWIG_PY_DIR)/libsvn;ln -sf ../.libs/*.so .
           @touch $(SWIG_PY_DIR)/libsvn/__init__.py
    
    swig-py: autogen-swig-py copy-swig-py
]]]

(On Windows, as far as I read win_tests.py, it copies modules
for test with same hierarchy as install time, so it doesn't
affect, I guess.)

So I think swig Python bindings has no problem with SWIG 3.0.9.

Cheers,
-- 
Yasuhito FUTATSUKI <fu...@yf.bsdclub.org>

-- 
Yasuhito FUTATSUKI <fu...@poem.co.jp>

Re: Supported SWIG version on swig-py3

Posted by Yasuhito FUTATSUKI <fu...@poem.co.jp>.
I ran check-swig-{py|pl|rb} with some SWIG versions on swig-py3 branch.

Environment:
   OS: FreeBSD 11.2
   Python 2: 2.7.16
   Python 3: 3.7.3
   Perl: 5.28.2
   Ruby: 2.5.5p157

Results are below.


SWIG 1.3.24:
   I couldn't build swig itself.

SWIG 2.0.0:
   Python 2  ... OK (skipped=1)
   Python 3  ... can't build modules
   Perl      ... can't build modules (Perhaps due to issue on build
                 with Perl >= 5.16, fixed in SWIG 2.0.8)[1]
   Ruby      ... 100% passed

SWIG 2.0.12:
   Python 2  ... OK (skipped=1)
   Python 3  ... FAILED (failures=16, errors=22)
   Perl      ... PASS
   Ruby      ... 100% passed

SWIG 3.0.0:
   Python 2  ... OK (skipped=1)
   Python 3  ... FAILED (failures=16, errors=22)
   Perl      ... PASS
   Ruby      ... 100% passed

SWIG 3.0.9:
   Python 2  ... can't import modules
                 (regression, fixed in SWIG 3.0.10)[2]
   Python 3  ... can't import modules
                 (regression, fixed in SWIG 3.0.10)[2]
   Perl      ... PASS
   Ruby      ... 100% passed

SWIG 3.0.10:
   Python 2  ... OK (skipped=1)
   Python 3  ... OK
   Perl      ... PASS
   Ruby      ... 100% passed

SWIG 3.0.12:
   Python 2  ... OK (skipped=1)
   Python 3  ... OK
   Perl      ... PASS
   Ruby      ... 100% passed

SWIG 4.0.1:
   Python 2  ... can't build modules correctly (SVN-4818)
   Python 3  ... can't build modules correctly (SVN-4818)
   Perl      ... PASS
   Ruby      ... 100% passed

[1] https://github.com/swig/swig/blob/rel-2.0.8/CHANGES.current#L30
[2] https://github.com/swig/swig/blob/rel-3.0.10/RELEASENOTES#L11

I also make a patch to update subversion/binding/swig/INSTALL,
however I don't touch build/ac-macros/swig.m4.

Cheers,
-- 
Yasuhito FUTATSUKI <fu...@poem.co.jp>

Re: Supported SWIG version on swig-py3

Posted by Yasuhito FUTATSUKI <fu...@poem.co.jp>.
On 2019/10/22 7:25, Yasuhito FUTATSUKI wrote:
> On 2019/10/21 19:55, Jun Omae wrote:
>> Hi,
>>
>> I'm trying to build and test swig-py3 branch (r1868677) on Ubuntu 16.04 with
>> Python 3.7, however I get FAILED(failures=16, errors=22) from the unit tests.
>>
>> Investigating the issue with helps of Yasuhito, that is caused by using old
>> SWIG version with no SWIG_PYTHON_STRICT_BYTE_CHAR feature. The
>> SWIG_PYTHON_STRICT_BYTE_CHAR feature is available since SWIG 3.0.9 but SWIG is
>> 3.0.8 in Ubuntu 16.04.
>>
>> I consider that we should warn the required SWIG version at least.
>>
>>
>> [1] https://github.com/swig/swig/blob/rel-3.0.10/CHANGES#L160
> 
> Thank you for your report. I think if that feature or some other changes on
> swig-py3 breaks Python 2 (or accidentally Ruby and/or Perl bindings),
> we should bump required SWIG verersion or resolve issues with older SWIGs.
> However if it affect only with Python 3, we only need to restrict
> per Language bindings requirement.
> 
> Anyway, I think we need more test with older SWIG (or restrict required
> SWIG version if we can't test).

With SWIG 3.0.0/2.0.12 + Python 2.7,  make check-py passes all tests.

[[[
$ head -2 subversion/bindings/swig/python/core.py && make check-swig-py
# This file was automatically generated by SWIG (http://www.swig.org).
# Version 2.0.12
if [ "LD_LIBRARY_PATH" = "DYLD_LIBRARY_PATH" ]; then  for d in /home/futatuki/work/subversion/vwc/branches/swig-py3/subversion/bindings/swig/python/libsvn_swig_py /home/futatuki/work/subversion/vwc/branches/swig-py3/subversion/bindings/swig/python/../../../libsvn_*; do  if [ -n "$DYLD_LIBRARY_PATH" ]; then  LD_LIBRARY_PATH="$LD_LIBRARY_PATH:$d/.libs";  else  LD_LIBRARY_PATH="$d/.libs";  fi;  done;  export LD_LIBRARY_PATH;  fi;  export PYTHONLIB=/home/futatuki/work/subversion/vwc/branches/swig-py3/subversion/bindings/swig/python/.libs:/home/futatuki/work/subversion/vwc/branches/swig-py3/subversion/bindings/swig/python;  cd /home/futatuki/work/subversion/vwc/branches/swig-py3/subversion/bindings/swig/python;  /usr/local/bin/python2.7 /home/futatuki/work/subversion/vwc/branches/swig-py3/subversion/bindings/swig/python/tests/run_all.py
..............s..........................................................................................................................................
----------------------------------------------------------------------
Ran 153 tests in 13.568s

OK (skipped=1)
]]]

[[[
$ head -2 subversion/bindings/swig/python/core.py && make check-swig-py
# This file was automatically generated by SWIG (http://www.swig.org).
# Version 3.0.0
if [ "LD_LIBRARY_PATH" = "DYLD_LIBRARY_PATH" ]; then  for d in /home/futatuki/work/subversion/vwc/branches/swig-py3/subversion/bindings/swig/python/libsvn_swig_py /home/futatuki/work/subversion/vwc/branches/swig-py3/subversion/bindings/swig/python/../../../libsvn_*; do  if [ -n "$DYLD_LIBRARY_PATH" ]; then  LD_LIBRARY_PATH="$LD_LIBRARY_PATH:$d/.libs";  else  LD_LIBRARY_PATH="$d/.libs";  fi;  done;  export LD_LIBRARY_PATH;  fi;  export PYTHONLIB=/home/futatuki/work/subversion/vwc/branches/swig-py3/subversion/bindings/swig/python/.libs:/home/futatuki/work/subversion/vwc/branches/swig-py3/subversion/bindings/swig/python;  cd /home/futatuki/work/subversion/vwc/branches/swig-py3/subversion/bindings/swig/python;  /usr/local/bin/python2.7 /home/futatuki/work/subversion/vwc/branches/swig-py3/subversion/bindings/swig/python/tests/run_all.py
..............s..........................................................................................................................................
----------------------------------------------------------------------
Ran 153 tests in 14.341s

OK (skipped=1)
]]]

Cheers,
-- 
Yasuhito FUTATSUKI <fu...@poem.co.jp>