mesonbuild / wrapdb

New wrap requests
https://mesonbuild.com/Adding-new-projects-to-wrapdb.html
MIT License
69 stars 174 forks source link

cwalk: fix compilation when both shared and static is wanted #1566

Open antoniovazquezblanco opened 1 week ago

antoniovazquezblanco commented 1 week ago

This is a follow up of https://github.com/mesonbuild/wrapdb/pull/1387

I am trying to improve on the already merged meson script by allowing compilation of both static and dynamic libs simultaneously with the proper arguments. This tryes to implement what @benoit-pierre suggested... Also adding @likle from upstream...

From meson maintainers, would this be desirable or would a "def" file provide any other desirable behaviour?

Thanks

antoniovazquezblanco commented 1 week ago

In MS compilers, this is producing a lot of warnings:

warning: 'cwk_path_change_segment' redeclared without 'dllimport' attribute: 'dllexport' attribute added [-Winconsistent-dllimport]
benoit-pierre commented 1 week ago

From meson maintainers, would this be desirable or would a "def" file provide any other desirable behaviour?

AFAIK, that's a Windows only solution, and CWK_SHARED also impact the build on other platforms:

#if defined(_WIN32) || defined(__CYGWIN__)
#define CWK_EXPORT __declspec(dllexport)
#define CWK_IMPORT __declspec(dllimport)
#elif __GNUC__ >= 4
#define CWK_EXPORT __attribute__((visibility("default")))
#define CWK_IMPORT __attribute__((visibility("default")))
#else
#define CWK_EXPORT
#define CWK_IMPORT
#endif

#if defined(CWK_SHARED)
#if defined(CWK_EXPORTS)
#define CWK_PUBLIC CWK_EXPORT
#else
#define CWK_PUBLIC CWK_IMPORT
#endif
#else
#define CWK_PUBLIC
#endif

However, the library(…) call is missing gnu_symbol_visibility: 'hidden' to take full advantage of __attribute__((visibility("default"))) when using clang/gcc.

benoit-pierre commented 1 week ago

redeclared without 'dllimport

That's because -DCWK_EXPORTS is needed too: from CMakeLists.txt, it should be unconditionally added do c_args.

Additionally, CWK_SHARED is part of the public interface:

target_compile_definitions(cwalk PUBLIC CWK_SHARED)

So it should be added to the compile_args of cwalk_dep.

benoit-pierre commented 1 week ago

You'll need to declare 2 dependencies, and use meson.override_dependency(…) with static, so the right compile arguments are used for both static and shared variants.

benoit-pierre commented 1 week ago

And switch to this syntax in the wrap:

[provide]
dependency_names = cwalk