Merge pull request #129 from webern/fix-remove-dir

provide crate fs functions and fix a remove_dir_all bug

twoliter/src/cmd/debug.rs

@@ -1,9 +1,9 @@
+use crate::common::fs;
 use crate::tools::install_tools;
-use anyhow::{Context, Result};
+use anyhow::Result;
 use clap::Parser;
 use std::env;
 use std::path::PathBuf;
-use tokio::fs;
 use uuid::Uuid;
 
 #[derive(Debug, Clone, Parser)]
@@ -49,9 +49,7 @@ impl CheckToolArgs {
             .install_dir
             .clone()
             .unwrap_or_else(|| env::temp_dir().join(unique_name()));
-        fs::create_dir_all(&dir)
-            .await
-            .context(format!("Unable to create directory '{}'", dir.display()))?;
+        fs::create_dir_all(&dir).await?;
         install_tools(&dir).await?;
         println!("{}", dir.display());
         Ok(())

twoliter/src/cmd/make.rs

@@ -1,10 +1,10 @@
 use crate::cargo_make::CargoMake;
+use crate::common::fs;
 use crate::project::{self};
 use crate::tools::install_tools;
 use anyhow::Result;
 use clap::Parser;
 use std::path::PathBuf;
-use tokio::fs;
 
 /// Run a cargo make command in Twoliter's build environment. Known Makefile.toml environment
 /// variables will be passed-through to the cargo make invocation.
@@ -35,7 +35,7 @@ impl Make {
     pub(super) async fn run(&self) -> Result<()> {
         let project = project::load_or_find_project(self.project_path.clone()).await?;
         let toolsdir = project.project_dir().join("build/tools");
-        fs::remove_dir_all(&toolsdir).await?;
+        let _ = fs::remove_dir_all(&toolsdir).await;
         install_tools(&toolsdir).await?;
         let makefile_path = toolsdir.join("Makefile.toml");
         CargoMake::new(&project, &self.arch)?

twoliter/src/common.rs

@@ -52,3 +52,114 @@ pub(crate) async fn exec(cmd: &mut Command, quiet: bool) -> Result<Option<String
         None
     })
 }
+
+/// These are thin wrappers for `tokio::fs` functions which provide more useful error messages. For
+/// example, tokio will provide an unhelpful `std` error message such as `Error: No such file or
+/// directory (os error 2)` and we want to augment this with the filepath that was not found.
+///
+/// We allow `dead_code` here because it is inconvenient to delete and replace these simple helper
+/// functions as we change calling code. The compiler will strip dead code in release builds anyway,
+/// so there is no real issue having these unused here.
+#[allow(dead_code)]
+pub(crate) mod fs {
+    use anyhow::{Context, Result};
+    use std::fs::Metadata;
+    use std::path::{Path, PathBuf};
+    use tokio::fs;
+
+    pub(crate) async fn canonicalize(path: impl AsRef<Path>) -> Result<PathBuf> {
+        fs::canonicalize(path.as_ref()).await.context(format!(
+            "Unable to canonicalize '{}'",
+            path.as_ref().display()
+        ))
+    }
+
+    pub(crate) async fn copy<P1, P2>(from: P1, to: P2) -> Result<u64>
+    where
+        P1: AsRef<Path>,
+        P2: AsRef<Path>,
+    {
+        let from = from.as_ref();
+        let to = to.as_ref();
+        fs::copy(from, to).await.context(format!(
+            "Unable to copy '{}' to '{}'",
+            from.display(),
+            to.display()
+        ))
+    }
+
+    pub(crate) async fn create_dir(path: impl AsRef<Path>) -> Result<()> {
+        fs::create_dir(path.as_ref()).await.context(format!(
+            "Unable to create directory '{}'",
+            path.as_ref().display()
+        ))
+    }
+
+    pub(crate) async fn create_dir_all(path: impl AsRef<Path>) -> Result<()> {
+        fs::create_dir_all(path.as_ref()).await.context(format!(
+            "Unable to create directory '{}'",
+            path.as_ref().display()
+        ))
+    }
+
+    pub(crate) async fn metadata(path: impl AsRef<Path>) -> Result<Metadata> {
+        fs::metadata(path.as_ref()).await.context(format!(
+            "Unable to read metadata for '{}'",
+            path.as_ref().display()
+        ))
+    }
+
+    pub(crate) async fn read(path: impl AsRef<Path>) -> Result<Vec<u8>> {
+        fs::read(path.as_ref())
+            .await
+            .context(format!("Unable to read from '{}'", path.as_ref().display()))
+    }
+
+    pub(crate) async fn read_to_string(path: impl AsRef<Path>) -> Result<String> {
+        fs::read_to_string(path.as_ref()).await.context(format!(
+            "Unable to read the following file as a string '{}'",
+            path.as_ref().display()
+        ))
+    }
+
+    pub(crate) async fn remove_dir(path: impl AsRef<Path>) -> Result<()> {
+        fs::remove_dir(path.as_ref()).await.context(format!(
+            "Unable to remove directory (remove_dir) '{}'",
+            path.as_ref().display()
+        ))
+    }
+
+    pub(crate) async fn remove_dir_all(path: impl AsRef<Path>) -> Result<()> {
+        fs::remove_dir_all(path.as_ref()).await.context(format!(
+            "Unable to remove directory (remove_dir_all) '{}'",
+            path.as_ref().display()
+        ))
+    }
+
+    pub(crate) async fn rename(from: impl AsRef<Path>, to: impl AsRef<Path>) -> Result<()> {
+        let from = from.as_ref();
+        let to = to.as_ref();
+        fs::rename(from, to).await.context(format!(
+            "Unable to rename '{}' to '{}'",
+            from.display(),
+            to.display()
+        ))
+    }
+
+    pub(crate) async fn remove_file(path: impl AsRef<Path>) -> Result<()> {
+        fs::remove_file(path.as_ref()).await.context(format!(
+            "Unable to XXSOMETHINGXX '{}'",
+            path.as_ref().display()
+        ))
+    }
+
+    pub(crate) async fn write<P, C>(path: P, contents: C) -> Result<()>
+    where
+        P: AsRef<Path>,
+        C: AsRef<[u8]>,
+    {
+        fs::write(path.as_ref(), contents)
+            .await
+            .context(format!("Unable to write to '{}'", path.as_ref().display()))
+    }
+}

twoliter/src/project.rs

@@ -1,3 +1,4 @@
+use crate::common::fs;
 use crate::docker::ImageArchUri;
 use crate::schema_version::SchemaVersion;
 use anyhow::{ensure, Context, Result};
@@ -7,7 +8,6 @@ use non_empty_string::NonEmptyString;
 use serde::{Deserialize, Serialize};
 use sha2::{Digest, Sha512};
 use std::path::{Path, PathBuf};
-use tokio::fs;
 use toml::Table;
 
 /// Common functionality in commands, if the user gave a path to the `Twoliter.toml` file,
@@ -241,9 +241,9 @@ impl UnvalidatedProject {
 #[cfg(test)]
 mod test {
     use super::*;
+    use crate::common::fs;
     use crate::test::data_dir;
     use tempfile::TempDir;
-    use tokio::fs;
 
     /// Ensure that `Twoliter.toml` can be deserialized.
     #[tokio::test]
@@ -341,18 +341,10 @@ mod test {
         let release_toml_to = p.join("Release.toml");
         fs::copy(&twoliter_toml_from, &twoliter_toml_to)
             .await
-            .expect(&format!(
-                "Unable to copy {} to {}",
-                twoliter_toml_from.display(),
-                twoliter_toml_to.display()
-            ));
+            .unwrap();
         fs::copy(&release_toml_from, &release_toml_to)
             .await
-            .expect(&format!(
-                "Unable to copy {} to {}",
-                release_toml_from.display(),
-                release_toml_to.display()
-            ));
+            .unwrap();
         let result = Project::find_and_load(&p).await;
         assert!(
             result.is_err(),
@@ -372,18 +364,10 @@ mod test {
         let release_toml_to = p.join("Release.toml");
         fs::copy(&twoliter_toml_from, &twoliter_toml_to)
             .await
-            .expect(&format!(
-                "Unable to copy {} to {}",
-                twoliter_toml_from.display(),
-                twoliter_toml_to.display()
-            ));
+            .unwrap();
         fs::copy(&release_toml_from, &release_toml_to)
             .await
-            .expect(&format!(
-                "Unable to copy {} to {}",
-                release_toml_from.display(),
-                release_toml_to.display()
-            ));
+            .unwrap();
 
         // The project should load because Release.toml and Twoliter.toml versions match.
         Project::find_and_load(&p).await.unwrap();

twoliter/src/tools.rs

@@ -1,11 +1,13 @@
+use crate::common::fs;
 use anyhow::{Context, Result};
 use filetime::{set_file_handle_times, set_file_mtime, FileTime};
 use flate2::read::ZlibDecoder;
 use log::debug;
 use std::path::Path;
 use tar::Archive;
-use tokio::fs;
+use tokio::fs::OpenOptions;
 use tokio::io::AsyncWriteExt;
+use tokio::runtime::Handle;
 
 const TAR_GZ_DATA: &[u8] = include_bytes!(concat!(env!("OUT_DIR"), "/tools.tar.gz"));
 const BOTTLEROCKET_VARIANT: &[u8] =
@@ -49,7 +51,7 @@ pub(crate) async fn install_tools(tools_dir: impl AsRef<Path>) -> Result<()> {
 
 async fn write_bin(name: &str, data: &[u8], dir: impl AsRef<Path>, mtime: FileTime) -> Result<()> {
     let path = dir.as_ref().join(name);
-    let mut f = fs::OpenOptions::new()
+    let mut f = OpenOptions::new()
         .create(true)
         .read(false)
         .write(true)
@@ -65,9 +67,15 @@ async fn write_bin(name: &str, data: &[u8], dir: impl AsRef<Path>, mtime: FileTi
         .context(format!("Unable to finalize '{}'", path.display()))?;
 
     let f = f.into_std().await;
-    set_file_handle_times(&f, None, Some(mtime))
-        .context(format!("Unable to set mtime for '{}'", path.display()))?;
-    Ok(())
+    let rt = Handle::current();
+    rt.spawn_blocking(move || {
+        set_file_handle_times(&f, None, Some(mtime))
+            .context(format!("Unable to set mtime for '{}'", path.display()))
+    })
+    .await
+    .context(format!(
+        "Unable to run and join async task for reading handle time"
+    ))?
 }
 
 async fn unpack_tarball(tools_dir: impl AsRef<Path>) -> Result<()> {