fix: krane using lazy_static! caused uncleaned temp files

Author: Sparksssj

Cargo.lock

@@ -9,7 +9,9 @@ publish = false
 [dependencies]
 anyhow.workspace = true
 include-env-compressed.workspace = true
-lazy_static.workspace = true
+sha2.workspace = true
+
+[dev-dependencies]
 tempfile.workspace = true
 
 [build-dependencies]

tools/krane/Cargo.toml

@@ -1,60 +1,106 @@
 use anyhow::Result;
 use include_env_compressed::{include_archive_from_env, Archive};
-use std::fs::{File, Permissions};
+use sha2::{Digest, Sha256};
+use std::fs;
 use std::os::unix::fs::PermissionsExt;
-use std::path::PathBuf;
-
-use tempfile::TempDir;
 
 pub const KRANE_BIN: Archive = include_archive_from_env!("KRANE_PATH");
 
-lazy_static::lazy_static! {
-    pub static ref KRANE: Krane = Krane::seal().unwrap();
+fn expected_checksum() -> String {
+    let mut hasher = Sha256::new();
+    let mut reader = KRANE_BIN.reader();
+    std::io::copy(&mut reader, &mut hasher).unwrap();
+    format!("{:x}", hasher.finalize())
 }
 
-#[derive(Debug)]
-pub struct Krane {
-    // Hold the file in memory to keep the fd open
-    _tmp_dir: TempDir,
-    path: PathBuf,
+fn file_checksum(path: &std::path::Path) -> Result<String> {
+    let mut hasher = Sha256::new();
+    let mut file = fs::File::open(path)?;
+    std::io::copy(&mut file, &mut hasher)?;
+    Ok(format!("{:x}", hasher.finalize()))
 }
 
-impl Krane {
-    fn seal() -> Result<Krane> {
-        let tmp_dir = TempDir::new()?;
-        let path = tmp_dir.path().join("krane");
+pub fn ensure_krane_in_dir(tools_dir: impl AsRef<std::path::Path>) -> Result<std::path::PathBuf> {
+    let tools_dir = tools_dir.as_ref();
+    let krane_path = tools_dir.join("krane");
 
-        let mut krane_file = File::create(&path)?;
-        let permissions = Permissions::from_mode(0o755);
-        krane_file.set_permissions(permissions)?;
+    let needs_update = if krane_path.exists() {
+        match file_checksum(&krane_path) {
+            Ok(actual) => actual != expected_checksum(),
+            Err(_) => true, // If we can't read it, assume it needs update
+        }
+    } else {
+        true
+    };
 
-        let mut krane_reader = KRANE_BIN.reader();
+    if needs_update {
+        fs::create_dir_all(tools_dir)?;
 
-        std::io::copy(&mut krane_reader, &mut krane_file)?;
+        let temp_path = tools_dir.join(format!(".krane.tmp.{}", std::process::id()));
 
-        Ok(Krane {
-            _tmp_dir: tmp_dir,
-            path,
-        })
-    }
+        {
+            let mut temp_file = fs::File::create(&temp_path)?;
+            let permissions = fs::Permissions::from_mode(0o755);
+            temp_file.set_permissions(permissions)?;
+
+            let mut krane_reader = KRANE_BIN.reader();
+            std::io::copy(&mut krane_reader, &mut temp_file)?;
+
+            use std::io::Write;
+            temp_file.flush()?;
+        }
 
-    pub fn path(&self) -> &PathBuf {
-        &self.path
+        fs::rename(temp_path, &krane_path)?;
     }
+
+    Ok(krane_path)
 }
 
 #[cfg(test)]
 mod test {
     use super::*;
-    use std::process::Command;
 
     #[test]
-    fn test_krane_runs() {
-        let status = Command::new(KRANE.path())
+    fn test_checksum_functions() {
+        let checksum1 = expected_checksum();
+        let checksum2 = expected_checksum();
+        assert_eq!(checksum1, checksum2);
+        assert!(!checksum1.is_empty());
+        println!("Expected checksum: {}", checksum1);
+    }
+
+    #[test]
+    fn test_ensure_krane_in_dir() {
+        use tempfile::TempDir;
+
+        let temp_dir = TempDir::new().unwrap();
+        println!("Testing ensure_krane_in_dir in: {:?}", temp_dir.path());
+
+        let path1 = crate::ensure_krane_in_dir(temp_dir.path()).unwrap();
+        assert!(path1.exists());
+        assert!(path1.ends_with("krane"));
+        println!("Created krane at: {:?}", path1);
+
+        let metadata = std::fs::metadata(&path1).unwrap();
+        let permissions = metadata.permissions();
+        assert!(permissions.mode() & 0o111 != 0);
+        println!("File is executable: {}", permissions.mode() & 0o111 != 0);
+
+        let mtime1 = std::fs::metadata(&path1).unwrap().modified().unwrap();
+        std::thread::sleep(std::time::Duration::from_millis(10));
+
+        let path2 = crate::ensure_krane_in_dir(temp_dir.path()).unwrap();
+        let mtime2 = std::fs::metadata(&path2).unwrap().modified().unwrap();
+
+        assert_eq!(path1, path2);
+        assert_eq!(mtime1, mtime2);
+        println!("File was reused (same mtime): {}", mtime1 == mtime2);
+
+        let status = std::process::Command::new(&path1)
             .arg("--help")
             .output()
             .expect("failed to run krane");
-
         assert_eq!(status.status.code().unwrap(), 0);
+        println!("Krane binary works correctly");
     }
 }

tools/krane/src/lib.rs

@@ -17,7 +17,7 @@ use std::{collections::HashMap, path::Path};
 use async_trait::async_trait;
 use cli::CommandLine;
 use crane::CraneCLI;
-use krane_bundle::KRANE;
+use krane_bundle::ensure_krane_in_dir;
 use olpc_cjson::CanonicalFormatter;
 use serde::{Deserialize, Serialize};
 use snafu::ResultExt;
@@ -31,12 +31,12 @@ pub struct ImageTool {
 }
 
 impl ImageTool {
-    /// Uses the builtin `krane` provided by the `tools/krane` crate.
     pub fn from_builtin_krane() -> Self {
+        let tools_dir = std::path::Path::new("./build/tools");
+        let krane_path = ensure_krane_in_dir(tools_dir).expect("Failed to setup krane binary");
+
         let image_tool_impl = Box::new(CraneCLI {
-            cli: CommandLine {
-                path: KRANE.path().to_path_buf(),
-            },
+            cli: CommandLine { path: krane_path },
         });
         Self { image_tool_impl }
     }