Skip to content

gh-125206: Correct detection of complex numbers support in libffi #126104

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Oct 30, 2024

Conversation

skirpichev
Copy link
Member

@skirpichev skirpichev commented Oct 29, 2024

@skirpichev skirpichev marked this pull request as draft October 29, 2024 05:25
@skirpichev
Copy link
Member Author

!buildbot SPARCv9 Oracle Solaris 11.4

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @skirpichev for commit 66c1cf7 🤖

The command will test the builders whose names match following regular expression: SPARCv9 Oracle Solaris 11.4

The builders matched are:

  • SPARCv9 Oracle Solaris 11.4 PR

@skirpichev

This comment was marked as outdated.

@bedevere-bot

This comment was marked as outdated.

@python python deleted a comment from bedevere-bot Oct 29, 2024
@skirpichev skirpichev marked this pull request as ready for review October 29, 2024 06:35
@skirpichev
Copy link
Member Author

!buildbot PPC64LE CentOS9 LTO + PGO

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @skirpichev for commit 66c1cf7 🤖

The command will test the builders whose names match following regular expression: PPC64LE CentOS9 LTO \+ PGO

The builders matched are:

  • PPC64LE CentOS9 LTO + PGO PR

@skirpichev
Copy link
Member Author

PPC64LE build works as expected (no support)

@skirpichev
Copy link
Member Author

!buildbot SPARCv9 Oracle Solaris 11.4

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @skirpichev for commit fc4ebff 🤖

The command will test the builders whose names match following regular expression: SPARCv9 Oracle Solaris 11.4

The builders matched are:

  • SPARCv9 Oracle Solaris 11.4 PR

@skirpichev
Copy link
Member Author

Hmm, now on SPARCv9 - libffi support was detected. Test failures seems unrelated and build was successful. (C.f. https://buildbot.python.org/all/#/builders/1262/builds/13 and https://buildbot.python.org/#/builders/1262/builds/21)

@skirpichev skirpichev requested a review from vstinner October 29, 2024 13:22
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@efimov-mikhail: Can you test the fix on Debian 10?

@skirpichev
Copy link
Member Author

I don't expect something new, as on his local system patch was working: #125322 (comment) (test code was unchanged)

@efimov-mikhail
Copy link
Contributor

@efimov-mikhail: Can you test the fix on Debian 10?

Yes, I can. It looks like the correct fix, but it's better to test again.

@efimov-mikhail
Copy link
Contributor

efimov-mikhail commented Oct 29, 2024

This fix works fine on Debian 10.

Copy link
Contributor

@erlend-aasland erlend-aasland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (if the Sparc failure is unrelated).

@vstinner vstinner merged commit dcad8fe into python:main Oct 30, 2024
47 of 48 checks passed
@vstinner
Copy link
Member

Merged, thanks.

LGTM (if the Sparc failure is unrelated).

There are tons of unrelated errors/test failures on Solaris.

@skirpichev skirpichev deleted the amend-libffi-complex-fix branch October 30, 2024 09:13
@skirpichev
Copy link
Member Author

On SPARCv9 only configure test was checked. It reports "yes" (supported). But it seems that ctypes tests weren't run. Sadly that configure script doesn't report library versions (even if this is available e.g. with pkg-config --modversion <lib>.

@erlend-aasland
Copy link
Contributor

On SPARCv9 only configure test was checked. It reports "yes" (supported). But it seems that ctypes tests weren't run. Sadly that configure script doesn't report library versions (even if this is available e.g. with pkg-config --modversion <lib>.

We could fix that if we wanted.

@skirpichev
Copy link
Member Author

We could fix that if we wanted.

If that does make sense, I can open an issue. But I admit, that I don't know how to solve this in general: maybe every snowflake^Wlibrary will require a special treatment.

BTW, version information is unavailable in the config.log (even for versioned checks like for libmpdec).

@erlend-aasland
Copy link
Contributor

IIRC (from the top of my head), the pkg-config macro will print version information if you specify version bounds (see the sqlite3 third-party detection code in configure.ac). My gut feeling for implementing such a thing would be to create a wrapper for PKG_CHECK_MODULES and ensure that we print version info in the action-if-found block. I'm not sure if it is worth it, but it would be convenient for debugging third-party dependency detection.

@skirpichev
Copy link
Member Author

the pkg-config macro will print version information if you specify version bounds (see the sqlite3 third-party detection code in configure.ac).

Quick test for mpdec (which uses same PKG_CHECK_MODULES macro for system libmpdec, just as for sqlite3) shows, that it's not true:

$ ./configure 2>&1|grep mpdec
checking for --with-system-libmpdec... yes
checking for libmpdec >= 2.5.0... yes

picnixz pushed a commit to picnixz/cpython that referenced this pull request Dec 8, 2024
ebonnal pushed a commit to ebonnal/cpython that referenced this pull request Jan 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.14 bugs and security fixes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants