libplacebo icon indicating copy to clipboard operation
libplacebo copied to clipboard

Don't export symbols from static library

Open BtbN opened this issue 1 year ago • 7 comments

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.

BtbN avatar Nov 15 '23 16:11 BtbN

LGTM

ePirat avatar Nov 15 '23 18:11 ePirat

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.

kasper93 avatar Nov 16 '23 06:11 kasper93

(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.

haasn avatar Nov 17 '23 12:11 haasn

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.

Nevcairiel avatar Nov 17 '23 12:11 Nevcairiel

@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…

ePirat avatar Nov 17 '23 14:11 ePirat

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)

kasper93 avatar Nov 17 '23 15:11 kasper93

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

kasper93 avatar Feb 27 '24 01:02 kasper93