mbrobbel commented on code in PR #3148:
URL: https://github.com/apache/arrow-adbc/pull/3148#discussion_r2203988524
##########
rust/driver/snowflake/build.rs:
##########
@@ -50,7 +51,14 @@ fn bundled() -> Result<(), Box<dyn Error>> {
println!("cargo:rustc-link-lib=static=snowflake");
// Link other dependencies.
- println!("cargo:rustc-link-lib=resolv");
+ #[cfg(not(target_os = "windows"))]
+ {
+ println!("cargo:rustc-link-lib=resolv");
+ }
Review Comment:
```suggestion
#[cfg(not(target_os = "windows"))]
println!("cargo:rustc-link-lib=resolv");
```
##########
rust/driver/snowflake/build.rs:
##########
@@ -50,7 +51,14 @@ fn bundled() -> Result<(), Box<dyn Error>> {
println!("cargo:rustc-link-lib=static=snowflake");
// Link other dependencies.
- println!("cargo:rustc-link-lib=resolv");
+ #[cfg(not(target_os = "windows"))]
+ {
+ println!("cargo:rustc-link-lib=resolv");
+ }
+ #[cfg(target_os = "windows")]
+ {
+ println!("cargo:rustc-link-lib=legacy_stdio_definitions");
+ }
Review Comment:
```suggestion
#[cfg(target_os = "windows")]
println!("cargo:rustc-link-lib=legacy_stdio_definitions");
```
##########
rust/driver/snowflake/build.rs:
##########
@@ -70,7 +78,10 @@ fn linked() -> Result<(), Box<dyn Error>> {
if let Some(path) = option_env!("ADBC_SNOWFLAKE_GO_LIB_DIR") {
println!("cargo:rustc-link-search={path}");
}
-
+ #[cfg(target_os = "windows")]
+ {
+ println!("cargo:rustc-link-lib=legacy_stdio_definitions");
+ }
Review Comment:
```suggestion
#[cfg(target_os = "windows")]
println!("cargo:rustc-link-lib=legacy_stdio_definitions");
```
##########
rust/core/src/driver_manager.rs:
##########
@@ -609,15 +618,27 @@ fn load_driver_from_registry(
entrypoint: Option<&[u8]>,
) -> Result<DriverInfo> {
const ADBC_DRIVER_REGISTRY: &str = "SOFTWARE\\ADBC\\Drivers";
- let drivers_key = root.open(ADBC_DRIVER_REGISTRY)?;
+ let drivers_key = root
+ .open(ADBC_DRIVER_REGISTRY)
+ .and_then(|k| k.open(driver_name.to_str().unwrap_or_default()))
+ .map_err(|e| {
+ Error::with_message_and_status(
+ format!("Failed to open registry key: {e}"),
+ Status::NotFound,
+ )
+ })?;
+
+ let entrypoint_val = match drivers_key.get_string("entrypoint") {
+ Ok(s) => Some(s.into_bytes()),
+ Err(_) => None,
+ };
Ok(DriverInfo {
- driver_name: drivers_key.get_string("name").unwrap_or_default(),
- entrypoint: entrypoint
- .or_else(|| drivers_key.get_string("entrypoint").map(|e|
e.into_bytes())),
- version: drivers_key.get_string("version").unwrap_or_default(),
- source: drivers_key.get_string("source").unwrap_or_default(),
lib_path:
PathBuf::from(drivers_key.get_string("driver").unwrap_or_default()),
+ entrypoint: match entrypoint_val {
+ Some(e) => Some(e),
+ None => entrypoint.map(|s| s.to_vec()),
+ },
Review Comment:
You could use
https://doc.rust-lang.org/stable/std/option/enum.Option.html#method.or_else
##########
rust/core/src/driver_manager.rs:
##########
@@ -609,15 +618,27 @@ fn load_driver_from_registry(
entrypoint: Option<&[u8]>,
) -> Result<DriverInfo> {
const ADBC_DRIVER_REGISTRY: &str = "SOFTWARE\\ADBC\\Drivers";
- let drivers_key = root.open(ADBC_DRIVER_REGISTRY)?;
+ let drivers_key = root
+ .open(ADBC_DRIVER_REGISTRY)
+ .and_then(|k| k.open(driver_name.to_str().unwrap_or_default()))
+ .map_err(|e| {
+ Error::with_message_and_status(
+ format!("Failed to open registry key: {e}"),
+ Status::NotFound,
+ )
+ })?;
+
+ let entrypoint_val = match drivers_key.get_string("entrypoint") {
+ Ok(s) => Some(s.into_bytes()),
+ Err(_) => None,
+ };
Review Comment:
You could use
https://doc.rust-lang.org/stable/std/result/enum.Result.html#method.ok
##########
rust/core/src/driver_manager.rs:
##########
@@ -1658,25 +1679,25 @@ impl Drop for ManagedStatement {
}
}
-#[cfg(target_os = "windows")]
-use windows_sys as windows;
-
#[cfg(target_os = "windows")]
mod target_windows {
+ #[cfg(target_os = "windows")]
Review Comment:
We can remove this here because the `mod` has the same cfg attribute
```suggestion
```
--
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: github-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
