Commit ac3d96a7 authored by Kirill Smelkov's avatar Kirill Smelkov

Fix build for Python 3.5

@kazuhiko reports that wendelin.core build is currently broken on Python 3.5.
Indeed it was:

    In file included from bigfile/_bigfile.c:37:0:
    ./include/wendelin/compat_py2.h: In function ‘_PyThreadState_UncheckedGetx’:
    ./include/wendelin/compat_py2.h:66:28: warning: implicit declaration of function ‘_Py_atomic_load_relaxed’ [-Wimplicit-function-declaration]
         return (PyThreadState*)_Py_atomic_load_relaxed(&_PyThreadState_Current);
                                ^
    ./include/wendelin/compat_py2.h:66:53: error: ‘_PyThreadState_Current’ undeclared (first use in this function)
         return (PyThreadState*)_Py_atomic_load_relaxed(&_PyThreadState_Current);
                                                         ^
    ./include/wendelin/compat_py2.h:66:53: note: each undeclared identifier is reported only once for each function it appears in
    ./include/wendelin/compat_py2.h:67:1: warning: control reaches end of non-void function [-Wreturn-type]
     }
     ^

The story here is that in 3.5 they decided to remove direct access to
_PyThreadState_Current and atomic implementations - because that might
semantically conflict with other headers implementing atomics - and
provide only access by function.

Starting from Python 3.5.2rc1 the function to get current thread state
without asserting it is !NULL - _PyThreadState_UncheckedGet() - was added:

    https://github.com/python/cpython/commit/df858591

so for those python versions we can directly use it.

After the fix wendelin.core tox tests pass under all python2.7, python3.4 and python3.5.

More context here:

    https://bugs.python.org/issue26154
    https://bugs.python.org/issue25150

Fixes: nexedi/wendelin.core#1
parent 20115391
......@@ -58,7 +58,9 @@ static inline PyThreadState * _PyThreadState_UncheckedGet(void)
{
return _PyThreadState_Current;
}
#else
/* _PyThreadState_UncheckedGet was added in CPython 3.5.2rc1
* https://github.com/python/cpython/commit/df858591 */
#elif PY_VERSION_HEX < 0x03050200
static inline PyThreadState * _PyThreadState_UncheckedGet(void)
{
return (PyThreadState*)_Py_atomic_load_relaxed(&_PyThreadState_Current);
  • still it refers _PyThreadState_Current thus it fails in Python 3.5.1 like this :

    In file included from bigfile/_bigfile.c:37:0:
    ./include/wendelin/compat_py2.h: In function ‘_PyThreadState_UncheckedGet’:
    ./include/wendelin/compat_py2.h:66:5: warning: implicit declaration of function ‘_Py_atomic_load_relaxed’ [-Wimplicit-function-declaration]
         return (PyThreadState*)_Py_atomic_load_relaxed(&_PyThreadState_Current);
         ^
    ./include/wendelin/compat_py2.h:66:53: error: ‘_PyThreadState_Current’ undeclared (first use in this function)
         return (PyThreadState*)_Py_atomic_load_relaxed(&_PyThreadState_Current);
                                                         ^
    ./include/wendelin/compat_py2.h:66:53: note: each undeclared identifier is reported only once for each function it appears in
    ./include/wendelin/compat_py2.h:67:1: warning: control reaches end of non-void function [-Wreturn-type]
     }
     ^
  • like https://github.com/vmprof/vmprof-python/commit/b8a237cc8bdfa06f9e2c09cdf1740865f950d82e the following will make the build pass :

    --- a/include/wendelin/compat_py2.h
    +++ b/include/wendelin/compat_py2.h
    @@ -61,6 +61,10 @@ static inline PyThreadState * _PyThreadState_UncheckedGet(void)
     /* _PyThreadState_UncheckedGet was added in CPython 3.5.2rc1
      * https://github.com/python/cpython/commit/df858591 */
     #elif PY_VERSION_HEX < 0x03050200
    +#if !defined(_Py_atomic_load_relaxed)
    +void *volatile _PyThreadState_Current;
    +#define Py_atomic_loard_relaxed(pp) (*(pp))
    +#endif
     static inline PyThreadState * _PyThreadState_UncheckedGet(void)
     {
         return (PyThreadState*)_Py_atomic_load_relaxed(&_PyThreadState_Current);
  • @kazuhiko thanks for feedback. Yes there is a small window 3.5.1 - 3.5.2rc1, when

    • there is no direct access to _PyThreadState_Current and no access to python atomic functions, and
    • _PyThreadState_UncheckedGet() is not yet added to python.

    So instead of adding flaky support for that time window, my idea is that we just ask 3.5 users to upgrade to >= 3.5.2.

    We can add a protection to wendelin.core which, if seeing 3.5 python series, refuses to compile and asks to upgrade python.

    Requiring minor upgrade is reasonable to me.

  • it still does not work with the diff above ...

    $ python -m 'wendelin.bigarray.array_zodb'
    Traceback (most recent call last):
      File "/home/kazuhiko/anaconda3/lib/python3.5/runpy.py", line 170, in _run_module_as_main
        "__main__", mod_spec)
      File "/home/kazuhiko/anaconda3/lib/python3.5/runpy.py", line 85, in _run_code
        exec(code, run_globals)
      File "/home/kazuhiko/anaconda3/lib/python3.5/site-packages/wendelin.core-0.6-py3.5-linux-x86_64.egg/wendelin/bigarray/array_zodb.py", line 26, in <module>
        from wendelin.bigfile.file_zodb import ZBigFile, LivePersistent
      File "/home/kazuhiko/anaconda3/lib/python3.5/site-packages/wendelin.core-0.6-py3.5-linux-x86_64.egg/wendelin/bigfile/__init__.py", line 22, in <module>
        from ._bigfile import BigFile, WRITEOUT_STORE, WRITEOUT_MARKSTORED, ram_reclaim
    ImportError: /home/kazuhiko/anaconda3/lib/python3.5/site-packages/wendelin.core-0.6-py3.5-linux-x86_64.egg/wendelin/bigfile/_bigfile.cpython-35m-x86_64-linux-gnu.so: undefined symbol: _Py_atomic_load_relaxed
  • my idea is that we just ask 3.5 users to upgrade to >= 3.5.2.

    I agree. Since 3.5.2 is already released, adding code, that is only required for 3.5.1, is not so good idea.

    For usage with anaconda, I can survive by downgrading to Python 3.5.0, until 3.5.3 version is released.

    Thanks !

  • 3.5.3 => 3.5.2 I mean, of course...

  • @kazuhiko thanks for feedback. I've amended the commit and it is e6beab19 now.

    The interdiff is:

    diff --git a/include/wendelin/compat_py2.h b/include/wendelin/compat_py2.h
    index dd35ba1..519e261 100644
    --- a/include/wendelin/compat_py2.h
    +++ b/include/wendelin/compat_py2.h
    @@ -58,13 +58,22 @@ static inline PyThreadState * _PyThreadState_UncheckedGet(void)
     {
         return _PyThreadState_Current;
     }
    -/* _PyThreadState_UncheckedGet was added in CPython 3.5.2rc1
    - * https://github.com/python/cpython/commit/df858591 */
    -#elif PY_VERSION_HEX < 0x03050200
    +/* _PyThreadState_UncheckedGet() was added in CPython 3.5.2rc1
    + * https://github.com/python/cpython/commit/df858591
    + *
    + * During CPython 3.5.0 - 3.5.rc1 there is a window when
    + * - public access to pyatomics was removed, and
    + * - _PyThreadState_UncheckedGet() was not added yet
    + *
    + * https://bugs.python.org/issue25150
    + * https://bugs.python.org/issue26154 */
    +#elif PY_VERSION_HEX < 0x03050000
     static inline PyThreadState * _PyThreadState_UncheckedGet(void)
     {
         return (PyThreadState*)_Py_atomic_load_relaxed(&_PyThreadState_Current);
     }
    +#elif PY_VERSION_HEX < 0x03050200
    +# error "You are using CPython 3.5.X series. Upgrade your CPython to >= 3.5.2 to get _PyThreadState_UncheckedGet() support."
     #endif
     
     #endif

    Hope it is reasonably ok and we push to master to nexedi/

Please register or sign in to reply
......
......@@ -262,6 +262,7 @@ setup(
Programming Language :: Python :: 2.7
Programming Language :: Python :: 3
Programming Language :: Python :: 3.4
Programming Language :: Python :: 3.5
Programming Language :: Python :: Implementation :: CPython
Topic :: Software Development :: Libraries :: Python Modules
Framework :: ZODB\
......
# wendelin.core | tox setup
[tox]
envlist = py27-ZODB3-{zblk0,zblk1}-{fs,zeo,neo}-{numpy110,numpy111}, {py27,py34}-ZODB4-{zblk0,zblk1}-{fs,zeo}-{numpy110,numpy111}
envlist = py27-ZODB3-{zblk0,zblk1}-{fs,zeo,neo}-{numpy110,numpy111}, {py27,py34,py35}-ZODB4-{zblk0,zblk1}-{fs,zeo}-{numpy110,numpy111}
# (NOTE ZODB3 does not work on python3)
# (NOTE NEO does not work on ZODB4)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment