Support build proto as shared lib on Windows
Fixes #3665 Fixes #3620
Changes
- Set
dllexport_declfor proto targets and can be built as shared on windows.
For significant contributions please make sure you have completed the following items:
- [x]
CHANGELOG.mdupdated for non-trivial changes - [x] Unit tests have been added
- [x] Changes in public API reviewed
Codecov Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 89.95%. Comparing base (9de3493) to head (3df0e30).
:warning: Report is 11 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@
## main #3714 +/- ##
==========================================
+ Coverage 89.93% 89.95% +0.02%
==========================================
Files 225 225
Lines 7158 7158
==========================================
+ Hits 6437 6438 +1
+ Misses 721 720 -1
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
My MSVC have LNK2001 unresolved external symbol for __std_find_last_trivial_1 , __std_search_1 , __std_search_1 , __std_min_8i and many other STL's symbols, I will wait the next VS update and continue this work then.
Thanks for the PR @owent! I've taken a quick pass and shared some feedback. It may be worth a SIG meeting discussion if the project should support windows dlls of protobuf and especially grpc given that projects warnings about unexpected behavior:
you've been warned that there are some important drawbacks and some things might not work at all or will be broken in interesting ways.
Separately we should investigate why the formatting tools are not applied consistently - is it possible the powershell scripts and .cmake files are not covered?
@lalitb @ThomsonTan @dbarker There is another suggestion for https://github.com/open-telemetry/opentelemetry-cpp/pull/3714#discussion_r2536061020_ and https://github.com/open-telemetry/opentelemetry-cpp/pull/3714#discussion_r2536064869_ .
We currently do not support -DCMAKE_BUILD_TYPE=ON on Windows. A better approach would be to use a macro to define __attribute__((__dllexport__)), __attribute__((__dllimport__)), or __attribute__((visibility("default"))) for each component and apply this macro to all exported APIs. This would allow us to build each component as a standalone DLL. By adopting this method, there would be no need to manually run ext/src/dll/make_def.ps1 or maintain ext/src/dll/input.src. When building ext/src/dll with -DCMAKE_BUILD_TYPE=ON, it would only need to link all components of otel-cpp. Conversely, when -DCMAKE_BUILD_TYPE=OFF is used, all symbols can be exported using a single CMake function, simplifying the process of maintaining symbols in the future.
For example, we can add the following CMake utility function:
function(project_build_tools_set_default_library_declaration DEFINITION_VARNAME)
if(BUILD_SHARED_LIBS OR ((CMAKE_SYSTEM_NAME STREQUAL "Windows") AND DEFINED OPENTELEMETRY_BUILD_DLL))
project_build_tools_set_shared_library_declaration(${DEFINITION_VARNAME}
${ARGN})
else()
project_build_tools_set_static_library_declaration(${DEFINITION_VARNAME}
${ARGN})
endif()
endfunction()
Then use it in opentelemetry_common as follows:
project_build_tools_set_default_library_declaration(OPENTELEMETRY_SDK_COMMON_API opentelemetry_common).
Public functions can then be declared like this:
class GlobalLogHandler
{
public:
OPENTELEMETRY_SDK_COMMON_API static nostd::shared_ptr<LogHandler> GetLogHandler() noexcept;
OPENTELEMETRY_SDK_COMMON_API static void SetLogHandler(const nostd::shared_ptr<LogHandler> &eh) noexcept;
// ...
};
What do you think of this approach? Should we open a new issue or discuss it further?
It touches many files, and I can complete the changes if we agree to proceed.