conda-forge / python-feedstock

A conda-smithy repository for python.
BSD 3-Clause "New" or "Revised" License
46 stars 105 forks source link

Update to Python 3.12.1 #656

Closed ismail closed 11 months ago

ismail commented 11 months ago

Refresh 0005-Unvendor-openssl.patch to apply

Checklist

conda-forge-webservices[bot] commented 11 months ago

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

ismail commented 11 months ago

@conda-forge-admin, please rerender

ismail commented 11 months ago

@conda-forge-admin, please rerender

ismail commented 11 months ago

macOS x86-64 builds fail with an ncurses related error

[...]/lib-dynload/_curses.cpython-312-darwin.so, 2): Symbol not found: _extended_color_content

Python has this check to see if it can use _extended_color_content (uninteresting parts omitted):

#if NCURSES_EXT_FUNCS+0 >= 20170401 && NCURSES_EXT_COLORS+0 >= 20170401
#define _NCURSES_EXTENDED_COLOR_FUNCS   1
#else
#define _NCURSES_EXTENDED_COLOR_FUNCS   0
#endif

#if _NCURSES_EXTENDED_COLOR_FUNCS
#define _CURSES_INIT_COLOR_FUNC         init_extended_color
#else
#define _CURSES_INIT_COLOR_FUNC         init_color
#endif  /* _NCURSES_EXTENDED_COLOR_FUNCS */

If I check ncurses package on macOS arm64:

❯ nm -g ./.pixi/env/lib/libncursesw.6.dylib | grep init_extended_color
0000000000009990 T _init_extended_color
0000000000009740 T _init_extended_color_sp

I downloaded the osx-64/ncurses-6.4-h93d8f39_1.conda to double check and sure enough symbol exists there too

❯ file ./lib/libncursesw.6.dylib
./lib/libncursesw.6.dylib: Mach-O 64-bit x86_64 dynamically linked shared library, flags:<NOUNDEFS|DYLDLINK|TWOLEVEL>

❯ nm -g ./lib/libncursesw.6.dylib | grep init_extended_color
0000000000008160 T _init_extended_color
0000000000007eb0 T _init_extended_color_sp

Interestingly, _curses.cpython-312-darwin.so is linking to libncurses.6.dylib

❯ otool -L ./osx-arm64/lib/python3.12/lib-dynload/_curses.cpython-312-darwin.so
./osx-arm64/lib/python3.12/lib-dynload/_curses.cpython-312-darwin.so:
    @rpath/libncurses.6.dylib (compatibility version 6.0.0, current version 6.0.0)
    /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1292.0.0)

which also should make arm64 test fail, but that's not happening.

ismail commented 11 months ago

Apparently until https://github.com/python/cpython/commit/d34650e7409b6a7cc64d7a118e4500c2f3cdbcf3 Python3 was not correctly detecting ncursesw on macOS, so this explains why this was not seen before.

ismail commented 11 months ago

Someone else, please feel free to pick this up.

I think building non-wide curses on macOS doesn't make sense, but it'd be too big of a hammer. We just need to make sure Python links to ncursesw on macOS.

mbargull commented 11 months ago

@conda-forge-admin, please rerender

mbargull commented 11 months ago

@conda-forge-admin, please rerender

mbargull commented 11 months ago

@ismail, thanks for looking into this! I've added a patch so that we link to ncursesw again and added a test so don't regress on this again.

Apparently until python/cpython@d34650e Python3 was not correctly detecting ncursesw on macOS, so this explains why this was not seen before.

Yep, conda-forge::python=3.12.0 definitely has wide character support broken on macOS; confirmed with https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=844414&view=logs&j=f23f984a-2b8a-59f7-ed33-e22fa8dd9908&t=1d351711-1535-5fad-3bab-ff535f39fb55&l=11742 . The non-3.12 builds should be fine, though, since it broke with https://github.com/python/cpython/commit/e925241d95d8095adf67f492042f97254ff82ec1 which is in 3.12.0a1 onward.

which also should make arm64 test fail, but that's not happening.

Our macOS CI workers are currently x86-only such that we have to cross-compile the osx-arm64 builds with no automated testing happening for them for now. Meaning, the tests didn't fail because they weren't run.

mbargull commented 11 months ago

@conda-forge-admin, please restart ci

mbargull commented 11 months ago

@conda-forge-admin, please rerender

mbargull commented 11 months ago

My suggestion would be to merge this and revisit further possible actions from https://github.com/conda-forge/python-feedstock/pull/656#discussion_r1428491692 later.

@conda-forge/python, anyone else to review?