Skip to content

Commit

Permalink
Fix windows linking (#104)
Browse files Browse the repository at this point in the history
* Fix finding of OpenVINO 2024.0.0 on Windows.

* Fixed linking on Windows.

* Fix Linux compile error.

* fix clippy error

* Refactor to pass linkage kind as a parameter

---------

Co-authored-by: Bradley Odell <btodell@hotmail.com>
Co-authored-by: Andrew Brown <andrew.brown@intel.com>
  • Loading branch information
3 people committed May 16, 2024
1 parent 6c551bb commit 836dd87
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 16 deletions.
3 changes: 0 additions & 3 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,6 @@ jobs:
# path and Cargo links the binary to it in the `build.rs` script.
- name: Check dynamic linking
run: cargo test
# TODO: the finder does not yet know how to distinguish between `*.lib` (required here) and
# `*.dll` (required by runtime-linking).
if: ${{ !startsWith(runner.os, 'windows') }}
# Finally, run the runtime-linking tests: the binddings do not link at build time, instead
# as the tests are run.
- name: Check runtime linking
Expand Down
39 changes: 31 additions & 8 deletions crates/openvino-finder/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,22 @@ macro_rules! check_and_return {
};
}

/// Distinguish which kind of library to link to.
///
/// The difference is important on Windows, e.g., which [requires] `*.lib` libraries when linking
/// dependent libraries.
///
/// [requires]:
/// https://learn.microsoft.com/en-us/windows/win32/dlls/dynamic-link-library-creation#creating-an-import-library
#[derive(Clone, Copy, Debug, PartialEq)]
pub enum Linking {
/// Find _static_ libraries: OpenVINO comes with static libraries on some platforms (e.g.,
/// Windows).
Static,
/// Find _dynamic_ libraries.
Dynamic,
}

/// Find the path to an OpenVINO library.
///
/// Because OpenVINO can be installed in quite a few ways (see module documentation), this function
Expand Down Expand Up @@ -87,13 +103,19 @@ macro_rules! check_and_return {
/// # Panics
///
/// Panics if it cannot list the contents of a search directory.
pub fn find(library_name: &str) -> Option<PathBuf> {
let file = format!(
"{}{}{}",
env::consts::DLL_PREFIX,
library_name,
pub fn find(library_name: &str, kind: Linking) -> Option<PathBuf> {
let suffix = if kind == Linking::Static {
// This is a bit rudimentary but works for the top three supported platforms: `linux`,
// `macos`, and `windows`.
if cfg!(target_os = "windows") {
".lib"
} else {
".a"
}
} else {
env::consts::DLL_SUFFIX
);
};
let file = format!("{}{}{}", env::consts::DLL_PREFIX, library_name, suffix);
log::info!("Attempting to find library: {}", file);

// Search using the `OPENVINO_BUILD_DIR` environment variable; this may be set by users of the
Expand Down Expand Up @@ -220,6 +242,7 @@ const KNOWN_INSTALLATION_SUBDIRECTORIES: &[&str] = &[
"runtime/lib/intel64/Release",
"runtime/lib/intel64",
"runtime/3rdparty/tbb/lib",
"runtime/bin/intel64/Release",
"runtime/bin/intel64",
"runtime/3rdparty/tbb/bin",
];
Expand Down Expand Up @@ -259,7 +282,7 @@ pub fn find_plugins_xml() -> Option<PathBuf> {

// Check in the same directory as the `openvino_c` library; e.g.,
// `/opt/intel/openvino_.../runtime/lib/intel64/plugins.xml`.
let library = find("openvino_c")?;
let library = find("openvino_c", Linking::Dynamic)?;
let library_parent_dir = library.parent()?;
check_and_return!(library_parent_dir.join(FILE_NAME));

Expand Down Expand Up @@ -314,7 +337,7 @@ mod test {
#[test]
fn find_openvino_c_locally() {
env_logger::init();
assert!(find("openvino_c").is_some());
assert!(find("openvino_c", Linking::Dynamic).is_some());
}

/// This test shows how the finder would discover the latest shared library on an
Expand Down
19 changes: 15 additions & 4 deletions crates/openvino-sys/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,13 @@ fn main() {
};

// Find the OpenVINO libraries to link to, either from a pre-installed location or by building
// from source.
// from source. We always look for the dynamic libraries here.
let link_kind = openvino_finder::Linking::Dynamic;
let (c_api_library_path, library_search_paths) = if linking == Linking::None {
(openvino_finder::find("openvino_c"), vec![])
} else if let Some(path) = openvino_finder::find("openvino_c") {
// Why try to find the library if we're not going to link against it? Well, this is for the
// helpful Cargo warnings that get printed below if we can't find the library on the system.
(openvino_finder::find("openvino_c", link_kind), vec![])
} else if let Some(path) = openvino_finder::find("openvino_c", link_kind) {
(Some(path), find_libraries_in_existing_installation())
} else {
panic!("Unable to find an OpenVINO installation on your system; build with runtime linking using `--features runtime-linking` or build from source with `OPENVINO_BUILD_DIR`.")
Expand Down Expand Up @@ -138,8 +141,16 @@ fn add_dynamically_linked_library(library: &str) {
/// unclear how we would discover this in a system-install scenario.
fn find_libraries_in_existing_installation() -> Vec<PathBuf> {
let mut dirs = vec![];
let link_kind = if cfg!(target_os = "windows") {
// Retrieve `*.lib` files on Windows. This is important because, when linking, Windows
// expects `*.lib` files. See
// https://learn.microsoft.com/en-us/windows/win32/dlls/dynamic-link-library-creation#creating-an-import-library.
openvino_finder::Linking::Static
} else {
openvino_finder::Linking::Dynamic
};
for library in LIBRARIES {
if let Some(path) = openvino_finder::find(library) {
if let Some(path) = openvino_finder::find(library, link_kind) {
println!(
"cargo:warning=Found library to link against: {}",
path.display()
Expand Down
2 changes: 1 addition & 1 deletion crates/openvino-sys/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ pub mod library {
/// `find().unwrap().parent()`.
pub fn find() -> Option<PathBuf> {
if cfg!(feature = "runtime-linking") {
openvino_finder::find("openvino_c")
openvino_finder::find("openvino_c", openvino_finder::Linking::Dynamic)
} else {
Some(PathBuf::from(env!("OPENVINO_LIB_PATH")))
}
Expand Down

0 comments on commit 836dd87

Please sign in to comment.