Introduce Rust #453
Introduce Rust #453
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This pull request introduces Rust to the ShoopDaLoop project, marking the beginning of a planned migration from Python and C++ to Rust.
- Added
Cargo.tomlandbuild.rsto set up Rust project structure and build process - Modified
/src/CMakeLists.txtto integrate Rust components into the existing build system - Updated
README.mdwith a detailed roadmap explaining the transition to Rust and future development plans - Added
/src/bin/build_appdir.rsto handle application directory structure creation in Rust - Introduced
/src/cmake/CompilerCaching.cmaketo improve build times using sccache or ccache
15 file(s) reviewed, 9 comment(s)
Edit PR Review Bot Settings
Cargo.toml
Outdated
| license = "TODO" | ||
|
|
||
| [dependencies] | ||
| pyo3 = "0.22" |
There was a problem hiding this comment.
style: Consider specifying a more precise version for pyo3 to ensure compatibility, e.g., '0.22.1'
build.rs
Outdated
| .stdout(std::process::Stdio::inherit()) | ||
| .stderr(std::process::Stdio::inherit()) | ||
| .current_dir(src_dir.clone()) | ||
| .status().unwrap(); |
There was a problem hiding this comment.
logic: unwrap() here may cause the build to fail silently if the command execution fails. Consider using expect() with a meaningful error message.
build.rs
Outdated
| let venv_dir = Path::new(&out_dir).join("shoopdaloop_env"); | ||
| Command::new(&host_python).args( | ||
| &["-m", "venv", venv_dir.to_str().expect("Could not get venv path")] | ||
| ).status().unwrap(); |
There was a problem hiding this comment.
logic: unwrap() here may cause the build to fail silently if venv creation fails. Consider using expect() with a meaningful error message.
build.rs
Outdated
| ) | ||
| .stdout(std::process::Stdio::inherit()) | ||
| .stderr(std::process::Stdio::inherit()) | ||
| .status().unwrap(); |
There was a problem hiding this comment.
logic: unwrap() here may cause the build to fail silently if wheel installation fails. Consider using expect() with a meaningful error message.
scripts/list_dependencies.sh
Outdated
| EXECUTABLE=$1 | ||
|
|
||
| # Temporary files | ||
| TRACE_OUTPUT=$(mktemp) |
There was a problem hiding this comment.
logic: TRACE_OUTPUT is created but never used
src/bin/build_appdir.rs
Outdated
| std::process::exit(1); | ||
| }; | ||
|
|
||
| let bad_usage = |msg : _| { |
There was a problem hiding this comment.
style: Specify the type for 'msg' instead of using '_'
src/bin/build_appdir.rs
Outdated
| let binding = String::from_utf8(output.stdout).unwrap(); | ||
| let build_out_dir = binding.trim(); |
There was a problem hiding this comment.
logic: Handle potential UTF-8 conversion errors instead of using unwrap()
src/bin/build_appdir.rs
Outdated
| recursive_dir_cpy( | ||
| &from, | ||
| &to | ||
| ).unwrap(); |
There was a problem hiding this comment.
logic: Handle potential errors from recursive_dir_cpy instead of using unwrap()
| if (DEFINED PTHREADS4W_PATH) | ||
| set(ZITA_CMAKE_ARGS | ||
| -DCMAKE_PREFIX_PATH=${PTHREADS4W_PATH} | ||
| -DCMAKE_LIBRARY_PATH=${PTHREADS4W_PATH} | ||
| -DCMAKE_CXX_FLAGS=-I${PTHREADS4W_PATH} | ||
| -DCMAKE_C_FLAGS=-I${PTHREADS4W_PATH} | ||
| ) | ||
| endif() |
There was a problem hiding this comment.
style: Consider using CMAKE_CXX_STANDARD and CMAKE_C_STANDARD instead of manually setting flags for including pthreads4w.
No description provided.