From 6243fb1d078e5b4d695c87e81dedad090fb09f97 Mon Sep 17 00:00:00 2001 From: Lucas Kent Date: Fri, 26 Jul 2024 13:28:20 +1000 Subject: [PATCH 1/2] Remove lazy_static + cleanup lib.rs --- Cargo.toml | 1 - src/errors.rs | 28 ++++--- src/lib.rs | 222 +++++++++++++++++++++++--------------------------- src/main.rs | 4 +- 4 files changed, 120 insertions(+), 135 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 886abd7..1921681 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,7 +23,6 @@ required-features = ["build-binary"] [dependencies] glob = "0.3" -lazy_static = "1.4" docopt = { version = "1.1", optional = true } [features] diff --git a/src/errors.rs b/src/errors.rs index ade53f8..454724d 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -11,8 +11,8 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. -use std::{fmt, result, string}; use std::error::Error; +use std::{fmt, result}; use glob; @@ -20,14 +20,12 @@ pub type Result = result::Result; #[derive(Debug)] pub struct JavaLocatorError { - description: String + description: String, } impl JavaLocatorError { - pub(crate) fn new(description: &str) -> JavaLocatorError { - JavaLocatorError { - description: description.to_string(), - } + pub(crate) fn new(description: String) -> JavaLocatorError { + JavaLocatorError { description } } } @@ -45,18 +43,24 @@ impl Error for JavaLocatorError { impl From for JavaLocatorError { fn from(err: std::io::Error) -> JavaLocatorError { - JavaLocatorError { description: format!("{:?}", err) } + JavaLocatorError { + description: format!("{:?}", err), + } } } -impl From for JavaLocatorError { - fn from(err: string::FromUtf8Error) -> JavaLocatorError { - JavaLocatorError { description: format!("{:?}", err) } +impl From for JavaLocatorError { + fn from(err: std::str::Utf8Error) -> JavaLocatorError { + JavaLocatorError { + description: format!("{:?}", err), + } } } impl From for JavaLocatorError { fn from(err: glob::PatternError) -> JavaLocatorError { - JavaLocatorError { description: format!("{:?}", err) } + JavaLocatorError { + description: format!("{:?}", err), + } } -} \ No newline at end of file +} diff --git a/src/lib.rs b/src/lib.rs index 15e8be5..f97255c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -91,53 +91,11 @@ use std::env; use std::path::PathBuf; use std::process::Command; +use errors::{JavaLocatorError, Result}; use glob::{glob, Pattern}; -use lazy_static::lazy_static; pub mod errors; -const WINDOWS: &'static str = "windows"; -const MACOS: &'static str = "macos"; -const ANDROID: &'static str = "android"; -const UNIX: &'static str = "unix"; - -lazy_static! { - static ref TARGET_OS: String = { - let target_os_res = env::var("CARGO_CFG_TARGET_OS"); - let tos = target_os_res.as_ref().map(|x| &**x).unwrap_or_else(|_| { - if cfg!(windows) { - WINDOWS - } else if cfg!(target_os = "macos") { - MACOS - } else if cfg!(target_os = "android") { - ANDROID - } else { - UNIX - } - }); - - tos.to_string() - }; -} - -fn is_windows() -> bool { - &*TARGET_OS == WINDOWS -} - -fn is_macos() -> bool { - &*TARGET_OS == MACOS -} - -#[allow(dead_code)] -fn is_android() -> bool { - &*TARGET_OS == ANDROID -} - -#[allow(dead_code)] -fn is_unix() -> bool { - &*TARGET_OS == UNIX -} - /// Returns the name of the jvm dynamic library: /// /// * libjvm.so for Linux @@ -146,9 +104,9 @@ fn is_unix() -> bool { /// /// * jvm.dll for Windows pub fn get_jvm_dyn_lib_file_name() -> &'static str { - if is_windows() { + if cfg!(target_os = "windows") { "jvm.dll" - } else if is_macos() { + } else if cfg!(target_os = "macos") { "libjvm.dylib" } else { "libjvm.so" @@ -160,7 +118,7 @@ pub fn get_jvm_dyn_lib_file_name() -> &'static str { /// If `JAVA_HOME` env var is defined, the function returns it without any checks whether the var points to a valid directory or not. /// /// If `JAVA_HOME` is not defined, the function tries to locate it using the `java` executable. -pub fn locate_java_home() -> errors::Result { +pub fn locate_java_home() -> Result { match &env::var("JAVA_HOME") { Ok(s) if s.is_empty() => do_locate_java_home(), Ok(java_home_env_var) => Ok(java_home_env_var.clone()), @@ -168,52 +126,94 @@ pub fn locate_java_home() -> errors::Result { } } -fn do_locate_java_home() -> errors::Result { - // Prepare the command depending on the host - let command_str = if is_windows() { - "where" - } else if is_macos() { - "/usr/libexec/java_home" - } else { - "which" - }; +#[cfg(target_os = "windows")] +fn do_locate_java_home() -> Result { + let output = Command::new("where") + .arg("java") + .output() + .map_err(|e| JavaLocatorError::new(format!("Failed to run command `where` ({e})")))?; + + let java_exec_path = std::str::from_utf8(&output.stdout)? + // Windows will return multiple lines if there are multiple `java` in the PATH. + .lines() + // The first line is the one that would be run, so take just that line. + .next() + .unwrap() + .trim(); - let mut command = Command::new(command_str); + if java_exec_path.is_empty() { + return Err(JavaLocatorError::new( + "Java is not installed or not in the system PATH".into(), + )); + } + + let mut home_path = follow_symlinks(java_exec_path); + + // Here we should have found ourselves in a directory like /usr/lib/jvm/java-8-oracle/jre/bin/java + home_path.pop(); + home_path.pop(); + + home_path + .into_os_string() + .into_string() + .map_err(|path| JavaLocatorError::new(format!("Java path {path:?} is invalid utf8"))) +} - if !is_macos() { - command.arg("java"); +#[cfg(target_os = "macos")] +fn do_locate_java_home() -> Result { + let output = Command::new("/usr/libexec/java_home") + .output() + .map_err(|e| { + JavaLocatorError::new(format!( + "Failed to run command `/usr/libexec/java_home` ({e})" + )) + })?; + + let java_exec_path = std::str::from_utf8(&output.stdout)?.trim(); + + if java_exec_path.is_empty() { + return Err(JavaLocatorError::new( + "Java is not installed or not in the system PATH".into(), + )); } - let output = command.output().map_err(|error| { - let message = format!( - "Command '{}' is not found in the system PATH ({})", - command_str, error - ); - errors::JavaLocatorError::new(&message) - })?; - let java_exec_path = String::from_utf8(output.stdout).map(|jp| { - let mut lines: Vec<&str> = jp.lines().collect(); - if lines.len() > 1 { - println!( - "WARNING: java_locator found {} possible java locations: {}. Using the last one.", - lines.len(), - lines.join(", ") - ); - lines.remove(lines.len() - 1).to_string() - } else { - jp - } - })?; + let home_path = follow_symlinks(java_exec_path); + + home_path + .into_os_string() + .into_string() + .map_err(|path| JavaLocatorError::new(format!("Java path {path:?} is invalid utf8"))) +} + +#[cfg(not(any(target_os = "windows", target_os = "macos")))] // Unix +fn do_locate_java_home() -> Result { + let output = Command::new("which") + .arg("java") + .output() + .map_err(|e| JavaLocatorError::new(format!("Failed to run command `which` ({e})")))?; + let java_exec_path = std::str::from_utf8(&output.stdout)?.trim(); - // Return early in case that the java executable is not found if java_exec_path.is_empty() { - Err(errors::JavaLocatorError::new( - "Java is not installed or not added in the system PATH", - ))? + return Err(JavaLocatorError::new( + "Java is not installed or not in the system PATH".into(), + )); } - let mut test_path = PathBuf::from(java_exec_path.trim()); + let mut home_path = follow_symlinks(java_exec_path); + + // Here we should have found ourselves in a directory like /usr/lib/jvm/java-8-oracle/jre/bin/java + home_path.pop(); + home_path.pop(); + home_path + .into_os_string() + .into_string() + .map_err(|path| JavaLocatorError::new(format!("Java path {path:?} is invalid utf8"))) +} + +// Its not clear to me which systems need this so for now its run on all systems. +fn follow_symlinks(path: &str) -> PathBuf { + let mut test_path = PathBuf::from(path); while let Ok(path) = test_path.read_link() { test_path = if path.is_absolute() { path @@ -223,55 +223,39 @@ fn do_locate_java_home() -> errors::Result { test_path }; } - - if !is_macos() { - // Here we should have found ourselves in a directory like /usr/lib/jvm/java-8-oracle/jre/bin/java - test_path.pop(); - test_path.pop(); - } - - match test_path.to_str() { - Some(s) => Ok(String::from(s)), - None => Err(errors::JavaLocatorError::new(&format!( - "Could not convert path {:?} to String", - test_path - ))), - } + test_path } /// Returns the path that contains the `libjvm.so` (or `jvm.dll` in windows). -pub fn locate_jvm_dyn_library() -> errors::Result { - let jvm_dyn_lib_file_name = if is_windows() { "jvm.dll" } else { "libjvm.*" }; - - locate_file(jvm_dyn_lib_file_name) +pub fn locate_jvm_dyn_library() -> Result { + if cfg!(target_os = "windows") { + locate_file("jvm.dll") + } else { + locate_file("libjvm.*") + } } /// Returns the path that contains the file with the provided name. /// /// This function argument can be a wildcard. -pub fn locate_file(file_name: &str) -> errors::Result { +pub fn locate_file(file_name: &str) -> Result { // Find the JAVA_HOME let java_home = locate_java_home()?; let query = format!("{}/**/{}", Pattern::escape(&java_home), file_name); - let paths_vec: Vec = glob(&query)? - .filter_map(Result::ok) - .map(|path_buf| { - let mut pb = path_buf.clone(); - pb.pop(); - pb.to_str().unwrap_or("").to_string() - }) - .filter(|s: &String| !s.is_empty()) - .collect(); - - if paths_vec.is_empty() { - Err(errors::JavaLocatorError::new(&format!( - "Could not find the {} library in any subdirectory of {}", - file_name, java_home - ))) - } else { - Ok(paths_vec[0].clone()) + let path = glob(&query)?.filter_map(|x| x.ok()).next().ok_or_else(|| { + JavaLocatorError::new(format!( + "Could not find the {file_name} library in any subdirectory of {java_home}", + )) + })?; + + let parent_path = path.parent().unwrap(); + match parent_path.to_str() { + Some(parent_path) => Ok(parent_path.to_owned()), + None => Err(JavaLocatorError::new(format!( + "Java path {parent_path:?} is invalid utf8" + ))), } } diff --git a/src/main.rs b/src/main.rs index 9c21c28..5fbea03 100644 --- a/src/main.rs +++ b/src/main.rs @@ -13,9 +13,7 @@ // limitations under the License. use docopt::Docopt; -use java_locator; - -const USAGE: &'static str = " +const USAGE: &str = " java-locator locates the active Java installation in the host. Usage: From 2c97eb88ff70d9144cb6b749b78d5dcbf10cc3ae Mon Sep 17 00:00:00 2001 From: Lucas Kent Date: Wed, 23 Oct 2024 09:16:07 +1100 Subject: [PATCH 2/2] review feedback --- src/lib.rs | 46 +++++++++++++++++++++++----------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index f97255c..db9b754 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -133,23 +133,24 @@ fn do_locate_java_home() -> Result { .output() .map_err(|e| JavaLocatorError::new(format!("Failed to run command `where` ({e})")))?; - let java_exec_path = std::str::from_utf8(&output.stdout)? - // Windows will return multiple lines if there are multiple `java` in the PATH. + let java_exec_path_raw = std::str::from_utf8(&output.stdout)?; + java_exec_path_validation(java_exec_path_raw)?; + + // Windows will return multiple lines if there are multiple `java` in the PATH. + let paths_found = java_exec_path_raw.lines().count(); + if paths_found > 1 { + eprintln!("WARNING: java_locator found {paths_found} possible java locations. Using the first one. To silence this warning set JAVA_HOME env var.") + } + + let java_exec_path = java_exec_path_raw .lines() // The first line is the one that would be run, so take just that line. .next() - .unwrap() + .expect("gauranteed to have at least one line by java_exec_path_validation") .trim(); - if java_exec_path.is_empty() { - return Err(JavaLocatorError::new( - "Java is not installed or not in the system PATH".into(), - )); - } - let mut home_path = follow_symlinks(java_exec_path); - // Here we should have found ourselves in a directory like /usr/lib/jvm/java-8-oracle/jre/bin/java home_path.pop(); home_path.pop(); @@ -171,12 +172,7 @@ fn do_locate_java_home() -> Result { let java_exec_path = std::str::from_utf8(&output.stdout)?.trim(); - if java_exec_path.is_empty() { - return Err(JavaLocatorError::new( - "Java is not installed or not in the system PATH".into(), - )); - } - + java_exec_path_validation(java_exec_path)?; let home_path = follow_symlinks(java_exec_path); home_path @@ -193,12 +189,7 @@ fn do_locate_java_home() -> Result { .map_err(|e| JavaLocatorError::new(format!("Failed to run command `which` ({e})")))?; let java_exec_path = std::str::from_utf8(&output.stdout)?.trim(); - if java_exec_path.is_empty() { - return Err(JavaLocatorError::new( - "Java is not installed or not in the system PATH".into(), - )); - } - + java_exec_path_validation(java_exec_path)?; let mut home_path = follow_symlinks(java_exec_path); // Here we should have found ourselves in a directory like /usr/lib/jvm/java-8-oracle/jre/bin/java @@ -211,7 +202,16 @@ fn do_locate_java_home() -> Result { .map_err(|path| JavaLocatorError::new(format!("Java path {path:?} is invalid utf8"))) } -// Its not clear to me which systems need this so for now its run on all systems. +fn java_exec_path_validation(path: &str) -> Result<()> { + if path.is_empty() { + return Err(JavaLocatorError::new( + "Java is not installed or not in the system PATH".into(), + )); + } + + Ok(()) +} + fn follow_symlinks(path: &str) -> PathBuf { let mut test_path = PathBuf::from(path); while let Ok(path) = test_path.read_link() {