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

[Enhancement] Add option to build as shared libraries #67

Merged
merged 5 commits into from
Jul 14, 2021

Conversation

0x00002a
Copy link
Contributor

This add the option MALLOY_BUILD_SHARED, which causes all libraries to be build as their shared versions (.dll on windows, .so on linux, etc). Since generator expressions cannot be used for add_library(.. [SHARED|STATIC]) I used an if statement and put it in a function (malloy_add_library).

Additionally, all shared libraries and binaries are now place in <builddir>/bin so stuff will actually run under windows. This is set on a per-target basis to avoid leaking into consumer builds. I think it is better to do this across the board rather than special case windows + shared configuration, since that introduces possible confusion and additional complexity. I am happy to change this though.

As discussed in #55 there might be issues with linking on windows, the CI seems to be fine though so hopefully its no longer a thing.

Closes: #55

@Tectu
Copy link
Owner

Tectu commented Jul 14, 2021

I don't really like the design with malloy_add_library(). What I would do is to have that MALLOY_BUILD_SHARED option exposed to the user and in the top-level CMakeLists.txt have a variable named MALLOY_LIBRARY_TYPE which gets set either to STATIC or SHARED (in the future I might add in OBJECT again) and then use that variable accordingly:

add_library(${TARGET} ${MALLOY_LIBRARY_TYPE})

This variable can then also be used for printing the build options so the user gets a nice text :)

@0x00002a
Copy link
Contributor Author

I don't really like the design with malloy_add_library(). What I would do is to have that MALLOY_BUILD_SHARED option exposed to the user and in the top-level CMakeLists.txt have a variable named MALLOY_LIBRARY_TYPE which gets set either to STATIC or SHARED [..]

I didn't think of that, much cleaner :p

@Tectu Tectu mentioned this pull request Jul 14, 2021
@Tectu Tectu merged commit a104278 into Tectu:main Jul 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow building as shared library
2 participants