Skip to content

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

Open
antoniovazquezblanco wants to merge 1 commit into
mesonbuild:masterfrom
antoniovazquezblanco:cwalk
Open

antoniovazquezblanco wants to merge 1 commit into
mesonbuild:masterfrom
antoniovazquezblanco:cwalk

Conversation

@antoniovazquezblanco

Copy link
Copy Markdown
Contributor

This is a follow up of #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

Comment thread releases.json Outdated
Comment thread releases.json Outdated
@antoniovazquezblanco

Copy link
Copy Markdown
Contributor Author

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

Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown
Contributor

And switch to this syntax in the wrap:

[provide]
dependency_names = cwalk

@antoniovazquezblanco

Copy link
Copy Markdown
Contributor Author

Finally found some time to get back to this but...

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.

I do not know how to do this. Could you provide a detailed description please? Thanks @benoit-pierre !

@bgilbert bgilbert added the needs fixes Pull request received review feedback and needs adjustments label Aug 10, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs fixes Pull request received review feedback and needs adjustments

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants