Add shared library option

[uilianries]

This PR includes the new option LIBCORO_BUILD_SHARED_LIBS that reflects the CMake option for BUILD_SHARED_LIBS. Once enabled, it will build only shared libraries and not static.

When using the vendor c-ares, it will keep both aligned, by using shared/shared or static/static.

The O.S. Windows requires __declspec to export symbols when providing a .dll. However, the libcoro project exposes all headers as public, so I just took a shortcut by using CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS instead. Using dllimport/dllexport in this case, would result in the same exported symbols, but only creating more logic to disable/enable in headers.

closes #233

[uilianries]

Hello @jbaldwin !

I added not the shared library option, but also new builds in the Github Action to make sure it will work. However, now I see several linkage errors, for instance:

https://github.com/jbaldwin/libcoro/actions/runs/7474611322/job/20341130462?pr=236

FAILED: examples/coro_task 
: && /usr/bin/g++-10  -O3 -DNDEBUG   examples/CMakeFiles/coro_task.dir/coro_task.cpp.o  -o examples/coro_task  -Wl,-rpath,/__w/libcoro/libcoro/Release:/__w/libcoro/libcoro/Release/vendor/c-ares/c-ares/lib  libcoro.so  -lpthread  vendor/c-ares/c-ares/lib/libcares.so.2.4.2  /usr/lib/x86_64-linux-gnu/libssl.so  /usr/lib/x86_64-linux-gnu/libcrypto.so && :
/usr/bin/ld: libcoro.so: undefined reference to `coro::io_scheduler::schedule(coro::task<void>&&)'
/usr/bin/ld: libcoro.so: undefined reference to `coro::io_scheduler::poll(int, coro::poll_op, std::chrono::duration<long, std::ratio<1l, 1000l> >)'

The configuration is clear about not using platform support (LIBCORO_FEATURE_PLATFORM=OFF), but it's actually always required.

The examples/coro_task.cpp does not include io_scheduler.hpp, but only coro/coro.hpp. The main include coro.hpp has a big ifdef to include or not scheduler too. So we know when LIBCORO_FEATURE_PLATFORM=OFF, those headers and source files are not compiler/included.

However, I can see schedule usage both in include/coro/task_container.hpp and include/coro/concepts/executor.hpp. The schedule method is implemented by io_scheduler.cpp.

Am I missing something, or LIBCORO_FEATURE_PLATFORM is actually mandatory to any example and test?

[jbaldwin]

It shouldn't be required. I recently added builds that should run on linux LIBCORO_FEATURE_PLATFORM=OFF, but its possible the windows build is exposing things I missed. Let me take a deeper look and see if its an easy fix.

[jbaldwin]

include/coro/concepts/executor.hpp only includes include/coro/concepts/awaitable.hpp which doesn't include any other coro headers, so I'm not sure I follow there?

coro::task_container does indeed forward declare io_scheduler but it doesn't include the header file.

I did look at the output from the actions job and it does seem to be linking into io_scheduler somehow, but that would mean that coro:task_container got template instantiated as coro::task_container<io_scheduler> which I'm not exactly sure where that is happening? One thing for sure we could add is ifdef's around the forward declare and the friend lines in task_container.hpp to only include them if LIBCORO_FEATURE_PLATFORM=ON

[jbaldwin]

I think its the coro::net::dns::resolver class, it includes io_scheduler and task_container, that class should be made to require LIBCORO_FEATURE_PLATFORM=ON.

It could also use the executor concept instead... thats probably the right thing to do. If we do that then only the example needs to be moved under PLATFORM I think

-- I'm going to try and prototype this out since I think its the right thing to do here --

[jbaldwin]

I think you're right after digging around a lot, PLATFORM and NETWORK are basically identical at this point. I'm probably just going to drop PLATFORM and use NETWORKING everywhere.

[jbaldwin]

Ok, can you try rebasing and we'll give this another go?

[uilianries]

@jbaldwin sorry the delay, I was offline on last Friday. I'll rebase now.

[uilianries]

Forced pushed due git rebase command. There was a conflict related to using platform option in CI. Removed as following in the main branch.

[jbaldwin]

I haven't been able to really look at why the windows build failed but is it just a missing header file? I can probably look deeper tomorrow

[uilianries]

@jbaldwin I just checked the build logs now and it's a limitation with CMake exporting static variables from semaphore class (coro::semaphore::acquire_result_unknown and others). We will need to export symbols using __declspec. Fortunately, CMake provides a helper for it: https://www.kitware.com/create-dlls-on-windows-without-declspec-using-new-cmake-export-all-feature/

I'll take a look later on it, right now I don't have a Windows machine to fix, only at home.

[uilianries]

@jbaldwin It's working now. Some note to understand this PR at this point:

• It generates static only or shared libraries according to LIBCORO_BUILD_SHARED_LIBS option
• When generating shared library in Windows, it will produce bin/libcoro.dll and lib/libcoro.lib (exposed symbols to load in the dll)
• The CMake definition CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS does not export static variables
• To export semaphore's variables I needed to produce a new header file, the coro/export.hpp. It's generated by cmake, using the method generate_export_header, and should manage __declspec
• The ctest does not include the DLL folder by default, so I needed to configure it by set_tests_properties

[jbaldwin]

Very nice work, thanks for sticking through on this feature!