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

C++ SDK sanity checks now header/source version against rerun_c binary version #4330

Merged
merged 8 commits into from
Nov 27, 2023
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
2 changes: 1 addition & 1 deletion crates/rerun_c/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ pub struct CError {
#[no_mangle]
pub extern "C" fn rr_version_string() -> *const c_char {
static VERSION: Lazy<CString> =
Lazy::new(|| CString::new(re_sdk::build_info().to_string()).unwrap()); // unwrap: there won't be any NUL bytes in the string
Lazy::new(|| CString::new(re_sdk::build_info().version.to_string()).unwrap()); // unwrap: there won't be any NUL bytes in the string

VERSION.as_ptr()
}
Expand Down
9 changes: 9 additions & 0 deletions crates/rerun_c/src/rerun.h
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,16 @@ typedef struct rr_error {
// ----------------------------------------------------------------------------
// Functions:

/// Returns the version of the Rerun C SDK.
///
/// This should match the string returned by `rr_version_string`.
/// If not, the SDK's binary and the C header are out of sync.
#define RERUN_SDK_HEADER_VERSION "@RERUN_VERSION@"

/// Returns a human-readable version string of the Rerun C SDK.
///
/// This should match the string in `RERUN_SDK_HEADER_VERSION`.
/// If not, the SDK's binary and the C header are out of sync.
extern const char* rr_version_string(void);

/// Spawns a new Rerun Viewer process from an executable available in PATH, ready to
Expand Down
10 changes: 7 additions & 3 deletions rerun_cpp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,12 @@ install(EXPORT rerun_sdkTargets

include(CMakePackageConfigHelpers)

# TODO(#3868): Extract version from rerun.h
set(RERUN_VERSION 0.11.0)
# Extract the version from rerun.h.
# Intentionally only grab major.minor.patch, not the full version, since version file can't handle it otherwise.
file(READ "${CMAKE_CURRENT_SOURCE_DIR}/src/rerun/c/rerun.h" RERUN_H_CONTENTS)
string(REGEX MATCH "\n#define RERUN_SDK_HEADER_VERSION \"([0-9]+.[0-9]+.[0-9]+)" _ ${RERUN_H_CONTENTS})
set(RERUN_INSTALL_VERSION ${CMAKE_MATCH_1})
message(STATUS "Rerun SDK install version: ${RERUN_INSTALL_VERSION}")

# Package config file, so find_package(rerun_sdk) produces a target.
configure_package_config_file(${CMAKE_CURRENT_SOURCE_DIR}/Config.cmake.in
Expand All @@ -165,7 +169,7 @@ configure_package_config_file(${CMAKE_CURRENT_SOURCE_DIR}/Config.cmake.in
# Version file for find_package.
write_basic_package_version_file(
${CMAKE_CURRENT_BINARY_DIR}/rerun_sdkConfigVersion.cmake
VERSION ${RERUN_VERSION}
VERSION ${RERUN_INSTALL_VERSION}
COMPATIBILITY ExactVersion
)

Expand Down
9 changes: 9 additions & 0 deletions rerun_cpp/src/rerun/c/rerun.h

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

5 changes: 0 additions & 5 deletions rerun_cpp/src/rerun/config.cpp
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
#include "config.hpp"

#include <algorithm>
#include <cstdlib>
#include <string>

namespace rerun {

RerunGlobalConfig& RerunGlobalConfig::instance() {
Expand All @@ -12,5 +8,4 @@ namespace rerun {
}

RerunGlobalConfig::RerunGlobalConfig() : default_enabled(true) {}

} // namespace rerun
5 changes: 3 additions & 2 deletions rerun_cpp/src/rerun/error.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,9 @@ namespace rerun {
/// Category codes are used to group errors together, but are never returned directly.
enum class ErrorCode : uint32_t {
Ok = 0x0000'0000,
OutOfMemory = 0x0000'0001,
NotImplemented = 0x0000'0002,
OutOfMemory,
NotImplemented,
SdkVersionMismatch,

// Invalid argument errors.
_CategoryArgument = 0x0000'0010,
Expand Down
5 changes: 5 additions & 0 deletions rerun_cpp/src/rerun/recording_stream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include "components/instance_key.hpp"
#include "config.hpp"
#include "data_cell.hpp"
#include "sdk_info.hpp"
#include "string_utils.hpp"

#include <arrow/buffer.h>
Expand Down Expand Up @@ -30,6 +31,8 @@ namespace rerun {

RecordingStream::RecordingStream(std::string_view app_id, StoreKind store_kind)
: _store_kind(store_kind) {
check_binary_and_header_version_match().handle();

rr_store_info store_info;
store_info.application_id = detail::to_rr_string(app_id);
store_info.store_kind = store_kind_to_c(store_kind);
Expand All @@ -54,6 +57,8 @@ namespace rerun {

RecordingStream::RecordingStream(uint32_t id, StoreKind store_kind)
: _id(id), _store_kind(store_kind) {
check_binary_and_header_version_match().handle();

rr_error status = {};
this->_enabled = rr_recording_stream_is_enabled(this->_id, &status);
Error(status).handle();
Expand Down
25 changes: 25 additions & 0 deletions rerun_cpp/src/rerun/sdk_info.cpp
Original file line number Diff line number Diff line change
@@ -1,8 +1,33 @@
#include "sdk_info.hpp"
#include "c/rerun.h"

#include <cstring> // strcmp
#include <string>

#include "c/rerun.h"

namespace rerun {
const char* version_string() {
return rr_version_string();
}

Error check_binary_and_header_version_match() {
const char* binary_version = version_string();

if (strcmp(binary_version, RERUN_SDK_HEADER_VERSION) == 0) {
return Error::ok();
} else {
return Error(
ErrorCode::SdkVersionMismatch,
std::string(
"Rerun_c SDK version and SDK header/source versions don't match. "
"Make sure to link against the correct version of the rerun_c library.\n"
"rerun_c binary version:\n"
)
.append(binary_version)
.append("\nrerun_c header version:\n")
.append(RERUN_SDK_HEADER_VERSION)
);
}
}
} // namespace rerun
9 changes: 9 additions & 0 deletions rerun_cpp/src/rerun/sdk_info.hpp
Original file line number Diff line number Diff line change
@@ -1,7 +1,16 @@
// General information about the SDK.
#pragma once

#include "error.hpp"

namespace rerun {
/// The Rerun C++ SDK version as a human-readable string.
const char* version_string();

/// Internal check whether the version reported by the rerun_c binary matches `sdk_version_string`.
///
/// This method is called on various C++ API entry points, calling `Error::handle` on the return value.
/// There is no need to call this method yourself unless you want to ensure that rerun_c binary and
/// rerun_c header versions match ahead of time.
Error check_binary_and_header_version_match();
Wumpf marked this conversation as resolved.
Show resolved Hide resolved
} // namespace rerun
3 changes: 3 additions & 0 deletions rerun_cpp/src/rerun/spawn.cpp
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
#include "spawn.hpp"
#include "c/rerun.h"
#include "sdk_info.hpp"

namespace rerun {
Error spawn(const SpawnOptions& options) {
RR_RETURN_NOT_OK(check_binary_and_header_version_match());

rr_spawn_options rerun_c_options = {};
options.fill_rerun_c_struct(rerun_c_options);
rr_error error = {};
Expand Down
Loading