-
Notifications
You must be signed in to change notification settings - Fork 225
Simplify the use of C/C++ API #942
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
base: main
Are you sure you want to change the base?
Conversation
The C++ Dataflow example doesn't seem to work on Linux |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is good! This is a shame that there is no tools like cargo-dist
to distribute libraries.
Just make sure the CI is passing on Ubuntu
# - os: windows-latest | ||
# target: x86_64-pc-windows-msvc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What would be the workaround to make it possible for Windows CXX users to use dora then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not familiar with the linking process on Windows, nor do I know how a library is installed on Windows.
The really error message is bfd plugin: LLVM gold plugin has failed to create LTO module: Opaque pointers are only supported in -opaque-po inters mode (Producer: 'LLVM19.1.5-rust-1.84.1-stable' Reader: 'LLVM 14.0.0') This is because for Ubuntu 22.04, the clang++ version it comes with is 14, and the related plug-ins are also 14, but for rust 1.84.1, it is built based on LLVM 19, so rust requires tools compatible with LLVM 19 to link libraries, which is not easy for Ubuntu 22.04. |
I'm slightly confused why the example doesn't work on ubuntu 22 when it uses to work before. Also, i think |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for this!
I understand it wasn't easy especially as a lot of the heavy lifting is done within the example folder.
Could be nice to do a follow up PR and makes the compilation of the cxx library be made cleaner.
examples/c++-dataflow/run.rs
Outdated
use std::{ | ||
env::consts::{DLL_PREFIX, DLL_SUFFIX, EXE_SUFFIX}, | ||
path::Path, | ||
// process::Command, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we remove this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok,I am sorry for forget delete this
|
||
let dataflow = Path::new("dataflow.yml").to_owned(); | ||
build_package("dora-runtime").await?; | ||
archive_node_bridge(root, build_dir).await?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's keep it like this but i'm really not a fan of having to do all this heavy lifting within the example...
|
||
let static_lib = root | ||
.join("target") | ||
.join("debug") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this linked to the debug version?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is because in the run.rs of the example, debug has been specified, and the compilation option --release has no effect.I can fix it.
dora/examples/c++-dataflow/run.rs
Line 95 in a30e760
root.join("target").join("debug").to_str().unwrap(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you create a c++ example with a readme on how to use this, because it isn't quite clear to me
install_libdora.sh
Outdated
echo " => PLATFORM: $PLATFORM" | ||
|
||
echo "==> Fetching latest Dora release info from GitHub..." | ||
LATEST_JSON=$(curl -s https://api.github.com/repos/starlitxiling/dora-rerun/releases/latest) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry can we update this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, I will do this
case "$RUNNER_OS" in | ||
"Linux" | "macOS") | ||
cp target/${{ matrix.target }}/release/*.a dist/lib/ || echo "No libdora_node_api_c.a found" | ||
cp target/${{ matrix.target }}/debug/libdora_node_api_cxx.a dist/lib || echo "No libdora_node_api_cxx.a found" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we using debug or release?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need release version
Sorry for following up on this question just now. Since |
Can we make a standalone C++ release script, please? I don't think we can merge this as is as it's very confusing. |
Yes, we made some unnecessary changes to the example, I hate this, so I will try other ways to achieve this goal.But I'm out of ideas for now, so any help would be appreciated! |
You can create a standalone script independant of the example. Could you check https://github.com/dora-rs/dora/blob/7c44e7a2e60dfa11a1557615a7efbc4ebb418f48/libraries/core/src/bin/generate_schema.rs cargo run -p dora-core generate_schemas |
This PR packages the header files and library files needed to use the Dora c/c++ api. I tested the whole process on macos and it is normal.
For the windows platform, there are some problems in the linking process, so I cancelled it.
I provide a script for users to install the files needed to use the api. Users only need to add
I/usr/local/include/dora
-L/usr/local/lib -ldora_node_api_c
or-L/usr/local/lib -ldora_node_api_cxx
when compiling the command, without having to consider specifying the path of the header file or library file.I think we need to add in the documentation what is needed to use the API, such as
pip install dora-rs
for the python API