Skip to content

gh-127604: Optimize -ldl usage on platforms that use dlopen for libFFI. #133081

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

freakboy3742
Copy link
Contributor

@freakboy3742 freakboy3742 commented Apr 28, 2025

#133040 modified the handling of -ldl, resulting in multiple copies of -ldl being included in link commands.

#133071 modified this handling to minimise a lot of those usages; but on platforms that use dlopen() (macOS and iOS), there was still some duplicated use. This PR cleans up that usage.

@freakboy3742
Copy link
Contributor Author

!buildbot iOS

@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @freakboy3742 for commit 297f242 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F133081%2Fmerge

The command will test the builders whose names match following regular expression: iOS

The builders matched are:

  • iOS ARM64 Simulator PR

@freakboy3742
Copy link
Contributor Author

freakboy3742 commented Apr 28, 2025

Nope... looks like that hasn't done it. I'll take another look in the morning if nobody beats me to it.

picnixz
picnixz previously approved these changes Apr 28, 2025
Copy link
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

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

It looks fine and I'm pretty sure there is a cleaner and alternative way to do it but I'm also pretty sure that it would require more steps and a more complicate configuration so I think we should live with this first.

More generally, I think we need to have a function that cleans up the flags to remove duplicate ones.

@picnixz
Copy link
Member

picnixz commented Apr 28, 2025

Nope... looks like that hasn't done it. I'll take another look in the morning if nobody beats me to it.

Wait how come? then where is the one coming from...?

@picnixz picnixz dismissed their stale review April 28, 2025 12:14

Apparently it doesn't work.

@picnixz
Copy link
Member

picnixz commented Apr 28, 2025

Actually, it looks like it worked because before we had (in the "Compile build Python" step):

ld: warning: ignoring duplicate libraries: '-ldl'
ld: warning: ignoring duplicate libraries: '-ldl'
ld: warning: ignoring duplicate libraries: '-ldl'
ld: warning: ignoring duplicate libraries: '-ldl'
ld: warning: ignoring duplicate libraries: '-ldl'
ld: warning: ignoring duplicate libraries: '-ldl'

and now we have

ld: warning: ignoring duplicate libraries: '-ldl'
ld: warning: ignoring duplicate libraries: '-ldl'
ld: warning: ignoring duplicate libraries: '-ldl'
ld: warning: ignoring duplicate libraries: '-ldl'

So two warnings go eliminated. It remains to check what the others are. We also have 2 warnings less in the "Compile host Python" step.

@picnixz
Copy link
Member

picnixz commented Apr 28, 2025

Note that the duplicate -ldl appear in only one command:

gcc -ldl    -o _bootstrap_python Modules/getbuildinfo.o [...] \
		Programs/_bootstrap_python.o Modules/getpath.o -ldl  -framework CoreFoundation                             

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants