libplacebo
libplacebo copied to clipboard
Don't export symbols from static library
The "both" case is likely still broken, but I see no good way to fix that with meson.
Background: FFmpeg just started linking to libplacebo directly from ffplay. This broke linking on Windows, since libavfilter ended up exporting the libplacebo symbols due to the forced dllexport on them.
Arguably, on Linux the visibility should likely also be set to hidden if PL_STATIC is set, since there is no point in exporting the symbols there either. But apparently the linker there is a bit smarter and figures things out.
LGTM
The "both" case is likely still broken, but I see no good way to fix that with meson.
both
case unfortunately is not fixable, because meson doesn't have any option to provide different flags for static/shared compilation. Objects are the same and in this case it is exported. Maybe one day they will support it, but not today. https://github.com/mesonbuild/meson/issues/3304
Don't dllexport symbols when building static library
commit is just cosmetic, no? You should not define PL_EXPORT
when building static lib if you don't want it to have marked symbols for export. If you don't do that as you did in the other commit, it will work correctly. There is small use-case where you want to build static lib and still be able to export its symbols when linking into common shared library.
The meson change LGTM, I don't think we can do anything better with meson...
Arguably, on Linux the visibility should likely also be set to hidden if PL_STATIC is set, since there is no point in exporting the symbols there either. But apparently the linker there is a bit smarter and figures things out.
Most likely on Linux it is linked with -Wl,--exclude-libs,ALL
as any sane person would do. On Windows it is just different, because dllexport just marks symbols that should be exported, so whenever they are linked they are made visible.
(Style) Can you change the commit title to e.g. meson: set PL_STATIC when building static library
?
As for the config.h change, @kasper93 it's saying it's not needed? Seems like it wouldn't really fix anything, if you're saying it is still broken in the "both" case.
meson should never set PL_EXPORT and PL_STATIC at the same time, so the config.h.in change just re-orders the defines but with no change in actual functionality.
@kasper93 I don't think both would ever be workable on windows because you need different installed headers for the static vs non-static library (due to the need for dllimport, or rely on the client using the library to set some define before including the header…)
The both case is mostly for a nice benefit of not rebuilding everything, which makes it somewhat pointless once you have different preprocessor macros for static vs dynamic cases as then you anyway need to rebuild everything…
I don't think both would ever be workable on windows because you need different installed headers for the static vs non-static library (due to the need for dllimport, or rely on the client using the library to set some define before including the header…)
You need exactly the same headers. That's why PL_STATIC
exist to add dllimport
when requested. And is currently appended when pkgconf --static
is requested. (Cflags.private
)
We never merged that? Either way, meson now have c_static_args
and c_shared_args
, we could use that instead. https://github.com/mesonbuild/meson/pull/11981