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

Rust integration: Adding variables & clients. #102

Merged
merged 12 commits into from
Nov 3, 2023
19 changes: 0 additions & 19 deletions cmake/cxxrs.cmake

This file was deleted.

94 changes: 71 additions & 23 deletions everestrs/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,35 +1,83 @@
include(cxxrs)

find_program(CXXBRIDGE cxxbridge PATHS "$ENV{HOME}/.cargo/bin/")
if (CXXBRIDGE STREQUAL "CXXBRIDGE-NOTFOUND")
message("Could not find cxxbridge, trying to install with `cargo install cxxbridge-cmd'")
find_program(CARGO cargo PATHS "$ENV{HOME}/.cargo/bin/")
if (CARGO STREQUAL "CARGO-NOTFOUND")
message(FATAL_ERROR "Requires cargo available in path, install via rustup https://rustup.rs/")
endif()
execute_process(COMMAND ${CARGO} install cxxbridge-cmd --version 1.0.107)
find_program(CXXBRIDGE cxxbridge PATHS "$ENV{HOME}/.cargo/bin/")
endif()

emit_cxxrs_header()
emit_cxxrs_for_module(everestrs)
set(CXXBRIDGE_BINARY ${CMAKE_CURRENT_BINARY_DIR}/cargo/bin/cxxbridge)
if (NOT EXISTS ${CXXBRIDGE_BINARY})
message(STATUS "Fetching rust cxxbridge cli tool")
execute_process(
COMMAND
cargo install cxxbridge-cmd --root ${CMAKE_CURRENT_BINARY_DIR}/cargo
WORKING_DIRECTORY
${CMAKE_CURRENT_BINARY_DIR}
RESULT_VARIABLE
CARGO_INSTALL_RESULT
OUTPUT_QUIET
ERROR_VARIABLE
CARGO_INSTALL_ERROR
)

if (CARGO_INSTALL_RESULT)
message(FATAL_ERROR "cargo install cxxbridge-cmd failed with:\n${CARGO_INSTALL_ERROR}")
endif ()
endif ()

set(CXXBRIDGE_OUTPUT_DIR ${CMAKE_CURRENT_BINARY_DIR}/cxxbridge)
set(EVERESTRS_FFI_HEADER ${CXXBRIDGE_OUTPUT_DIR}/lib.rs.h)
set(EVERESTRS_FFI_SOURCE ${CXXBRIDGE_OUTPUT_DIR}/lib.rs.cc)
set(CXXBRIDGE_HEADER ${CXXBRIDGE_OUTPUT_DIR}/rust.h)

add_custom_command(
OUTPUT
${CXXBRIDGE_OUTPUT_DIR}
${EVERESTRS_FFI_HEADER}
${EVERESTRS_FFI_SOURCE}
${CXXBRIDGE_HEADER}
COMMAND
${CMAKE_COMMAND} -E make_directory ${CXXBRIDGE_OUTPUT_DIR}
COMMAND
${CXXBRIDGE_BINARY} --header -o ${CXXBRIDGE_HEADER}
COMMAND
${CXXBRIDGE_BINARY} ${CMAKE_CURRENT_SOURCE_DIR}/everestrs/src/lib.rs --header -o ${EVERESTRS_FFI_HEADER}
COMMAND
${CXXBRIDGE_BINARY} ${CMAKE_CURRENT_SOURCE_DIR}/everestrs/src/lib.rs -o ${EVERESTRS_FFI_SOURCE}
DEPENDS
everestrs/src/lib.rs
COMMENT "Generating cxxbridge glue for everestrs"
VERBATIM
DEPENDS
${CXXBRIDGE_BINARY}
)

add_library(everestrs_sys STATIC
${CMAKE_CURRENT_BINARY_DIR}/cxxbridge/everestrs/lib.rs.cc
everestrs_sys/everestrs_sys.cpp
${EVERESTRS_FFI_SOURCE}
everestrs_sys/everestrs_sys.cpp
)
add_library(everest::everestrs_sys ALIAS everestrs_sys)

target_include_directories(everestrs_sys PRIVATE
${CMAKE_CURRENT_SOURCE_DIR}
${CMAKE_CURRENT_BINARY_DIR}/cxxbridge
set_property(
TARGET
everestrs_sys
PROPERTY
EVERESTRS_DIR ${CMAKE_CURRENT_SOURCE_DIR}/everestrs
)

set_property(
TARGET
everestrs_sys
PROPERTY
EVERESTRS_DIR ${CMAKE_CURRENT_SOURCE_DIR}/everestrs
)

target_include_directories(everestrs_sys
PRIVATE
${CMAKE_CURRENT_SOURCE_DIR}
${CMAKE_CURRENT_BINARY_DIR}
)

# This is a requirement that linking works on systems enforcing PIE.
# This is a requirement that linking works on systems enforcing PIE
# FIXME (aw): investicate why this is really necessary
set_property(TARGET everestrs_sys PROPERTY POSITION_INDEPENDENT_CODE ON)
target_link_libraries(everestrs_sys
PRIVATE
everest::framework
everest::log
everest::framework
everest::log
)

install(TARGETS everestrs_sys LIBRARY)
44 changes: 24 additions & 20 deletions everestrs/everestrs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,26 +4,30 @@

## Trying it out

- Install rust as outlined on <https://rustup.rs/>, which should just be this
one line: `curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh`
- Built your workspace as outlined in `everest-core` README, make sure to tell
cMake to enable `EVEREST_ENABLE_RS_SUPPORT`. Note, that the Rust code relies
on being built in a workspace where `make install` was run once.
- You can now try building the code, but it will not do anything: `cd everestrs
&& cargo build --all`
- If you want to play with a node, check out `https://github.com/EVerest/everest-core/pull/344` in your workspace and run make install.
- Go to `everest-core/modules/RsSomkeTest` and run `cargo build` there.
- There is no support for building or installing Rust modules with cMake
currently, so let's fake the installation:
- Go to `everest-core/build/dist/libexec/everest/modules` and create the stuff needed for a module:
~~~bash
mkdir RsSmokeTest
ln -s ../../../../../../modules/RsSmokeTest/target/debug/smoke_test RsSmokeTest
ln -s ../../../../../../modules/RsSmokeTest/manifest.yaml .
~~~
- You should now be able to configure the `RsSmokeTest` module in your config
YAML.
- Install rust as outlined on <https://rustup.rs/>, which should just be this

Check warning on line 7 in everestrs/everestrs/README.md

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

everestrs/everestrs/README.md#L7

[list-item-bullet-indent] Incorrect indentation before bullet: remove 2 spaces

Check warning on line 7 in everestrs/everestrs/README.md

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

everestrs/everestrs/README.md#L7

[list-item-indent] Incorrect list-item indent: add 2 spaces
one line: `curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh`

Check warning on line 8 in everestrs/everestrs/README.md

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

everestrs/everestrs/README.md#L8

[list-item-spacing] Missing new line after list item
- Built your workspace as outlined in `everest-core` README, make sure to tell

Check warning on line 9 in everestrs/everestrs/README.md

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

everestrs/everestrs/README.md#L9

[list-item-bullet-indent] Incorrect indentation before bullet: remove 2 spaces

Check warning on line 9 in everestrs/everestrs/README.md

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

everestrs/everestrs/README.md#L9

[list-item-indent] Incorrect list-item indent: add 2 spaces
cMake to enable `EVEREST_ENABLE_RS_SUPPORT`. Note, that the Rust code relies
on being built in a workspace where `make install` was run once.

Check warning on line 11 in everestrs/everestrs/README.md

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

everestrs/everestrs/README.md#L11

[list-item-spacing] Missing new line after list item
- You can now try building the code, but it will not do anything: `cd everestrs

Check warning on line 12 in everestrs/everestrs/README.md

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

everestrs/everestrs/README.md#L12

[list-item-bullet-indent] Incorrect indentation before bullet: remove 2 spaces

Check warning on line 12 in everestrs/everestrs/README.md

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

everestrs/everestrs/README.md#L12

[list-item-indent] Incorrect list-item indent: add 2 spaces
&& cargo build --all`

Check warning on line 13 in everestrs/everestrs/README.md

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

everestrs/everestrs/README.md#L13

[list-item-spacing] Missing new line after list item
- You should now be able to configure the `RsExample` or `RsExampleUser` modules in your config

Check warning on line 14 in everestrs/everestrs/README.md

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

everestrs/everestrs/README.md#L14

[list-item-bullet-indent] Incorrect indentation before bullet: remove 2 spaces

Check warning on line 14 in everestrs/everestrs/README.md

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

everestrs/everestrs/README.md#L14

[list-item-indent] Incorrect list-item indent: add 2 spaces
YAML.

## Differences to other EVerest language wrappers

- The `enable_external_mqtt` is ignored for Rust modules. If you want to interact

Check warning on line 19 in everestrs/everestrs/README.md

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

everestrs/everestrs/README.md#L19

[list-item-bullet-indent] Incorrect indentation before bullet: remove 2 spaces

Check warning on line 19 in everestrs/everestrs/README.md

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

everestrs/everestrs/README.md#L19

[list-item-indent] Incorrect list-item indent: add 2 spaces
with MQTT externally, just pull an external mqtt module (for example the
really excellent [rumqttc](https://docs.rs/rumqttc/latest/rumqttc/)) crate
into your module and use it directly. This is a concious decision to future
proof, should everst at some point move to something different than MQTT as
transport layer and for cleaner abstraction.

## Status

This code is currently only supporting providing an interface to be implemented, i.e. no variables publish or receiving and no calling of other interfaces. Those features are straightforward, quick and easy to implement, but for now this is probably enough to iron out the integration questions.
Full support for requiring and providing interfaces is implemented, missing
currently is:

- Support for EVerest logging

Check warning on line 31 in everestrs/everestrs/README.md

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

everestrs/everestrs/README.md#L31

[list-item-bullet-indent] Incorrect indentation before bullet: remove 2 spaces

Check warning on line 31 in everestrs/everestrs/README.md

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

everestrs/everestrs/README.md#L31

[list-item-indent] Incorrect list-item indent: add 2 spaces
- Support for Configuration options of Modules

Check warning on line 32 in everestrs/everestrs/README.md

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

everestrs/everestrs/README.md#L32

[list-item-bullet-indent] Incorrect indentation before bullet: remove 2 spaces

Check warning on line 32 in everestrs/everestrs/README.md

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

everestrs/everestrs/README.md#L32

[list-item-indent] Incorrect list-item indent: add 2 spaces
- Support for implementations with `max_connections != 1` or `min_connections != 1`

Check warning on line 33 in everestrs/everestrs/README.md

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

everestrs/everestrs/README.md#L33

[list-item-bullet-indent] Incorrect indentation before bullet: remove 2 spaces

Check warning on line 33 in everestrs/everestrs/README.md

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

everestrs/everestrs/README.md#L33

[list-item-indent] Incorrect list-item indent: add 2 spaces
19 changes: 17 additions & 2 deletions everestrs/everestrs/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ struct Libraries {
}

fn find_everest_workspace_root() -> PathBuf {
if let Ok(everest_framework_dir) = env::var("EVEREST_RS_FRAMEWORK_SOURCE_LOCATION") {
Copy link
Contributor

Choose a reason for hiding this comment

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

The naming is a bit off here, is it everest framework dir OR everestrs framework source location?

Copy link
Contributor

Choose a reason for hiding this comment

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

My intention with naming here was that this environment variable is meant for everestrs and contains the location to the source of everest-framework

return PathBuf::from(everest_framework_dir);
}

let mut cur_dir =
PathBuf::from(env::var("CARGO_MANIFEST_DIR").expect("always set in build.rs execution"));

Expand All @@ -24,8 +28,19 @@ fn find_everest_workspace_root() -> PathBuf {
/// Returns the Libraries path if this is a standalone build of everest-framework or None if it is
/// not.
fn find_libs_in_everest_framework(root: &Path) -> Option<Libraries> {
let everestrs_sys = root.join("everest-framework/build/everestrs/libeverestrs_sys.a");
let framework = root.join("everest-framework/build/lib/libframework.so");
let (everestrs_sys, framework) =
if let Ok(everest_framework_dir) = env::var("EVEREST_RS_FRAMEWORK_BINARY_LOCATION") {
Copy link
Contributor

Choose a reason for hiding this comment

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

Compare the naming:
if let Ok(everest_framework_dir) = env::var("EVEREST_RS_FRAMEWORK_SOURCE_LOCATION") {
and
if let Ok(everest_framework_dir) = env::var("EVEREST_RS_FRAMEWORK_BINARY_LOCATION") {

Copy link
Contributor

Choose a reason for hiding this comment

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

we do need the binary location here, not the source location, which are usually not identical

Copy link
Contributor

Choose a reason for hiding this comment

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

If you pass locations by environment variables, why don't you pass the the complete path to libeverestrs_sys.a and libframework.so directly, this would be more stable. Note that at least inside cmake you can get the full path to library-targets (using generator expressions: FRAMEWORK_LIB=$<TARGET_FILE:everest::framework>)

Copy link
Contributor

Choose a reason for hiding this comment

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

Absolutely, would add yet another environment variable but I guess the cost of that is quite low

let everest_framework_path = PathBuf::from(everest_framework_dir);
(
everest_framework_path.join("everestrs/libeverestrs_sys.a"),
everest_framework_path.join("lib/libframework.so"),
)
} else {
(
root.join("everest-framework/build/everestrs/libeverestrs_sys.a"),
root.join("everest-framework/build/lib/libframework.so"),
)
};
Comment on lines +32 to +43
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure about rust best practices, but for me this if let Ok(..) = ... { doesn't naturally read. Also the implicit returns are hard to spot (but could just be me ...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nit: I'd prefer a match statement here too. The implicit returns are rustic - i.e. idiomatic. They are like meaningful whitespace in Python - one first finds them appalling, once used to them they become rather nice.

if everestrs_sys.exists() && framework.exists() {
Some(Libraries {
everestrs_sys,
Expand Down
Loading