Skip to content

Commit

Permalink
C++ SDK sanity checks now header/source version against rerun_c binar…
Browse files Browse the repository at this point in the history
…y version (#4330)

### What

* Fixes #3868
* Fixes left over to set generic package version from #4326  
* Yes there's two levels of regex'ing existing files now, but I'm happy
with it because it avoids etching the version into more places: The repo
cmake setup first figures out the rerun version by taking a look at
Cargo.toml and puts that into rerun.h which we commit. Then when doing
an install (in repo or outside!) we check rerun.h's version and put that
into the CMake Install shenanigans


Tests erro'red correctly when I still had a longer string, looked like
this:
```
-------------------------------------------------------------------------------
Scenario: RecordingStream can connect
      Given: a new RecordingStream
-------------------------------------------------------------------------------
/Users/andreas/dev/rerun-io/rerun/rerun_cpp/tests/recording_stream.cpp:433
...............................................................................

/Users/andreas/dev/rerun-io/rerun/rerun_cpp/tests/recording_stream.cpp:433: FAILED:
due to unexpected exception with message:
  Rerun_c SDK version and SDK header/source versions don't match match. Make
  sure to link against the correct version of the rerun_c library.
  Rerun_c version:
  re_sdk 0.11.0-alpha.1+dev [rustc 1.72.1 (d5c2e9c34 2023-09-13), LLVM 16.0.5]
  aarch64-apple-darwin
  SDK header/source version:
  0.11.0-alpha.1+dev
```


### Checklist
* [x] I have read and agree to [Contributor
Guide](https://github.com/rerun-io/rerun/blob/main/CONTRIBUTING.md) and
the [Code of
Conduct](https://github.com/rerun-io/rerun/blob/main/CODE_OF_CONDUCT.md)
* [x] I've included a screenshot or gif (if applicable)
* [x] I have tested [demo.rerun.io](https://demo.rerun.io/pr/4330) (if
applicable)
* [x] The PR title and labels are set such as to maximize their
usefulness for the next release's CHANGELOG

- [PR Build Summary](https://build.rerun.io/pr/4330)
- [Docs
preview](https://rerun.io/preview/72b1b7eedaa4934a8a34d1ab498c9b7752dd8125/docs)
<!--DOCS-PREVIEW-->
- [Examples
preview](https://rerun.io/preview/72b1b7eedaa4934a8a34d1ab498c9b7752dd8125/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://build.rerun.io/graphs/crates.html)
- [Wasm size tracking](https://build.rerun.io/graphs/sizes.html)
  • Loading branch information
Wumpf authored Nov 27, 2023
1 parent bf4b187 commit 7e6924f
Show file tree
Hide file tree
Showing 10 changed files with 71 additions and 11 deletions.
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();
} // 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

0 comments on commit 7e6924f

Please sign in to comment.