fix path handling, introduce another (smaller scope) filesystem crate

Cargo.lock

@@ -2314,6 +2314,8 @@ dependencies = [
  "enums",
  "envvar",
  "errors",
+ "files",
+ "filesys",
  "google_drive_common",
  "hashing",
  "hostname",
@@ -2682,6 +2684,13 @@ dependencies = [
  "ring 0.17.0-alpha.11",
 ]
 
+[[package]]
+name = "filesys"
+version = "0.0.1"
+dependencies = [
+ "errors",
+]
+
 [[package]]
 name = "firestorm"
 version = "0.4.6"
@@ -3599,6 +3608,8 @@ dependencies = [
  "enums",
  "envvar",
  "errors",
+ "files",
+ "filesys",
  "hashing",
  "hostname",
  "hyper 0.14.24",

Cargo.toml

@@ -30,6 +30,7 @@ members = [
   "crates/lib/envvar",
   "crates/lib/errors",
   "crates/lib/files",
+  "crates/lib/filesys",
   "crates/lib/hashing",
   "crates/lib/idempotency",
   "crates/lib/jobs_common",

crates/lib/errors/Cargo.toml

@@ -16,6 +16,7 @@ path = "src/lib.rs"
 [dependencies]
 # NB: This is one of the most central crates in the monorepo and everything depends on it.
 # Consider the dependencies here wisely.
+# Ideally we have almost nothing as a dependency of this crate.
 anyhow = "1.0.40"
 
 [dev-dependencies]

crates/lib/files/src/lib.rs

@@ -1,6 +1,8 @@
 //! files
 //!
 //! The purpose of this library is to group common file (or in-memory file) operations.
+//! This crate is slightly higher level than the "filesys" crate and will contain more
+//! dependencies and file-specific features.
 //!
 
 // Never allow these

crates/lib/filesys/Cargo.toml

@@ -0,0 +1,24 @@
+[package]
+name = "filesys"
+edition = "2018"
+version = "0.0.1"
+authors = [
+    "Brandon Thomas <bt@brand.io>",
+    "Brandon Thomas <echelon@gmail.com>",
+]
+publish = false
+
+[lib]
+# NB: Public "filesys" crate does not exist, so it's safe to use for now.
+name = "filesys"
+path = "src/lib.rs"
+
+[dependencies]
+# Internal
+errors = { path = "../../lib/errors" }
+
+# External
+# KEEP THIS TO A MINIMUM OR NIL.
+
+[dev-dependencies]
+# None yet

crates/lib/filesys/src/filename_concat.rs

@@ -0,0 +1,78 @@
+use std::path::Path;
+use crate::path_to_string::path_to_string;
+
+/// Concatenate filenames while preserving path information. Do not join paths as directories.
+/// This contains no file normalizing logic, it's purely a concatenation.
+///
+///   eg. "foo" + "bar" results in "foobar" instead of the "foo/bar" that Path::join() returns.
+///   eg. "/usr/bin" + "baz" results in "/usr/binbaz" instead of "/usr/bin/baz"
+///
+/// This is not efficient.
+pub fn filename_concat<P: AsRef<Path>, Q: AsRef<Path>>(part_1: P, part_2: Q) -> String {
+  let mut base = path_to_string(part_1);
+  let suffix= path_to_string(part_2);
+  base.push_str(&suffix);
+  base
+}
+
+#[cfg(test)]
+mod tests {
+  use std::path::{PathBuf};
+  use crate::filename_concat::filename_concat;
+
+  // Layout: Base, Suffix, Expected Result
+  const TEST_CASES : [(&'static str, &'static str, &'static str); 11] = [
+    // Absolute
+    ("/", "foo", "/foo"),
+    ("/usr/bin", "baz", "/usr/binbaz"),
+    ("/usr/local", "/bin", "/usr/local/bin"),
+    // Relative
+    ("picture", ".jpg", "picture.jpg"),
+    ("/", "..", "/.."),
+    ("../", "..", "../.."),
+    ("./", "bin", "./bin"),
+    (".", "bar", ".bar"),
+    // Misc
+    ("", "", ""),
+    ("/", "", "/"),
+    ("", "/", "/"),
+  ];
+
+  #[test]
+  fn pathbuf() {
+    for (part_1, part_2, expected) in TEST_CASES.iter() {
+      let test_part_1 = PathBuf::from(part_1);
+      let test_part_2 = PathBuf::from(part_2);
+      assert_eq!(filename_concat(test_part_1, test_part_2), expected.to_string());
+    }
+  }
+
+  #[test]
+  fn path() {
+    for (part_1, part_2, expected) in TEST_CASES.iter() {
+      let test_part_1 = PathBuf::from(part_1);
+      let test_part_2 = PathBuf::from(part_2);
+
+      assert_eq!(filename_concat(test_part_1.as_path(), test_part_2.as_path()),
+                 expected.to_string());
+    }
+  }
+
+  #[test]
+  fn str() {
+    for (part_1, part_2, expected) in TEST_CASES.iter() {
+      let test_part_1 = *part_1;
+      let test_part_2 = *part_2;
+      assert_eq!(filename_concat(test_part_1, test_part_2), expected.to_string());
+    }
+  }
+
+  #[test]
+  fn mixed_type() {
+    for (part_1, part_2, expected) in TEST_CASES.iter() {
+      let test_part_1 = PathBuf::from(part_1);
+      let test_part_2 = *part_2;
+      assert_eq!(filename_concat(test_part_1, test_part_2), expected.to_string());
+    }
+  }
+}

crates/lib/filesys/src/lib.rs

@@ -0,0 +1,22 @@
+//! filesys
+//!
+//! The purpose of this library is to make basic filesystem operations easier.
+//! We won't be including content type or magic type features that incur a higher cost.
+//!
+
+// Never allow these
+#![forbid(private_in_public)]
+#![forbid(unused_must_use)] // NB: It's unsafe to not close/check some things
+
+// Okay to toggle
+#![forbid(unreachable_patterns)]
+#![forbid(unused_imports)]
+#![forbid(unused_mut)]
+#![forbid(unused_variables)]
+
+// Always allow
+#![allow(dead_code)]
+#![allow(non_snake_case)]
+
+pub mod filename_concat;
+pub mod path_to_string;

crates/lib/filesys/src/path_to_string.rs

@@ -0,0 +1,52 @@
+use std::path::Path;
+
+/// Convert a path to a string without adding quotes or garbage.
+pub fn path_to_string<P: AsRef<Path>>(path: P) -> String {
+  path.as_ref().display().to_string()
+}
+
+#[cfg(test)]
+mod tests {
+  use std::path::{PathBuf};
+  use crate::path_to_string::path_to_string;
+
+  const TEST_CASES : [&'static str; 9] = [
+    // Absolute
+    "/",
+    "/usr/bin",
+    // Relative
+    "picture.jpg",
+    "foo/bar/baz",
+    "../",
+    "./",
+    ".",
+    // Misc
+    "",
+    ".././../foo/.././bar",
+  ];
+
+  #[test]
+  fn pathbuf() {
+    for example in TEST_CASES.iter() {
+      let test = PathBuf::from(example);
+      assert_eq!(path_to_string(test), example.to_string());
+    }
+  }
+
+  #[test]
+  fn path() {
+    for example in TEST_CASES.iter() {
+      let path = PathBuf::from(example);
+      let test = path.as_path();
+      assert_eq!(path_to_string(test), example.to_string());
+    }
+  }
+
+  #[test]
+  fn str() {
+    for example in TEST_CASES.iter() {
+      let test = *example;
+      assert_eq!(path_to_string(test), example.to_string());
+    }
+  }
+}

crates/service/job/download_job/Cargo.toml

@@ -21,6 +21,8 @@ container_common = { path = "../../../lib/deprecated/container_common" }
 enums = { path = "../../../_types/enums" }
 envvar = { path = "../../../lib/envvar" }
 errors = { path = "../../../lib/errors" }
+files = { path = "../../../lib/files" }
+filesys = { path = "../../../lib/filesys" }
 google_drive_common = { path = "../../../lib/api_clients/google_drive_common" }
 hashing = { path = "../../../lib/hashing" }
 jobs_common = { path = "../../../lib/jobs_common" }

crates/service/job/download_job/src/job_types/tts/vits/process_vits_model.rs

@@ -18,6 +18,7 @@ use std::fs::File;
 use std::io::Read;
 use std::path::{Path, PathBuf};
 use tempdir::TempDir;
+use filesys::filename_concat::filename_concat;
 
 /// Returns the token of the entity.
 pub async fn process_vits_model<'a, 'b>(
@@ -37,7 +38,7 @@ pub async fn process_vits_model<'a, 'b>(
   let original_model_file_path = PathBuf::from(download_filename.clone());
 
   // NB: We'll be creating the traced model in the "check" step and uploading it to GDrive along with the original.
-  let traced_model_file_path = PathBuf::from(format!("{:?}_traced", original_model_file_path.as_os_str()));
+  let traced_model_file_path = PathBuf::from(filename_concat(&original_model_file_path, "_traced"));
 
   let config_path = PathBuf::from("configs/ljs_li44_tmbert_nmp_s1_arpa.json"); // TODO: This could be variable.
 

crates/service/job/inference_job/Cargo.toml

@@ -21,6 +21,8 @@ container_common = { path = "../../../lib/deprecated/container_common" }
 enums = { path = "../../../_types/enums" }
 envvar = { path = "../../../lib/envvar" }
 errors = { path = "../../../lib/errors" }
+files = { path = "../../../lib/files" }
+filesys = { path = "../../../lib/filesys" }
 hashing = { path = "../../../lib/hashing" }
 jobs_common = { path = "../../../lib/jobs_common" }
 memory_caching = { path = "../../../lib/caching/memory_caching" }

crates/service/job/inference_job/src/job/job_types/tts/tacotron2_v2_early_fakeyou/tacotron2_inference_command.rs

@@ -1,4 +1,5 @@
 use crate::AnyhowResult;
+use filesys::path_to_string::path_to_string;
 use log::info;
 use std::path::{Path, PathBuf};
 use subprocess::{Popen, PopenConfig};
@@ -108,7 +109,7 @@ impl Tacotron2InferenceCommand {
   ) -> AnyhowResult<()> {
 
     let mut command = String::new();
-    command.push_str(&format!("cd {}", path_to_str(&self.tacotron_code_root_directory)));
+    command.push_str(&format!("cd {}", path_to_string(&self.tacotron_code_root_directory)));
 
     if let Some(venv_command) = self.maybe_virtual_env_activation_command.as_deref() {
       command.push_str(" && ");
@@ -123,12 +124,12 @@ impl Tacotron2InferenceCommand {
     command.push_str(" && ");
     command.push_str(python_binary);
     command.push_str(" ");
-    command.push_str(&path_to_str(&self.inference_script_name));
+    command.push_str(&path_to_string(&self.inference_script_name));
 
     // ===== Begin Python Inference Args =====
 
     command.push_str(" --synthesizer_checkpoint_path ");
-    command.push_str(&path_to_str(args.synthesizer_checkpoint_path));
+    command.push_str(&path_to_string(args.synthesizer_checkpoint_path));
 
     command.push_str(" --text_pipeline_type ");
     command.push_str(args.text_pipeline_type);
@@ -139,7 +140,7 @@ impl Tacotron2InferenceCommand {
         command.push_str("waveglow");
 
         command.push_str(" --waveglow_vocoder_checkpoint_path ");
-        command.push_str(&path_to_str(waveglow_vocoder_checkpoint_path));
+        command.push_str(&path_to_string(waveglow_vocoder_checkpoint_path));
       }
       VocoderForInferenceOption::HifiganSuperres {
         hifigan_vocoder_checkpoint_path,
@@ -149,10 +150,10 @@ impl Tacotron2InferenceCommand {
         command.push_str("hifigan-superres");
 
         command.push_str(" --hifigan_vocoder_checkpoint_path ");
-        command.push_str(&path_to_str(hifigan_vocoder_checkpoint_path));
+        command.push_str(&path_to_string(hifigan_vocoder_checkpoint_path));
 
         command.push_str(" --hifigan_superres_vocoder_checkpoint_path ");
-        command.push_str(&path_to_str(hifigan_superres_vocoder_checkpoint_path));
+        command.push_str(&path_to_string(hifigan_superres_vocoder_checkpoint_path));
       }
     }
 
@@ -169,16 +170,16 @@ impl Tacotron2InferenceCommand {
     }
 
     command.push_str(" --input_text_filename ");
-    command.push_str(&path_to_str(args.input_text_filename));
+    command.push_str(&path_to_string(args.input_text_filename));
 
     command.push_str(" --output_audio_filename ");
-    command.push_str(&path_to_str(args.output_audio_filename));
+    command.push_str(&path_to_string(args.output_audio_filename));
 
     command.push_str(" --output_spectrogram_filename ");
-    command.push_str(&path_to_str(args.output_spectrogram_filename));
+    command.push_str(&path_to_string(args.output_spectrogram_filename));
 
     command.push_str(" --output_metadata_filename ");
-    command.push_str(&path_to_str(args.output_metadata_filename));
+    command.push_str(&path_to_string(args.output_metadata_filename));
 
     // ===== End Python Inference Args =====
 
@@ -207,7 +208,3 @@ impl Tacotron2InferenceCommand {
     Ok(())
   }
 }
-
-fn path_to_str<P: AsRef<Path>>(path: P) -> String {
-  path.as_ref().display().to_string()
-}