diff --git a/Cargo.lock b/Cargo.lock index f7d107c5d..4019150b1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -147,6 +147,7 @@ version = "0.1.0" dependencies = [ "assert_fs", "atty", + "camino", "cc", "directories-next", "serial_test", @@ -213,6 +214,12 @@ version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b700ce4376041dcd0a327fd0097c41095743c4c8af8887265942faf1100bd040" +[[package]] +name = "camino" +version = "1.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cd065703998b183ed0b348a22555691373a9345a1431141e5778b48bb17e4703" + [[package]] name = "cc" version = "1.0.67" @@ -799,6 +806,7 @@ name = "houston" version = "0.0.0" dependencies = [ "assert_fs", + "camino", "directories-next", "regex", "serde", @@ -1585,6 +1593,7 @@ name = "robot-panic" version = "1.0.3" dependencies = [ "backtrace", + "camino", "os_type", "serde", "termcolor", @@ -1604,6 +1613,7 @@ dependencies = [ "atty", "billboard", "binstall", + "camino", "chrono", "console 0.14.0", "git-url-parse", @@ -1639,6 +1649,7 @@ name = "rover-client" version = "0.0.0" dependencies = [ "anyhow", + "camino", "chrono", "graphql_client", "houston", @@ -1877,6 +1888,7 @@ name = "sputnik" version = "0.0.0" dependencies = [ "assert_fs", + "camino", "ci_info", "reqwest", "semver", diff --git a/Cargo.toml b/Cargo.toml index 5d30c2d31..4501a62b2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,6 +19,7 @@ timber = { path = "./crates/timber" } anyhow = "1.0.38" atty = "0.2.14" ansi_term = "0.12.1" +camino = "1.0.2" billboard = { git = "https://github.com/EverlastingBugstopper/billboard.git", branch = "main" } chrono = "0.4" console = "0.14.0" @@ -52,4 +53,5 @@ members = [".", "crates/*", "installers/binstall"] [build-dependencies] anyhow = "1" +camino = "1.0" which = "4.0.2" diff --git a/build.rs b/build.rs index 471310565..567b7f89c 100644 --- a/build.rs +++ b/build.rs @@ -1,11 +1,13 @@ -use anyhow::{anyhow, Context, Result}; +use anyhow::{anyhow, Context, Error, Result}; use std::{ env, fs, - path::Path, + path::PathBuf, process::{Command, Output}, str, }; +use camino::{Utf8Path, Utf8PathBuf}; + /// files to copy from the repo's root directory into the npm tarball const FILES_TO_COPY: &[&str; 2] = &["LICENSE", "README.md"]; @@ -39,12 +41,18 @@ fn cargo_warn(message: &str) { /// these steps are only _required_ when running in release mode fn prep_npm(is_release_build: bool) -> Result<()> { let npm_install_path = match which::which("npm") { - Ok(install_path) => Some(install_path), + Ok(install_path) => { + Some(Utf8PathBuf::from_path_buf(install_path).map_err(|pb| invalid_path_buf(&pb))?) + } Err(_) => None, }; // we have to work with absolute paths like this because of windows :P - let current_dir = env::current_dir().context("Could not find the current directory.")?; + let current_dir = Utf8PathBuf::from_path_buf( + env::current_dir().context("Could not find the current directory.")?, + ) + .map_err(|pb| invalid_path_buf(&pb))?; + let npm_dir = current_dir.join("installers").join("npm"); let is_npm_installed = npm_install_path.is_some(); @@ -52,7 +60,7 @@ fn prep_npm(is_release_build: bool) -> Result<()> { if !npm_dir.exists() { return Err(anyhow!( "The npm package does not seem to be located here:\n{}", - npm_dir.display() + &npm_dir )); } @@ -86,6 +94,10 @@ fn prep_npm(is_release_build: bool) -> Result<()> { Ok(()) } +fn invalid_path_buf(pb: &PathBuf) -> Error { + anyhow!("Current directory \"{}\" is not valid UTF-8", pb.display()) +} + fn process_command_output(output: &Output) -> Result<()> { if !output.status.success() { let stdout = @@ -98,7 +110,7 @@ fn process_command_output(output: &Output) -> Result<()> { Ok(()) } -fn update_dependency_tree(npm_install_path: &Path, npm_dir: &Path) -> Result<()> { +fn update_dependency_tree(npm_install_path: &Utf8Path, npm_dir: &Utf8Path) -> Result<()> { let command_output = Command::new(npm_install_path) .current_dir(npm_dir) .arg("update") @@ -110,7 +122,7 @@ fn update_dependency_tree(npm_install_path: &Path, npm_dir: &Path) -> Result<()> Ok(()) } -fn install_dependencies(npm_install_path: &Path, npm_dir: &Path) -> Result<()> { +fn install_dependencies(npm_install_path: &Utf8Path, npm_dir: &Utf8Path) -> Result<()> { let command_output = Command::new(npm_install_path) .current_dir(npm_dir) .arg("install") @@ -120,7 +132,7 @@ fn install_dependencies(npm_install_path: &Path, npm_dir: &Path) -> Result<()> { process_command_output(&command_output).context("Could not print output of 'npm install'.") } -fn update_version(npm_install_path: &Path, npm_dir: &Path) -> Result<()> { +fn update_version(npm_install_path: &Utf8Path, npm_dir: &Utf8Path) -> Result<()> { let command_output = Command::new(npm_install_path) .current_dir(npm_dir) .arg("version") @@ -138,7 +150,7 @@ fn update_version(npm_install_path: &Path, npm_dir: &Path) -> Result<()> { .with_context(|| format!("Could not print output of 'npm version {}'.", PKG_VERSION)) } -fn dry_run_publish(npm_install_path: &Path, npm_dir: &Path) -> Result<()> { +fn dry_run_publish(npm_install_path: &Utf8Path, npm_dir: &Utf8Path) -> Result<()> { let command_output = Command::new(npm_install_path) .current_dir(npm_dir) .arg("publish") @@ -150,7 +162,11 @@ fn dry_run_publish(npm_install_path: &Path, npm_dir: &Path) -> Result<()> { .context("Could not print output of 'npm publish --dry-run'.") } -fn copy_files_to_npm_package(files: &[&str], current_dir: &Path, npm_dir: &Path) -> Result<()> { +fn copy_files_to_npm_package( + files: &[&str], + current_dir: &Utf8Path, + npm_dir: &Utf8Path, +) -> Result<()> { for file in files { let context = format!("Could not copy {} to npm package.", &file); fs::copy(current_dir.join(file), npm_dir.join(file)).context(context)?; diff --git a/crates/houston/Cargo.toml b/crates/houston/Cargo.toml index e2cd72c6f..abb6038b6 100644 --- a/crates/houston/Cargo.toml +++ b/crates/houston/Cargo.toml @@ -5,6 +5,7 @@ authors = ["Apollo Developers "] edition = "2018" [dependencies] +camino = "1.0" directories-next = "2.0" serde = { version = "1.0", features = ["derive"] } thiserror = "1.0" diff --git a/crates/houston/src/config.rs b/crates/houston/src/config.rs index 563867192..219bd7124 100644 --- a/crates/houston/src/config.rs +++ b/crates/houston/src/config.rs @@ -2,8 +2,8 @@ use directories_next::ProjectDirs; use crate::HoustonProblem; +use camino::{Utf8Path, Utf8PathBuf}; use std::fs; -use std::path::{Path, PathBuf}; /// Config allows end users to override default settings /// usually determined by Houston. They are intended to @@ -12,7 +12,7 @@ use std::path::{Path, PathBuf}; #[derive(Debug, Clone)] pub struct Config { /// home is the path to the user's global config directory - pub home: PathBuf, + pub home: Utf8PathBuf, /// override_api_key is used for overriding the API key returned /// when loading a profile @@ -22,15 +22,15 @@ pub struct Config { impl Config { /// Creates a new instance of `Config` pub fn new( - override_home: Option<&impl AsRef>, + override_home: Option<&impl AsRef>, override_api_key: Option, ) -> Result { let home = match override_home { Some(home) => { - let home_path = PathBuf::from(home.as_ref()); + let home_path = Utf8PathBuf::from(home.as_ref()); if home_path.exists() && !home_path.is_dir() { Err(HoustonProblem::InvalidOverrideConfigDir( - home_path.display().to_string(), + home_path.to_string(), )) } else { Ok(home_path) @@ -40,17 +40,22 @@ impl Config { // Lin: /home/alice/.config/rover // Win: C:\Users\Alice\AppData\Roaming\Apollo\Rover\config // Mac: /Users/Alice/Library/Application Support/com.Apollo.Rover - Ok(ProjectDirs::from("com", "Apollo", "Rover") + let project_dirs = ProjectDirs::from("com", "Apollo", "Rover") .ok_or(HoustonProblem::DefaultConfigDirNotFound)? .config_dir() - .to_path_buf()) + .to_path_buf(); + + Ok(Utf8PathBuf::from_path_buf(project_dirs).map_err(|pb| { + HoustonProblem::PathNotUnicode { + path_display: pb.display().to_string(), + } + })?) } }?; if !home.exists() { - fs::create_dir_all(&home).map_err(|_| { - HoustonProblem::CouldNotCreateConfigHome(home.display().to_string()) - })?; + fs::create_dir_all(&home) + .map_err(|_| HoustonProblem::CouldNotCreateConfigHome(home.to_string()))?; } Ok(Config { @@ -63,7 +68,7 @@ impl Config { pub fn clear(&self) -> Result<(), HoustonProblem> { tracing::debug!(home_dir = ?self.home); fs::remove_dir_all(&self.home) - .map_err(|_| HoustonProblem::NoConfigFound(self.home.display().to_string())) + .map_err(|_| HoustonProblem::NoConfigFound(self.home.to_string())) } } @@ -71,10 +76,12 @@ impl Config { mod tests { use super::Config; use assert_fs::TempDir; + use camino::Utf8PathBuf; #[test] fn it_can_clear_global_config() { let tmp_home = TempDir::new().unwrap(); - let config = Config::new(Some(&tmp_home.path()), None).unwrap(); + let tmp_path = Utf8PathBuf::from_path_buf(tmp_home.path().to_path_buf()).unwrap(); + let config = Config::new(Some(&tmp_path), None).unwrap(); assert!(config.home.exists()); config.clear().unwrap(); assert_eq!(config.home.exists(), false); diff --git a/crates/houston/src/error.rs b/crates/houston/src/error.rs index 46086f919..e319fdf9c 100644 --- a/crates/houston/src/error.rs +++ b/crates/houston/src/error.rs @@ -21,6 +21,13 @@ pub enum HoustonProblem { #[error("Could not find a configuration directory at \"{0}\".")] NoConfigFound(String), + /// PathNotUnicode occurs when Houston encounteres a file path that is not valid UTF-8 + #[error("File path \"{path_display}\" is not valid Unicode")] + PathNotUnicode { + /// The display name of the invalid path + path_display: String, + }, + /// ProfileNotFound occurs when a profile with a specified name can't be found. #[error("There is no profile named \"{0}\".")] ProfileNotFound(String), diff --git a/crates/houston/src/profile/mod.rs b/crates/houston/src/profile/mod.rs index 6632d34e5..32e39ae3e 100644 --- a/crates/houston/src/profile/mod.rs +++ b/crates/houston/src/profile/mod.rs @@ -4,7 +4,7 @@ use crate::{Config, HoustonProblem}; use sensitive::Sensitive; use serde::{Deserialize, Serialize}; -use std::path::PathBuf; +use camino::Utf8PathBuf as PathBuf; use std::{fmt, fs, io}; /// Collects configuration related to a profile. diff --git a/crates/houston/src/profile/sensitive.rs b/crates/houston/src/profile/sensitive.rs index 9226bfe23..f5f96c046 100644 --- a/crates/houston/src/profile/sensitive.rs +++ b/crates/houston/src/profile/sensitive.rs @@ -1,7 +1,7 @@ use crate::{profile::Profile, Config, HoustonProblem}; use serde::{Deserialize, Serialize}; -use std::path::PathBuf; +use camino::Utf8PathBuf as PathBuf; use std::{fmt, fs}; /// Holds sensitive information regarding authentication. diff --git a/crates/houston/tests/auth.rs b/crates/houston/tests/auth.rs index 197b0436a..d5aa7a126 100644 --- a/crates/houston/tests/auth.rs +++ b/crates/houston/tests/auth.rs @@ -2,6 +2,7 @@ use config::Config; use houston as config; use assert_fs::TempDir; +use camino::Utf8Path; #[test] fn it_can_set_and_get_an_api_key_via_creds_file() { @@ -51,5 +52,6 @@ fn it_can_get_an_api_key_via_env_var() { fn get_config(override_api_key: Option) -> Config { let tmp_home = TempDir::new().unwrap(); - Config::new(Some(&tmp_home.path()), override_api_key).unwrap() + let tmp_home_path = Utf8Path::from_path(tmp_home.path()).unwrap().to_owned(); + Config::new(Some(&tmp_home_path), override_api_key).unwrap() } diff --git a/crates/houston/tests/profile.rs b/crates/houston/tests/profile.rs index 3ba17ac40..66e3a2a4e 100644 --- a/crates/houston/tests/profile.rs +++ b/crates/houston/tests/profile.rs @@ -1,4 +1,5 @@ use assert_fs::TempDir; +use camino::Utf8Path; use config::Config; use houston as config; @@ -28,5 +29,6 @@ fn it_lists_many_profiles() { fn get_config(override_api_key: Option) -> Config { let tmp_home = TempDir::new().unwrap(); - Config::new(Some(&tmp_home.path()), override_api_key).unwrap() + let tmp_home_path = Utf8Path::from_path(tmp_home.path()).unwrap().to_owned(); + Config::new(Some(&tmp_home_path), override_api_key).unwrap() } diff --git a/crates/robot-panic/Cargo.toml b/crates/robot-panic/Cargo.toml index d30431603..053834da2 100644 --- a/crates/robot-panic/Cargo.toml +++ b/crates/robot-panic/Cargo.toml @@ -5,6 +5,7 @@ edition = "2018" [dependencies] backtrace = "0.3" +camino = "1.0" os_type = "2.2" serde = "1.0" termcolor = "1.1" diff --git a/crates/robot-panic/src/lib.rs b/crates/robot-panic/src/lib.rs index 4dafc03b7..c338f1c6e 100644 --- a/crates/robot-panic/src/lib.rs +++ b/crates/robot-panic/src/lib.rs @@ -180,8 +180,7 @@ pub fn print_msg(crash_report: &Report, meta: &Metadata) -> IoResult<()> { "We have generated a report file at \"{}\". Submit an \ issue with the subject of \"{} Crash Report\" and include the \ report as an attachment.", - path.display(), - name + path, name )?; } Err(_) => { diff --git a/crates/robot-panic/src/report.rs b/crates/robot-panic/src/report.rs index ee434fea3..25dd64a34 100644 --- a/crates/robot-panic/src/report.rs +++ b/crates/robot-panic/src/report.rs @@ -7,9 +7,10 @@ use std::borrow::Cow; use std::error::Error; use std::fmt::Write as FmtWrite; use std::mem; -use std::{env, fs::File, io::Write, path::Path, path::PathBuf}; +use std::{env, fs::File, io::Write}; use backtrace::Backtrace; +use camino::Utf8PathBuf; use serde::Serialize; use url::Url; use uuid::Uuid; @@ -131,11 +132,16 @@ impl Report { } /// Write a file to disk. - pub fn persist(&self) -> Result> { + pub fn persist(&self) -> Result> { let uuid = Uuid::new_v4().to_hyphenated().to_string(); let tmp_dir = env::temp_dir(); let file_name = format!("report-{}.toml", &uuid); - let file_path = Path::new(&tmp_dir).join(file_name); + let base_file_path = Utf8PathBuf::from_path_buf(tmp_dir).map_err(|pb| { + let err: Box = + format!("file \"{}\" is not valid UTF-8", pb.display()).into(); + err + })?; + let file_path = base_file_path.join(file_name); let mut file = File::create(&file_path)?; let toml = self.serialize().unwrap(); file.write_all(toml.as_bytes())?; diff --git a/crates/rover-client/Cargo.toml b/crates/rover-client/Cargo.toml index 165be8c3c..633c84218 100644 --- a/crates/rover-client/Cargo.toml +++ b/crates/rover-client/Cargo.toml @@ -12,6 +12,7 @@ houston = { path = "../houston" } # crates.io deps anyhow = "1" +camino = "1" graphql_client = "0.9" http = "0.2" reqwest = { version = "0.11", features = ["json", "blocking", "native-tls-vendored"] } @@ -23,6 +24,7 @@ chrono = "0.4" regex = "1" [build-dependencies] +camino = "1" online = "0.2.2" reqwest = { version = "0.11", features = ["blocking", "native-tls-vendored"] } uuid = { version = "0.8", features = ["v4"] } diff --git a/crates/rover-client/build.rs b/crates/rover-client/build.rs index 27d7aa585..926ffa234 100644 --- a/crates/rover-client/build.rs +++ b/crates/rover-client/build.rs @@ -1,7 +1,7 @@ use std::env; use std::fs::{self, read, write}; -use std::path::PathBuf; +use camino::Utf8PathBuf as PathBuf; use reqwest::blocking::Client; use uuid::Uuid; diff --git a/crates/sputnik/Cargo.toml b/crates/sputnik/Cargo.toml index e816cd2ff..d62833b86 100644 --- a/crates/sputnik/Cargo.toml +++ b/crates/sputnik/Cargo.toml @@ -5,6 +5,7 @@ authors = ["Apollo Developers "] edition = "2018" [dependencies] +camino = "1.0" ci_info = { version = "0.14", features = ["serde-1"] } reqwest = { version = "0.11", features = ["blocking"] } semver = { version = "0.11", features = ["serde"] } diff --git a/crates/sputnik/src/report.rs b/crates/sputnik/src/report.rs index e05cec033..2b96f5b94 100644 --- a/crates/sputnik/src/report.rs +++ b/crates/sputnik/src/report.rs @@ -1,9 +1,9 @@ use url::Url; use uuid::Uuid; +use camino::{Utf8Path, Utf8PathBuf}; use std::fs::{self, File}; use std::io::Write; -use std::path::{Path, PathBuf}; use crate::{Command, SputnikError}; @@ -34,7 +34,7 @@ pub trait Report { /// returns the location the tool stores a globally persistent /// machine identifier - fn machine_id_config(&self) -> Result; + fn machine_id_config(&self) -> Result; /// returns the globally persistent machine identifier /// and writes it if it does not exist @@ -46,8 +46,8 @@ pub trait Report { } } -fn get_or_write_machine_id(path: &PathBuf) -> Result { - if Path::exists(path) { +fn get_or_write_machine_id(path: &Utf8PathBuf) -> Result { + if Utf8Path::exists(path) { if let Ok(contents) = fs::read_to_string(path) { if let Ok(machine_uuid) = Uuid::parse_str(&contents) { return Ok(machine_uuid); @@ -58,7 +58,7 @@ fn get_or_write_machine_id(path: &PathBuf) -> Result { write_machine_id(path) } -fn write_machine_id(path: &PathBuf) -> Result { +fn write_machine_id(path: &Utf8PathBuf) -> Result { let machine_id = Uuid::new_v4(); let mut file = File::create(path)?; file.write_all(machine_id.to_string().as_bytes())?; @@ -70,6 +70,7 @@ mod tests { use super::{get_or_write_machine_id, write_machine_id}; use assert_fs::prelude::*; + use camino::Utf8PathBuf; /// if a machine ID hasn't been written already, one will be created /// and saved. @@ -77,7 +78,7 @@ mod tests { fn it_can_write_machine_id() { let fixture = assert_fs::TempDir::new().unwrap(); let test_file = fixture.child("test_write_machine_id.txt"); - let test_path = test_file.path().to_path_buf(); + let test_path = Utf8PathBuf::from_path_buf(test_file.path().to_path_buf()).unwrap(); assert!(write_machine_id(&test_path).is_ok()); } @@ -87,7 +88,7 @@ mod tests { fn it_can_read_machine_id() { let fixture = assert_fs::TempDir::new().unwrap(); let test_file = fixture.child("test_read_machine_id.txt"); - let test_path = test_file.path().to_path_buf(); + let test_path = Utf8PathBuf::from_path_buf(test_file.path().to_path_buf()).unwrap(); let written_uuid = write_machine_id(&test_path).expect("could not write machine id"); let read_uuid = get_or_write_machine_id(&test_path).expect("could not read machine id"); assert_eq!(written_uuid, read_uuid); @@ -100,7 +101,7 @@ mod tests { fn it_can_read_and_write_machine_id() { let fixture = assert_fs::TempDir::new().unwrap(); let test_file = fixture.child("test_read_and_write_machine_id.txt"); - let test_path = test_file.path().to_path_buf(); + let test_path = Utf8PathBuf::from_path_buf(test_file.path().to_path_buf()).unwrap(); assert!(get_or_write_machine_id(&test_path).is_ok()); } } diff --git a/installers/binstall/Cargo.toml b/installers/binstall/Cargo.toml index d309db5d5..cfd2cc8d9 100644 --- a/installers/binstall/Cargo.toml +++ b/installers/binstall/Cargo.toml @@ -6,6 +6,7 @@ edition = "2018" [dependencies] atty = "0.2" +camino = "1.0" directories-next = "2.0" thiserror = "1.0" tracing = "0.1" diff --git a/installers/binstall/src/error.rs b/installers/binstall/src/error.rs index 6b2fcdc96..34efa7c3f 100644 --- a/installers/binstall/src/error.rs +++ b/installers/binstall/src/error.rs @@ -17,6 +17,6 @@ pub enum InstallerError { #[error("Zsh setup failed")] ZshSetup, - #[error("Computed install path is not valid Unicode")] - PathNotUnicode, + #[error("File path \"{path_display}\" is not valid Unicode")] + PathNotUnicode { path_display: String }, } diff --git a/installers/binstall/src/install.rs b/installers/binstall/src/install.rs index 43899fe7b..8a3c8712f 100644 --- a/installers/binstall/src/install.rs +++ b/installers/binstall/src/install.rs @@ -3,25 +3,25 @@ use crate::InstallerError; use std::env; use std::fs; use std::io; -use std::path::PathBuf; use atty::{self, Stream}; +use camino::Utf8PathBuf; pub struct Installer { pub binary_name: String, pub force_install: bool, - pub executable_location: PathBuf, - pub override_install_path: Option, + pub executable_location: Utf8PathBuf, + pub override_install_path: Option, } impl Installer { - pub fn install(&self) -> Result, InstallerError> { + pub fn install(&self) -> Result, InstallerError> { let install_path = self.do_install()?; Ok(install_path) } - fn do_install(&self) -> Result, InstallerError> { + fn do_install(&self) -> Result, InstallerError> { let bin_destination = self.get_bin_path()?; if !self.force_install @@ -34,7 +34,7 @@ impl Installer { tracing::debug!("Creating directory for binary"); self.create_bin_dir()?; - eprintln!("Writing binary to {}", &bin_destination.display()); + eprintln!("Writing binary to {}", &bin_destination); self.write_bin_to_fs()?; tracing::debug!("Adding binary to PATH"); @@ -43,7 +43,7 @@ impl Installer { Ok(Some(bin_destination)) } - pub(crate) fn get_base_dir_path(&self) -> Result { + pub(crate) fn get_base_dir_path(&self) -> Result { let base_dir = if let Some(base_dir) = &self.override_install_path { Ok(base_dir.to_owned()) } else { @@ -52,7 +52,7 @@ impl Installer { Ok(base_dir.join(&format!(".{}", &self.binary_name))) } - pub(crate) fn get_bin_dir_path(&self) -> Result { + pub(crate) fn get_bin_dir_path(&self) -> Result { let bin_dir = self.get_base_dir_path()?.join("bin"); Ok(bin_dir) } @@ -62,7 +62,7 @@ impl Installer { Ok(()) } - fn get_bin_path(&self) -> Result { + fn get_bin_path(&self) -> Result { Ok(self .get_bin_dir_path()? .join(&self.binary_name) @@ -71,13 +71,13 @@ impl Installer { fn write_bin_to_fs(&self) -> Result<(), InstallerError> { let bin_path = self.get_bin_path()?; - tracing::debug!("copying from: {}", &self.executable_location.display()); - tracing::debug!("copying to: {}", &bin_path.display()); + tracing::debug!("copying from: {}", &self.executable_location); + tracing::debug!("copying to: {}", &bin_path); fs::copy(&self.executable_location, &bin_path)?; Ok(()) } - fn should_overwrite(&self, destination: &PathBuf) -> Result { + fn should_overwrite(&self, destination: &Utf8PathBuf) -> Result { // If we're not attached to a TTY then we can't get user input, so there's // nothing to do except inform the user about the `-f` flag. if !atty::is(Stream::Stdin) { @@ -88,8 +88,7 @@ impl Installer { // like to overwrite the previous installation. eprintln!( "existing {} installation found at `{}`", - &self.binary_name, - destination.display() + &self.binary_name, destination ); eprintln!("Would you like to overwrite this file? [y/N]: "); let mut line = String::new(); diff --git a/installers/binstall/src/lib.rs b/installers/binstall/src/lib.rs index e0211d7c3..10a5be053 100644 --- a/installers/binstall/src/lib.rs +++ b/installers/binstall/src/lib.rs @@ -16,8 +16,8 @@ //! it's pretty simple! We're largely just moving over our currently running //! executable to a different path. +use camino::Utf8PathBuf; use directories_next::BaseDirs; -use std::path::PathBuf; mod error; mod install; @@ -32,9 +32,15 @@ pub(crate) use system::unix; #[cfg(windows)] pub(crate) use system::windows; -pub(crate) fn get_home_dir_path() -> Result { +pub(crate) fn get_home_dir_path() -> Result { if let Some(base_dirs) = BaseDirs::new() { - Ok(base_dirs.home_dir().to_path_buf()) + Ok( + Utf8PathBuf::from_path_buf(base_dirs.home_dir().to_path_buf()).map_err(|pb| { + InstallerError::PathNotUnicode { + path_display: pb.display().to_string(), + } + })?, + ) } else if cfg!(windows) { Err(InstallerError::NoHomeWindows) } else { @@ -44,28 +50,35 @@ pub(crate) fn get_home_dir_path() -> Result { #[cfg(test)] mod tests { + #[cfg(not(windows))] + use serial_test::serial; + + #[cfg(not(windows))] use super::Installer; + + #[cfg(not(windows))] use assert_fs::TempDir; - use serial_test::serial; - use std::path::PathBuf; + #[cfg(not(windows))] + use camino::Utf8PathBuf; #[cfg(not(windows))] #[test] #[serial] fn install_bins_creates_rover_home() { let fixture = TempDir::new().unwrap(); - let base_dir = fixture.path().display().to_string(); + let base_dir = Utf8PathBuf::from_path_buf(fixture.path().to_path_buf()).unwrap(); let install_path = Installer { binary_name: "test".to_string(), force_install: false, - override_install_path: Some(PathBuf::from(&base_dir)), - executable_location: std::env::current_exe().unwrap(), + override_install_path: Some(base_dir.clone()), + executable_location: Utf8PathBuf::from_path_buf(std::env::current_exe().unwrap()) + .unwrap(), } .install() .unwrap() .unwrap(); - assert!(install_path.to_string_lossy().contains(&base_dir)); + assert!(install_path.to_string().contains(&base_dir.to_string())); } } diff --git a/installers/binstall/src/system/unix.rs b/installers/binstall/src/system/unix.rs index 7f46567d8..a8dd11d73 100644 --- a/installers/binstall/src/system/unix.rs +++ b/installers/binstall/src/system/unix.rs @@ -1,9 +1,9 @@ -use std::env; +use camino::Utf8PathBuf; use std::fmt::Debug; use std::fs; use std::io::{self, Write}; -use std::path::PathBuf; use std::process::Command; +use std::{env, path::PathBuf}; use crate::{Installer, InstallerError}; @@ -15,7 +15,7 @@ pub fn add_binary_to_path(installer: &Installer) -> Result<(), InstallerError> { for rc in shell.update_rcs() { if !rc.is_file() || !fs::read_to_string(&rc)?.contains(&source_cmd) { - tracing::debug!("updating {}", &rc.display()); + tracing::debug!("updating {}", &rc); let mut dest_file = fs::OpenOptions::new() .write(true) .append(true) @@ -59,9 +59,10 @@ impl ShellScript { pub fn write(&self, installer: &Installer) -> Result<(), InstallerError> { let home = installer.get_base_dir_path()?; let path_to_bin = installer.get_bin_dir_path()?; - let bin_str = path_to_bin.to_str().ok_or(InstallerError::PathNotUnicode)?; let path = home.join(&self.name); - let contents = &self.content.replace("{path_to_bin}", bin_str); + let contents = &self + .content + .replace("{path_to_bin}", &path_to_bin.to_string()); let mut file = fs::OpenOptions::new() .write(true) .truncate(true) @@ -83,10 +84,10 @@ trait UnixShell: Debug { // Gives all rcfiles of a given shell that we are concerned with. // Used primarily in checking rcfiles for cleanup. - fn rcfiles(&self) -> Vec; + fn rcfiles(&self) -> Vec; // Gives rcs that should be written to. - fn update_rcs(&self) -> Vec; + fn update_rcs(&self) -> Vec; // Gets the name of the shell. fn name(&self) -> &str; @@ -102,7 +103,7 @@ trait UnixShell: Debug { fn source_string(&self, installer: &Installer) -> Result { Ok(format!( r#"source "{}/env""#, - installer.get_base_dir_path()?.to_string_lossy() + installer.get_base_dir_path()? )) } } @@ -119,14 +120,14 @@ impl UnixShell for Posix { true } - fn rcfiles(&self) -> Vec { + fn rcfiles(&self) -> Vec { match crate::get_home_dir_path().ok() { Some(dir) => vec![dir.join(".profile")], _ => vec![], } } - fn update_rcs(&self) -> Vec { + fn update_rcs(&self) -> Vec { // Write to .profile even if it doesn't exist. It's the only rc in the // POSIX spec so it should always be set up. self.rcfiles() @@ -145,7 +146,7 @@ impl UnixShell for Bash { !self.update_rcs().is_empty() } - fn rcfiles(&self) -> Vec { + fn rcfiles(&self) -> Vec { // Bash also may read .profile, however we already includes handling // .profile as part of POSIX and always does setup for POSIX shells. [".bash_profile", ".bash_login", ".bashrc"] @@ -154,7 +155,7 @@ impl UnixShell for Bash { .collect() } - fn update_rcs(&self) -> Vec { + fn update_rcs(&self) -> Vec { self.rcfiles() .into_iter() .filter(|rc| rc.is_file()) @@ -166,13 +167,13 @@ impl UnixShell for Bash { struct Zsh; impl Zsh { - fn zdotdir() -> Result { + fn zdotdir() -> Result { use std::ffi::OsStr; use std::os::unix::ffi::OsStrExt; if matches!(env::var("SHELL"), Ok(sh) if sh.contains("zsh")) { match env::var("ZDOTDIR") { - Ok(dir) if !dir.is_empty() => Ok(PathBuf::from(dir)), + Ok(dir) if !dir.is_empty() => Ok(Utf8PathBuf::from(dir)), _ => Err(InstallerError::ZshSetup), } } else { @@ -180,7 +181,15 @@ impl Zsh { .args(&["-c", "'echo $ZDOTDIR'"]) .output() { - Ok(io) if !io.stdout.is_empty() => Ok(PathBuf::from(OsStr::from_bytes(&io.stdout))), + Ok(io) if !io.stdout.is_empty() => { + let raw_pb = PathBuf::from(OsStr::from_bytes(&io.stdout)); + let pb = Utf8PathBuf::from_path_buf(raw_pb).map_err(|pb| { + InstallerError::PathNotUnicode { + path_display: pb.display().to_string(), + } + })?; + Ok(pb) + } _ => Err(InstallerError::ZshSetup), } } @@ -205,14 +214,14 @@ impl UnixShell for Zsh { matches!(env::var("SHELL"), Ok(sh) if sh.contains("zsh")) || matches!(has_cmd("zsh"), true) } - fn rcfiles(&self) -> Vec { + fn rcfiles(&self) -> Vec { [Zsh::zdotdir().ok(), crate::get_home_dir_path().ok()] .iter() .filter_map(|dir| dir.as_ref().map(|p| p.join(".zshenv"))) .collect() } - fn update_rcs(&self) -> Vec { + fn update_rcs(&self) -> Vec { // zsh can change $ZDOTDIR both _before_ AND _during_ reading .zshenv, // so we: write to $ZDOTDIR/.zshenv if-exists ($ZDOTDIR changes before) // OR write to $HOME/.zshenv if it exists (change-during) diff --git a/installers/binstall/src/system/windows.rs b/installers/binstall/src/system/windows.rs index dcf60ce0c..bdb41c0a6 100644 --- a/installers/binstall/src/system/windows.rs +++ b/installers/binstall/src/system/windows.rs @@ -7,11 +7,7 @@ use std::io; /// Adds the downloaded binary in Installer to a Windows PATH pub fn add_binary_to_path(installer: &Installer) -> Result<(), InstallerError> { let windows_path = get_windows_path_var()?; - let bin_path = installer - .get_bin_dir_path()? - .to_str() - .ok_or(InstallerError::PathNotUnicode)? - .to_string(); + let bin_path = installer.get_bin_dir_path()?.to_string(); if let Some(old_path) = windows_path { if let Some(new_path) = add_to_path(&old_path, &bin_path) { apply_new_path(&new_path)?; diff --git a/installers/npm/package-lock.json b/installers/npm/package-lock.json index 77dba7f77..5cff5039f 100644 --- a/installers/npm/package-lock.json +++ b/installers/npm/package-lock.json @@ -101,9 +101,6 @@ "version": "1.1.0", "resolved": "https://registry.npmjs.org/easy-table/-/easy-table-1.1.0.tgz", "integrity": "sha1-hvmrTBAvA3G3KXuSplHVgkvIy3M=", - "dependencies": { - "wcwidth": ">=1.0.1" - }, "optionalDependencies": { "wcwidth": ">=1.0.1" } diff --git a/src/cli.rs b/src/cli.rs index 9409351c8..21a07eaf1 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -14,7 +14,7 @@ use config::Config; use houston as config; use timber::{Level, LEVELS}; -use std::path::PathBuf; +use camino::Utf8PathBuf; #[derive(Debug, Serialize, StructOpt)] #[structopt(name = "Rover", global_settings = &[structopt::clap::AppSettings::ColoredHelp], about = " @@ -55,10 +55,10 @@ pub struct Rover { impl Rover { pub(crate) fn get_rover_config(&self) -> Result { - let override_home: Option = self + let override_home: Option = self .env_store .get(RoverEnvKey::ConfigHome)? - .map(|p| PathBuf::from(&p)); + .map(|p| Utf8PathBuf::from(&p)); let override_api_key = self.env_store.get(RoverEnvKey::Key)?; Ok(Config::new(override_home.as_ref(), override_api_key)?) } @@ -69,11 +69,11 @@ impl Rover { Ok(StudioClientConfig::new(override_endpoint, config)) } - pub(crate) fn get_install_override_path(&self) -> Result> { + pub(crate) fn get_install_override_path(&self) -> Result> { Ok(self .env_store .get(RoverEnvKey::Home)? - .map(|p| PathBuf::from(&p))) + .map(|p| Utf8PathBuf::from(&p))) } pub(crate) fn get_git_context(&self) -> Result { diff --git a/src/command/config/auth.rs b/src/command/config/auth.rs index e805f6b00..ae15cb16f 100644 --- a/src/command/config/auth.rs +++ b/src/command/config/auth.rs @@ -62,6 +62,7 @@ fn is_valid(api_key: &str) -> bool { #[cfg(test)] mod tests { use assert_fs::TempDir; + use camino::Utf8Path; use serial_test::serial; use houston::{Config, Profile}; @@ -98,6 +99,7 @@ mod tests { fn get_config(override_api_key: Option) -> Config { let tmp_home = TempDir::new().unwrap(); - Config::new(Some(&tmp_home.path()), override_api_key).unwrap() + let tmp_home_path = Utf8Path::from_path(tmp_home.path()).unwrap().to_owned(); + Config::new(Some(&tmp_home_path), override_api_key).unwrap() } } diff --git a/src/command/install/mod.rs b/src/command/install/mod.rs index ea14c0557..d10650618 100644 --- a/src/command/install/mod.rs +++ b/src/command/install/mod.rs @@ -7,8 +7,8 @@ use crate::command::RoverStdout; use crate::PKG_NAME; use crate::{anyhow, Context, Result}; +use camino::Utf8PathBuf; use std::env; -use std::path::PathBuf; #[derive(Debug, Serialize, StructOpt)] pub struct Install { @@ -17,9 +17,11 @@ pub struct Install { } impl Install { - pub fn run(&self, override_install_path: Option) -> Result { + pub fn run(&self, override_install_path: Option) -> Result { let binary_name = PKG_NAME.to_string(); if let Ok(executable_location) = env::current_exe() { + let executable_location = Utf8PathBuf::from_path_buf(executable_location) + .map_err(|pb| anyhow!("File path \"{}\" is not valid UTF-8", pb.display()))?; let install_location = Installer { binary_name: binary_name.clone(), force_install: self.force, @@ -32,8 +34,7 @@ impl Install { if let Some(install_location) = install_location { eprintln!( "{} was successfully installed to `{}`.", - &binary_name, - install_location.display() + &binary_name, install_location ) } else { eprintln!("{} was not installed. To override the existing installation, you can pass the `--force` flag to the installer.", &binary_name); diff --git a/src/error/metadata/mod.rs b/src/error/metadata/mod.rs index 47bdd1afb..05a50ab71 100644 --- a/src/error/metadata/mod.rs +++ b/src/error/metadata/mod.rs @@ -106,6 +106,7 @@ impl From<&mut anyhow::Error> for Metadata { HoustonProblem::NoConfigProfiles => (Some(Suggestion::NewUserNoProfiles), None), HoustonProblem::ProfileNotFound(_) => (Some(Suggestion::ListProfiles), None), HoustonProblem::NoNonSensitiveConfigFound(_) + | HoustonProblem::PathNotUnicode { path_display: _ } | HoustonProblem::TomlDeserialization(_) | HoustonProblem::TomlSerialization(_) | HoustonProblem::IOError(_) => (Some(Suggestion::SubmitIssue), None), diff --git a/src/utils/loaders.rs b/src/utils/loaders.rs index 89f1029e7..b26b242a8 100644 --- a/src/utils/loaders.rs +++ b/src/utils/loaders.rs @@ -1,11 +1,11 @@ use crate::utils::parsers::SchemaSource; use crate::{anyhow, Context, Result}; +use camino::Utf8Path; use std::io::Read; -use std::path::Path; /// this fn takes 2 args: the first, an enum describing where to look to load -/// a schema - from stdin or a file's PathBuf, and the second, the reference to +/// a schema - from stdin or a file's Utf8PathBuf, and the second, the reference to /// stdin to load from, should it be needed. pub fn load_schema_from_flag(loc: &SchemaSource, mut stdin: impl Read) -> Result { match loc { @@ -17,11 +17,11 @@ pub fn load_schema_from_flag(loc: &SchemaSource, mut stdin: impl Read) -> Result Ok(buffer) } SchemaSource::File(path) => { - if Path::exists(&path) { + if Utf8Path::exists(&path) { let contents = std::fs::read_to_string(path)?; Ok(contents) } else { - Err(anyhow!("Invalid path. No file found at {}", path.display()).into()) + Err(anyhow!("Invalid path. No file found at {}", path).into()) } } } @@ -31,7 +31,7 @@ pub fn load_schema_from_flag(loc: &SchemaSource, mut stdin: impl Read) -> Result mod tests { use super::{load_schema_from_flag, SchemaSource}; use assert_fs::prelude::*; - use std::path::PathBuf; + use camino::Utf8PathBuf; #[test] fn load_schema_from_flag_loads() { @@ -42,7 +42,7 @@ mod tests { .write_str("type Query { hello: String! }") .unwrap(); - let test_path = test_file.path().to_path_buf(); + let test_path = Utf8PathBuf::from_path_buf(test_file.path().to_path_buf()).unwrap(); let loc = SchemaSource::File(test_path); let schema = load_schema_from_flag(&loc, std::io::stdin()).unwrap(); @@ -52,7 +52,7 @@ mod tests { #[test] fn load_schema_from_flag_errs_on_bad_path() { let empty_path = "./wow.graphql"; - let loc = SchemaSource::File(PathBuf::from(empty_path)); + let loc = SchemaSource::File(Utf8PathBuf::from(empty_path)); let schema = load_schema_from_flag(&loc, std::io::stdin()); assert_eq!(schema.is_err(), true); diff --git a/src/utils/parsers.rs b/src/utils/parsers.rs index 49d92c368..8fa29bd8e 100644 --- a/src/utils/parsers.rs +++ b/src/utils/parsers.rs @@ -1,13 +1,15 @@ +use camino::Utf8PathBuf; use regex::Regex; use serde::Serialize; -use std::{convert::TryInto, fmt, path::PathBuf}; + +use std::{convert::TryInto, fmt}; use crate::{error::RoverError, Result}; #[derive(Debug, PartialEq)] pub enum SchemaSource { Stdin, - File(PathBuf), + File(Utf8PathBuf), } pub fn parse_schema_source(loc: &str) -> Result { @@ -18,7 +20,7 @@ pub fn parse_schema_source(loc: &str) -> Result { "The path provided to find a schema is empty", )) } else { - let path = PathBuf::from(loc); + let path = Utf8PathBuf::from(loc); Ok(SchemaSource::File(path)) } } @@ -137,7 +139,7 @@ mod tests { let loc = parse_schema_source("./schema.graphql").unwrap(); match loc { SchemaSource::File(buf) => { - assert_eq!(buf.to_str().unwrap(), "./schema.graphql"); + assert_eq!(buf.to_string(), "./schema.graphql".to_string()); } _ => panic!("parsed incorrectly as stdin"), } diff --git a/src/utils/telemetry.rs b/src/utils/telemetry.rs index fcbb296b9..bd9eb0d2c 100644 --- a/src/utils/telemetry.rs +++ b/src/utils/telemetry.rs @@ -1,7 +1,6 @@ +use camino::Utf8PathBuf; use url::Url; -use std::path::PathBuf; - use crate::utils::env::RoverEnvKey; use crate::{cli::Rover, PKG_NAME, PKG_VERSION}; use sputnik::{Command, Report, SputnikError}; @@ -103,7 +102,7 @@ impl Report for Rover { PKG_VERSION.to_string() } - fn machine_id_config(&self) -> Result { + fn machine_id_config(&self) -> Result { let config = self .get_rover_config() .map_err(|_| SputnikError::ConfigError)?; diff --git a/src/utils/version.rs b/src/utils/version.rs index cfba9a449..606140660 100644 --- a/src/utils/version.rs +++ b/src/utils/version.rs @@ -1,14 +1,13 @@ -use std::{fs, path::PathBuf, time::SystemTime}; - -use rover_client::releases::get_latest_release; +use std::{fs, time::SystemTime}; use ansi_term::Colour::{Cyan, Yellow}; use billboard::{Alignment, Billboard}; +use camino::Utf8PathBuf; use semver::Version; use crate::{Result, PKG_VERSION}; - use houston as config; +use rover_client::releases::get_latest_release; const ONE_HOUR: u64 = 60 * 60; const ONE_DAY: u64 = ONE_HOUR * 24; @@ -81,7 +80,7 @@ fn do_update_check(checked: &mut bool) -> Result<()> { Ok(()) } -fn get_last_checked_time_from_disk(version_file: &PathBuf) -> Option { +fn get_last_checked_time_from_disk(version_file: &Utf8PathBuf) -> Option { match fs::read_to_string(&version_file) { Ok(contents) => match toml::from_str(&contents) { Ok(last_checked_version) => Some(last_checked_version), diff --git a/tests/config/profile.rs b/tests/config/profile.rs index 6da057c2e..581e495a2 100644 --- a/tests/config/profile.rs +++ b/tests/config/profile.rs @@ -1,7 +1,7 @@ use assert_cmd::Command; use assert_fs::TempDir; +use camino::Utf8PathBuf; use predicates::prelude::*; -use std::path::PathBuf; use houston::{Config, Profile}; use rover::utils::env::RoverEnvKey; @@ -15,10 +15,7 @@ fn it_can_list_no_profiles() { let mut cmd = Command::cargo_bin("rover").unwrap(); let result = cmd .arg("config") - .env( - RoverEnvKey::ConfigHome.to_string(), - temp_dir.to_string_lossy().to_string(), - ) + .env(RoverEnvKey::ConfigHome.to_string(), temp_dir.to_string()) .arg("list") .assert(); result.stderr(predicate::str::contains("No profiles")); @@ -32,16 +29,13 @@ fn it_can_list_one_profile() { let mut cmd = Command::cargo_bin("rover").unwrap(); let result = cmd - .env( - RoverEnvKey::ConfigHome.to_string(), - temp_dir.to_string_lossy().to_string(), - ) + .env(RoverEnvKey::ConfigHome.to_string(), temp_dir.to_string()) .arg("config") .arg("list") .assert(); result.stdout(predicate::str::contains(CUSTOM_PROFILE)); } -fn get_temp_dir() -> PathBuf { - TempDir::new().unwrap().path().to_path_buf() +fn get_temp_dir() -> Utf8PathBuf { + Utf8PathBuf::from_path_buf(TempDir::new().unwrap().path().to_path_buf()).unwrap() } diff --git a/tests/installers.rs b/tests/installers.rs index 9b6f205e9..7be2eacee 100644 --- a/tests/installers.rs +++ b/tests/installers.rs @@ -1,7 +1,7 @@ use std::fs; -use std::path::PathBuf; use std::process::Command; +use camino::Utf8PathBuf; use serde_json::Value; // The behavior of these tests _must_ remain unchanged @@ -27,7 +27,7 @@ fn it_has_windows_installer() { assert!(!windows_script.is_empty()) } -fn get_binstall_scripts_root() -> PathBuf { +fn get_binstall_scripts_root() -> Utf8PathBuf { let cargo_locate_project_output = Command::new("cargo") .arg("locate-project") .output() @@ -41,7 +41,7 @@ fn get_binstall_scripts_root() -> PathBuf { .as_str() .expect("`root` either does not exist or is not a String"); - let root_directory = PathBuf::from(cargo_toml_location) + let root_directory = Utf8PathBuf::from(cargo_toml_location) .parent() .expect("Could not find parent of `Cargo.toml`") .to_path_buf();