diff --git a/.gitignore b/.gitignore index eb4221f2ac..df4c1bc8a7 100644 --- a/.gitignore +++ b/.gitignore @@ -9,3 +9,4 @@ /local-rustup /snapcraft flake.lock +.vscode/ diff --git a/src/cli/self_update/env.fish b/src/cli/self_update/env.fish new file mode 100644 index 0000000000..1cbd32b825 --- /dev/null +++ b/src/cli/self_update/env.fish @@ -0,0 +1 @@ +contains {cargo_bin} $fish_user_paths; or set -Ua fish_user_paths {cargo_bin} diff --git a/src/cli/self_update/shell.rs b/src/cli/self_update/shell.rs index 3927e531c8..6371d132d1 100644 --- a/src/cli/self_update/shell.rs +++ b/src/cli/self_update/shell.rs @@ -1,3 +1,7 @@ +#![warn(clippy::all)] +#![warn(clippy::pedantic)] +#![allow(clippy::doc_markdown)] + //! Paths and Unix shells //! //! MacOS, Linux, FreeBSD, and many other OS model their design on Unix, @@ -26,7 +30,7 @@ use std::borrow::Cow; use std::path::PathBuf; -use anyhow::{bail, Result}; +use anyhow::{bail, Context, Result}; use super::utils; use crate::process; @@ -71,7 +75,12 @@ pub(crate) fn cargo_home_str() -> Result> { // TODO?: Make a decision on Ion Shell, Power Shell, Nushell // Cross-platform non-POSIX shells have not been assessed for integration yet fn enumerate_shells() -> Vec { - vec![Box::new(Posix), Box::new(Bash), Box::new(Zsh)] + vec![ + Box::new(Posix), + Box::new(Bash), + Box::new(Zsh), + Box::new(Fish), + ] } pub(crate) fn get_available_shells() -> impl Iterator { @@ -90,6 +99,43 @@ pub(crate) trait UnixShell { // Gives rcs that should be written to. fn update_rcs(&self) -> Vec; + // By default follows POSIX and sources /env + fn add_to_path(&self) -> Result<(), anyhow::Error> { + let source_cmd = self.source_string()?; + let source_cmd_with_newline = format!("\n{}", &source_cmd); + for rc in self.update_rcs() { + let cmd_to_write = match utils::read_file("rcfile", &rc) { + Ok(contents) if contents.contains(&source_cmd) => continue, + Ok(contents) if !contents.ends_with('\n') => &source_cmd_with_newline, + _ => &source_cmd, + }; + + utils::append_file("rcfile", &rc, cmd_to_write) + .with_context(|| format!("could not amend shell profile: '{}'", rc.display()))?; + } + Ok(()) + } + + fn remove_from_path(&self) -> Result<(), anyhow::Error> { + let source_bytes = format!("{}\n", self.source_string()?).into_bytes(); + for rc in self.rcfiles().iter().filter(|rc| rc.is_file()) { + let file = utils::read_file("rcfile", rc)?; + let file_bytes = file.into_bytes(); + // FIXME: This is whitespace sensitive where it should not be. + if let Some(idx) = file_bytes + .windows(source_bytes.len()) + .position(|w| w == source_bytes.as_slice()) + { + // Here we rewrite the file without the offending line. + let mut new_bytes = file_bytes[..idx].to_vec(); + new_bytes.extend(&file_bytes[idx + source_bytes.len()..]); + let new_file = String::from_utf8(new_bytes).unwrap(); + utils::write_file("rcfile", rc, &new_file)?; + } + } + Ok(()) + } + // Writes the relevant env file. fn env_script(&self) -> ShellScript { ShellScript { @@ -201,14 +247,117 @@ impl UnixShell for Zsh { } } +struct Fish; + +impl Fish { + #![allow(non_snake_case)] + /// Gets fish vendor config location from `XDG_DATA_DIRS` + /// Returns None if `XDG_DATA_DIRS` is not set or if there is no fis`fish/vendor_conf.d`.d directory found + pub fn get_vendor_config_from_XDG_DATA_DIRS() -> Option { + // Skip the directory during testing as we don't want to write into the XDG_DATA_DIRS by accident + // TODO: Change the definition of XDG_DATA_DIRS in test so that doesn't happen + + // TODO: Set up XDG DATA DIRS in a test to test the location being correct + return process() + .var("XDG_DATA_DIRS") + .map(|var| { + var.split(':') + .map(PathBuf::from) + .find(|path| path.ends_with("fish/vendor_conf.d")) + }) + .ok() + .flatten(); + } +} + +impl UnixShell for Fish { + fn does_exist(&self) -> bool { + matches!(process().var("SHELL"), Ok(sh) if sh.contains("fish")) + || matches!(utils::find_cmd(&["fish"]), Some(_)) + } + + fn rcfiles(&self) -> Vec { + // As per https://fishshell.com/docs/current/language.html#configuration + // Vendor config files should be written to `/usr/share/fish/vendor_config.d/` + // if that does not exist then it should be written to `/usr/share/fish/vendor_conf.d/` + // otherwise it should be written to `$HOME/.config/fish/conf.d/ as per discussions in github issue #478 + [ + // #[cfg(test)] // Write to test location so we don't pollute during testing. + // utils::home_dir().map(|home| home.join(".config/fish/conf.d/")), + Self::get_vendor_config_from_XDG_DATA_DIRS(), + Some(PathBuf::from("/usr/share/fish/vendor_conf.d/")), + Some(PathBuf::from("/usr/local/share/fish/vendor_conf.d/")), + utils::home_dir().map(|home| home.join(".config/fish/conf.d/")), + ] + .iter_mut() + .flatten() + .map(|x| x.join("cargo.fish")) + .collect() + } + + fn update_rcs(&self) -> Vec { + // TODO: Change rcfiles to just read parent dirs + self.rcfiles() + .into_iter() + .filter(|rc| { + // We only want to check if the directory exists as in fish we create a new file for every applications env + rc.parent().map_or(false, |parent| { + parent + .metadata() // Returns error if path doesn't exist so separate check is not needed + .map_or(false, |metadata| !metadata.permissions().readonly()) + }) + }) + .collect() + } + + fn add_to_path(&self) -> Result<(), anyhow::Error> { + let source_cmd = self.source_string()?; + // Write to first path location if it exists. + if let Some(rc) = self.update_rcs().get(0) { + let cmd_to_write = match utils::read_file("rcfile", rc) { + Ok(contents) if contents.contains(&source_cmd) => return Ok(()), + _ => &source_cmd, + }; + + utils::write_file("fish conf.d", rc, cmd_to_write) + .with_context(|| format!("Could not add source to {}", rc.display()))?; + } + Ok(()) + } + + fn remove_from_path(&self) -> Result<(), anyhow::Error> { + for rc in self.update_rcs() { + utils::remove_file("Cargo.fish", &rc) + .with_context(|| format!("Could not remove {}", rc.display()))?; + } + Ok(()) + } + + fn env_script(&self) -> ShellScript { + ShellScript { + name: "env.fish", + content: include_str!("env.fish"), + } + } + + fn source_string(&self) -> Result { + Ok(format!( + "contains {}/bin $fish_user_paths; or set -Ua fish_user_paths {}/bin", + cargo_home_str()?, + cargo_home_str()? + )) + } +} + pub(crate) fn legacy_paths() -> impl Iterator { - let zprofiles = Zsh::zdotdir() + let z_profiles = Zsh::zdotdir() .into_iter() .chain(utils::home_dir()) .map(|d| d.join(".zprofile")); - let profiles = [".bash_profile", ".profile"] + + let bash_profiles = [".bash_profile", ".profile"] .iter() .filter_map(|rc| utils::home_dir().map(|d| d.join(rc))); - profiles.chain(zprofiles) + bash_profiles.chain(z_profiles) } diff --git a/src/cli/self_update/unix.rs b/src/cli/self_update/unix.rs index ce4a79c14b..44c6473413 100644 --- a/src/cli/self_update/unix.rs +++ b/src/cli/self_update/unix.rs @@ -54,24 +54,7 @@ pub(crate) fn delete_rustup_and_cargo_home() -> Result<()> { pub(crate) fn do_remove_from_path() -> Result<()> { for sh in shell::get_available_shells() { - let source_bytes = format!("{}\n", sh.source_string()?).into_bytes(); - - // Check more files for cleanup than normally are updated. - for rc in sh.rcfiles().iter().filter(|rc| rc.is_file()) { - let file = utils::read_file("rcfile", rc)?; - let file_bytes = file.into_bytes(); - // FIXME: This is whitespace sensitive where it should not be. - if let Some(idx) = file_bytes - .windows(source_bytes.len()) - .position(|w| w == source_bytes.as_slice()) - { - // Here we rewrite the file without the offending line. - let mut new_bytes = file_bytes[..idx].to_vec(); - new_bytes.extend(&file_bytes[idx + source_bytes.len()..]); - let new_file = String::from_utf8(new_bytes).unwrap(); - utils::write_file("rcfile", rc, &new_file)?; - } - } + sh.remove_from_path()?; } remove_legacy_paths()?; @@ -81,19 +64,7 @@ pub(crate) fn do_remove_from_path() -> Result<()> { pub(crate) fn do_add_to_path() -> Result<()> { for sh in shell::get_available_shells() { - let source_cmd = sh.source_string()?; - let source_cmd_with_newline = format!("\n{}", &source_cmd); - - for rc in sh.update_rcs() { - let cmd_to_write = match utils::read_file("rcfile", &rc) { - Ok(contents) if contents.contains(&source_cmd) => continue, - Ok(contents) if !contents.ends_with('\n') => &source_cmd_with_newline, - _ => &source_cmd, - }; - - utils::append_file("rcfile", &rc, cmd_to_write) - .with_context(|| format!("could not amend shell profile: '{}'", rc.display()))?; - } + sh.add_to_path()?; } remove_legacy_paths()?; diff --git a/tests/cli-paths.rs b/tests/cli-paths.rs index 0af6e5ec76..554d500545 100644 --- a/tests/cli-paths.rs +++ b/tests/cli-paths.rs @@ -71,6 +71,31 @@ export PATH="$HOME/apple/bin" }); } + #[test] + fn install_creates_necessary_fish_scripts_new() { + clitools::setup(Scenario::Empty, &|config| { + let mut cmd = clitools::cmd(config, "rustup-init", &INIT_NONE[1..]); + let rcs: Vec = [".cargo/env.fish", ".config/fish/config.fish"] + .iter() + .map(|file| config.homedir.join(file)) + .collect(); + for file in &rcs { + assert!(!file.exists()); + } + cmd.env_remove("CARGO_HOME"); + cmd.env("SHELL", "fish"); + cmd.env("XDG_DATA_DIRS", ""); // Overwrite XDG as host shouldn't be affected by the test. + assert!(cmd.output().unwrap().status.success()); + let expected = include_str!("../src/cli/self_update/env.fish") + .replace("{cargo_bin}", "$HOME/.cargo/bin"); + + assert!(rcs + .iter() + .filter_map(|rc| fs::read_to_string(rc).ok()) // Read contents of files that exist + .any(|rc_content| rc_content.contains(&expected))); + }) + } + #[test] fn install_updates_bash_rcs() { clitools::setup(Scenario::Empty, &|config| { diff --git a/tests/mock/clitools.rs b/tests/mock/clitools.rs index d7acecd32f..bd84964e31 100644 --- a/tests/mock/clitools.rs +++ b/tests/mock/clitools.rs @@ -99,7 +99,8 @@ pub fn setup(s: Scenario, f: &dyn Fn(&mut Config)) { env::remove_var("RUSTUP_TOOLCHAIN"); env::remove_var("SHELL"); env::remove_var("ZDOTDIR"); - // clap does its own terminal colour probing, and that isn't + env::remove_var("XDG_DATA_DIRS"); + // clap does it's own terminal colour probing, and that isn't // trait-controllable, but it does honour the terminal. To avoid testing // claps code, lie about whatever terminal this process was started under. env::set_var("TERM", "dumb"); @@ -637,8 +638,7 @@ where let mut vars: HashMap = HashMap::default(); self::env(config, &mut vars); vars.extend(env.iter().map(|(k, v)| (k.to_string(), v.to_string()))); - let mut arg_strings: Vec> = Vec::new(); - arg_strings.push(name.to_owned().into_boxed_str()); + let mut arg_strings: Vec> = vec![name.to_owned().into_boxed_str()]; for arg in args { arg_strings.push( arg.as_ref()