Xuanwo commented on code in PR #5195:
URL: https://github.com/apache/opendal/pull/5195#discussion_r1804245811
##########
bindings/cpp/build.rs:
##########
@@ -15,8 +15,56 @@
// specific language governing permissions and limitations
// under the License.
+use std::{
+ env::var,
+ io,
+ path::{Path, PathBuf},
+};
+
+#[cfg(unix)]
+fn symlink<P: AsRef<Path>, Q: AsRef<Path>>(original: P, link: Q) ->
io::Result<()> {
+ std::os::unix::fs::symlink(original, link)
+}
+
+#[cfg(target_os = "windows")]
+fn symlink<P: AsRef<Path>, Q: AsRef<Path>>(original: P, link: Q) ->
io::Result<()> {
+ std::os::windows::fs::symlink_file(original, link)
+}
+
+fn symlink_force<P: AsRef<Path>, Q: AsRef<Path>>(original: P, link: Q) ->
io::Result<()> {
+ if link.as_ref().exists() {
+ return std::fs::remove_file(link);
+ }
+
+ symlink(original, link)
+}
+
+fn print_to_cargo() {
+ println!("cargo:rerun-if-changed=src/lib.rs");
+ #[cfg(feature = "async")]
+ println!("cargo:rerun-if-changed=src/async.rs");
+}
+
+fn symlink_async_includes() {
+ let async_inc = var("DEP_CXX_ASYNC_INCLUDE").unwrap();
+ let src_dir = PathBuf::from(async_inc).join("rust");
+
+ let prj_dir = var("CARGO_MANIFEST_DIR").unwrap();
+ let dst_dir = PathBuf::from(prj_dir)
+ .join("target")
+ .join("cxxbridge")
+ .join("rust");
+
+ symlink_force(src_dir.join("cxx_async.h"),
dst_dir.join("cxx_async.h")).unwrap();
+}
+
fn main() {
let _ = cxx_build::bridge("src/lib.rs");
+ #[cfg(feature = "async")]
+ {
+ let _ = cxx_build::bridge("src/async.rs");
+ symlink_async_includes();
+ }
- println!("cargo:rerun-if-changed=src/lib.rs");
+ print_to_cargo()
Review Comment:
`print_to_cargo` has only been called once; are we over-optimizing the code
here?
##########
bindings/cpp/build.rs:
##########
@@ -15,8 +15,56 @@
// specific language governing permissions and limitations
// under the License.
+use std::{
+ env::var,
+ io,
+ path::{Path, PathBuf},
+};
+
+#[cfg(unix)]
+fn symlink<P: AsRef<Path>, Q: AsRef<Path>>(original: P, link: Q) ->
io::Result<()> {
+ std::os::unix::fs::symlink(original, link)
+}
+
+#[cfg(target_os = "windows")]
+fn symlink<P: AsRef<Path>, Q: AsRef<Path>>(original: P, link: Q) ->
io::Result<()> {
+ std::os::windows::fs::symlink_file(original, link)
+}
+
+fn symlink_force<P: AsRef<Path>, Q: AsRef<Path>>(original: P, link: Q) ->
io::Result<()> {
+ if link.as_ref().exists() {
+ return std::fs::remove_file(link);
+ }
+
+ symlink(original, link)
+}
+
+fn print_to_cargo() {
+ println!("cargo:rerun-if-changed=src/lib.rs");
+ #[cfg(feature = "async")]
+ println!("cargo:rerun-if-changed=src/async.rs");
+}
+
+fn symlink_async_includes() {
+ let async_inc = var("DEP_CXX_ASYNC_INCLUDE").unwrap();
+ let src_dir = PathBuf::from(async_inc).join("rust");
+
+ let prj_dir = var("CARGO_MANIFEST_DIR").unwrap();
+ let dst_dir = PathBuf::from(prj_dir)
+ .join("target")
+ .join("cxxbridge")
+ .join("rust");
+
+ symlink_force(src_dir.join("cxx_async.h"),
dst_dir.join("cxx_async.h")).unwrap();
Review Comment:
Can we just copy them instead of symlink?
##########
bindings/cpp/CMakeLists.txt:
##########
@@ -34,6 +31,18 @@ option(OPENDAL_ENABLE_DOCUMENTATION "Enable generating
document for opendal" OFF
option(OPENDAL_DOCS_ONLY "Only build documentation (dev only for quick ci)"
OFF)
option(OPENDAL_ENABLE_TESTING "Enable building test binary for opendal" OFF)
option(OPENDAL_DEV "Enable dev mode" OFF)
+option(OPENDAL_ENABLE_ASYNC "Enable async mode (requires C++20)" OFF)
+
+if(OPENDAL_ENABLE_ASYNC)
+ set(CMAKE_CXX_STANDARD 20)
+
+ if (NOT ((CMAKE_CXX_COMPILER_ID STREQUAL "Clang") OR
(CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang")))
+ message(FATAL_ERROR "currently C++ compiler must be clang for async
mode")
+ endif()
+else()
+ set(CMAKE_CXX_STANDARD 17)
Review Comment:
I'm fine with upgrading directly to C++20 if it can reduce our maintenance
burden.
##########
bindings/cpp/src/async.rs:
##########
@@ -0,0 +1,83 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use anyhow::Result;
+use opendal as od;
+use std::collections::HashMap;
+use std::future::Future;
+use std::ops::Deref;
+use std::str::FromStr;
+
+#[cxx::bridge(namespace = "opendal::ffi::async")]
+mod ffi {
+ struct HashMapValue {
+ key: String,
+ value: String,
+ }
+
+ // here we have to use raw pointers since:
+ // 1. cxx-async futures requires 'static lifetime (and it's hard to change
for now)
+ // 2. cxx SharedPtr cannot accept Rust types as type parameters for now
+ pub struct OperatorPtr {
+ op: *const Operator,
+ }
+
+ extern "Rust" {
+ type Operator;
+
+ fn new_operator(scheme: &str, configs: Vec<HashMapValue>) ->
Result<Box<Operator>>;
+ unsafe fn read_operator(op: OperatorPtr, path: String) ->
RustFutureRead;
Review Comment:
`read_operator` sounds like it's reading an operator. Perhaps
`operator_read` or something similar would be better?
##########
bindings/cpp/Cargo.toml:
##########
@@ -34,6 +34,8 @@ crate-type = ["staticlib"]
anyhow = "1.0"
chrono = "0.4"
cxx = "1.0"
+# cxx-async v0.1.1 in crates.io has build problems, so we use git repo directly
+cxx-async = { git = "https://github.com/pcwalton/cxx-async";, rev =
"961dd106b8eb2a86991728e1a18948e597426c1a", optional = true }
Review Comment:
It appears that cxx-async is no longer maintained. Should we consider
forking it and maintaining it ourselves?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
