Skip to content

[RSDK-10385] Windows build system improvements and rust_utils workarounds #402

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

Merged
merged 4 commits into from
Apr 7, 2025
Merged
Show file tree
Hide file tree
Changes from all 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
28 changes: 17 additions & 11 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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)


Expand Down Expand Up @@ -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")
Expand All @@ -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
"$<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}>"
"$<INSTALL_INTERFACE:${CMAKE_INSTALL_LIBDIR}>"
)

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
Expand Down
20 changes: 18 additions & 2 deletions conanfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]

Expand All @@ -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)

Expand All @@ -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"]
11 changes: 10 additions & 1 deletion src/viam/sdk/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -267,10 +267,19 @@ 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. 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)
target_link_libraries(viamsdk PUBLIC "-framework Security")
Expand Down
5 changes: 5 additions & 0 deletions src/viam/sdk/common/private/service_helper.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,13 @@ class ServiceHelper : public ServiceHelperBase {
}

private:
#if __cplusplus >= 201703L
template <typename Callable, typename... Args>
using is_void_result = std::is_void<std::invoke_result_t<Callable, Args...>>;
#else
template <typename Callable, typename... Args>
using is_void_result = std::is_void<std::result_of_t<Callable(Args...)>>;
#endif

// Implementation of `invoke_` for a Callable returning non-void,
// presumably an error return, which we return as a
Expand Down
10 changes: 10 additions & 0 deletions src/viam/sdk/module/service.cpp
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
#ifdef _WIN32
#define NOMINMAX
// clang-format off
// Otherwise clang-format tries to alphabetize these headers,
// but `winsock2.h` should definitely precede `windows.h`.
#include <winsock2.h>
Copy link
Member Author

Choose a reason for hiding this comment

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

I don't love this. I'm surprised the gRPC headers don't handle this for themselves, but without this this file doesn't compile.

The right way to do this would be with a global config precompiled header that got pulled into every .cpp we have, but I don't want to spend time on that right now, so I will file a follow-up ticket.

Copy link
Member Author

Choose a reason for hiding this comment

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

#include <windows.h>
// clang-format on
#endif

#include <viam/sdk/module/service.hpp>

#include <exception>
Expand Down
11 changes: 1 addition & 10 deletions src/viam/sdk/rpc/dial.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,8 @@
#include <viam/api/robot/v1/robot.pb.h>
#include <viam/sdk/common/exception.hpp>
#include <viam/sdk/rpc/private/viam_grpc_channel.hpp>
#include <viam/sdk/rpc/private/viam_rust_utils.h>

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 {

Expand Down
21 changes: 21 additions & 0 deletions src/viam/sdk/rpc/private/viam_rust_utils.h
Original file line number Diff line number Diff line change
@@ -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
19 changes: 19 additions & 0 deletions src/viam/sdk/rpc/private/viam_rust_utils_stubs.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#include <viam/sdk/rpc/private/viam_rust_utils.h>

#include <cstdlib>

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();
}
Loading