Copilot commented on code in PR #857:
URL: https://github.com/apache/incubator-graphar/pull/857#discussion_r2806818660
##########
rust/build.rs:
##########
@@ -20,9 +20,25 @@
use std::env;
use std::path::{Path, PathBuf};
+/// Query a pkg-config variable for the `arrow` package.
+fn pkg_config_arrow(variable: &str) -> Option<String> {
+ std::process::Command::new("pkg-config")
+ .args([&format!("--variable={variable}"), "arrow"])
+ .output()
+ .ok()
+ .filter(|o| o.status.success())
+ .and_then(|o| String::from_utf8(o.stdout).ok())
+ .map(|s| s.trim().to_string())
+ .filter(|s| !s.is_empty())
+}
+
fn link_libraries() {
- // TODO: Link to system Arrow, `libarrow` is under
`/usr/lib/x86_64-linux-gnu/` on Ubuntu
- println!("cargo:rustc-link-search=native=/usr/lib/x86_64-linux-gnu/");
+ if let Some(dir) = pkg_config_arrow("libdir") {
+ println!("cargo:rustc-link-search=native={dir}");
+ } else if let Ok(dir) = env::var("ARROW_LIB_DIR") {
+ println!("cargo:rustc-link-search=native={dir}");
+ }
Review Comment:
The `ARROW_LIB_DIR` environment variable is used to determine the library
search path, but there's no corresponding
`cargo:rerun-if-env-changed=ARROW_LIB_DIR` directive. This means that if a user
changes the `ARROW_LIB_DIR` environment variable, the build script won't be
re-executed, potentially causing the build to use stale library paths. Add
`println!("cargo:rerun-if-env-changed=ARROW_LIB_DIR");` after line 40 to ensure
the build script reruns when this environment variable changes.
```suggestion
}
println!("cargo:rerun-if-env-changed=ARROW_LIB_DIR");
```
##########
rust/build.rs:
##########
@@ -20,9 +20,25 @@
use std::env;
use std::path::{Path, PathBuf};
+/// Query a pkg-config variable for the `arrow` package.
+fn pkg_config_arrow(variable: &str) -> Option<String> {
+ std::process::Command::new("pkg-config")
+ .args([&format!("--variable={variable}"), "arrow"])
+ .output()
+ .ok()
+ .filter(|o| o.status.success())
+ .and_then(|o| String::from_utf8(o.stdout).ok())
+ .map(|s| s.trim().to_string())
+ .filter(|s| !s.is_empty())
+}
+
fn link_libraries() {
- // TODO: Link to system Arrow, `libarrow` is under
`/usr/lib/x86_64-linux-gnu/` on Ubuntu
- println!("cargo:rustc-link-search=native=/usr/lib/x86_64-linux-gnu/");
+ if let Some(dir) = pkg_config_arrow("libdir") {
+ println!("cargo:rustc-link-search=native={dir}");
+ } else if let Ok(dir) = env::var("ARROW_LIB_DIR") {
+ println!("cargo:rustc-link-search=native={dir}");
Review Comment:
When `pkg-config` is unavailable or fails to find Arrow, and `ARROW_LIB_DIR`
is also not set, the `link_libraries` function proceeds without setting any
library search path. This will cause the linker to search only in system
default paths, which may fail on systems where Arrow is installed in
non-standard locations. Consider adding a warning or informational message when
both pkg-config and ARROW_LIB_DIR fallbacks fail, to help users diagnose build
issues. For example: `eprintln!("cargo:warning=Could not determine Arrow
library directory via pkg-config or ARROW_LIB_DIR. Relying on system default
library paths.");`
```suggestion
println!("cargo:rustc-link-search=native={dir}");
} else {
eprintln!("cargo:warning=Could not determine Arrow library directory
via pkg-config or ARROW_LIB_DIR. Relying on system default library paths.");
```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
