Bug 253604 - lang/python39: use libmpdec from ports
Summary: lang/python39: use libmpdec from ports
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Many People
Assignee: Po-Chuan Hsieh
URL:
Keywords:
: 253492 (view as bug list)
Depends on:
Blocks:
 
Reported: 2021-02-17 22:10 UTC by Stefan Krah
Modified: 2021-05-11 21:57 UTC (History)
3 users (show)

See Also:
bugzilla: maintainer-feedback? (python)


Attachments
build --with-system-libmpdec (3.76 KB, patch)
2021-02-17 22:10 UTC, Stefan Krah
no flags Details | Diff
my build log (73.07 KB, application/gzip)
2021-02-18 19:45 UTC, Steve Wills
no flags Details
updated patch (3.84 KB, patch)
2021-02-18 21:16 UTC, Steve Wills
no flags Details | Diff
updated patch 2 (3.26 KB, patch)
2021-02-19 02:03 UTC, Steve Wills
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Stefan Krah 2021-02-17 22:10:09 UTC
Created attachment 222541 [details]
build --with-system-libmpdec

Finally, the same as https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=253601  for Python-3.9.
Comment 1 Steve Wills freebsd_committer freebsd_triage 2021-02-18 14:32:40 UTC
Why the change to Modules/_decimal/_decimal.c? Also, we have to account for the package changing (PORTREVISION change, pkg-plist change), see my patch in 253492 (already linked).
Comment 2 Stefan Krah 2021-02-18 16:37:42 UTC
I didn't notice that you had already opened an issue, sorry.

The _decimal.c changes are generally required because the mpdecimal.h header has been cleaned up. There were some leftovers like the typedef of "uchar", which collided with some system header on Solaris and broke the build.


It does not build here without the patch:

In file included from ./Include/unicodeobject.h:58,
                 from ./Include/Python.h:97,
                 from /usr/home/stefan/Python-3.9.1/Modules/_decimal/_decimal.c:29:
/usr/home/stefan/Python-3.9.1/Modules/_decimal/_decimal.c: In function 'dec_format':
/usr/home/stefan/Python-3.9.1/Modules/_decimal/_decimal.c:3283:43: error: 'uchar' undeclared (first use in this function); did you mean 'u_char'?
 3283 |         if (n > 1 || (n == 1 && !isascii((uchar)spec.dot[0]))) {
      |                                           ^~~~~



Would you like to close this and continue with your original issue or keep this one open?
Comment 3 Steve Wills freebsd_committer freebsd_triage 2021-02-18 17:51:36 UTC
(In reply to Stefan Krah from comment #2)

Duplicate bug is no problem, they're easy to change.

Interestingly, I don't see that build failure without the extra patch. I'm not sure why.
Comment 4 Stefan Krah 2021-02-18 18:41:56 UTC
(In reply to Steve Wills from comment #3)

Yes, interesting indeed.  I'm on FreeBSD-12.2 (amd64), fully updated.

Circumventing the ports machinery:

tar xvf Python-3.9.1.tar.xz
cd Python-3.9.1
./configure CC=clang --with-system-libmpdec
[...]
clang -pthread -fPIC -Wno-unused-result -Wsign-compare -Wunreachable-code -DNDEBUG -g -fwrapv -O3 -Wall -std=c99 -Wextra -Wno-unused-result -Wno-unused-parameter -Wno-missing-field-initializers -Wstrict-prototypes -Werror=implicit-function-declaration -fvisibility=hidden -I./Include/internal -DCONFIG_64=1 -DASM=1 -I./Include -I. -I/usr/local/include -I/usr/home/stefan/Python-3.9.1/Include -I/usr/home/stefan/Python-3.9.1 -c /usr/home/stefan/Python-3.9.1/Modules/_decimal/_decimal.c -o build/temp.freebsd-12.2-RELEASE-p3-amd64-3.9/usr/home/stefan/Python-3.9.1/Modules/_decimal/_decimal.o
/usr/home/stefan/Python-3.9.1/Modules/_decimal/_decimal.c:3283:43: error: use of undeclared identifier 'uchar'
        if (n > 1 || (n == 1 && !isascii((uchar)spec.dot[0]))) {
                                          ^
/usr/home/stefan/Python-3.9.1/Modules/_decimal/_decimal.c:3292:43: error: use of undeclared identifier 'uchar'
        if (n > 1 || (n == 1 && !isascii((uchar)spec.sep[0]))) {
                                          ^
2 errors generated.


And I can't find any system header that typedefs uchar:

[stefan@freebsd-amd64 ~]$ find /usr/include -type f | xargs grep -n "typedef.*uchar"
/usr/include/c++/v1/atomic:493:typedef atomic<unsigned char>      atomic_uchar;
/usr/include/c++/v1/atomic:2390:typedef atomic<unsigned char>      atomic_uchar;
/usr/include/sys/stdatomic.h:189:typedef _Atomic(unsigned char)         atomic_uchar;


[stefan@freebsd-amd64 ~]$ find /usr/local/include -type f | xargs grep -n "typedef.*uchar"
/usr/local/include/boost/atomic/detail/atomic_template.hpp:1178:typedef atomic< unsigned char > atomic_uchar;
/usr/local/include/boost/compute/algorithm/detail/radix_sort.hpp:57:    typedef uchar_ type;
/usr/local/include/boost/compute/types/fundamental.hpp:29:typedef cl_uchar uchar_;
/usr/local/include/boost/locale/generic_codecvt.hpp:155:    typedef CharType uchar;
/usr/local/include/boost/locale/generic_codecvt.hpp:457:    typedef CharType uchar;
/usr/local/include/boost/locale/generic_codecvt.hpp:659:    typedef CharType uchar;
/usr/local/include/boost/numeric/ublas/detail/returntype_deduction.hpp:33:    typedef char(&uchar_value_type)[7];
/usr/local/include/boost/regex/v4/char_regex_traits.hpp:48:   typedef unsigned char uchar_type;
/usr/local/include/boost/regex/v4/char_regex_traits.hpp:60:   typedef unsigned short uchar_type;
/usr/local/include/boost/regex/v4/cpp_regex_traits.hpp:636:      typedef typename make_unsigned<charT>::type uchar_type;
/usr/local/include/boost/wave/cpplexer/re2clex/scanner.hpp:31:typedef unsigned char uchar;
/usr/local/include/boost/xpressive/match_results.hpp:1178:        typedef typename boost::uint_t<CHAR_BIT * sizeof(char_type)>::least uchar_t;
/usr/local/include/boost/xpressive/match_results.hpp:1180:        typedef numeric::conversion_traits<uchar_t, int> converstion_traits;
/usr/local/include/boost/xpressive/detail/dynamic/parse_charset.hpp:81:    typedef typename boost::uint_t<CHAR_BIT * sizeof(char_type)>::least uchar_t;
/usr/local/include/boost/xpressive/detail/dynamic/parse_charset.hpp:83:    typedef numeric::conversion_traits<uchar_t, int> converstion_traits;
Comment 5 Steve Wills freebsd_committer freebsd_triage 2021-02-18 19:45:52 UTC
Created attachment 222560 [details]
my build log

Here's my build log, maybe that will help? (This is with system mpdec but without the other patch)
Comment 6 Steve Wills freebsd_committer freebsd_triage 2021-02-18 19:47:40 UTC
(In reply to Steve Wills from comment #5)
Oh, I see it, sorry:

[00:00:22] Modules/_decimal/_decimal.c:3283:43: error: use of undeclared identifier 'uchar'
[00:00:22]         if (n > 1 || (n == 1 && !isascii((uchar)spec.dot[0]))) {
[00:00:22]                                           ^
[00:00:22] Modules/_decimal/_decimal.c:3292:43: error: use of undeclared identifier 'uchar'
[00:00:22]         if (n > 1 || (n == 1 && !isascii((uchar)spec.sep[0]))) {
[00:00:22]                                           ^
[00:00:22] 2 errors generated.
[00:00:22] 
[00:00:22] Python build finished successfully!

Ok, that explains it...
Comment 7 Steve Wills freebsd_committer freebsd_triage 2021-02-18 20:04:14 UTC
*** Bug 253492 has been marked as a duplicate of this bug. ***
Comment 8 Stefan Krah 2021-02-18 20:11:23 UTC
(In reply to Steve Wills from comment #6)

The failure is quite well hidden, I don't think that there's an option to
get a non-zero exit status for the extension builds.  Worse, test_decimal
will still pass because the pure Python version is also there. :)
Comment 9 Steve Wills freebsd_committer freebsd_triage 2021-02-18 21:16:44 UTC
Created attachment 222562 [details]
updated patch

Here's an updated version of the patch, that includes source patches, PORTREVISION and pkg-plist changes.
Comment 10 Stefan Krah 2021-02-18 23:04:46 UTC
(In reply to Steve Wills from comment #9)

I think with ...

   %%NO_LIBMPDEC%%lib/python%%XYDOT%%/lib-dynload/_decimal.so

... _decimal.so is not installed (I checked here).



Shouldn't it be the same as for LIBFFI, where you either have the
system libffi, the vendored libffi (or a build failure)?


_ctypes.so, which relies on libffi, is also installed unconditionally:

   lib/python%%XYDOT%%/lib-dynload/_ctypes.so
Comment 11 Steve Wills freebsd_committer freebsd_triage 2021-02-19 02:03:40 UTC
Created attachment 222583 [details]
updated patch 2

(In reply to Stefan Krah from comment #10)
Yeah, something made me think it wasn't built or installed when the option was disabled, but I must have been confused. Here's an updated patch.
Comment 12 commit-hook freebsd_committer freebsd_triage 2021-05-11 21:54:58 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=9738c1c2eadc9a8bd599397bcfbf84c37844a554

commit 9738c1c2eadc9a8bd599397bcfbf84c37844a554
Author:     Stefan Krah <skrah@bytereef.org>
AuthorDate: 2021-05-11 21:44:39 +0000
Commit:     Po-Chuan Hsieh <sunpoet@FreeBSD.org>
CommitDate: 2021-05-11 21:47:33 +0000

    lang/python39: Use libmpdec from ports

    - Bump PORTREVISION for dependency and package change

    PR:             253604

 lang/python39/Makefile                               |  9 +++++++--
 .../files/patch-Modules___decimal___decimal.c (new)  | 20 ++++++++++++++++++++
 lang/python39/files/patch-setup.py (new)             | 11 +++++++++++
 3 files changed, 38 insertions(+), 2 deletions(-)
Comment 13 Po-Chuan Hsieh freebsd_committer freebsd_triage 2021-05-11 21:57:21 UTC
Committed. Thanks!