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

Support shared library building on Windows #6053

Merged
merged 18 commits into from
Apr 23, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,14 @@ function(rerun_strict_warning_settings target)
# TODO(andreas): Try to enable /Wall
target_compile_options(${target} PRIVATE /W4)

if(BUILD_SHARED_LIBS)
# If we are building as shared libs, we are going to have to disable the C4251
# warning, as it would trigger for any datatype derived from a STL class
# See also https://github.com/protocolbuffers/protobuf/blob/v26.1/cmake/README.md#notes-on-compiler-warnings
# We need also to make it public, otherwise downstream code will be flooded by c4251 warnings
target_compile_options(${target} PUBLIC /wd4251)
endif()

# CMAKE_COMPILE_WARNING_AS_ERROR is only directly supported starting in CMake `3.24`
# https://cmake.org/cmake/help/latest/prop_tgt/COMPILE_WARNING_AS_ERROR.html
if(CMAKE_COMPILE_WARNING_AS_ERROR)
Expand Down Expand Up @@ -144,8 +152,20 @@ FetchContent_Declare(LoguruGitRepo
GIT_REPOSITORY "https://github.com/emilk/loguru" # can be a filesystem path
GIT_TAG "master"
)

# Loguru does not support being build with BUILD_SHARED_LIBS=ON on Windows
# so we always compile it with BUILD_SHARED_LIBS=OFF on Windows
if(MSVC AND BUILD_SHARED_LIBS)
set(BUILD_SHARED_LIBS_RERUN_SDK ${BUILD_SHARED_LIBS})
set(BUILD_SHARED_LIBS OFF)
endif()

FetchContent_MakeAvailable(LoguruGitRepo) # defines target 'loguru::loguru'

if(MSVC AND BUILD_SHARED_LIBS_RERUN_SDK)
set(BUILD_SHARED_LIBS ${BUILD_SHARED_LIBS_RERUN_SDK})
endif()

# Set any loguru compile-time flags before calling MakeAvailable()
# Stacktraces are not yet supported on Windows.
if(NOT WIN32)
Expand Down
24 changes: 24 additions & 0 deletions rerun_cpp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,29 @@

target_link_libraries(rerun_sdk PRIVATE rerun_arrow_target)

if(MSVC AND BUILD_SHARED_LIBS)
# This code is required by to support BUILD_SHARED_LIBS=ON on Windows
# Differently from Linux/macOS, by default Windows does not support
# exporting all symbols of a shared library, and instead requires
# annotating manually each class that needs to be exported
# The WINDOWS_EXPORT_ALL_SYMBOLS PROPERTY (set in the next line)
# simply the process of having shared library on Windows, by emulating
# in CMake the behaviour of Linux/macOS. However, it does not cover

Check warning on line 126 in rerun_cpp/CMakeLists.txt

View workflow job for this annotation

GitHub Actions / Checks / Spell Check

"behaviour" should be "behavior".
# static variables.
set_property(TARGET rerun_sdk PROPERTY WINDOWS_EXPORT_ALL_SYMBOLS ON)
# For exporting static variables in shared libraries in Windows, it
# is not possible to just use WINDOWS_EXPORT_ALL_SYMBOLS, we need instead
# to manually annotate with the appropriate storage-class attributes
# all static variables. The easiest way to do so is use GenerateExportHeader
# module to generate a rerun_sdk_export.hpp header file that define the RERUN_SDK_EXPORT
# macro, and add that macro to all static variables. The RERUN_SDK_EXPORT is defined
# in src/rerun/rerun_sdk_export.hpp . The definition of the macro changes depending
# of whether the library is compiled as static or shared, so for shared builds we
# set the RERUN_SDK_COMPILED_AS_SHARED_LIBRARY to let the header know if the build
# is a shared library one
target_compile_definitions(rerun_sdk PUBLIC RERUN_SDK_COMPILED_AS_SHARED_LIBRARY)
endif()

# -----------------------------------------------------------------------------
# Installation.
set(RERUN_SDK_INSTALL_CMAKE_DIR "lib/cmake/rerun_sdk")
Expand All @@ -125,6 +148,7 @@
EXPORT rerun_sdkTargets
LIBRARY DESTINATION lib
ARCHIVE DESTINATION lib
RUNTIME DESTINATION bin
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI I added this here just for consistencies, since CMake 3.14 these argument have default values that should just work fine, see https://cmake.org/cmake/help/v3.16/command/install.html#installing-targets .

INCLUDES DESTINATION include
)

Expand Down
5 changes: 3 additions & 2 deletions rerun_cpp/src/rerun/archetypes/clear.hpp

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 7 additions & 2 deletions rerun_cpp/src/rerun/archetypes/clear_ext.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
#include "clear.hpp"

// <CODEGEN_COPY_TO_HEADER>
#include "../rerun_sdk_export.hpp"

// </CODEGEN_COPY_TO_HEADER>

// Uncomment for better auto-complete while editing the extension.
// #define EDIT_EXTENSION

Expand All @@ -12,9 +17,9 @@ namespace rerun {

// <CODEGEN_COPY_TO_HEADER>

static const Clear FLAT;
RERUN_SDK_EXPORT static const Clear FLAT;

static const Clear RECURSIVE;
RERUN_SDK_EXPORT static const Clear RECURSIVE;

Clear(bool _is_recursive = false)
: Clear(components::ClearIsRecursive(_is_recursive)) {}
Expand Down
3 changes: 2 additions & 1 deletion rerun_cpp/src/rerun/archetypes/transform3d.hpp

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 6 additions & 1 deletion rerun_cpp/src/rerun/archetypes/transform3d_ext.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
#include "transform3d.hpp"

// <CODEGEN_COPY_TO_HEADER>
#include "../rerun_sdk_export.hpp"

// </CODEGEN_COPY_TO_HEADER>

// Uncomment for better auto-complete while editing the extension.
// #define EDIT_EXTENSION

Expand All @@ -12,7 +17,7 @@ namespace rerun {
/// Identity transformation.
///
/// Applying this transform does not alter an entity's transformation.
static const Transform3D IDENTITY;
RERUN_SDK_EXPORT static const Transform3D IDENTITY;

/// New 3D transform from translation/matrix datatype.
///
Expand Down
123 changes: 62 additions & 61 deletions rerun_cpp/src/rerun/archetypes/view_coordinates.hpp

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading