Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix running under nightly: copy prebuilt libtensorflow.so to OUT_DIR #72

Merged
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions tensorflow-sys/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,9 @@ fn install_prebuilt() {
// Extract the tarball.
let unpacked_dir = download_dir.join(base_name);
let lib_dir = unpacked_dir.join("lib");
if !lib_dir.join(format!("lib{}.so", LIBRARY)).exists() {
let library_file = format!("lib{}.so", LIBRARY);
let library_full_path = lib_dir.join(&library_file);
if !library_full_path.exists() {
extract(file_name, &unpacked_dir);
}

Expand All @@ -122,7 +124,16 @@ fn install_prebuilt() {
}); // TODO: remove

println!("cargo:rustc-link-lib=dylib={}", LIBRARY);
println!("cargo:rustc-link-search={}", lib_dir.display());
let output = PathBuf::from(&get!("OUT_DIR"));
let new_library_full_path = output.join(&library_file);
if new_library_full_path.exists() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think deleting the old one is necessary since the copy will overwrite it anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That what I though, and that's what the documentation mentions. But unwrap()ing the result fails. For example if I comment out the exists() block and simply copy() I get this:

 -> cargo test
   Compiling tensorflow-sys v0.7.0 (file://${HOME}/tensorflow_rust.git/tensorflow-sys)
error: failed to run custom build command for `tensorflow-sys v0.7.0 (file://${HOME}/tensorflow_rust.git/tensorflow-sys)`
process didn't exit successfully: `${HOME}/tensorflow_rust.git/target/debug/build/tensorflow-sys-28fb5808195bfa7e/build-script-build` (exit code: 101)
--- stdout
libtensorflow-sys/build.rs:80: binary_url = "https://storage.googleapis.com/tensorflow/libtensorflow/libtensorflow-cpu-darwin-x86_64-1.0.0.tar.gz"
libtensorflow-sys/build.rs:84: base_name = "libtensorflow-cpu-darwin-x86_64-1.0.0"
libtensorflow-sys/build.rs:93: file_name = "${HOME}/tensorflow_rust.git/tensorflow-sys/target/libtensorflow-cpu-darwin-x86_64-1.0.0.tar.gz"
libtensorflow-sys/build.rs:209: Executing "ls" "-l" "${HOME}/tensorflow_rust.git/tensorflow-sys/target/libtensorflow-cpu-darwin-x86_64-1.0.0/lib"
total 209520
-r-xr-xr-x  1 nicolas  staff  107270400 31 déc  1969 libtensorflow.so
libtensorflow-sys/build.rs:213: Command "ls" "-l" "${HOME}/tensorflow_rust.git/tensorflow-sys/target/libtensorflow-cpu-darwin-x86_64-1.0.0/lib" finished successfully
cargo:rustc-link-lib=dylib=tensorflow
libtensorflow-sys/build.rs:134: Copying ${HOME}/tensorflow_rust.git/tensorflow-sys/target/libtensorflow-cpu-darwin-x86_64-1.0.0/lib/libtensorflow.so to ${HOME}/tensorflow_rust.git/target/debug/build/tensorflow-sys-adb04dcfd9da710b/out/libtensorflow.so...

--- stderr
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Error { repr: Os { code: 13, message: "Permission denied" } }', /Users/rustbuild/src/rust-buildbot/slave/stable-dist-rustc-mac/build/src/libcore/result.rs:868
note: Run with `RUST_BACKTRACE=1` for a backtrace.

After investigation, it seems the .so file is -r-xr-xr-x (read and execute only), so the copy fails if the file already exists (it can't be overwritten as it's not writable. Making the file writable allow the copy operation to succeed.

So for the copy to succeed, I either have to chmod the .so file or delete the old one... Any preference?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, I guess that makes sense. Let's keep the delete code. The fewer platform-dependent requirements (like chmod) that we introduce, the better.

log!("File {} already exists, deleting.", new_library_full_path.display());
std::fs::remove_file(&new_library_full_path).unwrap();
}

log!("Copying {} to {}...", library_full_path.display(), new_library_full_path.display());
std::fs::copy(&library_full_path, &new_library_full_path).unwrap();
println!("cargo:rustc-link-search={}", output.display());
}

fn build_from_src() {
Expand Down