You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Jun Omae <ju...@gmail.com> on 2020/05/10 04:27:46 UTC
[PATCH] fix unable to load *.pyd files with Python 3.8.x on Windows
Hi,
I noticed Python bindings is unable to load with Python 3.8.x on Windows, while trying
to run check-swig-py.
[[[
C:\usr\src\subversion\trunk-py3> python.exe -c "from svn import core; print(core.SVN_VERSION)"
Traceback (most recent call last):
File "C:\usr\src\subversion\trunk-py3\Release\python\libsvn\core.py", line 14, in swig_import_helper
return importlib.import_module(mname)
File "C:\usr\apps\python38\lib\importlib\__init__.py", line 127, in import_module
return _bootstrap._gcd_import(name[level:], package, level)
File "<frozen importlib._bootstrap>", line 1014, in _gcd_import
File "<frozen importlib._bootstrap>", line 991, in _find_and_load
File "<frozen importlib._bootstrap>", line 975, in _find_and_load_unlocked
File "<frozen importlib._bootstrap>", line 657, in _load_unlocked
File "<frozen importlib._bootstrap>", line 556, in module_from_spec
File "<frozen importlib._bootstrap_external>", line 1101, in create_module
File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
ImportError: DLL load failed while importing _core: The specified module could not be found.
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "<string>", line 1, in <module>
File "C:\usr\src\subversion\trunk-py3\Release\python\svn\core.py", line 26, in <module>
from libsvn.core import *
File "C:\usr\src\subversion\trunk-py3\Release\python\libsvn\core.py", line 17, in <module>
_core = swig_import_helper()
File "C:\usr\src\subversion\trunk-py3\Release\python\libsvn\core.py", line 16, in swig_import_helper
return importlib.import_module('_core')
File "C:\usr\apps\python38\lib\importlib\__init__.py", line 127, in import_module
return _bootstrap._gcd_import(name[level:], package, level)
ModuleNotFoundError: No module named '_core'
]]]
Investigating the issue, I found related changes in What's New In Python 3.8 [1].
According to the document, DLL dependencies for *.pyd file doesn't search the directories
in PATH environment since Python 3.8 on Windows. We should call os.add_dll_directory() to
add search paths to resolve the dependencies.
I created patch to resolve the issue using moduleimport option of %module directive.
After attached patch, *.pyd file is successfully loaded and tests for Python bindings pass.
[[[
* subversion/bindings/swig/include/svn_global.swg
(SVN_PYTHON_MODULEIMPORT): Add custom code to add search paths to resolve DLL
dependencies when importing libsvn/*.pyd file.
* subversion/bindings/swig/core.i
* subversion/bindings/swig/svn_client.i
* subversion/bindings/swig/svn_delta.i
* subversion/bindings/swig/svn_diff.i
* subversion/bindings/swig/svn_fs.i
* subversion/bindings/swig/svn_ra.i
* subversion/bindings/swig/svn_repos.i
* subversion/bindings/swig/svn_wc.i
Add moduleimport option with SVN_PYTHON_MODULEIMPORT to %module directive for
Python.
]]]
[1] https://docs.python.org/3/whatsnew/3.8.html#bpo-36085-whatsnew
--
Jun Omae <ju...@gmail.com> ($BBgA0(B $B=a(B)
Python bindings issue with multiple apr_pool_t* arguments (was:
[PATCH] fix unable to load *.pyd files with Python 3.8.x on Windows)
Posted by Johan Corveleyn <jc...@gmail.com>.
[ Changing subject to get this unburried from a long semi-related
thread. More below. ]
On Tue, Dec 22, 2020 at 6:27 PM Yasuhito FUTATSUKI
<fu...@yf.bsdclub.org> wrote:
>
> On 2020/12/21 15:50, Daniel Shahaf wrote:
> > Johan Corveleyn wrote on Sun, 20 Dec 2020 23:05 +0100:
> >> On Sat, May 23, 2020 at 2:17 PM Yasuhito FUTATSUKI <fu...@poem.co.jp> wrote:
> >>>
> >>> On 2020/05/23 12:16, Daniel Shahaf wrote:
> >>>> Yasuhito FUTATSUKI wrote on Sat, 23 May 2020 11:08 +0900:
> >>>>> I think it can be fixed by overriding _global_py_pool to $input in "in"
> >>>>> typemap, when $1 is updated. It assumes that there are no APIs that
> >>>>> uses two or more result_pool like apr_pool_t * argument for allocationg
> >>>>> return value.
> >>>>
> >>>> What about svn_repos_node_editor()? It has two pools, and if I'm
> >>>> reading the docstring correctly, "pool" will be used both for temporary
> >>>> allocations and for allocating *editor and *edit_baton, while
> >>>> "node_pool" will be used to allocate the tree that function constructs.
> >>>>
> >>>> That function is from 1.0 [according to its docstring], so it precedes
> >>>> the result_pool/scratch_pool convention by several years.
> >>>
> >>> We can't add reference of "node_pool" Python object in apr_pool_t *
> >>> members of the *editor and the *edit_baton C structure, at least
> >>> with current wrapper mechanism. I think all we can do is to make
> >>> "_parent_pool" attribute of wrapper objects for *editor and
> >>> *edit_baton point to "pool" object.
> >>>
> >>> Fortunately as apr_pool_t *node_pool is used this API only, we can
> >>> add typemap for it safely, in this case.
> >>
> >> Now that the "unable to load *.pyd files" is fixed on trunk in
> >> r1884642 (and nominated for backport to 1.14.x), I've retried this
> >> latest patch for fixing the refcount issue. This patch makes the
> >> swig-python tests succeed for me (on Windows with Python 3.9.1, in
> >> debug configuration).
> >>
> >> So, as far as I'm concerned, this is good to go, and certainly a step
> >> forward again :-).
> >>
> >> I don't feel up to doing a proper review though, it that's what's asked here.
> >> Can Jun or Daniel perhaps take another look?
> >
> > Not this Daniel :)
>
> Really? :)
>
> I know that Jun is not satisfied with my patch. Also, I don't think
> my patch really resolve the problems.
>
> For svn_repos_node_editor(), those programmer who want to pass different
> pools to the wrapper function should keep the reference for node_pool
> explicitly until the wrapper objects which contains tree structure
> allocated from node_pool.
>
> For other APIs which accept result_pool and scratch_pool, the problem
> of my patch is:
>
> (1) If a wrapper API accept a pure Python objects and construct some
> C struct object from them, wrapper function uses scratch_pool
> for their allocation.
> (2) If a C API correspoinding to such a wrapper returns some C
> structure which contain some object created in case (1) as a
> part of them, those result objects depends on scratch_pool
> as well as result_pool. However Python don't know such a
> dependency because there is no reference from wrapper object of
> result object to scratch_pool object.
>
> If object alocation in (1) uses result_pool instead of srcatch_pool,
> the problem in (2) will be resolved, and perhaps Jun wants to implement
> it. However in this case, temporary objects for C API call will left
> in result_pool even if they aren't parts of result objects.
>
> Any ways, I think this is a limitation of non-custom wrapper function.
> So it might be one of good solution that we never allow to pass
> multiple pools to Python wrapper function.
>
> Cheers,
> --
> Yasuhito FUTATSUKI <fu...@yf.bsclub.org>
Hi,
Just wanted to highlight that this issue with the Python bindings and
multiple apr_pool_t* arguments is not fixed yet. As I understand it,
it leads to a "negative refcount" assertion in debug mode, and in a
potential memory leak in both debug and release mode.
There were some patch iterations by Jun and Yasuhito, but apparently
the approach still has some problems (as explained by Yasuhito here
above). Attaching the latest patch from the original thread, to have
everything together.
To be clear: I'm not personally affected by this problem, and I don't
understand enough about it to help ... I just don't want this problem
/ these efforts to be lost in the mists of our mailinglists.
Hopefully, some people can take another look at this.
--
Johan
Re: [PATCH] fix unable to load *.pyd files with Python 3.8.x on
Windows
Posted by Yasuhito FUTATSUKI <fu...@yf.bsdclub.org>.
On 2020/12/21 15:50, Daniel Shahaf wrote:
> Johan Corveleyn wrote on Sun, 20 Dec 2020 23:05 +0100:
>> On Sat, May 23, 2020 at 2:17 PM Yasuhito FUTATSUKI <fu...@poem.co.jp> wrote:
>>>
>>> On 2020/05/23 12:16, Daniel Shahaf wrote:
>>>> Yasuhito FUTATSUKI wrote on Sat, 23 May 2020 11:08 +0900:
>>>>> I think it can be fixed by overriding _global_py_pool to $input in "in"
>>>>> typemap, when $1 is updated. It assumes that there are no APIs that
>>>>> uses two or more result_pool like apr_pool_t * argument for allocationg
>>>>> return value.
>>>>
>>>> What about svn_repos_node_editor()? It has two pools, and if I'm
>>>> reading the docstring correctly, "pool" will be used both for temporary
>>>> allocations and for allocating *editor and *edit_baton, while
>>>> "node_pool" will be used to allocate the tree that function constructs.
>>>>
>>>> That function is from 1.0 [according to its docstring], so it precedes
>>>> the result_pool/scratch_pool convention by several years.
>>>
>>> We can't add reference of "node_pool" Python object in apr_pool_t *
>>> members of the *editor and the *edit_baton C structure, at least
>>> with current wrapper mechanism. I think all we can do is to make
>>> "_parent_pool" attribute of wrapper objects for *editor and
>>> *edit_baton point to "pool" object.
>>>
>>> Fortunately as apr_pool_t *node_pool is used this API only, we can
>>> add typemap for it safely, in this case.
>>
>> Now that the "unable to load *.pyd files" is fixed on trunk in
>> r1884642 (and nominated for backport to 1.14.x), I've retried this
>> latest patch for fixing the refcount issue. This patch makes the
>> swig-python tests succeed for me (on Windows with Python 3.9.1, in
>> debug configuration).
>>
>> So, as far as I'm concerned, this is good to go, and certainly a step
>> forward again :-).
>>
>> I don't feel up to doing a proper review though, it that's what's asked here.
>> Can Jun or Daniel perhaps take another look?
>
> Not this Daniel :)
Really? :)
I know that Jun is not satisfied with my patch. Also, I don't think
my patch really resolve the problems.
For svn_repos_node_editor(), those programmer who want to pass different
pools to the wrapper function should keep the reference for node_pool
explicitly until the wrapper objects which contains tree structure
allocated from node_pool.
For other APIs which accept result_pool and scratch_pool, the problem
of my patch is:
(1) If a wrapper API accept a pure Python objects and construct some
C struct object from them, wrapper function uses scratch_pool
for their allocation.
(2) If a C API correspoinding to such a wrapper returns some C
structure which contain some object created in case (1) as a
part of them, those result objects depends on scratch_pool
as well as result_pool. However Python don't know such a
dependency because there is no reference from wrapper object of
result object to scratch_pool object.
If object alocation in (1) uses result_pool instead of srcatch_pool,
the problem in (2) will be resolved, and perhaps Jun wants to implement
it. However in this case, temporary objects for C API call will left
in result_pool even if they aren't parts of result objects.
Any ways, I think this is a limitation of non-custom wrapper function.
So it might be one of good solution that we never allow to pass
multiple pools to Python wrapper function.
Cheers,
--
Yasuhito FUTATSUKI <fu...@yf.bsclub.org>
Re: [PATCH] fix unable to load *.pyd files with Python 3.8.x on
Windows
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Johan Corveleyn wrote on Sun, 20 Dec 2020 23:05 +0100:
> On Sat, May 23, 2020 at 2:17 PM Yasuhito FUTATSUKI <fu...@poem.co.jp> wrote:
> >
> > On 2020/05/23 12:16, Daniel Shahaf wrote:
> > > Yasuhito FUTATSUKI wrote on Sat, 23 May 2020 11:08 +0900:
> > >> I think it can be fixed by overriding _global_py_pool to $input in "in"
> > >> typemap, when $1 is updated. It assumes that there are no APIs that
> > >> uses two or more result_pool like apr_pool_t * argument for allocationg
> > >> return value.
> > >
> > > What about svn_repos_node_editor()? It has two pools, and if I'm
> > > reading the docstring correctly, "pool" will be used both for temporary
> > > allocations and for allocating *editor and *edit_baton, while
> > > "node_pool" will be used to allocate the tree that function constructs.
> > >
> > > That function is from 1.0 [according to its docstring], so it precedes
> > > the result_pool/scratch_pool convention by several years.
> >
> > We can't add reference of "node_pool" Python object in apr_pool_t *
> > members of the *editor and the *edit_baton C structure, at least
> > with current wrapper mechanism. I think all we can do is to make
> > "_parent_pool" attribute of wrapper objects for *editor and
> > *edit_baton point to "pool" object.
> >
> > Fortunately as apr_pool_t *node_pool is used this API only, we can
> > add typemap for it safely, in this case.
>
> Now that the "unable to load *.pyd files" is fixed on trunk in
> r1884642 (and nominated for backport to 1.14.x), I've retried this
> latest patch for fixing the refcount issue. This patch makes the
> swig-python tests succeed for me (on Windows with Python 3.9.1, in
> debug configuration).
>
> So, as far as I'm concerned, this is good to go, and certainly a step
> forward again :-).
>
> I don't feel up to doing a proper review though, it that's what's asked here.
> Can Jun or Daniel perhaps take another look?
Not this Daniel :)
Re: [PATCH] fix unable to load *.pyd files with Python 3.8.x on Windows
Posted by Johan Corveleyn <jc...@gmail.com>.
On Sat, May 23, 2020 at 2:17 PM Yasuhito FUTATSUKI <fu...@poem.co.jp> wrote:
>
> On 2020/05/23 12:16, Daniel Shahaf wrote:
> > Yasuhito FUTATSUKI wrote on Sat, 23 May 2020 11:08 +0900:
> >> I think it can be fixed by overriding _global_py_pool to $input in "in"
> >> typemap, when $1 is updated. It assumes that there are no APIs that
> >> uses two or more result_pool like apr_pool_t * argument for allocationg
> >> return value.
> >
> > What about svn_repos_node_editor()? It has two pools, and if I'm
> > reading the docstring correctly, "pool" will be used both for temporary
> > allocations and for allocating *editor and *edit_baton, while
> > "node_pool" will be used to allocate the tree that function constructs.
> >
> > That function is from 1.0 [according to its docstring], so it precedes
> > the result_pool/scratch_pool convention by several years.
>
> We can't add reference of "node_pool" Python object in apr_pool_t *
> members of the *editor and the *edit_baton C structure, at least
> with current wrapper mechanism. I think all we can do is to make
> "_parent_pool" attribute of wrapper objects for *editor and
> *edit_baton point to "pool" object.
>
> Fortunately as apr_pool_t *node_pool is used this API only, we can
> add typemap for it safely, in this case.
Now that the "unable to load *.pyd files" is fixed on trunk in
r1884642 (and nominated for backport to 1.14.x), I've retried this
latest patch for fixing the refcount issue. This patch makes the
swig-python tests succeed for me (on Windows with Python 3.9.1, in
debug configuration).
So, as far as I'm concerned, this is good to go, and certainly a step
forward again :-).
I don't feel up to doing a proper review though, it that's what's asked here.
Can Jun or Daniel perhaps take another look?
--
Johan
Re: [PATCH] fix unable to load *.pyd files with Python 3.8.x on
Windows
Posted by Yasuhito FUTATSUKI <fu...@poem.co.jp>.
On 2020/05/23 12:16, Daniel Shahaf wrote:
> Yasuhito FUTATSUKI wrote on Sat, 23 May 2020 11:08 +0900:
>> I think it can be fixed by overriding _global_py_pool to $input in "in"
>> typemap, when $1 is updated. It assumes that there are no APIs that
>> uses two or more result_pool like apr_pool_t * argument for allocationg
>> return value.
>
> What about svn_repos_node_editor()? It has two pools, and if I'm
> reading the docstring correctly, "pool" will be used both for temporary
> allocations and for allocating *editor and *edit_baton, while
> "node_pool" will be used to allocate the tree that function constructs.
>
> That function is from 1.0 [according to its docstring], so it precedes
> the result_pool/scratch_pool convention by several years.
We can't add reference of "node_pool" Python object in apr_pool_t *
members of the *editor and the *edit_baton C structure, at least
with current wrapper mechanism. I think all we can do is to make
"_parent_pool" attribute of wrapper objects for *editor and
*edit_baton point to "pool" object.
Fortunately as apr_pool_t *node_pool is used this API only, we can
add typemap for it safely, in this case.
Cheers,
--
Yasuhito FUTATSUKI <fu...@yf.bsdclub.org>
Re: [PATCH] fix unable to load *.pyd files with Python 3.8.x on
Windows
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Yasuhito FUTATSUKI wrote on Sat, 23 May 2020 11:08 +0900:
> I think it can be fixed by overriding _global_py_pool to $input in "in"
> typemap, when $1 is updated. It assumes that there are no APIs that
> uses two or more result_pool like apr_pool_t * argument for allocationg
> return value.
What about svn_repos_node_editor()? It has two pools, and if I'm
reading the docstring correctly, "pool" will be used both for temporary
allocations and for allocating *editor and *edit_baton, while
"node_pool" will be used to allocate the tree that function constructs.
That function is from 1.0 [according to its docstring], so it precedes
the result_pool/scratch_pool convention by several years.
Cheers,
Daniel
P.S. Found that with «grep -h -Eo 'apr_pool_t[ *]*[A-Za-z0-9_]*'
subversion/include/*.h | sort | uniq -c».
Re: [PATCH] fix unable to load *.pyd files with Python 3.8.x on
Windows
Posted by Yasuhito FUTATSUKI <fu...@poem.co.jp>.
On 2020/05/22 8:02, Yasuhito FUTATSUKI wrote:
> On 2020/05/20 21:54, Jun Omae wrote:
>> On Mon, May 18, 2020 at 8:25 AM Yasuhito FUTATSUKI <fu...@poem.co.jp> wrote:
>>> Jun and I found it is caused by long existing bug on SWIG Python bindings,
>>> since before swig-py3 branch was merged (thus this bug exists on 1.13.x
>>> and 1.10.3 branches). On SWIG Python bindings, APIs which have multiple
>>> apr_pool_t * argments cannot be wrapped correctly.
>>>
>>> If we call such a wrapper function with specifying multiple pool argments
>>> explicitly, only last one is used for *all* apr_pool_t * arguments when
>>> it calls C API.
>>
>> Created global-py-pool-ref-count.diff patch to solve the multiple apr_pool *
>> pointers issue in Python bindings.
>>
>> Verified no assertions while running check-swig-py with the following
>> environments:
>>
>> - (Python 3.5.2, 3.6.10, 3.7.7, 3.8.3, 3.9.0a6) x (SWIG 4.0.1)
>> - (Python 2.7.12) x (SWIG 1.3.40, 2.0.12, 3.0.12)
>
> Thank you very much!
>
> With this patch, I confirmed that each apr_pool * argment in C API supplied
> from corresponding pool object if it is supplied by the Python wrapper function
> (by reading _wrap_svn_client_propget5() in svn_client.c generated by
> SWIG 3.0.12, for Python 2).
>
> However, I couldn't confirm that the "_parent_pool" attribute in wrapper
> object returned by wrapper function point to "result_pool" object in such case,
> yet. I doubt it still points to "scratch_pool" object because variable
> "_global_py_pool" points to "scratch_pool" object when both of "result_pool"
> and "scratch_pool" are provided on such APIs.
In "out" and "argout" typemaps, "_parent_pool" attribute of wrapper objects
are set from "_global_py_pool".
With this additional hunks for SubversionClientTestCase.test_conflict(),
[[[
Index: subversion/bindings/swig/python/tests/client.py
===================================================================
--- subversion/bindings/swig/python/tests/client.py (revision 1877963)
+++ subversion/bindings/swig/python/tests/client.py (working copy)
@@ -592,10 +603,14 @@
client.update4((path,), rev, core.svn_depth_unknown, True, False, False,
False, False, self.client_ctx)
- pool = core.Pool()
- conflict = client.conflict_get(trunk_path, self.client_ctx, pool)
+ result_pool = core.Pool()
+ scratch_pool = core.Pool()
+ conflict = client.conflict_get(trunk_path, self.client_ctx,
+ result_pool, scratch_pool)
self.assertTrue(isinstance(conflict, client.svn_client_conflict_t))
+ self.assertNotEqual(conflict._parent_pool, scratch_pool)
+ self.assertEqual(conflict._parent_pool, result_pool)
conflict_opts = client.conflict_tree_get_resolution_options(conflict, self.client_ctx)
@@ -602,7 +617,8 @@
self.assertTrue(isinstance(conflict_opts, list))
self.assert_all_instances_of(conflict_opts, client.svn_client_conflict_option_t)
- pool.clear()
+ scratch_pool.clear()
+ result_pool.clear()
@unittest.skip("experimental API, not currently exposed")
def test_shelf(self):
]]]
this test fails.
[[[
======================================================================
FAIL: test_conflict (client.SubversionClientTestCase)
Test conflict api.
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/futatuki/work/subversion/vwc/trunk/subversion/bindings/swig/python/tests/client.py", line 612, in test_conflict
self.assertNotEqual(conflict._parent_pool, scratch_pool)
AssertionError: <libsvn.core.apr_pool_t; proxy of <Swig Object of type 'apr_pool_t *' at 0x80acf5180> > == <libsvn.core.apr_pool_t; proxy of <Swig Object of type 'apr_pool_t *' at 0x80acf5180> >
----------------------------------------------------------------------
]]]
I think it can be fixed by overriding _global_py_pool to $input in "in"
typemap, when $1 is updated. It assumes that there are no APIs that
uses two or more result_pool like apr_pool_t * argument for allocationg
return value.
I tweaked Jun's patch to do it.
(Attached file: global-py-pool-ref-count2-diff.txt)
Tested on FreeBSD 11, with Python 2.7(debug), Python 3.7(non debug)
Cheers,
--
Yasuhito FUTATSUKI <fu...@yf.bsdclub.org>
Re: [PATCH] fix unable to load *.pyd files with Python 3.8.x on
Windows
Posted by Yasuhito FUTATSUKI <fu...@poem.co.jp>.
On 2020/05/20 21:54, Jun Omae wrote:
> On Mon, May 18, 2020 at 8:25 AM Yasuhito FUTATSUKI <fu...@poem.co.jp> wrote:
>>
>> On 2020/05/18 2:51, Johan Corveleyn wrote:
>>> On Sun, May 17, 2020 at 3:43 PM Jun Omae <ju...@gmail.com> wrote:
>>
>>>> Assertion for negative ref count is raised from test_conflict
>>>> (client.SubversionClientTestCase).
>>>> 1.10.x through trunk have the issue.
>>
>>> Thank you for the thorough research!
>>> I'm not sure what can be done further about this. Perhaps others can
>>> chime in here.
>>
>> Jun and I found it is caused by long existing bug on SWIG Python bindings,
>> since before swig-py3 branch was merged (thus this bug exists on 1.13.x
>> and 1.10.3 branches). On SWIG Python bindings, APIs which have multiple
>> apr_pool_t * argments cannot be wrapped correctly.
>>
>> If we call such a wrapper function with specifying multiple pool argments
>> explicitly, only last one is used for *all* apr_pool_t * arguments when
>> it calls C API.
>
> Created global-py-pool-ref-count.diff patch to solve the multiple apr_pool *
> pointers issue in Python bindings.
>
> Verified no assertions while running check-swig-py with the following
> environments:
>
> - (Python 3.5.2, 3.6.10, 3.7.7, 3.8.3, 3.9.0a6) x (SWIG 4.0.1)
> - (Python 2.7.12) x (SWIG 1.3.40, 2.0.12, 3.0.12)
Thank you very much!
With this patch, I confirmed that each apr_pool * argment in C API supplied
from corresponding pool object if it is supplied by the Python wrapper function
(by reading _wrap_svn_client_propget5() in svn_client.c generated by
SWIG 3.0.12, for Python 2).
However, I couldn't confirm that the "_parent_pool" attribute in wrapper
object returned by wrapper function point to "result_pool" object in such case,
yet. I doubt it still points to "scratch_pool" object because variable
"_global_py_pool" points to "scratch_pool" object when both of "result_pool"
and "scratch_pool" are provided on such APIs.
> Investigating generated C code for Perl and Ruby bindings from SWIG, it seems
> that all bindings have the same issue which only apr_pool_t * pointer passed
> as last argument is used.
svn_swig_pl_get_pool(), svn_swig_py_get_pool(), svn_swig_rb_get_pool(),
which is used in "default" typemaps for apr_pool_t *, only examine whether the
last argment passed by wrapper function is pool wrapper object or not, and "in"
typemaps for apr_pool_t * don't see $input in Perl and Ruby bindings.
Thus I think they have same issue, too.
Cheers,
--
Yasuhito FUTATSUKI <fu...@yf.bsdclub.org>
Re: [PATCH] fix unable to load *.pyd files with Python 3.8.x on Windows
Posted by Jun Omae <ju...@gmail.com>.
On Mon, May 18, 2020 at 8:25 AM Yasuhito FUTATSUKI <fu...@poem.co.jp> wrote:
>
> On 2020/05/18 2:51, Johan Corveleyn wrote:
> > On Sun, May 17, 2020 at 3:43 PM Jun Omae <ju...@gmail.com> wrote:
>
> >> Assertion for negative ref count is raised from test_conflict
> >> (client.SubversionClientTestCase).
> >> 1.10.x through trunk have the issue.
>
> > Thank you for the thorough research!
> > I'm not sure what can be done further about this. Perhaps others can
> > chime in here.
>
> Jun and I found it is caused by long existing bug on SWIG Python bindings,
> since before swig-py3 branch was merged (thus this bug exists on 1.13.x
> and 1.10.3 branches). On SWIG Python bindings, APIs which have multiple
> apr_pool_t * argments cannot be wrapped correctly.
>
> If we call such a wrapper function with specifying multiple pool argments
> explicitly, only last one is used for *all* apr_pool_t * arguments when
> it calls C API.
Created global-py-pool-ref-count.diff patch to solve the multiple apr_pool *
pointers issue in Python bindings.
Verified no assertions while running check-swig-py with the following
environments:
- (Python 3.5.2, 3.6.10, 3.7.7, 3.8.3, 3.9.0a6) x (SWIG 4.0.1)
- (Python 2.7.12) x (SWIG 1.3.40, 2.0.12, 3.0.12)
Investigating generated C code for Perl and Ruby bindings from SWIG, it seems
that all bindings have the same issue which only apr_pool_t * pointer passed
as last argument is used.
--
Jun Omae <ju...@gmail.com> (大前 潤)
Re: [PATCH] fix unable to load *.pyd files with Python 3.8.x on
Windows
Posted by Yasuhito FUTATSUKI <fu...@poem.co.jp>.
On 2020/05/18 2:51, Johan Corveleyn wrote:
> On Sun, May 17, 2020 at 3:43 PM Jun Omae <ju...@gmail.com> wrote:
>> Assertion for negative ref count is raised from test_conflict
>> (client.SubversionClientTestCase).
>> 1.10.x through trunk have the issue.
> Thank you for the thorough research!
> I'm not sure what can be done further about this. Perhaps others can
> chime in here.
Jun and I found it is caused by long existing bug on SWIG Python bindings,
since before swig-py3 branch was merged (thus this bug exists on 1.13.x
and 1.10.3 branches). On SWIG Python bindings, APIs which have multiple
apr_pool_t * argments cannot be wrapped correctly.
If we call such a wrapper function with specifying multiple pool argments
explicitly, only last one is used for *all* apr_pool_t * arguments when
it calls C API. And if we call it without specifying pools, wrapper
function create same number of new temporary pool wrapper Python objects
as pool arguments, however except last one are all disposed without
dereferencing, and last one is dereferenced instead. Thus, it causes
negative reference count assertion in debug mode, and causes memory leak
both in release and debug mode.
In SubversionClientTestCase.test_conflict, the client.conflict_get() call
is the former and the client.conflict_tree_get_resolution_options() is the
latter case.
Actually, the patch below can make test_conflict() pass in debug mode,
however test_inherited_props() still causes negative ref assertion
(on trunk, FreeBSD, Python 2.7, debug build). In case of 1.10.x
and 1.13.x in Jun's report, it was also caused in test_inherited_props().
[[[
Index: subversion/bindings/swig/python/tests/client.py
===================================================================
--- subversion/bindings/swig/python/tests/client.py (revision 1877742)
+++ subversion/bindings/swig/python/tests/client.py (working copy)
@@ -597,11 +597,13 @@
self.assertTrue(isinstance(conflict, client.svn_client_conflict_t))
- conflict_opts = client.conflict_tree_get_resolution_options(conflict, self.client_ctx)
+ conflict_opts = client.conflict_tree_get_resolution_options(conflict, self.client_ctx, pool)
self.assertTrue(isinstance(conflict_opts, list))
self.assert_all_instances_of(conflict_opts, client.svn_client_conflict_option_t)
+ del conflict
+ del conflict_opts
pool.clear()
@unittest.skip("experimental API, not currently exposed")
]]]
I'll try to fix, later.
Cheers,
--
Yasuhito FUTATSUKI <fu...@yf.bsdclub.org>
Re: [PATCH] fix unable to load *.pyd files with Python 3.8.x on Windows
Posted by Johan Corveleyn <jc...@gmail.com>.
On Sun, May 17, 2020 at 3:43 PM Jun Omae <ju...@gmail.com> wrote:
>
> On Sat, May 16, 2020 at 9:13 PM Johan Corveleyn <jc...@gmail.com> wrote:
> > If I apply your patch, I get this assertion with Python 3.8.2 and SWIG 3.0.12:
> >
> > [[[
> > C:\research\svn\dev\trunk>python win-tests.py --log-level=DEBUG
> > --debug --swig=python R:\test_py
> > Testing Debug configuration on local repository.
> > -- Running Swig Python tests --
> > Assertion failed: PyTuple_Check(args), file
> > d:\a\1\s\objects\typeobject.c, line 3707
> > [Test runner reported failure]
> > ]]]
> >
> > I don't see the other assertion that you got with SWIG 3.0.12. Perhaps
> > because I'm using Python 3.8.2? Or because I'm using the 32-bit
> > builds? I don't know.
>
> PyTuple_Check(args) asserts when Python 3.7.x - 3.8.x with SWIG 3.0.12.
> The issue is due to SWIG 3.0.x and has been fixed in SWIG 4.0.1.
> ref. https://github.com/swig/swig/issues/1321
>
> Assertion for negative ref count is raised from test_conflict
> (client.SubversionClientTestCase).
> 1.10.x through trunk have the issue.
>
> Summary tested with Python debug builds on Linux amd64:
>
> | Source | Python | SWIG | Result
> |--------|--------|--------|--------
> | trunk | 3.8.3 | 4.0.1 | _Py_NegativeRefcount: Assertion failed:
> object has negative ref count
> | trunk | 3.8.3 | 3.0.12 | excess_args: Assertion `PyTuple_Check(args)' failed
> | trunk | 3.7.7 | 4.0.1 | Fatal Python error:
> ../Objects/dictobject.c:1905 object at ... has negative ref count -...
> | trunk | 3.7.7 | 3.0.12 | excess_args: Assertion `PyTuple_Check(args)' failed
> | trunk | 3.6.10 | 4.0.1 | Fatal Python error:
> ../Objects/dictobject.c:2017 object at ... has negative ref count -...
> | trunk | 3.6.10 | 3.0.12 | Fatal Python error:
> ../Objects/dictobject.c:2017 object at ... has negative ref count -...
> | trunk | 3.5.2 | 4.0.1 | Fatal Python error:
> ../Objects/dictobject.c:354 object at ... has negative ref count -...
> | trunk | 3.5.2 | 3.0.12 | Fatal Python error:
> ../Objects/dictobject.c:354 object at ... has negative ref count -...
> | trunk | 2.7.12 | 3.0.12 | Fatal Python error:
> subversion/bindings/swig/python/svn_client.c:28085 object at ... has
> negative ref count -...
> | 1.13.x | 2.7.12 | 3.0.12 | Fatal Python error:
> subversion/bindings/swig/python/svn_client.c:28085 object at ... has
> negative ref count -...
> | 1.10.x | 2.7.12 | 3.0.12 | Fatal Python error:
> subversion/bindings/swig/python/svn_client.c:26916 object at ... has
> negative ref count -...
>
>
> trunk on Python 3.8.3 with SWIG 4.0.1
> ========
> .............................../Include/object.h:541:
> _Py_NegativeRefcount: Assertion failed: object has negative ref count
> <object at 0x7f9efefb1050 is freed>
> Fatal Python error: _PyObject_AssertFailed
> Python runtime state: initialized
>
>
> trunk on Python 3.8.3 with SWIG 3.0.12
> ========
> python3.8-dbg: ../Objects/typeobject.c:3707: excess_args: Assertion
> `PyTuple_Check(args)' failed.
>
>
> trunk on Python 3.7.7 with SWIG 4.0.1
> ========
> .............................Fatal Python error:
> ../Objects/dictobject.c:1905 object at 0x7f8f739ced10 has negative ref
> count -2459565876494606884
>
>
> trunk on Python 3.7.7 with SWIG 3.0.12
> ========
> python3.7-dbg: ../Objects/typeobject.c:3670: excess_args: Assertion
> `PyTuple_Check(args)' failed.
>
>
> trunk on Python 3.6.10 with SWIG 4.0.1
> ========
> .............................Fatal Python error:
> ../Objects/dictobject.c:2017 object at 0x7f9c30277878 has negative ref
> count -2604246222170760230
>
>
> trunk on Python 3.6.10 with SWIG 3.0.12
> ========
> .............................Fatal Python error:
> ../Objects/dictobject.c:2017 object at 0x7f83bf511400 has negative ref
> count -2604246222170760230
>
>
> trunk on Python 3.5.2 with SWIG 4.0.1
> ========
> .............................Fatal Python error:
> ../Objects/dictobject.c:354 object at 0x7fbd5b50d608 has negative ref
> count -2604246222170760230
>
>
> trunk on Python 3.5.2 with SWIG 3.0.12
> ========
> .............................Fatal Python error:
> ../Objects/dictobject.c:354 object at 0x7f77710cff60 has negative ref
> count -2604246222170760230
>
>
> trunk on Python 2.7.12 with SWIG 3.0.12
> ========
> ........................Fatal Python error:
> subversion/bindings/swig/python/svn_client.c:28085 object at
> 0x7f43a91de0d8 has negative ref count -2604246222170760230
>
>
> branches/1.13.x on Python 2.7.12 with SWIG 3.0.12
> ========
> ........................Fatal Python error:
> subversion/bindings/swig/python/svn_client.c:28085 object at
> 0x7fd02bba81c8 has negative ref count -2604246222170760230
>
>
> branches/1.10.x on Python 2.7.12 with SWIG 3.0.12
> ========
> ........................Fatal Python error:
> subversion/bindings/swig/python/svn_client.c:26916 object at
> 0x7fe73b53a150 has negative ref count -2604246222170760230
>
> --
> Jun Omae <ju...@gmail.com> (大前 潤)
Thank you for the thorough research!
I'm not sure what can be done further about this. Perhaps others can
chime in here.
--
Johan
Re: [PATCH] fix unable to load *.pyd files with Python 3.8.x on Windows
Posted by Jun Omae <ju...@gmail.com>.
On Sat, May 16, 2020 at 9:13 PM Johan Corveleyn <jc...@gmail.com> wrote:
> If I apply your patch, I get this assertion with Python 3.8.2 and SWIG 3.0.12:
>
> [[[
> C:\research\svn\dev\trunk>python win-tests.py --log-level=DEBUG
> --debug --swig=python R:\test_py
> Testing Debug configuration on local repository.
> -- Running Swig Python tests --
> Assertion failed: PyTuple_Check(args), file
> d:\a\1\s\objects\typeobject.c, line 3707
> [Test runner reported failure]
> ]]]
>
> I don't see the other assertion that you got with SWIG 3.0.12. Perhaps
> because I'm using Python 3.8.2? Or because I'm using the 32-bit
> builds? I don't know.
PyTuple_Check(args) asserts when Python 3.7.x - 3.8.x with SWIG 3.0.12.
The issue is due to SWIG 3.0.x and has been fixed in SWIG 4.0.1.
ref. https://github.com/swig/swig/issues/1321
Assertion for negative ref count is raised from test_conflict
(client.SubversionClientTestCase).
1.10.x through trunk have the issue.
Summary tested with Python debug builds on Linux amd64:
| Source | Python | SWIG | Result
|--------|--------|--------|--------
| trunk | 3.8.3 | 4.0.1 | _Py_NegativeRefcount: Assertion failed:
object has negative ref count
| trunk | 3.8.3 | 3.0.12 | excess_args: Assertion `PyTuple_Check(args)' failed
| trunk | 3.7.7 | 4.0.1 | Fatal Python error:
../Objects/dictobject.c:1905 object at ... has negative ref count -...
| trunk | 3.7.7 | 3.0.12 | excess_args: Assertion `PyTuple_Check(args)' failed
| trunk | 3.6.10 | 4.0.1 | Fatal Python error:
../Objects/dictobject.c:2017 object at ... has negative ref count -...
| trunk | 3.6.10 | 3.0.12 | Fatal Python error:
../Objects/dictobject.c:2017 object at ... has negative ref count -...
| trunk | 3.5.2 | 4.0.1 | Fatal Python error:
../Objects/dictobject.c:354 object at ... has negative ref count -...
| trunk | 3.5.2 | 3.0.12 | Fatal Python error:
../Objects/dictobject.c:354 object at ... has negative ref count -...
| trunk | 2.7.12 | 3.0.12 | Fatal Python error:
subversion/bindings/swig/python/svn_client.c:28085 object at ... has
negative ref count -...
| 1.13.x | 2.7.12 | 3.0.12 | Fatal Python error:
subversion/bindings/swig/python/svn_client.c:28085 object at ... has
negative ref count -...
| 1.10.x | 2.7.12 | 3.0.12 | Fatal Python error:
subversion/bindings/swig/python/svn_client.c:26916 object at ... has
negative ref count -...
trunk on Python 3.8.3 with SWIG 4.0.1
========
.............................../Include/object.h:541:
_Py_NegativeRefcount: Assertion failed: object has negative ref count
<object at 0x7f9efefb1050 is freed>
Fatal Python error: _PyObject_AssertFailed
Python runtime state: initialized
trunk on Python 3.8.3 with SWIG 3.0.12
========
python3.8-dbg: ../Objects/typeobject.c:3707: excess_args: Assertion
`PyTuple_Check(args)' failed.
trunk on Python 3.7.7 with SWIG 4.0.1
========
.............................Fatal Python error:
../Objects/dictobject.c:1905 object at 0x7f8f739ced10 has negative ref
count -2459565876494606884
trunk on Python 3.7.7 with SWIG 3.0.12
========
python3.7-dbg: ../Objects/typeobject.c:3670: excess_args: Assertion
`PyTuple_Check(args)' failed.
trunk on Python 3.6.10 with SWIG 4.0.1
========
.............................Fatal Python error:
../Objects/dictobject.c:2017 object at 0x7f9c30277878 has negative ref
count -2604246222170760230
trunk on Python 3.6.10 with SWIG 3.0.12
========
.............................Fatal Python error:
../Objects/dictobject.c:2017 object at 0x7f83bf511400 has negative ref
count -2604246222170760230
trunk on Python 3.5.2 with SWIG 4.0.1
========
.............................Fatal Python error:
../Objects/dictobject.c:354 object at 0x7fbd5b50d608 has negative ref
count -2604246222170760230
trunk on Python 3.5.2 with SWIG 3.0.12
========
.............................Fatal Python error:
../Objects/dictobject.c:354 object at 0x7f77710cff60 has negative ref
count -2604246222170760230
trunk on Python 2.7.12 with SWIG 3.0.12
========
........................Fatal Python error:
subversion/bindings/swig/python/svn_client.c:28085 object at
0x7f43a91de0d8 has negative ref count -2604246222170760230
branches/1.13.x on Python 2.7.12 with SWIG 3.0.12
========
........................Fatal Python error:
subversion/bindings/swig/python/svn_client.c:28085 object at
0x7fd02bba81c8 has negative ref count -2604246222170760230
branches/1.10.x on Python 2.7.12 with SWIG 3.0.12
========
........................Fatal Python error:
subversion/bindings/swig/python/svn_client.c:26916 object at
0x7fe73b53a150 has negative ref count -2604246222170760230
--
Jun Omae <ju...@gmail.com> (大前 潤)
Re: [PATCH] fix unable to load *.pyd files with Python 3.8.x on Windows
Posted by Johan Corveleyn <jc...@gmail.com>.
On Fri, May 15, 2020 at 11:27 AM Jun Omae <ju...@gmail.com> wrote:
>
> On Wed, May 13, 2020 at 5:56 PM Jun Omae <ju...@gmail.com> wrote:
> > At least, I think we should use python_d.exe when debug configuration.
> > ...
> > Python bindings with debug configuration for Python 3.x has something wrong.
> >
> > [[[
> > -- Running Swig Python tests --
> > Traceback (most recent call last):
> > File "C:\usr\src\subversion\trunk-py3\Debug\swig\pylib\libsvn\core.py", line 14, in swig_import_helper
> > return importlib.import_module(mname)
> > File "C:\usr\apps\python37\lib\importlib\__init__.py", line 127, in import_module
> > return _bootstrap._gcd_import(name[level:], package, level)
> > File "<frozen importlib._bootstrap>", line 1006, in _gcd_import
> > File "<frozen importlib._bootstrap>", line 983, in _find_and_load
> > File "<frozen importlib._bootstrap>", line 965, in _find_and_load_unlocked
> > ModuleNotFoundError: No module named 'libsvn._core'
> > ]]]
>
> According to Build and C API Changes in Python 3.5 [1], extension module should end with
> `{name}_d.pyd` rather than `{name}.pyd` in debug mode.
>
> After py35-windows-debug-pyd.diff, win-tests.py --debug --swig=python is able to run tests.
>
> On Python 3.7.7 with SWIG 3.0.12, the following assertion raises. The assertion has been
> reported and in SWIG #1321 [2] and fixed in SWIG 4.0.1.
>
> [[[
> Testing Debug configuration on local repository.
> -- Running Swig Python tests --
> .............................Fatal Python error: ..\Objects\dictobject.c:1905 object at 0000018C25A37C28 has negative ref count -2459565876494606884
> ]]]
>
>
> On Python 3.7.7 with SWIG 4.0.1, another assertion raises.
>
> [[[
> Testing Debug configuration on local repository.
> -- Running Swig Python tests --
> Assertion failed: PyTuple_Check(args), file ..\Objects\typeobject.c, line 3670
> [Test runner reported failure]
> ]]]
>
>
> [1] https://docs.python.org/3/whatsnew/3.5.html#build-and-c-api-changes
> [2] https://github.com/swig/swig/issues/1321
Thanks. Another step forward.
If I apply your patch, I get this assertion with Python 3.8.2 and SWIG 3.0.12:
[[[
C:\research\svn\dev\trunk>python win-tests.py --log-level=DEBUG
--debug --swig=python R:\test_py
Testing Debug configuration on local repository.
-- Running Swig Python tests --
Assertion failed: PyTuple_Check(args), file
d:\a\1\s\objects\typeobject.c, line 3707
[Test runner reported failure]
]]]
I don't see the other assertion that you got with SWIG 3.0.12. Perhaps
because I'm using Python 3.8.2? Or because I'm using the 32-bit
builds? I don't know.
--
Johan
Re: [PATCH] fix unable to load *.pyd files with Python 3.8.x on
Windows
Posted by Jun Omae <ju...@gmail.com>.
On Wed, May 13, 2020 at 5:56 PM Jun Omae <ju...@gmail.com> wrote:
> At least, I think we should use python_d.exe when debug configuration.
> ...
> Python bindings with debug configuration for Python 3.x has something wrong.
>
> [[[
> -- Running Swig Python tests --
> Traceback (most recent call last):
> File "C:\usr\src\subversion\trunk-py3\Debug\swig\pylib\libsvn\core.py", line 14, in swig_import_helper
> return importlib.import_module(mname)
> File "C:\usr\apps\python37\lib\importlib\__init__.py", line 127, in import_module
> return _bootstrap._gcd_import(name[level:], package, level)
> File "<frozen importlib._bootstrap>", line 1006, in _gcd_import
> File "<frozen importlib._bootstrap>", line 983, in _find_and_load
> File "<frozen importlib._bootstrap>", line 965, in _find_and_load_unlocked
> ModuleNotFoundError: No module named 'libsvn._core'
> ]]]
According to Build and C API Changes in Python 3.5 [1], extension module should end with
`{name}_d.pyd` rather than `{name}.pyd` in debug mode.
After py35-windows-debug-pyd.diff, win-tests.py --debug --swig=python is able to run tests.
On Python 3.7.7 with SWIG 3.0.12, the following assertion raises. The assertion has been
reported and in SWIG #1321 [2] and fixed in SWIG 4.0.1.
[[[
Testing Debug configuration on local repository.
-- Running Swig Python tests --
.............................Fatal Python error: ..\Objects\dictobject.c:1905 object at 0000018C25A37C28 has negative ref count -2459565876494606884
]]]
On Python 3.7.7 with SWIG 4.0.1, another assertion raises.
[[[
Testing Debug configuration on local repository.
-- Running Swig Python tests --
Assertion failed: PyTuple_Check(args), file ..\Objects\typeobject.c, line 3670
[Test runner reported failure]
]]]
[1] https://docs.python.org/3/whatsnew/3.5.html#build-and-c-api-changes
[2] https://github.com/swig/swig/issues/1321
--
Jun Omae <ju...@gmail.com> (大前 潤)
Re: [PATCH] fix unable to load *.pyd files with Python 3.8.x on
Windows
Posted by Jun Omae <ju...@gmail.com>.
On 2020/05/12 19:17, Johan Corveleyn wrote:
>> On Tue, May 12, 2020 at 4:33 AM Johan Corveleyn <jc...@gmail.com> wrote:
>>> [[[
>>> C:\research\svn\dev\trunk>python win-tests.py --log-level=DEBUG
>>> --debug --swig=python R:\test_swigpython
>>> Testing Debug configuration on local repository.
>>> -- Running Swig Python tests --
>>> Fatal Python error: _PyInterpreterState_Get(): no current thread state
>>> Python runtime state: unknown
>>>
>>> [Test runner reported failure]
>>> ]]]
At least, I think we should use python_d.exe when debug configuration.
[[[
Index: win-tests.py
===================================================================
--- win-tests.py (revision 1877480)
+++ win-tests.py (working copy)
@@ -1285,7 +1285,8 @@
if 'PYTHONPATH' in os.environ:
pythonpath += os.pathsep + os.environ['PYTHONPATH']
- python_exe = 'python.exe'
+ python_exe = sys.executable if objdir != 'Debug' else \
+ os.path.join(os.path.dirname(sys.executable), 'python_d.exe')
old_cwd = os.getcwd()
try:
os.environ['PYTHONPATH'] = pythonpath
]]]
However, importing libsvn._core fails on debug configuration even with/without
py38-windows-add-dll-directory--v2.diff (tested on Python 3.7.7).
Python bindings with debug configuration for Python 3.x has something wrong.
[[[
-- Running Swig Python tests --
Traceback (most recent call last):
File "C:\usr\src\subversion\trunk-py3\Debug\swig\pylib\libsvn\core.py", line 14, in swig_import_helper
return importlib.import_module(mname)
File "C:\usr\apps\python37\lib\importlib\__init__.py", line 127, in import_module
return _bootstrap._gcd_import(name[level:], package, level)
File "<frozen importlib._bootstrap>", line 1006, in _gcd_import
File "<frozen importlib._bootstrap>", line 983, in _find_and_load
File "<frozen importlib._bootstrap>", line 965, in _find_and_load_unlocked
ModuleNotFoundError: No module named 'libsvn._core'
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "C:\usr\src\subversion\trunk-py3\subversion\bindings\swig\python\tests\run_all.py", line 23, in <module>
import mergeinfo, core, client, delta, checksum, pool, fs, ra, wc, repository, \
File "C:\usr\src\subversion\trunk-py3\subversion\bindings\swig\python\tests\mergeinfo.py", line 22, in <module>
from svn import core, repos, fs
File "C:\usr\src\subversion\trunk-py3\subversion\bindings\swig\python\tests/..\svn\core.py", line 26, in <module>
from libsvn.core import *
File "C:\usr\src\subversion\trunk-py3\Debug\swig\pylib\libsvn\core.py", line 17, in <module>
_core = swig_import_helper()
File "C:\usr\src\subversion\trunk-py3\Debug\swig\pylib\libsvn\core.py", line 16, in swig_import_helper
return importlib.import_module('_core')
File "C:\usr\apps\python37\lib\importlib\__init__.py", line 127, in import_module
return _bootstrap._gcd_import(name[level:], package, level)
ModuleNotFoundError: No module named '_core'
[Test runner reported failure]
]]]
--
Jun Omae <ju...@gmail.com> (大前 潤)
Re: [PATCH] fix unable to load *.pyd files with Python 3.8.x on
Windows
Posted by Yasuhito FUTATSUKI <fu...@poem.co.jp>.
On 2020/05/14 3:46, Johan Corveleyn wrote:
> On Tue, May 12, 2020 at 12:17 PM Johan Corveleyn <jc...@gmail.com> wrote:
> ...
>> I'll try again later with Release mode. But just wanted to let you
>> know that the problem was not with building, but rather at runtime.
>
> Okay, tried again with Release mode, and then it works :-). Great!
Then problem with Debug is another issue, and perhaps we should update
the document about it, I think.
The patch itself looks good to me. Nothing to say in addtion about it.
+1 to commit, too.
Cheers,
--
Yasuhito FUTATSUKI <fu...@yf.bsdclub.org>
Re: [PATCH] fix unable to load *.pyd files with Python 3.8.x on Windows
Posted by Johan Corveleyn <jc...@gmail.com>.
On Tue, May 12, 2020 at 12:17 PM Johan Corveleyn <jc...@gmail.com> wrote:
...
> I'll try again later with Release mode. But just wanted to let you
> know that the problem was not with building, but rather at runtime.
Okay, tried again with Release mode, and then it works :-). Great!
--
Johan
Re: [PATCH] fix unable to load *.pyd files with Python 3.8.x on Windows
Posted by Johan Corveleyn <jc...@gmail.com>.
On Tue, May 12, 2020 at 7:47 AM Jun Omae <ju...@gmail.com> wrote:
>
> Hi,
>
> On Tue, May 12, 2020 at 4:33 AM Johan Corveleyn <jc...@gmail.com> wrote:
> > [[[
> > C:\research\svn\dev\trunk>python win-tests.py --log-level=DEBUG
> > --debug --swig=python R:\test_swigpython
> > Testing Debug configuration on local repository.
> > -- Running Swig Python tests --
> > Fatal Python error: _PyInterpreterState_Get(): no current thread state
> > Python runtime state: unknown
> >
> > [Test runner reported failure]
> > ]]]
> >
> > Any ideas?
> > (Note: it's entirely possible that I did something wrong here locally :-))
>
> Python bindings is unable to build with Debug mode. Instead, try to
> build and test with Release mode.
>
> The limitation is described in subversion/bindings/swig/INSTALL:
>
> [[[
> NOTE: Our Python SWIG bindings will currently NOT compile in Debug mode
> unless you have python24_d.lib (which binary distributions of
> Python do not contain). Therefore, the Python bindings will only
> compile in Release mode. (This is due to pyconfig.h using the
> _DEBUG flag too and setting a #pragma comment(lib) value.)
> ]]]
I'm not sure that's the problem, because I did build them (seemingly
successfully) in Debug mode.
Yes, first the build failed (with an error about missing
python38_d.lib). But then I reran the Python 3.8.2 installer (Windows
32-bit -- I have everything built with 32-bit for now), and on the
last screen of the installer I checked "Install debug symbols" and
"Install debug binaries (Requires at least Visual Studio 2015)". After
that, the file python38_d.lib was present in the Python installation:
[[[
C:\research\svn\dev\trunk>dir C:\Python38-32\libs
Volume in drive C has no label.
Volume Serial Number is 2636-2424
Directory of C:\Python38-32\libs
10/05/2020 16:26 <DIR> .
10/05/2020 16:26 <DIR> ..
25/02/2020 23:30 174.392 python3.lib
25/02/2020 23:30 364.012 python38.lib
25/02/2020 23:30 368.618 python38_d.lib
25/02/2020 23:30 176.006 python3_d.lib
25/02/2020 23:30 1.746 _tkinter.lib
]]]
And building the __SWIG_PYTHON__ target in Debug configuration (x86)
did succeed.
But then I got that new error when trying to run the swig-python tests.
I'll try again later with Release mode. But just wanted to let you
know that the problem was not with building, but rather at runtime.
--
Johan
Re: [PATCH] fix unable to load *.pyd files with Python 3.8.x on Windows
Posted by Jun Omae <ju...@gmail.com>.
Hi,
On Tue, May 12, 2020 at 4:33 AM Johan Corveleyn <jc...@gmail.com> wrote:
> [[[
> C:\research\svn\dev\trunk>python win-tests.py --log-level=DEBUG
> --debug --swig=python R:\test_swigpython
> Testing Debug configuration on local repository.
> -- Running Swig Python tests --
> Fatal Python error: _PyInterpreterState_Get(): no current thread state
> Python runtime state: unknown
>
> [Test runner reported failure]
> ]]]
>
> Any ideas?
> (Note: it's entirely possible that I did something wrong here locally :-))
Python bindings is unable to build with Debug mode. Instead, try to
build and test with Release mode.
The limitation is described in subversion/bindings/swig/INSTALL:
[[[
NOTE: Our Python SWIG bindings will currently NOT compile in Debug mode
unless you have python24_d.lib (which binary distributions of
Python do not contain). Therefore, the Python bindings will only
compile in Release mode. (This is due to pyconfig.h using the
_DEBUG flag too and setting a #pragma comment(lib) value.)
]]]
--
Jun Omae <ju...@gmail.com> (大前 潤)
Re: [PATCH] fix unable to load *.pyd files with Python 3.8.x on Windows
Posted by Johan Corveleyn <jc...@gmail.com>.
On Mon, May 11, 2020 at 7:08 PM Jun Omae <ju...@gmail.com> wrote:
> See attached patch py38-windows-add-dll-directory--v2.diff.
I think I also ran into that error when I was testing 1.14.0-rc2,
while testing the swig-python bindings on Windows. But I had already
had so many problems that I gave up, and just signed the release
without validating swig-python. So: thanks for taking a look at this
:-).
Before applying your patch I get (same as you, I guess):
[[[
Testing Debug configuration on local repository.
-- Running Swig Python tests --
Traceback (most recent call last):
File "R:\test_debug-p--swigpython\swig\pylib\libsvn\core.py", line
14, in swig_import_helper
return importlib.import_module(mname)
File "C:\Python38-32\lib\importlib\__init__.py", line 127, in import_module
return _bootstrap._gcd_import(name[level:], package, level)
File "<frozen importlib._bootstrap>", line 1014, in _gcd_import
File "<frozen importlib._bootstrap>", line 991, in _find_and_load
File "<frozen importlib._bootstrap>", line 975, in _find_and_load_unlocked
File "<frozen importlib._bootstrap>", line 657, in _load_unlocked
File "<frozen importlib._bootstrap>", line 556, in module_from_spec
File "<frozen importlib._bootstrap_external>", line 1101, in create_module
File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
ImportError: DLL load failed while importing _core: Kan opgegeven
module niet vinden.
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "C:\research\svn\dev\trunk\subversion\bindings\swig\python\tests\run_all.py",
line 23, in <module>
import mergeinfo, core, client, delta, checksum, pool, fs, ra, wc,
repository, \
File "C:\research\svn\dev\trunk\subversion\bindings\swig\python\tests\mergeinfo.py",
line 22, in <module>
from svn import core, repos, fs
File "C:\research\svn\dev\trunk\subversion\bindings\swig\python\tests/..\svn\core.py",
line 26, in <module>
from libsvn.core import *
File "R:\test_debug-p--swigpython\swig\pylib\libsvn\core.py", line
17, in <module>
_core = swig_import_helper()
File "R:\test_debug-p--swigpython\swig\pylib\libsvn\core.py", line
16, in swig_import_helper
return importlib.import_module('_core')
File "C:\Python38-32\lib\importlib\__init__.py", line 127, in import_module
return _bootstrap._gcd_import(name[level:], package, level)
ModuleNotFoundError: No module named '_core'
[Test runner reported failure]
]]]
However, after applying your latest patch, I still get an error:
[[[
C:\research\svn\dev\trunk>python win-tests.py --log-level=DEBUG
--debug --swig=python R:\test_swigpython
Testing Debug configuration on local repository.
-- Running Swig Python tests --
Fatal Python error: _PyInterpreterState_Get(): no current thread state
Python runtime state: unknown
[Test runner reported failure]
]]]
Any ideas?
(Note: it's entirely possible that I did something wrong here locally :-))
--
Johan
Re: [PATCH] fix unable to load *.pyd files with Python 3.8.x on
Windows
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Jun Omae wrote on Tue, 12 May 2020 02:08 +0900:
> See attached patch py38-windows-add-dll-directory--v2.diff.
Thanks. The changes looks good to me. I don't have any further
comments, either. However, I'm not familiar with SWIG enough to be
comfortable +1'ing this for commit, so I'll let the other developers
take over reviewing from here.
Thanks again,
Daniel
Re: [PATCH] fix unable to load *.pyd files with Python 3.8.x on
Windows
Posted by Jun Omae <ju...@gmail.com>.
Thanks for the reviewing.
On 2020/05/11 0:57, Daniel Shahaf wrote:
> Jun Omae wrote on Sun, 10 May 2020 13:27 +0900:
>> +def _dll_paths():
>> + import os
>> + if hasattr(os, 'add_dll_directory'): # Python 3.8+ on Windows
>> + cookies = []
>> + for path in os.environ.get('PATH', '').split(';'):
>
> Use os.pathsep in the split() call? That would make the code
> self-documenting and be forward compatible with… well, with anything
> that implements os.add_dll_directory() and uses another value of
> os.pathsep. I guess Cygwin's Python package might fit this bill.
Indeed. Revised the patch by your suggestion.
However python3.8 on cygwin doesn't have os.add_dll_directory.
$ python3.8
Python 3.8.2 (default, Apr 9 2020, 21:39:49)
[GCC 9.3.0] on cygwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import os
>>> os.add_dll_directory
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
AttributeError: module 'os' has no attribute 'add_dll_directory'
>> + if path and os.path.isabs(path):
>> + try:
>> + cookie = os.add_dll_directory(path)
>> + except:
>> + continue
>
> Should this really catch _all_ exceptions?
>
>> + else:
>> + cookies.append(cookie)
>> + return cookies
>> + else:
>> + return ()
Okay. Confirmed OSError is raised when os.add_dll_directory() fails (e.g. for non existent directory).
Revised the patch to catch only OSError.
See attached patch py38-windows-add-dll-directory--v2.diff.
--
Jun Omae <ju...@gmail.com> (大前 潤)
Re: [PATCH] fix unable to load *.pyd files with Python 3.8.x on
Windows
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Jun Omae wrote on Sun, 10 May 2020 13:27 +0900:
> +++ subversion/bindings/swig/include/svn_global.swg (working copy)
> @@ -242,3 +242,40 @@
> +#ifdef SWIGPYTHON
> +/* Since Python 3.8+ on Windows, DLL dependencies when loading *.pyd file
> + * searches only the system paths, the directory containing the *.pyd file and
> + * the directories added with os.add_dll_directory().
> + * See also https://bugs.python.org/issue36085.
> + */
> +%define SVN_PYTHON_MODULEIMPORT
> +"
> +def _dll_paths():
> + import os
> + if hasattr(os, 'add_dll_directory'): # Python 3.8+ on Windows
> + cookies = []
> + for path in os.environ.get('PATH', '').split(';'):
Use os.pathsep in the split() call? That would make the code
self-documenting and be forward compatible with… well, with anything
that implements os.add_dll_directory() and uses another value of
os.pathsep. I guess Cygwin's Python package might fit this bill.
> + if path and os.path.isabs(path):
> + try:
> + cookie = os.add_dll_directory(path)
> + except:
> + continue
Should this really catch _all_ exceptions?
> + else:
> + cookies.append(cookie)
> + return cookies
> + else:
> + return ()
Cheers,
Daniel
Re: [PATCH] fix unable to load *.pyd files with Python 3.8.x on Windows
Posted by Jun Omae <ju...@gmail.com>.
On Sun, May 10, 2020 at 1:27 PM Jun Omae <ju...@gmail.com> wrote:
> I created patch to resolve the issue using moduleimport option of %module directive.
> After attached patch, *.pyd file is successfully loaded and tests for Python bindings pass.
Tested with the following environments:
- (Python 3.8.2, 3.7.7) x (SWIG 4.0.1, 3.0.12) x (Windows, Linux)
- (Python 2.7.18) x (SWIG 3.0.12, 2.0.12, 1.3.40) x (Windows, Linux)
--
Jun Omae <ju...@gmail.com> (大前 潤)