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

Spawn via $PATH 3: C and C++ implementations #3998

Merged
merged 13 commits into from
Oct 26, 2023
Merged

Conversation

teh-cmc
Copy link
Member

@teh-cmc teh-cmc commented Oct 25, 2023

  • Introduces standalone rr_spawn(rr_spawn_options) C API that allows one to start a Rerun Viewer process ready to listen for TCP connections, as well as the associated rr_recording_stream integration.
  • Introduces standalone rerun::spawn() C++ API that allows one to start a Rerun Viewer process ready to listen for TCP connections, as well as the associated RecordingStream integration.
23-10-25_12.03.39.patched.mp4

Spawn via $PATH series:


Checklist

  • I have read and agree to Contributor Guide and the Code of Conduct
  • I've included a screenshot or gif (if applicable)
  • I have tested demo.rerun.io (if applicable)
  • The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG

@teh-cmc teh-cmc added 🧑‍💻 dev experience developer experience (excluding CI) 🌊 C++ API C/C++ API specific include in changelog labels Oct 25, 2023
@teh-cmc teh-cmc changed the base branch from main to cmc/binary_spawn_2_rerunargs October 25, 2023 08:52
@teh-cmc teh-cmc force-pushed the cmc/binary_spawn_2_rerunargs branch from ec8de22 to b9dd5c8 Compare October 25, 2023 09:37
@teh-cmc teh-cmc force-pushed the cmc/binary_spawn_3_c_and_cpp branch from f62eec4 to f506eee Compare October 25, 2023 09:37
@teh-cmc teh-cmc marked this pull request as ready for review October 25, 2023 10:05
@teh-cmc teh-cmc added the do-not-merge Do not merge this PR label Oct 25, 2023
@teh-cmc teh-cmc force-pushed the cmc/binary_spawn_2_rerunargs branch from b9dd5c8 to c9f9336 Compare October 25, 2023 15:09
@teh-cmc teh-cmc force-pushed the cmc/binary_spawn_3_c_and_cpp branch from c714df8 to 3f41c3b Compare October 25, 2023 16:06
@Wumpf Wumpf self-requested a review October 26, 2023 09:40
Copy link
Member

@Wumpf Wumpf left a comment

Choose a reason for hiding this comment

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

almost good to go. Exited to get this in!

crates/rerun_c/src/rerun.h Show resolved Hide resolved
crates/rerun_c/src/rerun.h Show resolved Hide resolved
crates/rerun_c/src/lib.rs Show resolved Hide resolved
crates/rerun_c/src/rerun.h Show resolved Hide resolved
@@ -49,6 +49,7 @@ py-test = { cmd = "python -m pytest -vv rerun_py/tests/unit", depends_on = [
] }

cpp-build-all = { depends_on = [
"cpp-prepare",
Copy link
Member

Choose a reason for hiding this comment

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

isn't that in already implicitly via the other build actions?

Copy link
Member Author

Choose a reason for hiding this comment

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

$ just cpp-clean && just cpp-build-all
rm -rf build CMakeCache.txt CMakeFiles
pixi run cpp-build-all
Error: /home/cmc/dev/rerun-io/rerun/build is not a directory
error: Recipe `cpp-build-all` failed on line 39 with exit code 1

/// spawn_opts:
/// Configuration of the spawned process.
/// Refer to `rr_spawn_options` documentation for details.
/// Passing null is valid and will result in sane defaults.
Copy link
Member

Choose a reason for hiding this comment

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

"sane default" could mean anything though ;)

Copy link
Member Author

Choose a reason for hiding this comment

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

It won't crash and burn is my main point

Copy link
Member Author

Choose a reason for hiding this comment

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

Turned it into "recommended defaults" ✨

rerun_cpp/src/rerun/recording_stream.hpp Outdated Show resolved Hide resolved
rerun_cpp/src/rerun/spawn.cpp Outdated Show resolved Hide resolved
rerun_cpp/src/rerun/spawn.hpp Outdated Show resolved Hide resolved
teh-cmc added a commit that referenced this pull request Oct 26, 2023
Introduces standalone `rerun::spawn(SpawnOptions)` API that allows one
to start a Rerun Viewer process ready to listen for TCP connections, as
well as the associated `RecordingStream` integration.



https://github.com/rerun-io/rerun/assets/2910679/24eb5647-38c1-4049-b249-dc6e00e4ff54



---

Spawn via `$PATH` series:
- #3996
- #3997
- #3998
@teh-cmc teh-cmc force-pushed the cmc/binary_spawn_2_rerunargs branch from c9f9336 to db012ec Compare October 26, 2023 11:32
Base automatically changed from cmc/binary_spawn_2_rerunargs to main October 26, 2023 11:43
teh-cmc added a commit that referenced this pull request Oct 26, 2023
…mples (#3997)

- Get rid of the old thread-based `spawn` functionality.
- Redesign `RerunArgs` to get rid of the awful callback while still
dealing with Tokio's TLS shenanigans.
- Update all tests and examples.

The new `RerunArgs` combined with the new `spawn` from PATH now make for
a pretty nice experience:
```rust
let args = Args::parse();
let (rec, _serve_guard) = args.rerun.init("my_app")?;
// do stuff with rec
```

---

Spawn via `$PATH` series:
- #3996
- #3997
- #3998

---

- Fixes #2109
@teh-cmc teh-cmc force-pushed the cmc/binary_spawn_3_c_and_cpp branch from 3f41c3b to 6704c9f Compare October 26, 2023 11:52
@teh-cmc teh-cmc force-pushed the cmc/binary_spawn_3_c_and_cpp branch from ee2c98f to a49e28e Compare October 26, 2023 13:34
@teh-cmc teh-cmc removed the do-not-merge Do not merge this PR label Oct 26, 2023
@teh-cmc teh-cmc merged commit 2f0fce7 into main Oct 26, 2023
33 of 35 checks passed
@teh-cmc teh-cmc deleted the cmc/binary_spawn_3_c_and_cpp branch October 26, 2023 14:11
teh-cmc added a commit that referenced this pull request Oct 26, 2023
Last one.

Spawn via `$PATH` series:
- #3996
- #3997
- #3998
- #4013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌊 C++ API C/C++ API specific 🧑‍💻 dev experience developer experience (excluding CI) include in changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tracking issue: binary assets for everyone and everything Add spawn-like functionality to C++ and Rust
2 participants