Skip to content
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

meson: add -Bsymbolic for libmpv #12346

Merged
merged 1 commit into from
Sep 7, 2023
Merged

Conversation

kasper93
Copy link
Contributor

@kasper93 kasper93 commented Sep 5, 2023

This is required when linking ffmpeg statically and generally good practice to have on shared libraries anyway.

Prerequisit for fixes in mpv-build for:
mpv-player/mpv-build#215
haasn/libplacebo#198

This is required when linking ffmpeg statically and generally good
practice to have on shared libraries anyway.

Prerequisit for fixes in mpv-build for:
mpv-player/mpv-build#215
haasn/libplacebo#198
kasper93 added a commit to kasper93/mpv-build that referenced this pull request Sep 5, 2023
It shouldn't be added unconditionally and with all the fragile checks if
it is supported it is way better to handle it in the meson istself.

See also:
mpv-player/mpv#12346

Fixes:
mpv-player#215
haasn/libplacebo#198
kasper93 added a commit to kasper93/mpv-build that referenced this pull request Sep 5, 2023
It shouldn't be added unconditionally and with all the fragile checks if
it is supported it is way better to handle it in the meson itself.

See also:
mpv-player/mpv#12346

Fixes:
mpv-player#215
haasn/libplacebo#198
@Dudemanguy
Copy link
Member

generally good practice to have on shared libraries anyway

Is it? I don't pretend to know the details of all this linking stuff, but I thought you weren't actually supposed to use this flag in general.

@kasper93
Copy link
Contributor Author

kasper93 commented Sep 5, 2023

generally good practice to have on shared libraries anyway

Is it? I don't pretend to know the details of all this linking stuff, but I thought you weren't actually supposed to use this flag in general.

It is whole story of ELF weirdness and why symbol interposition is not really that good idea.

tl;dr; This flag sets DT_SYMBOLIC, which disallows interposing symbols in this shared library from executable that is linking said library.

You can read more what it does here:
https://maskray.me/blog/2021-05-16-elf-interposition-and-bsymbolic
https://maskray.me/blog/2021-05-09-fno-semantic-interposition

All in all, it doesn't mean much for libmpv. But those two patches fixes compilation when statically linking ffmpeg. I frankly don't know why it fails when it is added when linking executable, but well let's not add it in this case.

If only people wouldn't link statically everything... just use shared libraries.

@Dudemanguy
Copy link
Member

Dudemanguy commented Sep 7, 2023

Sorry for the delay. This still feels kind of like a sledgehammer, but ffmpeg does mention this flag in their documentation so whatever I guess.

@Dudemanguy Dudemanguy merged commit bced1ee into mpv-player:master Sep 7, 2023
13 checks passed
Dudemanguy pushed a commit to mpv-player/mpv-build that referenced this pull request Sep 7, 2023
It shouldn't be added unconditionally and with all the fragile checks if
it is supported it is way better to handle it in the meson itself.

See also:
mpv-player/mpv#12346

Fixes:
#215
haasn/libplacebo#198
@kasper93 kasper93 deleted the meson branch October 7, 2023 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants