Drop LAPACK symbol inspection

Author: Dirreke

openblas-build/Cargo.toml

@@ -22,5 +22,4 @@ ureq = { version = "2.5.0", default-features = false, features = [
     "gzip",
 ] }
 native-tls = { version = "0.2.11" }
-walkdir = "2.3.1"
 

openblas-build/nofortran.conf

@@ -2,7 +2,6 @@
 
 use crate::{check::*, error::*};
 use std::{env, fs, path::*, process::Command, str::FromStr};
-use walkdir::WalkDir;
 
 /// Interface for 32-bit interger (LP64) and 64-bit integer (ILP64)
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
@@ -345,12 +344,6 @@ impl Default for Configure {
     }
 }
 
-/// Deliverables of `make` command
-pub struct Deliverables {
-    /// Inspection what `make` command really show.
-    pub make_conf: MakeConf,
-}
-
 impl Configure {
     fn make_args(&self) -> Vec<String> {
         let mut args = Vec::new();
@@ -396,37 +389,6 @@ impl Configure {
         args
     }
 
-    /// Inspect existing build deliverables, and validate them.
-    ///
-    /// Error
-    /// ------
-    /// - No build deliverables exist
-    /// - Build deliverables are not valid
-    ///   - e.g. `self.no_lapack == false`, but the existing library does not contains LAPACK symbols.
-    ///
-    pub fn inspect(&self, out_dir: impl AsRef<Path>) -> Result<Deliverables, Error> {
-        let out_dir = out_dir.as_ref();
-        let make_conf = MakeConf::new(out_dir.join("Makefile.conf"))?;
-        if !self.no_static {
-            let lib_path = out_dir.join("libopenblas.a");
-            if !lib_path.exists() {
-                return Err(Error::LibraryNotExist { path: lib_path });
-            }
-        }
-        if !self.no_shared {
-            let lib_path = if cfg!(target_os = "macos") {
-                out_dir.join("libopenblas.dylib")
-            } else {
-                out_dir.join("libopenblas.so")
-            };
-            if !lib_path.exists() {
-                return Err(Error::LibraryNotExist { path: lib_path });
-            }
-        }
-
-        Ok(Deliverables { make_conf })
-    }
-
     /// Build OpenBLAS
     ///
     /// Libraries are created directly under `out_dir` e.g. `out_dir/libopenblas.a`
@@ -441,38 +403,12 @@ impl Configure {
         self,
         openblas_root: impl AsRef<Path>,
         out_dir: impl AsRef<Path>,
-    ) -> Result<Deliverables, Error> {
+    ) -> Result<MakeConf, Error> {
+        let root = openblas_root.as_ref();
         let out_dir = out_dir.as_ref();
-        if !out_dir.exists() {
-            fs::create_dir_all(out_dir)?;
-        }
-
         // Do not build if libraries and Makefile.conf already exist and are valid
-        if let Ok(deliv) = self.inspect(out_dir) {
-            return Ok(deliv);
-        }
-
-        // Copy OpenBLAS sources from this crate to `out_dir`
-        let root = openblas_root.as_ref();
-        for entry in WalkDir::new(root) {
-            let entry = entry.expect("Unknown IO error while walkdir");
-            let dest = out_dir.join(
-                entry
-                    .path()
-                    .strip_prefix(root)
-                    .expect("Directory entry is not under root"),
-            );
-            if dest.exists() {
-                // Do not overwrite
-                // Cache of previous build should be cleaned by `cargo clean`
-                continue;
-            }
-            if entry.file_type().is_dir() {
-                fs::create_dir(&dest)?;
-            }
-            if entry.file_type().is_file() {
-                fs::copy(entry.path(), &dest)?;
-            }
+        if let Ok(make_conf) = MakeConf::new(out_dir.join("Makefile.conf")) {
+            return Ok(make_conf);
         }
 
         // check if cross compile is needed
@@ -494,7 +430,7 @@ impl Configure {
         let out = fs::File::create(out_dir.join("out.log")).expect("Cannot create log file");
         let err = fs::File::create(out_dir.join("err.log")).expect("Cannot create log file");
         match Command::new("make")
-            .current_dir(out_dir)
+            .current_dir(root)
             .stdout(out)
             .stderr(err)
             .args(self.make_args())
@@ -514,7 +450,32 @@ impl Configure {
                 return Err(e);
             }
         }
-        self.inspect(out_dir)
+
+        // Copy OpenBLAS and Makefile.conf from this crate to `out_dir`
+        let mut file_names = Vec::new();
+        if !self.no_static {
+            file_names.push("libopenblas.a");
+        }
+        if !self.no_shared {
+            if cfg!(target_os = "macos") {
+                file_names.push("libopenblas.dylib");
+            } else {
+                file_names.push("libopenblas.so");
+            }
+        }
+        file_names.push("Makefile.conf");
+        for file_name in file_names {
+            let src = root.join(file_name);
+            let dest = out_dir.join(file_name);
+            if dest.exists() {
+                // Do not overwrite
+                // Cache of previous build should be cleaned by `cargo clean`
+                continue;
+            }
+            fs::copy(src, dest)?;
+        }
+
+        MakeConf::new(out_dir.join("Makefile.conf"))
     }
 }
 
@@ -540,53 +501,73 @@ mod tests {
     #[test]
     fn build_default() {
         let root = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
+        let out_dir = root.join("test_build/build_default");
         let opt = Configure::default();
-        let _detail = opt
-            .build(get_openblas_source(), root.join("test_build/build_default"))
-            .unwrap();
+        let _ = opt.build(get_openblas_source(), &out_dir).unwrap();
     }
 
     #[ignore]
     #[test]
     fn build_no_shared() {
         let root = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
+        let out_dir = root.join("test_build/build_no_shared");
         let mut opt = Configure::default();
         opt.no_shared = true;
-        let detail = opt
-            .build(
-                get_openblas_source(),
-                root.join("test_build/build_no_shared"),
-            )
-            .unwrap();
-        assert!(detail.shared_lib.is_none());
+        opt.build(get_openblas_source(), &out_dir).unwrap();
+        let _ = LibInspect::new(out_dir.join("libopenblas.a")).unwrap();
     }
 
     #[ignore]
     #[test]
     fn build_no_lapacke() {
         let root = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
+        let out_dir = root.join("test_build/build_no_lapacke");
         let mut opt = Configure::default();
         opt.no_lapacke = true;
-        let detail = opt
-            .build(
-                get_openblas_source(),
-                root.join("test_build/build_no_lapacke"),
-            )
-            .unwrap();
-        let shared_lib = detail.shared_lib.unwrap();
-        assert!(shared_lib.has_lapack());
-        assert!(!shared_lib.has_lapacke());
+        let _ = opt.build(get_openblas_source(), &out_dir).unwrap();
+        let lib_name = if cfg!(target_os = "macos") {
+            "libopenblas.dylib"
+        } else {
+            "libopenblas.so"
+        };
+        let lib_inspect = LibInspect::new(out_dir.join(lib_name)).unwrap();
+
+        assert!(lib_inspect.has_lapack());
+        assert!(!lib_inspect.has_lapacke());
+    }
+
+    #[ignore]
+    #[test]
+    fn build_no_cblas() {
+        let root = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
+        let out_dir = root.join("test_build/build_no_cblas");
+        let mut opt = Configure::default();
+        opt.no_lapacke = true;
+        let _ = opt.build(get_openblas_source(), &out_dir).unwrap();
+        let lib_name = if cfg!(target_os = "macos") {
+            "libopenblas.dylib"
+        } else {
+            "libopenblas.so"
+        };
+        let lib_inspect = LibInspect::new(out_dir.join(lib_name)).unwrap();
+
+        assert!(!lib_inspect.has_cblas());
     }
 
     #[ignore]
     #[test]
     fn build_openmp() {
         let root = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
+        let out_dir = root.join("test_build/build_openmp");
         let mut opt = Configure::default();
         opt.use_openmp = true;
-        let detail = opt
-            .build(get_openblas_source(), root.join("test_build/build_openmp"))
-            .unwrap();
-        assert!(detail.shared_lib.unwrap().has_lib("gomp"));
+        let _ = opt.build(get_openblas_source(), &out_dir).unwrap();
+        let lib_name = if cfg!(target_os = "macos") {
+            "libopenblas.dylib"
+        } else {
+            "libopenblas.so"
+        };
+        let lib_inspect = LibInspect::new(out_dir.join(lib_name)).unwrap();
+        assert!(lib_inspect.has_lib("gomp"));
     }
 }

openblas-build/src/build.rs

@@ -7,7 +7,6 @@ use std::{
     hash::Hash,
     io::{self, BufRead},
     path::*,
-    process::Command,
 };
 
 /// Parse compiler linker flags, `-L` and `-l`
@@ -105,18 +104,19 @@ impl MakeConf {
 /// - Linked shared libraries using `objdump -p` external command.
 /// - Global "T" symbols in the text (code) section of library using `nm -g` external command.
 #[derive(Debug, Clone)]
-#[allow(dead_code)]
-pub struct LibInspect {
+#[cfg(test)]
+pub(crate) struct LibInspect {
     pub libs: Vec<String>,
     pub symbols: Vec<String>,
 }
 
-#[allow(dead_code)]
+#[cfg(test)]
 impl LibInspect {
     /// Inspect library file
     ///
     /// Be sure that `nm -g` and `objdump -p` are executed in this function
     pub fn new<P: AsRef<Path>>(path: P) -> Result<Self, Error> {
+        use std::process::Command;
         let path = path.as_ref();
         if !path.exists() {
             return Err(Error::LibraryNotExist {
@@ -208,20 +208,11 @@ impl LibInspect {
 #[cfg(test)]
 mod tests {
     use super::*;
-
     #[test]
     fn detail_from_makefile_conf() {
         let path = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("Makefile.conf");
         assert!(path.exists());
         let detail = MakeConf::new(path).unwrap();
         assert!(!detail.no_fortran);
     }
-
-    #[test]
-    fn detail_from_nofortran_conf() {
-        let path = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("nofortran.conf");
-        assert!(path.exists());
-        let detail = MakeConf::new(path).unwrap();
-        assert!(detail.no_fortran);
-    }
 }

openblas-build/src/check.rs

@@ -50,7 +50,9 @@ fn windows_msvc_system() {
         env::set_var("VCPKGRS_DYNAMIC", "1");
     }
     #[cfg(target_env = "msvc")]
-    vcpkg::find_package("openblas").unwrap();
+    vcpkg::find_package("openblas").expect(
+        "vcpkg failed to find OpenBLAS package , Try to install it using `vcpkg install openblas:$(ARCH)-windows(-static)(-md)`"
+    );
     if !cfg!(target_env = "msvc") {
         unreachable!();
     }
@@ -194,19 +196,19 @@ fn build() {
     }
 
     let source = openblas_build::download(&output).unwrap();
-    let deliv = cfg.build(source, &output).unwrap();
+    let make_conf = cfg.build(source, &output).unwrap();
 
     println!("cargo:rustc-link-search={}", output.display());
-    for search_path in &deliv.make_conf.c_extra_libs.search_paths {
+    for search_path in &make_conf.c_extra_libs.search_paths {
         println!("cargo:rustc-link-search={}", search_path.display());
     }
-    for lib in &deliv.make_conf.c_extra_libs.libs {
+    for lib in &make_conf.c_extra_libs.libs {
         println!("cargo:rustc-link-lib={}", lib);
     }
-    for search_path in &deliv.make_conf.f_extra_libs.search_paths {
+    for search_path in &make_conf.f_extra_libs.search_paths {
         println!("cargo:rustc-link-search={}", search_path.display());
     }
-    for lib in &deliv.make_conf.f_extra_libs.libs {
+    for lib in &make_conf.f_extra_libs.libs {
         println!("cargo:rustc-link-lib={}", lib);
     }
 }