From f71506c0f32f88de6dfc519c4ef0b8cdf49ddd0f Mon Sep 17 00:00:00 2001 From: Andrew Morrow Date: Fri, 4 Apr 2025 15:05:23 -0400 Subject: [PATCH 1/3] [RSDK-10385] Windows build system improvements and rust_utils workarounds --- CMakeLists.txt | 28 +++++++++++-------- conanfile.py | 20 +++++++++++-- src/viam/sdk/CMakeLists.txt | 8 +++++- .../sdk/common/private/service_helper.hpp | 6 ++++ src/viam/sdk/module/service.cpp | 6 ++++ src/viam/sdk/rpc/dial.cpp | 11 +------- src/viam/sdk/rpc/private/viam_rust_utils.h | 21 ++++++++++++++ .../sdk/rpc/private/viam_rust_utils_stubs.cpp | 23 +++++++++++++++ 8 files changed, 99 insertions(+), 24 deletions(-) create mode 100644 src/viam/sdk/rpc/private/viam_rust_utils.h create mode 100644 src/viam/sdk/rpc/private/viam_rust_utils_stubs.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index b34192878..9dfd1b053 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -173,6 +173,10 @@ if (NOT CMAKE_CXX_STANDARD) set(CMAKE_CXX_STANDARD 14) endif() set(CMAKE_CXX_STANDARD_REQUIRED True) +if(MSVC) + # https://discourse.cmake.org/t/set-cmake-cxx-standard-should-set-zc-cplusplus-for-msvc/1876 + string(APPEND CMAKE_CXX_FLAGS " /Zc:__cplusplus") +endif() set(CMAKE_CXX_EXTENSIONS OFF) @@ -255,7 +259,7 @@ if (viam_rust_utils_files) ${viam_rust_utils_file} ONLY_IF_DIFFERENT ) -else() +elseif(NOT WIN32) # TODO(RSDK-10366): Currently, rust_utils is not published for windows, so don't even try downloading set(lvru_system_name ${CMAKE_SYSTEM_NAME}) if (CMAKE_SYSTEM_NAME STREQUAL "Darwin") set(lvru_system_name "macosx") @@ -275,21 +279,23 @@ else() endif() -add_library(viam_rust_utils SHARED IMPORTED) +# TODO(RSDK-10366): Currently, rust_utils is not published for windows, so don't even declare the library +if (NOT WIN32) + add_library(viam_rust_utils SHARED IMPORTED) -target_link_directories(viam_rust_utils - INTERFACE + target_link_directories(viam_rust_utils + INTERFACE "$" "$" -) - -set_property(TARGET viam_rust_utils PROPERTY IMPORTED_LOCATION ${viam_rust_utils_file}) + ) -install( - IMPORTED_RUNTIME_ARTIFACTS viam_rust_utils - LIBRARY COMPONENT viam-cpp-sdk_runtime -) + set_property(TARGET viam_rust_utils PROPERTY IMPORTED_LOCATION ${viam_rust_utils_file}) + install( + IMPORTED_RUNTIME_ARTIFACTS viam_rust_utils + LIBRARY COMPONENT viam-cpp-sdk_runtime + ) +endif() # Install the license file install(FILES diff --git a/conanfile.py b/conanfile.py index b1c98ecea..f3e2fa923 100644 --- a/conanfile.py +++ b/conanfile.py @@ -82,7 +82,11 @@ def package(self): CMake(self).install() def package_info(self): - self.cpp_info.components["viam_rust_utils"].libs = ["viam_rust_utils"] + + # TODO(RSDK-10366): Currently, rust_utils is not published for windows + # and the C++ SDK just doesn't include it as a dependency on that platform + if not self.settings.os == "Windows": + self.cpp_info.components["viam_rust_utils"].libs = ["viam_rust_utils"] self.cpp_info.components["viamsdk"].libs = ["viamsdk"] @@ -103,10 +107,16 @@ def package_info(self): else: lib_folder = os.path.join(self.package_folder, "lib") lib_fullpath = os.path.join(lib_folder, "libviamapi.a") + if self.settings.os == "Windows": + lib_fullpath = os.path.join(lib_folder, "viamapi.lib") + if is_apple_os(self): whole_archive = f"-Wl,-force_load,{lib_fullpath}" + elif self.settings.os == "Windows": + whole_archive = f"/WHOLEARCHIVE:{lib_fullpath}" else: whole_archive = f"-Wl,--push-state,--whole-archive,{lib_fullpath},--pop-state" + self.cpp_info.components["viamapi"].exelinkflags.append(whole_archive) self.cpp_info.components["viamapi"].sharedlinkflags.append(whole_archive) @@ -118,7 +128,13 @@ def package_info(self): "xtensor::xtensor", "viamapi", - "viam_rust_utils" ]) + # TODO(RSDK-10366): Currently, rust_utils is not published for windows + # and the C++ SDK just doesn't include it as a dependency on that platform + if self.settings.os != "Windows": + self.cpp_info.components["viamsdk"].requires.extend([ + "viam_rust_utils" + ]) + self.cpp_info.components["viamsdk"].frameworks = ["Security"] diff --git a/src/viam/sdk/CMakeLists.txt b/src/viam/sdk/CMakeLists.txt index 2af5eb483..aad8919e7 100644 --- a/src/viam/sdk/CMakeLists.txt +++ b/src/viam/sdk/CMakeLists.txt @@ -132,6 +132,7 @@ target_sources(viamsdk rpc/dial.cpp rpc/server.cpp rpc/private/viam_grpc_channel.cpp + rpc/private/viam_rust_utils_stubs.cpp services/discovery.cpp services/generic.cpp services/mlmodel.cpp @@ -263,10 +264,15 @@ target_link_libraries(viamsdk PRIVATE ${VIAMCPPSDK_GRPCXX_LIBRARIES} PRIVATE ${VIAMCPPSDK_GRPC_LIBRARIES} PRIVATE ${VIAMCPPSDK_PROTOBUF_LIBRARIES} - PRIVATE viam_rust_utils PRIVATE Threads::Threads ) +# TODO(RSDK-10366): Currently, rust_utils is not published for windows, so don't link to it. +if (NOT WIN32) + target_link_libraries(viamsdk + PRIVATE viam_rust_utils + ) +endif() if (APPLE) target_link_libraries(viamsdk PUBLIC "-framework Security") diff --git a/src/viam/sdk/common/private/service_helper.hpp b/src/viam/sdk/common/private/service_helper.hpp index 3212fe00f..9e4b851a1 100644 --- a/src/viam/sdk/common/private/service_helper.hpp +++ b/src/viam/sdk/common/private/service_helper.hpp @@ -55,8 +55,14 @@ class ServiceHelper : public ServiceHelperBase { } private: + +#if __cplusplus >= 201703L + template + using is_void_result = std::is_void>; +#else template using is_void_result = std::is_void>; +#endif // Implementation of `invoke_` for a Callable returning non-void, // presumably an error return, which we return as a diff --git a/src/viam/sdk/module/service.cpp b/src/viam/sdk/module/service.cpp index 7445e5057..042e5a92e 100644 --- a/src/viam/sdk/module/service.cpp +++ b/src/viam/sdk/module/service.cpp @@ -1,3 +1,9 @@ +#ifdef _WIN32 +#define NOMINMAX +#include +#include +#endif + #include #include diff --git a/src/viam/sdk/rpc/dial.cpp b/src/viam/sdk/rpc/dial.cpp index d360acca2..35a990363 100644 --- a/src/viam/sdk/rpc/dial.cpp +++ b/src/viam/sdk/rpc/dial.cpp @@ -13,17 +13,8 @@ #include #include #include +#include -extern "C" void* init_rust_runtime(); -extern "C" int free_rust_runtime(void* ptr); -extern "C" void free_string(const char* s); -extern "C" char* dial(const char* uri, - const char* entity, - const char* type, - const char* payload, - bool allow_insecure, - float timeout, - void* ptr); namespace viam { namespace sdk { diff --git a/src/viam/sdk/rpc/private/viam_rust_utils.h b/src/viam/sdk/rpc/private/viam_rust_utils.h new file mode 100644 index 000000000..7542a51f4 --- /dev/null +++ b/src/viam/sdk/rpc/private/viam_rust_utils.h @@ -0,0 +1,21 @@ +#if defined(__cplusplus) +extern "C" { +#endif + +// Prototypes for the entrypoints from viam_rust_utils +// that we use in the SDK. + +void* init_rust_runtime(); +int free_rust_runtime(void* ptr); +void free_string(const char* s); +char* dial(const char* uri, + const char* entity, + const char* type, + const char* payload, + bool allow_insecure, + float timeout, + void* ptr); + +#if defined(__cplusplus) +} // extern "C" +#endif diff --git a/src/viam/sdk/rpc/private/viam_rust_utils_stubs.cpp b/src/viam/sdk/rpc/private/viam_rust_utils_stubs.cpp new file mode 100644 index 000000000..f8f5bbdf7 --- /dev/null +++ b/src/viam/sdk/rpc/private/viam_rust_utils_stubs.cpp @@ -0,0 +1,23 @@ +#include + +#include + +// TODO(RSDK-10366): Currently, rust_utils is not published for windows +// so we just implement the associated entry points as `abort`. +#ifdef _WIN32 +void* init_rust_runtime() { + abort(); +} + +int free_rust_runtime(void*) { + abort(); +} + +void free_string(const char*) { + abort(); +} + +char* dial(const char*, const char*, const char*, const char*, bool, float, void*) { + abort(); +} +#endif From 71c43d5a4f5f4a677ebd434470bc910c3516d9de Mon Sep 17 00:00:00 2001 From: Andrew Morrow Date: Mon, 7 Apr 2025 10:58:15 -0400 Subject: [PATCH 2/3] format --- src/viam/sdk/common/private/service_helper.hpp | 1 - src/viam/sdk/module/service.cpp | 4 ++++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/viam/sdk/common/private/service_helper.hpp b/src/viam/sdk/common/private/service_helper.hpp index 9e4b851a1..0c8326b40 100644 --- a/src/viam/sdk/common/private/service_helper.hpp +++ b/src/viam/sdk/common/private/service_helper.hpp @@ -55,7 +55,6 @@ class ServiceHelper : public ServiceHelperBase { } private: - #if __cplusplus >= 201703L template using is_void_result = std::is_void>; diff --git a/src/viam/sdk/module/service.cpp b/src/viam/sdk/module/service.cpp index 042e5a92e..bb1178b65 100644 --- a/src/viam/sdk/module/service.cpp +++ b/src/viam/sdk/module/service.cpp @@ -1,7 +1,11 @@ #ifdef _WIN32 #define NOMINMAX +// clang-format off +// Otherwise clang-format tries to alphabetize these headers, +// but `winsock2.h` should definitely precede `windows.h`. #include #include +// clang-format on #endif #include From 27d1a2bee12226207d3825d8ad2b0126c28a082f Mon Sep 17 00:00:00 2001 From: Andrew Morrow Date: Mon, 7 Apr 2025 13:13:42 -0400 Subject: [PATCH 3/3] review feedback --- src/viam/sdk/CMakeLists.txt | 7 +++++-- src/viam/sdk/rpc/private/viam_rust_utils_stubs.cpp | 4 ---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/viam/sdk/CMakeLists.txt b/src/viam/sdk/CMakeLists.txt index aad8919e7..565491f7c 100644 --- a/src/viam/sdk/CMakeLists.txt +++ b/src/viam/sdk/CMakeLists.txt @@ -132,7 +132,6 @@ target_sources(viamsdk rpc/dial.cpp rpc/server.cpp rpc/private/viam_grpc_channel.cpp - rpc/private/viam_rust_utils_stubs.cpp services/discovery.cpp services/generic.cpp services/mlmodel.cpp @@ -267,11 +266,15 @@ target_link_libraries(viamsdk PRIVATE Threads::Threads ) -# TODO(RSDK-10366): Currently, rust_utils is not published for windows, so don't link to it. +# TODO(RSDK-10366): Currently, rust_utils is not published for +# windows, so don't link to it. Instead, link a stub implementation +# that just calls `abort`. if (NOT WIN32) target_link_libraries(viamsdk PRIVATE viam_rust_utils ) +else() + target_sources(viamsdk PRIVATE rpc/private/viam_rust_utils_stubs.cpp) endif() if (APPLE) diff --git a/src/viam/sdk/rpc/private/viam_rust_utils_stubs.cpp b/src/viam/sdk/rpc/private/viam_rust_utils_stubs.cpp index f8f5bbdf7..caa78aef2 100644 --- a/src/viam/sdk/rpc/private/viam_rust_utils_stubs.cpp +++ b/src/viam/sdk/rpc/private/viam_rust_utils_stubs.cpp @@ -2,9 +2,6 @@ #include -// TODO(RSDK-10366): Currently, rust_utils is not published for windows -// so we just implement the associated entry points as `abort`. -#ifdef _WIN32 void* init_rust_runtime() { abort(); } @@ -20,4 +17,3 @@ void free_string(const char*) { char* dial(const char*, const char*, const char*, const char*, bool, float, void*) { abort(); } -#endif