This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Move _decimal build setup into configure
Type: enhancement Stage: resolved
Components: Build Versions: Python 3.11
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: christian.heimes Nosy List: christian.heimes, mark.dickinson, ned.deily, pitrou
Priority: Keywords: patch

Created on 2021-11-13 09:15 by christian.heimes, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 29541 merged christian.heimes, 2021-11-13 09:20
PR 29949 merged ned.deily, 2021-12-07 02:07
Messages (8)
msg406271 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2021-11-13 09:15
Compiler and linker flags for _decimal and internal libmpdec are currently handled by a mix of configure checks and if/else chains in setup.py. The split makes it harder to build _decimal correctly from Modules/Setup. The Modules/Setup file also does not handle --with-system-mpdec.

I have a working PR that moves all logic into configure.ac. The new system:

* sets LIBMPDEC_CFLAGS and LIBMPDEC_LDFLAGS based on --with-system-libmpdec value.

* detects libmpdec_machine by looking at ac_sys_system, MACOSX_DEFAULT_ARCH, ac_cv_sizeof_size_t, ac_cv_gcc_asm_for_x64, ac_cv_type___uint128_t, and ac_cv_gcc_asm_for_x87.

* sets libmpdec compiler args based on libmpdec_machine, have_ipa_pure_const_bug, and have_glibc_memmove_bug.

* if --with-system-libmpdec is not given, then our Makefile compiles libmpdec objects and puts them into a libmpdec.a archive.

* finally it either links _decimal with our libmpdec.a or with system's libmpdec shared library.

I went for libmpdec.a because it makes the logic for the internal path look similar to the logic with linking with an external shared library.

Modules/Setup
msg406272 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2021-11-13 09:31
PS: I had to add an explicit make rule for each object file. "%.o: %c" templates are not portable.
msg406275 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2021-11-13 12:56
New changeset 0486570f7b2b5a75812e5a01a8dca58bfadc2437 by Christian Heimes in branch 'main':
bpo-45798: Move _decimal build setup into configure (GH-29541)
https://github.com/python/cpython/commit/0486570f7b2b5a75812e5a01a8dca58bfadc2437
msg406276 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2021-11-13 12:56
Thanks for the quick review, Mark!
msg406278 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2021-11-13 13:09
I tested the --with-system-libmpdec successfully on my system. Most vendors are using the internal copy of libmpdec any way. AFAIK only Debian-based systems use their own system libmpdec.

$ ./configure -C --with-system-libmpdec
$ make
...
building '_decimal' extension
gcc -pthread -fPIC -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -std=c99 -Wextra -Wno-unused-parameter -Wno-missing-field-initializers -Werror=implicit-function-declaration -fvisibility=hidden -I./Include/internal -I./Include -I. -I/usr/local/include -I/home/heimes/dev/python/cpython/Include -I/home/heimes/dev/python/cpython -c /home/heimes/dev/python/cpython/Modules/_decimal/_decimal.c -o build/temp.linux-x86_64-3.11/home/heimes/dev/python/cpython/Modules/_decimal/_decimal.o -DCONFIG_64=1 -DASM=1
gcc -pthread -shared build/temp.linux-x86_64-3.11/home/heimes/dev/python/cpython/Modules/_decimal/_decimal.o -L/usr/local/lib -o build/lib.linux-x86_64-3.11/_decimal.cpython-311-x86_64-linux-gnu.so -lmpdec
...
$ ldd build/lib.linux-x86_64-3.11/_decimal.cpython-311-x86_64-linux-gnu.so 
        linux-vdso.so.1 (0x00007ffde21e1000)
        libmpdec.so.3 => /lib64/libmpdec.so.3 (0x00007f4f3b4cf000)
        libpthread.so.0 => /lib64/libpthread.so.0 (0x00007f4f3b4ae000)
        libc.so.6 => /lib64/libc.so.6 (0x00007f4f3b2df000)
        libm.so.6 => /lib64/libm.so.6 (0x00007f4f3b19b000)
        /lib64/ld-linux-x86-64.so.2 (0x00007f4f3b554000)
msg407874 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2021-12-06 22:46
It looks like this change broke macOS universal2 builds of libmpdec. I'm looking at it now but wanted to flag it as a "release blocker" for tagging 3.11.0a3.
msg407890 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2021-12-07 02:35
New changeset ddbab69b6d44085564a9b5022b96b002a52b2f2b by Ned Deily in branch 'main':
bpo-45798: Let libmpdec decide which archs to build on macOS as done previously. (GH-29949)
https://github.com/python/cpython/commit/ddbab69b6d44085564a9b5022b96b002a52b2f2b
msg407891 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2021-12-07 02:43
The issue here turned out to be that the architecture selection code for libmpdec builds on macOS had been scrambled a bit on the move from setup.py to configure. To support universal builds correctly, libmpdec has code to decide itself which arch(s) to build with on macOS. PR 29949 restores that behavior and macOS universal2 builds (builds which have both native arm64 and x86_64 binaries in each executable or library file) now work again for 3.11.
History
Date User Action Args
2022-04-11 14:59:52adminsetgithub: 89956
2021-12-07 02:43:44ned.deilysetstatus: open -> closed
priority: release blocker ->
messages: + msg407891

resolution: fixed
stage: patch review -> resolved
2021-12-07 02:35:54ned.deilysetmessages: + msg407890
2021-12-07 02:07:31ned.deilysetstage: patch review
pull_requests: + pull_request28174
2021-12-06 22:46:24ned.deilysetstatus: closed -> open
priority: normal -> release blocker


nosy: + ned.deily
messages: + msg407874
resolution: fixed -> (no value)
stage: resolved -> (no value)
2021-11-13 13:27:04christian.heimessetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2021-11-13 13:09:39christian.heimessetmessages: + msg406278
2021-11-13 12:56:41christian.heimessetmessages: + msg406276
2021-11-13 12:56:26christian.heimessetmessages: + msg406275
2021-11-13 09:31:58christian.heimessetmessages: + msg406272
2021-11-13 09:20:01christian.heimessetkeywords: + patch
pull_requests: + pull_request27793
2021-11-13 09:15:19christian.heimescreate