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

Shared lib not being installed? #201

Closed
vlad0x00 opened this issue Aug 22, 2023 · 6 comments
Closed

Shared lib not being installed? #201

vlad0x00 opened this issue Aug 22, 2023 · 6 comments
Assignees
Labels
bug Something isn't working

Comments

@vlad0x00
Copy link

vlad0x00 commented Aug 22, 2023

Environment

toml++ version and/or commit hash: dbc4bce

Compiler: N/A

C++ standard mode: N/A

Target arch: N/A

Library configuration overrides: DTOML_HEADER_ONLY=0

Relevant compilation flags: N/A

Describe the bug

I've included tomlplusplus as a meson subproject using WrapDB as described in README: meson wrap install tomlplusplus. By default, it builds as a shared library but the built shared library is not installed and so I can't link with it during run time. The issue seems to be in tomlplusplus/src/meson.build:

tomlplusplus_lib = library(
	meson.project_name(),
	files('toml.cpp'),
	cpp_args: lib_internal_args,
	gnu_symbol_visibility: 'hidden',
	include_directories: include_dir,
	install: not is_subproject,
	version: meson.project_version(),
	override_options: global_overrides
)

Installation is disabled if tomlplusplus is a subproject: install: not is_subproject
This is not an issue if I build tomlplusplus as a static library, but this line gnu_symbol_visibility: 'hidden' is forcing my main project to also build with hidden symbols, otherwise the compiler complains. Perhaps building my project with hidden symbols is not a bad idea, but I'm not sure if forcing that on the user is ideal.

Steps to reproduce (or a small repro code sample)

Additional information

@vlad0x00 vlad0x00 added the bug Something isn't working label Aug 22, 2023
@marzer
Copy link
Owner

marzer commented Aug 23, 2023

Ah, so it looks like we have two separate things for me to address here:

1. gnu_symbol_visibility: 'hidden'

I did not realize meson applies that even if you choose to build as a static lib. That's... not particularly useful? I suppose there are scenarios where you are building one shared lib from separate compilations of a bunch of static libs, and having those static libs be part of the final library API, though that seems niche, heh. I can make that setting conditional

2. install: not is_subproject

The rationale here was that if you bring toml++ in as a subproject you may not want your project to clobber an existing system install of it against the local user's wishes, if they had a different or customized version of it as their system-wide build. I can change that, though it just means that you should probably check for a local install of toml++ in your project's meson.build before falling back to the subproject.

aside: DTOML_HEADER_ONLY=0

If you're using the library as a meson subproject you shouldn't need to explicitly set this anywhere; it will be done for you when you include the toml++ dependency:

lib_args = cpp.get_supported_arguments('-DTOML_HEADER_ONLY=0')

@marzer
Copy link
Owner

marzer commented Aug 23, 2023

Ok, after reading this thread, it seems my intuition was correct; I'll leave the 'not is_subproject' as-is, but fix the gnu_symbol_visibility behaviour when using the library as static, sound good?

@vlad0x00
Copy link
Author

vlad0x00 commented Aug 23, 2023

That sounds reasonable to me. Given that symbol visibility should be consistent across the project and subprojects, it sounds sensible that the project should decide what the visibility should be which they can control via, e.g. add_global_arguments('-fvisibility=hidden', ...). Up to you, but one alternative option that might be useful is adding a visibility option to meson_options.txt since I think it's not possible to control a subproject's symbol visibility via options in the subproject declaration.

Also, thanks for making this great library!

@marzer
Copy link
Owner

marzer commented Aug 23, 2023

Given that symbol visibility should be consistent across the project and subprojects

Only for static libs, though. I believe I can safely leave this enabled when the library is built in shared mode, and have no impact on any parent project or subproject. For static libs there's almost no reason you'd set the symbol visibility to hidden anyways, so yeah I think the path of least resistance is to be more explicit about applying this to shared-only and expose some sort of configurable if it becomes an issue for someone in static mode (though you're the first to report that the symbols were hidden in static mode so I suspect it won't 😅)

Also, thanks for making this great library!

🙇🏼 glad you like it :)

@marzer marzer closed this as completed in 882d9d1 Aug 26, 2023
@marzer
Copy link
Owner

marzer commented Aug 26, 2023

@vlad0x00 I've pushed a fix for the gnu_symbol_visibility issue, though it won't be propagated via the wrapfile on the wrapdb until I make a new release and someone updates it. In the mean-time you can replace your wrapfile with this:

[wrap-git]
url = https://github.com/marzer/tomlplusplus.git
revision = 882d9d1c34077d733559ec55d31c67799002239f
depth = 1
clone-recursive = false

[provide]
tomlplusplus  = tomlplusplus_dep

@vlad0x00
Copy link
Author

Woo! Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants