-
Notifications
You must be signed in to change notification settings - Fork 22
fix: compilation with BUILD_SHARED_LIBS with MSVC
#776
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
base: develop
Are you sure you want to change the base?
Conversation
|
An automated preview of the documentation is available at https://776.mrdocs.prtest2.cppalliance.org/index.html |
0e76e48 to
3db209c
Compare
|
An automated preview of the documentation is available at https://776.mrdocs.prtest2.cppalliance.org/index.html |
|
That's amazing, especially since it's now being checked in CI. The plugin system is not designed yet but it's a step we need to take. I'll review it in 2025. |
|
Just had a look at this. The changes are great. These MRDOCS_DECLs were being tested at all. Just one question though: isn't the requirement #58 the other way around? That is, the compiled plugin is what needs to be a shared library but it doesn't really matter if mrdocs it's static. MrDocs only needs to load it when the system is implemented. |
|
Yeah, that's what I initially thought, too. I was a bit confused by the graph TD;
implib["mrdocs.lib<br>(import lib)"]
lib["mrdocs-core.lib<br>(static library with “public“ symbols)"]
lib---|linked to|mrdocs.exe;
mrdocs.exe---|derived|implib;
implib---|“linked“ to|plugin.dll;
mrdocs.exe---|loads|plugin.dll;
I'm not entirely sure what the equivalent of an import library on Linux and macOS is. |
Yes. If the user asks for BUILD_SHARED_LIBS when invoking CMake, we want to honor that. So that's what MRDOCS_DECL does.
Now, I'm not really sure about this. I think we can only find out once the plugin system is really implemented. But it's possible that mrdocs needs to be built with BUILD_SHARED_LIBS for plugins to work because the plugin will end up interacting with MrDocs' public API.
Yes. Shared libraries exist on Linux and MacOS. The API for interacting with them is what changes (but that's something to worry about when the plugin system is implemented). Boost.DLL abstracts these differences. |
3db209c to
d00119d
Compare
|
An automated preview of the documentation is available at https://776.mrdocs.prtest2.cppalliance.org/index.html |
To "just" support custom generators, exporting the symbols in the executable is enough. That's what enabling
Yea, I wasn't sure how dynamic libraries on Linux and macOS would need to be compiled to use the provided functions. But it seems like one can
LLVM also provides a facility to load dynamic libraries. That's how Clang loads its plugins. |
In CMake, that's transparent. You just ask CMake for shared libraries and it gives them to you.
That's nice. I haven't researched that yet, but I thought LLVM would have its version. LLVM comes with its implementation of almost all basic things. |
a94f805 to
6b6979b
Compare
|
An automated preview of the documentation is available at https://776.mrdocs.prtest2.cppalliance.org/index.html |
6b6979b to
144bd74
Compare
|
An automated preview of the documentation is available at https://776.mrdocs.prtest2.cppalliance.org/index.html |
144bd74 to
054261a
Compare
|
An automated preview of the documentation is available at https://776.mrdocs.prtest2.cppalliance.org/index.html |
054261a to
3fae223
Compare
|
An automated preview of the documentation is available at https://776.mrdocs.prtest2.cppalliance.org/index.html |
3fae223 to
5d6ca46
Compare
|
An automated preview of the documentation is available at https://776.mrdocs.prtest2.cppalliance.org/index.html |
5d6ca46 to
592f987
Compare
|
Is there anything left here? |
|
An automated preview of the documentation is available at https://776.mrdocs.prtest2.cppalliance.org/index.html |
592f987 to
294929d
Compare
|
An automated preview of the documentation is available at https://776.mrdocs.prtest2.cppalliance.org/index.html |
294929d to
c9fab57
Compare
c9fab57 to
348832e
Compare
|
An automated preview of the documentation is available at https://776.mrdocs.prtest2.cppalliance.org/index.html |
348832e to
97e2a3f
Compare
|
An automated preview of the documentation is available at https://776.mrdocs.prtest2.cppalliance.org/index.html |
This fixes the compilation when building shared libraries instead of a statically linked executable. Building a shared library is required for #581. Since MSVC requires
__declspec(dllexport), there were some linker errors. Additionally, there were some compiler errors about implicit special member functions being generated ininclude/dom/(usingclang-cl/lld-linkas the compiler leads to much more helpful errors thancl/link).For GCC-ish compilers,
-fvisibility=hiddendoesn't seem to be specified formrdocs-core, because it's a static library. If it was specified,MRDOCS_DECLfor GCC would need to be updated (similar to what CMake'sGenerateExportHeaderdoes).Aside: A
clang-formatconfig would be great.Footnotes
Technically, the executable could still be statically linked while exporting the required symbols. That's basically what setting
MRDOCS_BUILD_SHARED=ONwithout enablingBUILD_SHARED_LIBSdoes. I thinkENABLE_EXPORTSwould still be required for that executable to make sure an "import library" is created. ↩