Skip to content

Commit

Permalink
Optionally (by default) download & build stripped arrow as part of re…
Browse files Browse the repository at this point in the history
…run_sdk (#4051)

### What

* Fixes  #4028

Confirmed to be working on Linux, Mac and Windows. On windows also made
sure this works with debug runtime and experimented a bit with Visual
Studio setups. Also checked Windows against the external sample, see
rerun-io/cpp-example-opencv-eigen#8

Made improvements to our CMakeLists files overall. Notably we no longer
need `DOWNLOAD_EXTRACT_TIMESTAMP ON` to suppress warnings - idea from
rerun-io/cpp-example-opencv-eigen#6, setting a
range for the cmake version fixes it


Drawbacks:
* LOTS of cmake log spam during build
* a bit longer builds on the first run, seems to be a bit more
pronounced on Windows (no hard numbers available yet)

Advantages:
* no longer needed to install libarrow
* works on Windows debug builds (note that other libraries like OpenCV
might still have this issue)
* stable against all c++ library/compiler changes, no fear of weird
runtime errors because of stdlib mismatches
* more lightweight artifacts - on Windows one would have needed to ship
a large number of dlls, now none

It's still worth considering if we should ship prebuilt debug & release
arrow dlls instead.

TODO: We need to document this new behavior and how to turn it off in
the respective doc pages. By default the user no longer needs to install
libArrow with this.

Important CMake options that need documenting (can partially copy& paste
this!):
* `RERUN_ARROW_LINK_SHARED`
* exited before but now it's default OFF (EDIT:) on windows. This makes
it easier to relocate windows executable (don't need to copy Arrow.dll
around!). EDIT: On mac this made the address sanitizer crazy for me and
I reckon it's an unnecessary slowdown
* `RERUN_ARROW_EXTERNAL_PROJECT` (new!)
    * Default on
* If enabled we download Arrow 10.0.1 and build it in a as small as
possible fashion
    * If disabled we run `find_package` == old behavior
* `RERUN_C_LIB`
   * where to find the static c library that rerun_sdk links against
   * in repo defaults to cargo built
   * outside repo defaults to /lib/ folder + platform artifact name

### 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/{{
pr.number }}) (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/{{ pr.number }})
- [Docs preview](https://rerun.io/preview/{{ pr.commit }}/docs)
<!--DOCS-PREVIEW-->
- [Examples preview](https://rerun.io/preview/{{ pr.commit }}/examples)
<!--EXAMPLES-PREVIEW-->
- [Recent benchmark results](https://ref.rerun.io/dev/bench/)
- [Wasm size tracking](https://ref.rerun.io/dev/sizes/)

---------

Co-authored-by: Clement Rey <[email protected]>
  • Loading branch information
Wumpf and teh-cmc authored Oct 29, 2023
1 parent 2ec5ce7 commit c0521ee
Show file tree
Hide file tree
Showing 20 changed files with 175 additions and 81 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/on_push_main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -340,13 +340,13 @@ jobs:
## CMake fetch-content for C++ SDK
```
include(FetchContent)
FetchContent_Declare(rerun_sdk DOWNLOAD_EXTRACT_TIMESTAMP ON URL https://build.rerun.io/commit/${{ env.SHORT_SHA }}/rerun_cpp_sdk.zip)
FetchContent_Declare(rerun_sdk URL https://build.rerun.io/commit/${{ env.SHORT_SHA }}/rerun_cpp_sdk.zip)
FetchContent_MakeAvailable(rerun_sdk)
```
or
```
include(FetchContent)
FetchContent_Declare(rerun_sdk DOWNLOAD_EXTRACT_TIMESTAMP ON URL https://github.com/rerun-io/rerun/releases/download/prerelease/rerun_cpp_sdk.zip)
FetchContent_Declare(rerun_sdk URL https://github.com/rerun-io/rerun/releases/download/prerelease/rerun_cpp_sdk.zip)
FetchContent_MakeAvailable(rerun_sdk)
```
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ crates/re_types_builder/source_hash.txt
*.a
*.bin
*.o
**/arrow/
**/build/
**/CMakeFiles/
**/CMakeCache.txt
Expand Down
4 changes: 2 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
cmake_minimum_required(VERSION 3.16)
cmake_minimum_required(VERSION 3.16...3.27)

project(rerun_cpp_proj LANGUAGES CXX)

Expand Down Expand Up @@ -61,7 +61,7 @@ endif()
# If using MSVC, always enable multi-process compiling for all targets.
# Note that this setting is repeated for rerun_sdk's CMakeLists.txt since it should also work stand-alone.
if(MSVC)
add_compile_options(/MP)
add_compile_options("/MP")
endif()

# Arrow requires a C++17 compliant compiler.
Expand Down
4 changes: 2 additions & 2 deletions crates/re_sdk/src/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,15 @@ impl ThreadLocalRecording {
}
}

#[cfg(target_os = "macos")]
#[cfg(any(target_os = "macos", target_os = "windows"))]
impl Drop for ThreadLocalRecording {
fn drop(&mut self) {
if let Some(stream) = self.stream.take() {
// Work-around for https://github.com/rerun-io/rerun/issues/2889
// Calling drop on `self.stream` will panic the calling thread.
// But we want to make sure we don't loose the data in the stream.
// So how?
re_log::warn!("Using thread-local RecordingStream on macOS can result in data loss because of https://github.com/rerun-io/rerun/issues/3937");
re_log::warn!("Using thread-local RecordingStream on macOS & Windows can result in data loss because of https://github.com/rerun-io/rerun/issues/3937");

// Give the batcher and sink threads a chance to process the data.
std::thread::sleep(std::time::Duration::from_millis(500));
Expand Down
2 changes: 1 addition & 1 deletion crates/re_viewer/data/quick_start_guides/cpp_connect.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ If you're using CMake, you can add the following to your `CMakeLists.txt`:

```cmake
include(FetchContent)
FetchContent_Declare(rerun_sdk DOWNLOAD_EXTRACT_TIMESTAMP ON URL https://github.com/rerun-io/rerun/releases/download/prerelease/rerun_cpp_sdk.zip) # TODO(#3962): update link
FetchContent_Declare(rerun_sdk URL https://github.com/rerun-io/rerun/releases/download/prerelease/rerun_cpp_sdk.zip) # TODO(#3962): update link
FetchContent_MakeAvailable(rerun_sdk)
```

Expand Down
2 changes: 1 addition & 1 deletion docs/code-examples/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
cmake_minimum_required(VERSION 3.16)
cmake_minimum_required(VERSION 3.16...3.27)

# Setup builds for examples
file(GLOB sources_list true ${CMAKE_CURRENT_SOURCE_DIR}/*.cpp)
Expand Down
11 changes: 10 additions & 1 deletion docs/code-examples/roundtrips.py
Original file line number Diff line number Diff line change
Expand Up @@ -153,12 +153,21 @@ def main() -> None:
if not args.no_py:
active_languages.append("py")

# Running CMake in parallel causes failures during rerun_sdk & arrow build.
# TODO(andreas): Tell cmake in a single command to build everything at once.
if not args.no_cpp_build:
for example in examples:
example_opt_out_entirely = opt_out_run.get(example, [])
if "cpp" in example_opt_out_entirely:
continue
run_example(example, "cpp", args)

with multiprocessing.Pool() as pool:
jobs = []
for example in examples:
example_opt_out_entirely = opt_out_run.get(example, [])
for language in active_languages:
if language in example_opt_out_entirely:
if language in example_opt_out_entirely or language == "cpp": # cpp already processed in series.
continue
job = pool.apply_async(run_example, (example, language, args))
jobs.append(job)
Expand Down
6 changes: 3 additions & 3 deletions docs/content/getting-started/cpp.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ add_executable(example_minimal main.cpp)
You can add Rerun to your project using `FetchContent`
```cmake
include(FetchContent)
FetchContent_Declare(rerun_sdk DOWNLOAD_EXTRACT_TIMESTAMP ON URL https://github.com/rerun-io/rerun/releases/download/prerelease/rerun_cpp_sdk.zip) # TODO(#3962): update link
FetchContent_Declare(rerun_sdk URL https://github.com/rerun-io/rerun/releases/download/prerelease/rerun_cpp_sdk.zip) # TODO(#3962): update link
FetchContent_MakeAvailable(rerun_sdk)
```
This will download a bundle with pre-built Rerun C static libraries for most desktop platforms, all Rerun C++ sources and headers, as well as CMake build instructions for them.
Expand All @@ -49,14 +49,14 @@ target_link_libraries(example_minimal PRIVATE rerun_sdk)

Combining the above, a minimal self-contained `CMakeLists.txt` looks like:
```cmake
cmake_minimum_required(VERSION 3.16)
cmake_minimum_required(VERSION 3.16...3.27)
project(example_minimal LANGUAGES CXX)
add_executable(example_minimal main.cpp)
# Download the rerun_sdk
include(FetchContent)
FetchContent_Declare(rerun_sdk DOWNLOAD_EXTRACT_TIMESTAMP ON URL https://github.com/rerun-io/rerun/releases/download/prerelease/rerun_cpp_sdk.zip) # TODO(#3962): update link
FetchContent_Declare(rerun_sdk URL https://github.com/rerun-io/rerun/releases/download/prerelease/rerun_cpp_sdk.zip) # TODO(#3962): update link
FetchContent_MakeAvailable(rerun_sdk)
# Rerun requires at least C++17, but it should be compatible with newer versions.
Expand Down
4 changes: 2 additions & 2 deletions examples/cpp/clock/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
cmake_minimum_required(VERSION 3.16)
cmake_minimum_required(VERSION 3.16...3.27)

# If you use the example outside of the Rerun SDK you need to specify
# where the rerun_c build is to be found by setting the `RERUN_CPP_URL` variable.
Expand All @@ -16,7 +16,7 @@ else()

# Download the rerun_sdk
include(FetchContent)
FetchContent_Declare(rerun_sdk DOWNLOAD_EXTRACT_TIMESTAMP YES URL ${RERUN_CPP_URL})
FetchContent_Declare(rerun_sdk URL ${RERUN_CPP_URL})
FetchContent_MakeAvailable(rerun_sdk)

# Rerun requires at least C++17, but it should be compatible with newer versions.
Expand Down
4 changes: 2 additions & 2 deletions examples/cpp/custom_component_adapter/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
cmake_minimum_required(VERSION 3.16)
cmake_minimum_required(VERSION 3.16...3.27)

# If you use the example outside of the Rerun SDK you need to specify
# where the rerun_c build is to be found by setting the `RERUN_CPP_URL` variable.
Expand All @@ -16,7 +16,7 @@ else()

# Download the rerun_sdk
include(FetchContent)
FetchContent_Declare(rerun_sdk DOWNLOAD_EXTRACT_TIMESTAMP YES URL ${RERUN_CPP_URL})
FetchContent_Declare(rerun_sdk URL ${RERUN_CPP_URL})
FetchContent_MakeAvailable(rerun_sdk)

# Rerun requires at least C++17, but it should be compatible with newer versions.
Expand Down
4 changes: 2 additions & 2 deletions examples/cpp/dna/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
cmake_minimum_required(VERSION 3.16)
cmake_minimum_required(VERSION 3.16...3.27)

# If you use the example outside of the Rerun SDK you need to specify
# where the rerun_c build is to be found by setting the `RERUN_CPP_URL` variable.
Expand All @@ -16,7 +16,7 @@ else()

# Download the rerun_sdk
include(FetchContent)
FetchContent_Declare(rerun_sdk DOWNLOAD_EXTRACT_TIMESTAMP YES URL ${RERUN_CPP_URL})
FetchContent_Declare(rerun_sdk URL ${RERUN_CPP_URL})
FetchContent_MakeAvailable(rerun_sdk)

# Rerun requires at least C++17, but it should be compatible with newer versions.
Expand Down
4 changes: 2 additions & 2 deletions examples/cpp/minimal/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
cmake_minimum_required(VERSION 3.16)
cmake_minimum_required(VERSION 3.16...3.27)

# If you use the example outside of the Rerun SDK you need to specify
# where the rerun_c build is to be found by setting the `RERUN_CPP_URL` variable.
Expand All @@ -17,7 +17,7 @@ else()

# Download the rerun_sdk
include(FetchContent)
FetchContent_Declare(rerun_sdk DOWNLOAD_EXTRACT_TIMESTAMP YES URL ${RERUN_CPP_URL})
FetchContent_Declare(rerun_sdk URL ${RERUN_CPP_URL})
FetchContent_MakeAvailable(rerun_sdk)

# Rerun requires at least C++17, but it should be compatible with newer versions.
Expand Down
4 changes: 2 additions & 2 deletions examples/cpp/spawn_viewer/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
cmake_minimum_required(VERSION 3.16)
cmake_minimum_required(VERSION 3.16...3.27)

# If you use the example outside of the Rerun SDK you need to specify
# where the rerun_c build is to be found by setting the `RERUN_CPP_URL` variable.
Expand All @@ -16,7 +16,7 @@ else()

# Download the rerun_sdk
include(FetchContent)
FetchContent_Declare(rerun_sdk DOWNLOAD_EXTRACT_TIMESTAMP YES URL ${RERUN_CPP_URL})
FetchContent_Declare(rerun_sdk URL ${RERUN_CPP_URL})
FetchContent_MakeAvailable(rerun_sdk)

# Rerun requires at least C++17, but it should be compatible with newer versions.
Expand Down
1 change: 0 additions & 1 deletion pixi.toml
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ cpp-test = { cmd = "export RERUN_STRICT=1 && ./build/rerun_cpp/tests/rerun_sdk_t
] }

[dependencies]
arrow-cpp = "10.0.1"
attrs = ">=23.1.0"
blackdoc = "0.3.8"
clang-tools = ">=15,<16"
Expand Down
Loading

0 comments on commit c0521ee

Please sign in to comment.