diff --git a/Cargo.lock b/Cargo.lock index 4b83f7d3a8..e45f422d30 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2375,8 +2375,11 @@ dependencies = [ "dunce", "getset", "glob", + "indexmap", + "nu", "nu-build", "nu-parser", + "nu-protocol", "nu-source", "tempfile", ] @@ -2385,6 +2388,7 @@ dependencies = [ name = "nu-value-ext" version = "0.8.0" dependencies = [ + "indexmap", "itertools 0.8.2", "nu-build", "nu-errors", diff --git a/crates/nu-protocol/src/value.rs b/crates/nu-protocol/src/value.rs index 63a858fe1b..9a1600aad2 100644 --- a/crates/nu-protocol/src/value.rs +++ b/crates/nu-protocol/src/value.rs @@ -122,7 +122,6 @@ impl UntaggedValue { } /// Helper for creating row values - #[allow(unused)] pub fn row(entries: IndexMap) -> UntaggedValue { UntaggedValue::Row(entries.into()) } diff --git a/crates/nu-test-support/Cargo.toml b/crates/nu-test-support/Cargo.toml index 7223aefc68..8e9cd16c57 100644 --- a/crates/nu-test-support/Cargo.toml +++ b/crates/nu-test-support/Cargo.toml @@ -10,14 +10,17 @@ license = "MIT" doctest = false [dependencies] +nu = { path = "../..", version = "0.8.0" } nu-parser = { path = "../nu-parser", version = "0.8.0" } nu-source = { path = "../nu-source", version = "0.8.0" } +nu-protocol = { path = "../nu-protocol", version = "0.8.0" } app_dirs = "1.2.1" dunce = "1.0.0" getset = "0.0.9" glob = "0.3.0" tempfile = "3.1.0" +indexmap = { version = "1.3.0", features = ["serde-1"] } [build-dependencies] nu-build = { version = "0.8.0", path = "../nu-build" } diff --git a/crates/nu-value-ext/Cargo.toml b/crates/nu-value-ext/Cargo.toml index b810da3a10..ae754ba5ef 100644 --- a/crates/nu-value-ext/Cargo.toml +++ b/crates/nu-value-ext/Cargo.toml @@ -17,6 +17,7 @@ nu-protocol = { path = "../nu-protocol", version = "0.8.0" } num-traits = "0.2.10" itertools = "0.8.2" +indexmap = { version = "1.3.0", features = ["serde-1"] } [build-dependencies] nu-build = { version = "0.8.0", path = "../nu-build" } diff --git a/crates/nu-value-ext/src/lib.rs b/crates/nu-value-ext/src/lib.rs index c7da5a33ef..a6d4c88771 100644 --- a/crates/nu-value-ext/src/lib.rs +++ b/crates/nu-value-ext/src/lib.rs @@ -8,6 +8,8 @@ use nu_source::{HasSpan, PrettyDebug, Spanned, SpannedItem, Tag, Tagged, TaggedI use num_traits::cast::ToPrimitive; pub trait ValueExt { + fn row_entries(&self) -> RowValueIter<'_>; + fn table_entries(&self) -> TableValueIter<'_>; fn into_parts(self) -> (UntaggedValue, Tag); fn get_data(&self, desc: &str) -> MaybeOwned<'_, Value>; fn get_data_by_key(&self, name: Spanned<&str>) -> Option; @@ -39,6 +41,14 @@ pub trait ValueExt { } impl ValueExt for Value { + fn row_entries(&self) -> RowValueIter<'_> { + row_entries(self) + } + + fn table_entries(&self) -> TableValueIter<'_> { + table_entries(self) + } + fn into_parts(self) -> (UntaggedValue, Tag) { (self.value, self.tag) } @@ -524,3 +534,52 @@ pub(crate) fn get_mut_data_by_member<'value>( _ => None, } } + +pub enum RowValueIter<'a> { + Empty, + Entries(indexmap::map::Iter<'a, String, Value>), +} + +pub enum TableValueIter<'a> { + Empty, + Entries(std::slice::Iter<'a, Value>), +} + +impl<'a> Iterator for RowValueIter<'a> { + type Item = (&'a String, &'a Value); + + fn next(&mut self) -> Option { + match self { + RowValueIter::Empty => None, + RowValueIter::Entries(iter) => iter.next(), + } + } +} + +impl<'a> Iterator for TableValueIter<'a> { + type Item = &'a Value; + + fn next(&mut self) -> Option { + match self { + TableValueIter::Empty => None, + TableValueIter::Entries(iter) => iter.next(), + } + } +} + +pub fn table_entries(value: &Value) -> TableValueIter<'_> { + match &value.value { + UntaggedValue::Table(t) => TableValueIter::Entries(t.iter()), + _ => TableValueIter::Empty, + } +} + +pub fn row_entries(value: &Value) -> RowValueIter<'_> { + match &value.value { + UntaggedValue::Row(o) => { + let iter = o.entries.iter(); + RowValueIter::Entries(iter) + } + _ => RowValueIter::Empty, + } +} diff --git a/src/cli.rs b/src/cli.rs index 67123ba14b..4695f8b8f6 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -3,7 +3,6 @@ use crate::commands::plugin::JsonRpc; use crate::commands::plugin::{PluginCommand, PluginSink}; use crate::commands::whole_stream_command; use crate::context::Context; -use crate::data::config; #[cfg(not(feature = "starship-prompt"))] use crate::git::current_branch; use crate::prelude::*; @@ -13,7 +12,7 @@ use nu_parser::{ hir, ClassifiedCommand, ClassifiedPipeline, InternalCommand, PipelineShape, SpannedToken, TokensIterator, }; -use nu_protocol::{Signature, UntaggedValue, Value}; +use nu_protocol::{Signature, Value}; use log::{debug, log_enabled, trace}; use rustyline::error::ReadlineError; @@ -145,8 +144,6 @@ fn load_plugins(context: &mut Context) -> Result<(), ShellError> { require_literal_leading_dot: false, }; - set_env_from_config()?; - for path in search_paths() { let mut pattern = path.to_path_buf(); @@ -245,7 +242,13 @@ fn create_default_starship_config() -> Option { /// The entry point for the CLI. Will register all known internal commands, load experimental commands, load plugins, then prepare the prompt and line reader for input. pub async fn cli() -> Result<(), Box> { + let mut syncer = crate::env::environment_syncer::EnvironmentSyncer::new(); + + syncer.load_environment(); + let mut context = Context::basic()?; + syncer.sync_env_vars(&mut context); + syncer.sync_path_vars(&mut context); { use crate::commands::*; @@ -479,6 +482,13 @@ pub async fn cli() -> Result<(), Box> { let line = process_line(readline, &mut context).await; + // Check the config to see if we need to update the path + // TODO: make sure config is cached so we don't path this load every call + // FIXME: we probably want to be a bit more graceful if we can't set the environment + syncer.reload(); + syncer.sync_env_vars(&mut context); + syncer.sync_path_vars(&mut context); + match line { LineResult::Success(line) => { rl.add_history_entry(line.clone()); @@ -541,58 +551,6 @@ fn chomp_newline(s: &str) -> &str { } } -fn set_env_from_config() -> Result<(), ShellError> { - let config = crate::data::config::read(Tag::unknown(), &None)?; - - if config.contains_key("env") { - // Clear the existing vars, we're about to replace them - for (key, _value) in std::env::vars() { - std::env::remove_var(key); - } - - let value = config.get("env"); - - if let Some(Value { - value: UntaggedValue::Row(r), - .. - }) = value - { - for (k, v) in &r.entries { - if let Ok(value_string) = v.as_string() { - std::env::set_var(k, value_string); - } - } - } - } - - if config.contains_key("path") { - // Override the path with what they give us from config - let value = config.get("path"); - - if let Some(Value { - value: UntaggedValue::Table(table), - .. - }) = value - { - let mut paths = vec![]; - - for val in table { - let path_str = val.as_string(); - - if let Ok(path_str) = path_str { - paths.push(PathBuf::from(path_str)); - } - } - - let path_os_string = std::env::join_paths(&paths); - if let Ok(path_os_string) = path_os_string { - std::env::set_var("PATH", path_os_string); - } - } - } - Ok(()) -} - enum LineResult { Success(String), Error(String, ShellError), @@ -648,13 +606,6 @@ async fn process_line(readline: Result, ctx: &mut Context })); } - // Check the config to see if we need to update the path - // TODO: make sure config is cached so we don't path this load every call - // FIXME: we probably want to be a bit more graceful if we can't set the environment - if let Err(err) = set_env_from_config() { - return LineResult::Error(line.to_string(), err); - } - match run_pipeline(pipeline, ctx, None, line).await { Ok(_) => LineResult::Success(line.to_string()), Err(err) => LineResult::Error(line.to_string(), err), diff --git a/src/data/config.rs b/src/data/config.rs index 9e0bdfcbea..c6428978b6 100644 --- a/src/data/config.rs +++ b/src/data/config.rs @@ -1,3 +1,12 @@ +mod conf; +mod nuconfig; + +#[cfg(test)] +pub mod tests; + +pub(crate) use conf::Conf; +pub(crate) use nuconfig::NuConfig; + use crate::commands::from_toml::convert_toml_value_to_nu_value; use crate::commands::to_toml::value_to_toml_value; use crate::prelude::*; @@ -6,17 +15,11 @@ use indexmap::IndexMap; use log::trace; use nu_errors::ShellError; use nu_protocol::{Dictionary, ShellTypeName, UntaggedValue, Value}; -use serde::{Deserialize, Serialize}; +use nu_source::Tag; use std::fs::{self, OpenOptions}; use std::io; use std::path::{Path, PathBuf}; -#[derive(Deserialize, Serialize)] -struct Config { - #[serde(flatten)] - extra: IndexMap, -} - pub const APP_INFO: AppInfo = AppInfo { name: "nu", author: "nu shell developers", diff --git a/src/data/config/conf.rs b/src/data/config/conf.rs new file mode 100644 index 0000000000..b3504441e2 --- /dev/null +++ b/src/data/config/conf.rs @@ -0,0 +1,23 @@ +use nu_protocol::Value; +use std::fmt::Debug; + +pub trait Conf: Debug + Send { + fn env(&self) -> Option; + fn path(&self) -> Option; + + fn reload(&self); +} + +impl Conf for Box { + fn env(&self) -> Option { + (**self).env() + } + + fn path(&self) -> Option { + (**self).path() + } + + fn reload(&self) { + (**self).reload(); + } +} diff --git a/src/data/config/nuconfig.rs b/src/data/config/nuconfig.rs new file mode 100644 index 0000000000..3444e1f7a9 --- /dev/null +++ b/src/data/config/nuconfig.rs @@ -0,0 +1,64 @@ +use crate::data::config::{read, Conf}; +use indexmap::IndexMap; +use nu_protocol::Value; +use nu_source::Tag; +use parking_lot::Mutex; +use std::fmt::Debug; +use std::sync::Arc; + +#[derive(Debug, Clone)] +pub struct NuConfig { + pub vars: Arc>>, +} + +impl Conf for NuConfig { + fn env(&self) -> Option { + self.env() + } + + fn path(&self) -> Option { + self.path() + } + + fn reload(&self) { + let mut vars = self.vars.lock(); + + if let Ok(variables) = read(Tag::unknown(), &None) { + vars.extend(variables); + } + } +} + +impl NuConfig { + pub fn new() -> NuConfig { + let vars = if let Ok(variables) = read(Tag::unknown(), &None) { + variables + } else { + IndexMap::default() + }; + + NuConfig { + vars: Arc::new(Mutex::new(vars)), + } + } + + pub fn env(&self) -> Option { + let vars = self.vars.lock(); + + if let Some(env_vars) = vars.get("env") { + return Some(env_vars.clone()); + } + + None + } + + pub fn path(&self) -> Option { + let vars = self.vars.lock(); + + if let Some(env_vars) = vars.get("path") { + return Some(env_vars.clone()); + } + + None + } +} diff --git a/src/data/config/tests.rs b/src/data/config/tests.rs new file mode 100644 index 0000000000..ecbe0cb7f7 --- /dev/null +++ b/src/data/config/tests.rs @@ -0,0 +1,44 @@ +use crate::data::config::{read, Conf, NuConfig}; +use indexmap::IndexMap; +use nu_protocol::Value; +use nu_source::Tag; +use parking_lot::Mutex; +use std::path::{Path, PathBuf}; +use std::sync::Arc; + +#[derive(Debug)] +pub struct FakeConfig { + pub config: NuConfig, +} + +impl Conf for FakeConfig { + fn env(&self) -> Option { + self.config.env() + } + + fn path(&self) -> Option { + self.config.path() + } + + fn reload(&self) { + // no-op + } +} + +impl FakeConfig { + pub fn new(config_file: &Path) -> FakeConfig { + let config_file = PathBuf::from(config_file); + + let vars = if let Ok(variables) = read(Tag::unknown(), &Some(config_file)) { + variables + } else { + IndexMap::default() + }; + + FakeConfig { + config: NuConfig { + vars: Arc::new(Mutex::new(vars)), + }, + } + } +} diff --git a/src/env.rs b/src/env.rs index e87ee06084..3f1dd1c311 100644 --- a/src/env.rs +++ b/src/env.rs @@ -1,3 +1,5 @@ +pub(crate) mod environment; +pub(crate) mod environment_syncer; pub(crate) mod host; pub(crate) use self::host::Host; diff --git a/src/env/environment.rs b/src/env/environment.rs new file mode 100644 index 0000000000..8a25db62dc --- /dev/null +++ b/src/env/environment.rs @@ -0,0 +1,255 @@ +use crate::data::config::Conf; +use indexmap::{indexmap, IndexSet}; +use nu_protocol::{UntaggedValue, Value}; +use std::ffi::OsString; +use std::fmt::Debug; + +pub trait Env: Debug + Send { + fn env(&self) -> Option; + fn path(&self) -> Option; + + fn add_env(&mut self, key: &str, value: &str); + fn add_path(&mut self, new_path: OsString); +} + +impl Env for Box { + fn env(&self) -> Option { + (**self).env() + } + + fn path(&self) -> Option { + (**self).path() + } + + fn add_env(&mut self, key: &str, value: &str) { + (**self).add_env(key, value); + } + + fn add_path(&mut self, new_path: OsString) { + (**self).add_path(new_path); + } +} + +#[derive(Debug)] +pub struct Environment { + environment_vars: Option, + path_vars: Option, +} + +impl Environment { + pub fn new() -> Environment { + Environment { + environment_vars: None, + path_vars: None, + } + } + + pub fn from_config(configuration: &T) -> Environment { + let env = configuration.env(); + let path = configuration.path(); + + Environment { + environment_vars: env, + path_vars: path, + } + } + + pub fn morph(&mut self, configuration: &T) { + self.environment_vars = configuration.env(); + self.path_vars = configuration.path(); + } +} + +impl Env for Environment { + fn env(&self) -> Option { + if let Some(vars) = &self.environment_vars { + return Some(vars.clone()); + } + + None + } + + fn path(&self) -> Option { + if let Some(vars) = &self.path_vars { + return Some(vars.clone()); + } + + None + } + + fn add_env(&mut self, key: &str, value: &str) { + let value = UntaggedValue::string(value); + + let new_envs = { + if let Some(Value { + value: UntaggedValue::Row(ref envs), + ref tag, + }) = self.environment_vars + { + let mut new_envs = envs.clone(); + new_envs.insert_data_at_key(key, value.into_value(tag.clone())); + Value { + value: UntaggedValue::Row(new_envs), + tag: tag.clone(), + } + } else { + UntaggedValue::Row(indexmap! { key.into() => value.into_untagged_value() }.into()) + .into_untagged_value() + } + }; + + self.environment_vars = Some(new_envs); + } + + fn add_path(&mut self, paths: std::ffi::OsString) { + let new_paths = { + if let Some(Value { + value: UntaggedValue::Table(ref current_paths), + ref tag, + }) = self.path_vars + { + let mut new_paths = vec![]; + + for path in std::env::split_paths(&paths) { + new_paths.push( + UntaggedValue::string(path.to_string_lossy()).into_value(tag.clone()), + ); + } + + new_paths.extend(current_paths.iter().cloned()); + + let paths: IndexSet = new_paths.into_iter().collect(); + + Value { + value: UntaggedValue::Table(paths.into_iter().collect()), + tag: tag.clone(), + } + } else { + let p = paths.into_string().unwrap_or_else(|_| String::from("")); + let p = UntaggedValue::string(p).into_untagged_value(); + UntaggedValue::Table(vec![p]).into_untagged_value() + } + }; + + self.path_vars = Some(new_paths); + } +} + +#[cfg(test)] +mod tests { + use super::{Env, Environment}; + use crate::data::config::{tests::FakeConfig, Conf}; + use nu_protocol::UntaggedValue; + use nu_test_support::fs::Stub::FileWithContent; + use nu_test_support::playground::Playground; + + #[test] + fn picks_up_environment_variables_from_configuration() { + Playground::setup("environment_test_1", |dirs, sandbox| { + sandbox.with_files(vec![FileWithContent( + "configuration.toml", + r#" + [env] + mosquetero_1 = "Andrés N. Robalino" + mosquetero_2 = "Jonathan Turner" + mosquetero_3 = "Yehuda katz" + mosquetero_4 = "Jason Gedge" + "#, + )]); + + let mut file = dirs.test().clone(); + file.push("configuration.toml"); + + let fake_config = FakeConfig::new(&file); + let actual = Environment::from_config(&fake_config); + + assert_eq!(actual.env(), fake_config.env()); + }); + } + + #[test] + fn picks_up_path_variables_from_configuration() { + Playground::setup("environment_test_2", |dirs, sandbox| { + sandbox.with_files(vec![FileWithContent( + "configuration.toml", + r#" + path = ["/Users/andresrobalino/.volta/bin", "/users/mosqueteros/bin"] + "#, + )]); + + let mut file = dirs.test().clone(); + file.push("configuration.toml"); + + let fake_config = FakeConfig::new(&file); + let actual = Environment::from_config(&fake_config); + + assert_eq!(actual.path(), fake_config.path()); + }); + } + + #[test] + fn updates_env_variable() { + Playground::setup("environment_test_3", |dirs, sandbox| { + sandbox.with_files(vec![FileWithContent( + "configuration.toml", + r#" + [env] + SHELL = "/usr/bin/you_already_made_the_nu_choice" + "#, + )]); + + let mut file = dirs.test().clone(); + file.push("configuration.toml"); + + let fake_config = FakeConfig::new(&file); + let mut actual = Environment::from_config(&fake_config); + + actual.add_env("USER", "NUNO"); + + assert_eq!( + actual.env(), + Some( + UntaggedValue::row( + indexmap! { + "USER".into() => UntaggedValue::string("NUNO").into_untagged_value(), + "SHELL".into() => UntaggedValue::string("/usr/bin/you_already_made_the_nu_choice").into_untagged_value(), + } + ).into_untagged_value() + ) + ); + }); + } + + #[test] + fn updates_path_variable() { + Playground::setup("environment_test_4", |dirs, sandbox| { + sandbox.with_files(vec![FileWithContent( + "configuration.toml", + r#" + path = ["/Users/andresrobalino/.volta/bin", "/users/mosqueteros/bin"] + "#, + )]); + + let mut file = dirs.test().clone(); + file.push("configuration.toml"); + + let fake_config = FakeConfig::new(&file); + let mut actual = Environment::from_config(&fake_config); + + actual.add_path(std::ffi::OsString::from("/path/to/be/added")); + + assert_eq!( + actual.path(), + Some( + UntaggedValue::table(&vec![ + UntaggedValue::string("/path/to/be/added").into_untagged_value(), + UntaggedValue::string("/Users/andresrobalino/.volta/bin") + .into_untagged_value(), + UntaggedValue::string("/users/mosqueteros/bin").into_untagged_value() + ]) + .into_untagged_value() + ) + ); + }); + } +} diff --git a/src/env/environment_syncer.rs b/src/env/environment_syncer.rs new file mode 100644 index 0000000000..976790744b --- /dev/null +++ b/src/env/environment_syncer.rs @@ -0,0 +1,305 @@ +use crate::context::Context; +use crate::data::config::{Conf, NuConfig}; +use crate::env::environment::{Env, Environment}; +use parking_lot::Mutex; +use std::sync::Arc; + +pub struct EnvironmentSyncer { + pub env: Arc>>, + pub config: Arc>, +} + +impl EnvironmentSyncer { + pub fn new() -> EnvironmentSyncer { + EnvironmentSyncer { + env: Arc::new(Mutex::new(Box::new(Environment::new()))), + config: Arc::new(Box::new(NuConfig::new())), + } + } + + #[cfg(test)] + pub fn set_config(&mut self, config: Box) { + self.config = Arc::new(config); + } + + pub fn load_environment(&mut self) { + let config = self.config.clone(); + + self.env = Arc::new(Mutex::new(Box::new(Environment::from_config(&*config)))); + } + + pub fn reload(&mut self) { + self.config.reload(); + + let mut environment = self.env.lock(); + environment.morph(&*self.config); + } + + pub fn sync_env_vars(&mut self, ctx: &mut Context) { + let mut environment = self.env.lock(); + + if environment.env().is_some() { + for (name, value) in ctx.with_host(|host| host.vars()) { + if name != "path" && name != "PATH" { + // account for new env vars present in the current session + // that aren't loaded from config. + environment.add_env(&name, &value); + + // clear the env var from the session + // we are about to replace them + ctx.with_host(|host| host.env_rm(std::ffi::OsString::from(name))); + } + } + + if let Some(variables) = environment.env() { + for var in nu_value_ext::row_entries(&variables) { + if let Ok(string) = var.1.as_string() { + ctx.with_host(|host| { + host.env_set( + std::ffi::OsString::from(var.0), + std::ffi::OsString::from(string), + ) + }); + } + } + } + } + } + + pub fn sync_path_vars(&mut self, ctx: &mut Context) { + let mut environment = self.env.lock(); + + if environment.path().is_some() { + let native_paths = ctx.with_host(|host| host.env_get(std::ffi::OsString::from("PATH"))); + + if let Some(native_paths) = native_paths { + environment.add_path(native_paths); + } + + ctx.with_host(|host| { + host.env_rm(std::ffi::OsString::from("PATH")); + }); + + if let Some(new_paths) = environment.path() { + let prepared = std::env::join_paths( + nu_value_ext::table_entries(&new_paths) + .map(|p| p.as_string()) + .filter_map(Result::ok), + ); + + if let Ok(paths_ready) = prepared { + ctx.with_host(|host| { + host.env_set(std::ffi::OsString::from("PATH"), paths_ready); + }); + } + } + } + } + + #[cfg(test)] + pub fn clear_env_vars(&mut self) { + for (key, _value) in std::env::vars() { + if key != "path" && key != "PATH" { + std::env::remove_var(key); + } + } + } + + #[cfg(test)] + pub fn clear_path_var(&mut self) { + std::env::remove_var("PATH") + } +} + +#[cfg(test)] +mod tests { + use super::EnvironmentSyncer; + use crate::context::Context; + use crate::data::config::tests::FakeConfig; + use crate::env::environment::Env; + use nu_errors::ShellError; + use nu_test_support::fs::Stub::FileWithContent; + use nu_test_support::playground::Playground; + use std::path::PathBuf; + + #[test] + fn syncs_env_if_new_env_entry_in_session_is_not_in_configuration_file() -> Result<(), ShellError> + { + let mut ctx = Context::basic()?; + + let expected = vec![ + ( + "SHELL".to_string(), + "/usr/bin/you_already_made_the_nu_choice".to_string(), + ), + ("USER".to_string(), "NUNO".to_string()), + ]; + + Playground::setup("syncs_env_test_1", |dirs, sandbox| { + sandbox.with_files(vec![FileWithContent( + "configuration.toml", + r#" + [env] + SHELL = "/usr/bin/you_already_made_the_nu_choice" + "#, + )]); + + let mut file = dirs.test().clone(); + file.push("configuration.toml"); + + let fake_config = FakeConfig::new(&file); + let mut actual = EnvironmentSyncer::new(); + actual.set_config(Box::new(fake_config)); + + // Here, the environment variables from the current session + // are cleared since we will load and set them from the + // configuration file (if any) + actual.clear_env_vars(); + + // We explicitly simulate and add the USER variable to the current + // session's environment variables with the value "NUNO". + std::env::set_var( + std::ffi::OsString::from("USER"), + std::ffi::OsString::from("NUNO"), + ); + + // Nu loads the environment variables from the configuration file (if any) + actual.load_environment(); + + // By this point, Nu has already loaded the environment variables + // stored in the configuration file. Before continuing we check + // if any new environment variables have been added from the ones loaded + // in the configuration file. + // + // Nu sees the missing "USER" variable and accounts for it. + actual.sync_env_vars(&mut ctx); + + // Confirms session environment variables are replaced from Nu configuration file + // including the newer one accounted for. + ctx.with_host(|test_host| { + let var_user = test_host + .env_get(std::ffi::OsString::from("USER")) + .expect("Couldn't get USER var from host.") + .into_string() + .expect("Couldn't convert to string."); + + let var_shell = test_host + .env_get(std::ffi::OsString::from("SHELL")) + .expect("Couldn't get SHELL var from host.") + .into_string() + .expect("Couldn't convert to string."); + + let actual = vec![ + ("SHELL".to_string(), var_shell), + ("USER".to_string(), var_user), + ]; + + assert_eq!(actual, expected); + }); + + // Now confirm in-memory environment variables synced appropiately + // including the newer one accounted for. + let environment = actual.env.lock(); + + let vars = nu_value_ext::row_entries( + &environment.env().expect("No variables in the environment."), + ) + .map(|(name, value)| { + ( + name.to_string(), + value.as_string().expect("Couldn't convert to string"), + ) + }) + .collect::>(); + + assert_eq!(vars, expected); + }); + + Ok(()) + } + + #[test] + fn syncs_path_if_new_path_entry_in_session_is_not_in_configuration_file( + ) -> Result<(), ShellError> { + let mut ctx = Context::basic()?; + + let expected = std::env::join_paths(vec![ + PathBuf::from("/path/to/be/added"), + PathBuf::from("/Users/andresrobalino/.volta/bin"), + PathBuf::from("/Users/mosqueteros/bin"), + ]) + .expect("Couldn't join paths.") + .into_string() + .expect("Couldn't convert to string."); + + Playground::setup("syncs_path_test_2", |dirs, sandbox| { + sandbox.with_files(vec![FileWithContent( + "configuration.toml", + r#" + path = ["/Users/andresrobalino/.volta/bin", "/Users/mosqueteros/bin"] + "#, + )]); + + let mut file = dirs.test().clone(); + file.push("configuration.toml"); + + let fake_config = FakeConfig::new(&file); + let mut actual = EnvironmentSyncer::new(); + actual.set_config(Box::new(fake_config)); + + // Here, the environment variables from the current session + // are cleared since we will load and set them from the + // configuration file (if any) + actual.clear_path_var(); + + // We explicitly simulate and add the PATH variable to the current + // session with the path "/path/to/be/added". + std::env::set_var( + std::ffi::OsString::from("PATH"), + std::env::join_paths(vec![PathBuf::from("/path/to/be/added")]) + .expect("Couldn't join paths."), + ); + + // Nu loads the path variables from the configuration file (if any) + actual.load_environment(); + + // By this point, Nu has already loaded environment path variable + // stored in the configuration file. Before continuing we check + // if any new paths have been added from the ones loaded in the + // configuration file. + // + // Nu sees the missing "/path/to/be/added" and accounts for it. + actual.sync_path_vars(&mut ctx); + + ctx.with_host(|test_host| { + let actual = test_host + .env_get(std::ffi::OsString::from("PATH")) + .expect("Couldn't get PATH var from host.") + .into_string() + .expect("Couldn't convert to string."); + + assert_eq!(actual, expected); + }); + + let environment = actual.env.lock(); + + let paths = std::env::join_paths( + &nu_value_ext::table_entries( + &environment + .path() + .expect("No path variable in the environment."), + ) + .map(|value| value.as_string().expect("Couldn't convert to string")) + .map(PathBuf::from) + .collect::>(), + ) + .expect("Couldn't join paths.") + .into_string() + .expect("Couldn't convert to string."); + + assert_eq!(paths, expected); + }); + + Ok(()) + } +} diff --git a/src/env/host.rs b/src/env/host.rs index cae253de9a..96202224b8 100644 --- a/src/env/host.rs +++ b/src/env/host.rs @@ -1,5 +1,6 @@ use crate::prelude::*; use nu_errors::ShellError; +use std::ffi::OsString; use std::fmt::Debug; pub trait Host: Debug + Send { @@ -12,6 +13,11 @@ pub trait Host: Debug + Send { fn stdout(&mut self, out: &str); fn stderr(&mut self, out: &str); + fn vars(&mut self) -> std::env::Vars; + fn env_get(&mut self, key: OsString) -> Option; + fn env_set(&mut self, k: OsString, v: OsString); + fn env_rm(&mut self, k: OsString); + fn width(&self) -> usize; } @@ -32,6 +38,22 @@ impl Host for Box { (**self).stderr(out) } + fn vars(&mut self) -> std::env::Vars { + (**self).vars() + } + + fn env_get(&mut self, key: OsString) -> Option { + (**self).env_get(key) + } + + fn env_set(&mut self, key: OsString, value: OsString) { + (**self).env_set(key, value); + } + + fn env_rm(&mut self, key: OsString) { + (**self).env_rm(key) + } + fn out_termcolor(&self) -> termcolor::StandardStream { (**self).out_termcolor() } @@ -71,6 +93,22 @@ impl Host for BasicHost { } } + fn vars(&mut self) -> std::env::Vars { + std::env::vars() + } + + fn env_get(&mut self, key: OsString) -> Option { + std::env::var_os(key) + } + + fn env_set(&mut self, key: OsString, value: OsString) { + std::env::set_var(key, value); + } + + fn env_rm(&mut self, key: OsString) { + std::env::remove_var(key); + } + fn out_termcolor(&self) -> termcolor::StandardStream { termcolor::StandardStream::stdout(termcolor::ColorChoice::Auto) } diff --git a/src/lib.rs b/src/lib.rs index 9793c56787..ed97476a68 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -21,7 +21,6 @@ mod stream; mod utils; pub use crate::cli::cli; -pub use crate::data::config::{config_path, APP_INFO}; pub use crate::data::dict::TaggedListBuilder; pub use crate::data::primitive; pub use crate::data::value; diff --git a/src/prelude.rs b/src/prelude.rs index e442a283cb..953da31892 100644 --- a/src/prelude.rs +++ b/src/prelude.rs @@ -77,6 +77,7 @@ pub(crate) use crate::commands::command::{ }; pub(crate) use crate::context::CommandRegistry; pub(crate) use crate::context::Context; +pub(crate) use crate::data::config; pub(crate) use crate::data::types::ExtractType; pub(crate) use crate::data::value; pub(crate) use crate::env::host::handle_unexpected; diff --git a/src/utils.rs b/src/utils.rs index 94963a6ef5..c0c247f0cd 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -4,22 +4,6 @@ use nu_errors::ShellError; use nu_protocol::{UntaggedValue, Value}; use std::path::{Component, Path, PathBuf}; -pub enum TaggedValueIter<'a> { - Empty, - List(indexmap::map::Iter<'a, String, Value>), -} - -impl<'a> Iterator for TaggedValueIter<'a> { - type Item = (&'a String, &'a Value); - - fn next(&mut self) -> Option { - match self { - TaggedValueIter::Empty => None, - TaggedValueIter::List(iter) => iter.next(), - } - } -} - fn is_value_tagged_dir(value: &Value) -> bool { match &value.value { UntaggedValue::Row(_) | UntaggedValue::Table(_) => true, @@ -27,16 +11,6 @@ fn is_value_tagged_dir(value: &Value) -> bool { } } -fn tagged_entries_for(value: &Value) -> TaggedValueIter<'_> { - match &value.value { - UntaggedValue::Row(o) => { - let iter = o.entries.iter(); - TaggedValueIter::List(iter) - } - _ => TaggedValueIter::Empty, - } -} - #[derive(Debug, Eq, Ord, PartialEq, PartialOrd)] pub struct ValueResource { pub at: usize, @@ -94,7 +68,7 @@ impl ValueStructure { } fn build(&mut self, src: &Value, lvl: usize) -> Result<(), ShellError> { - for entry in tagged_entries_for(src) { + for entry in nu_value_ext::row_entries(src) { let value = entry.1; let path = entry.0;