From 83795a3e7090fe88fc3c334cb413b921bbc3d96c Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Thu, 4 Jun 2020 01:07:39 +0200 Subject: [PATCH 01/57] Add args in .nurc file to environment --- crates/nu-cli/src/env/environment.rs | 12 ++++++++++++ crates/nu-cli/src/env/environment_syncer.rs | 2 ++ 2 files changed, 14 insertions(+) diff --git a/crates/nu-cli/src/env/environment.rs b/crates/nu-cli/src/env/environment.rs index 9b586eded2..d0286e1a71 100644 --- a/crates/nu-cli/src/env/environment.rs +++ b/crates/nu-cli/src/env/environment.rs @@ -3,6 +3,8 @@ use indexmap::{indexmap, IndexSet}; use nu_protocol::{UntaggedValue, Value}; use std::ffi::OsString; use std::fmt::Debug; +use std::fs::File; +use std::io::Write; pub trait Env: Debug + Send { fn env(&self) -> Option; @@ -44,6 +46,16 @@ impl Environment { } } + pub fn add_nurc(&mut self) { + let key = "envtest"; + let value = "I am here!"; + + let mut file = File::create("env.txt").unwrap(); + write!(&mut file, "{:?}", "somedata").unwrap(); + + self.add_env(key, value); + } + pub fn from_config(configuration: &T) -> Environment { let env = configuration.env(); let path = configuration.path(); diff --git a/crates/nu-cli/src/env/environment_syncer.rs b/crates/nu-cli/src/env/environment_syncer.rs index 852ccfa75c..50f644cf23 100644 --- a/crates/nu-cli/src/env/environment_syncer.rs +++ b/crates/nu-cli/src/env/environment_syncer.rs @@ -70,6 +70,8 @@ impl EnvironmentSyncer { } } } + + environment.add_nurc(); } pub fn sync_path_vars(&mut self, ctx: &mut Context) { From 3d15ac076ceed1994329696700090d5d0d58ca75 Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Thu, 4 Jun 2020 01:13:04 +0200 Subject: [PATCH 02/57] Working dummy version --- crates/nu-cli/src/env/environment.rs | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/crates/nu-cli/src/env/environment.rs b/crates/nu-cli/src/env/environment.rs index d0286e1a71..08479477b1 100644 --- a/crates/nu-cli/src/env/environment.rs +++ b/crates/nu-cli/src/env/environment.rs @@ -40,10 +40,24 @@ pub struct Environment { impl Environment { pub fn new() -> Environment { - Environment { + let mut e = Environment { environment_vars: None, path_vars: None, - } + }; + e.add_nurc(); + e + } + + pub fn from_config(configuration: &T) -> Environment { + let env = configuration.env(); + let path = configuration.path(); + + let mut e = Environment { + environment_vars: env, + path_vars: path, + }; + e.add_nurc(); + e } pub fn add_nurc(&mut self) { @@ -56,16 +70,6 @@ impl Environment { self.add_env(key, value); } - 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(); From 8eaaddca8f3cd2ffc8cbdf46c4d3bf4a78b3b609 Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Thu, 4 Jun 2020 01:16:07 +0200 Subject: [PATCH 03/57] Add add_nurc to sync_env command --- crates/nu-cli/src/env/environment.rs | 13 +++++-------- crates/nu-cli/src/env/environment_syncer.rs | 3 +-- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/crates/nu-cli/src/env/environment.rs b/crates/nu-cli/src/env/environment.rs index 08479477b1..9194580a0a 100644 --- a/crates/nu-cli/src/env/environment.rs +++ b/crates/nu-cli/src/env/environment.rs @@ -40,26 +40,23 @@ pub struct Environment { impl Environment { pub fn new() -> Environment { - let mut e = Environment { + Environment { environment_vars: None, path_vars: None, - }; - e.add_nurc(); - e + } } pub fn from_config(configuration: &T) -> Environment { let env = configuration.env(); let path = configuration.path(); - let mut e = Environment { + Environment { environment_vars: env, path_vars: path, - }; - e.add_nurc(); - e + } } + //Add env vars specified in the current dirs .nurc, if it exists. pub fn add_nurc(&mut self) { let key = "envtest"; let value = "I am here!"; diff --git a/crates/nu-cli/src/env/environment_syncer.rs b/crates/nu-cli/src/env/environment_syncer.rs index 50f644cf23..f7315dee66 100644 --- a/crates/nu-cli/src/env/environment_syncer.rs +++ b/crates/nu-cli/src/env/environment_syncer.rs @@ -44,6 +44,7 @@ impl EnvironmentSyncer { pub fn sync_env_vars(&mut self, ctx: &mut Context) { let mut environment = self.env.lock(); + environment.add_nurc(); if environment.env().is_some() { for (name, value) in ctx.with_host(|host| host.vars()) { if name != "path" && name != "PATH" { @@ -70,8 +71,6 @@ impl EnvironmentSyncer { } } } - - environment.add_nurc(); } pub fn sync_path_vars(&mut self, ctx: &mut Context) { From 48e4bb60d0a5e0060b43d503712fed102c69f3ce Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Thu, 4 Jun 2020 02:14:49 +0200 Subject: [PATCH 04/57] Parse .nurc file --- crates/nu-cli/src/env/environment.rs | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/crates/nu-cli/src/env/environment.rs b/crates/nu-cli/src/env/environment.rs index 9194580a0a..9c85c2c4f2 100644 --- a/crates/nu-cli/src/env/environment.rs +++ b/crates/nu-cli/src/env/environment.rs @@ -4,7 +4,7 @@ use nu_protocol::{UntaggedValue, Value}; use std::ffi::OsString; use std::fmt::Debug; use std::fs::File; -use std::io::Write; +use std::io::prelude::*; pub trait Env: Debug + Send { fn env(&self) -> Option; @@ -57,14 +57,18 @@ impl Environment { } //Add env vars specified in the current dirs .nurc, if it exists. - pub fn add_nurc(&mut self) { - let key = "envtest"; - let value = "I am here!"; + //TODO: Remove env vars after leaving the directory. Save added vars in env? + //TODO: Add authentication by saving the path to the .nurc file in some variable? + pub fn add_nurc(&mut self) -> std::io::Result<()> { + let mut file = File::open(".nurc")?; + let mut contents = String::new(); + file.read_to_string(&mut contents)?; - let mut file = File::create("env.txt").unwrap(); - write!(&mut file, "{:?}", "somedata").unwrap(); + let value = contents.parse::().unwrap(); + let nurc_vars = value.get("env").unwrap().as_table().unwrap(); - self.add_env(key, value); + nurc_vars.iter().for_each(|(k, v)| self.add_env(k, v.as_str().unwrap())); + Ok(()) } pub fn morph(&mut self, configuration: &T) { From c67d93dae1c8ab7e5f7cdb5421b48279d1f0c113 Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Thu, 4 Jun 2020 02:40:50 +0200 Subject: [PATCH 05/57] Delete env vars after leaving directory --- crates/nu-cli/src/env/environment.rs | 70 ++++++++++++++++++++- crates/nu-cli/src/env/environment_syncer.rs | 2 +- 2 files changed, 68 insertions(+), 4 deletions(-) diff --git a/crates/nu-cli/src/env/environment.rs b/crates/nu-cli/src/env/environment.rs index 9c85c2c4f2..bb8718230a 100644 --- a/crates/nu-cli/src/env/environment.rs +++ b/crates/nu-cli/src/env/environment.rs @@ -1,10 +1,12 @@ use crate::data::config::Conf; use indexmap::{indexmap, IndexSet}; use nu_protocol::{UntaggedValue, Value}; +use std::collections::HashMap; use std::ffi::OsString; use std::fmt::Debug; use std::fs::File; use std::io::prelude::*; +use std::path::PathBuf; pub trait Env: Debug + Send { fn env(&self) -> Option; @@ -36,6 +38,7 @@ impl Env for Box { pub struct Environment { environment_vars: Option, path_vars: Option, + nurc_env_vars: HashMap>, } impl Environment { @@ -43,6 +46,7 @@ impl Environment { Environment { environment_vars: None, path_vars: None, + nurc_env_vars: HashMap::new(), } } @@ -53,24 +57,84 @@ impl Environment { Environment { environment_vars: env, path_vars: path, + nurc_env_vars: HashMap::new(), } } //Add env vars specified in the current dirs .nurc, if it exists. //TODO: Remove env vars after leaving the directory. Save added vars in env? + //Map directory to vars //TODO: Add authentication by saving the path to the .nurc file in some variable? + //TODO: handle errors + + pub fn maintain_nurc_environment_vars(&mut self) { + self.clear_vars_from_unvisited_dirs(); + self.add_nurc(); + } + pub fn add_nurc(&mut self) -> std::io::Result<()> { let mut file = File::open(".nurc")?; let mut contents = String::new(); file.read_to_string(&mut contents)?; - let value = contents.parse::().unwrap(); - let nurc_vars = value.get("env").unwrap().as_table().unwrap(); + let toml_doc = contents.parse::().unwrap(); + let nurc_vars = toml_doc.get("env").unwrap().as_table().unwrap(); - nurc_vars.iter().for_each(|(k, v)| self.add_env(k, v.as_str().unwrap())); + nurc_vars.iter().for_each(|(k, v)| { + self.add_env(k, v.as_str().unwrap()); + }); + + self.nurc_env_vars.insert( + std::env::current_dir()?, + nurc_vars.keys().map(|k| k.clone()).collect(), + ); //Maybe could do without clone here, but leave for now Ok(()) } + //If the user has left directories which added env vars through .nurc, we clear those vars + //For each directory d in nurc_env_vars: + //if current_dir does not have d as a parent (possibly recursive), + //the vars set by d should be removed + pub fn clear_vars_from_unvisited_dirs(&mut self) -> std::io::Result<()> { + let current_dir = std::env::current_dir()?; + + let mut vars_to_keep = HashMap::new(); + for (d, v) in self.nurc_env_vars.iter() { + let mut working_dir = Some(current_dir.as_path()); + while working_dir.is_some() { + if working_dir.unwrap() == d { + vars_to_keep.insert(d.clone(), v.clone()); + break; + } else { + working_dir = working_dir.unwrap().parent(); + } + } + } + + let mut vars_to_delete = vec![]; + for (path, vals) in self.nurc_env_vars.iter() { + if !vars_to_keep.contains_key(path) { + vars_to_delete.extend(vals.clone()); + } + } + + vars_to_delete.iter().for_each(|var| self.remove_env(var)); + + self.nurc_env_vars = vars_to_keep; + Ok(()) + } + + // environment_vars: Option, + pub fn remove_env(&mut self, key: &str) { + if let Some(Value { + value: UntaggedValue::Row(envs), + tag: _, + }) = &mut self.environment_vars + { + envs.entries.remove(key); + } + } + pub fn morph(&mut self, configuration: &T) { self.environment_vars = configuration.env(); self.path_vars = configuration.path(); diff --git a/crates/nu-cli/src/env/environment_syncer.rs b/crates/nu-cli/src/env/environment_syncer.rs index f7315dee66..3570896b29 100644 --- a/crates/nu-cli/src/env/environment_syncer.rs +++ b/crates/nu-cli/src/env/environment_syncer.rs @@ -44,7 +44,7 @@ impl EnvironmentSyncer { pub fn sync_env_vars(&mut self, ctx: &mut Context) { let mut environment = self.env.lock(); - environment.add_nurc(); + environment.maintain_nurc_environment_vars(); if environment.env().is_some() { for (name, value) in ctx.with_host(|host| host.vars()) { if name != "path" && name != "PATH" { From de0c252e27e4f3cb63bb30925d1dd8f48ca1d5d3 Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Thu, 4 Jun 2020 04:37:02 +0200 Subject: [PATCH 06/57] Removing vals not working, strangely --- crates/nu-cli/src/env/environment.rs | 21 +++++++++++++++++---- crates/nu-protocol/src/value/dict.rs | 4 ++++ 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/crates/nu-cli/src/env/environment.rs b/crates/nu-cli/src/env/environment.rs index bb8718230a..f24f38b0e0 100644 --- a/crates/nu-cli/src/env/environment.rs +++ b/crates/nu-cli/src/env/environment.rs @@ -1,11 +1,13 @@ use crate::data::config::Conf; use indexmap::{indexmap, IndexSet}; use nu_protocol::{UntaggedValue, Value}; -use std::collections::HashMap; +use std::collections::{HashMap}; +use indexmap::IndexMap; use std::ffi::OsString; use std::fmt::Debug; use std::fs::File; use std::io::prelude::*; +use std::io::Write; use std::path::PathBuf; pub trait Env: Debug + Send { @@ -68,8 +70,12 @@ impl Environment { //TODO: handle errors pub fn maintain_nurc_environment_vars(&mut self) { - self.clear_vars_from_unvisited_dirs(); - self.add_nurc(); + match self.clear_vars_from_unvisited_dirs() { + _ => {} + }; + match self.add_nurc() { + _ => {} + }; } pub fn add_nurc(&mut self) -> std::io::Result<()> { @@ -125,13 +131,20 @@ impl Environment { } // environment_vars: Option, + // pub fn remove_env(&mut self, key: &str) { if let Some(Value { value: UntaggedValue::Row(envs), tag: _, }) = &mut self.environment_vars { - envs.entries.remove(key); + + let mut file = File::create("env.txt").unwrap(); + write!(&mut file, "env: {:?}", envs.entries).unwrap(); + + let mut file = File::create("removenv.txt").unwrap(); + let removed = envs.remove_key(key); + write!(&mut file, "removed {:?}", (key, removed)).unwrap(); } } diff --git a/crates/nu-protocol/src/value/dict.rs b/crates/nu-protocol/src/value/dict.rs index b66307decf..70873f7983 100644 --- a/crates/nu-protocol/src/value/dict.rs +++ b/crates/nu-protocol/src/value/dict.rs @@ -159,6 +159,10 @@ impl Dictionary { self.entries.contains_key(key) } + pub fn remove_key(&mut self, key: &str) -> Option { + self.entries.remove(key) + } + /// Find the matching Value for a key, if possible pub fn get_data_by_key(&self, name: Spanned<&str>) -> Option { let result = self From 6e723863604bcb3c027d8204ab2aae258cca1330 Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Thu, 4 Jun 2020 05:30:28 +0200 Subject: [PATCH 07/57] Refactoring, add comment --- crates/nu-cli/src/env/environment.rs | 37 ++++++++++------------------ 1 file changed, 13 insertions(+), 24 deletions(-) diff --git a/crates/nu-cli/src/env/environment.rs b/crates/nu-cli/src/env/environment.rs index f24f38b0e0..5b3c585bf8 100644 --- a/crates/nu-cli/src/env/environment.rs +++ b/crates/nu-cli/src/env/environment.rs @@ -2,12 +2,10 @@ use crate::data::config::Conf; use indexmap::{indexmap, IndexSet}; use nu_protocol::{UntaggedValue, Value}; use std::collections::{HashMap}; -use indexmap::IndexMap; use std::ffi::OsString; use std::fmt::Debug; use std::fs::File; use std::io::prelude::*; -use std::io::Write; use std::path::PathBuf; pub trait Env: Debug + Send { @@ -40,7 +38,7 @@ impl Env for Box { pub struct Environment { environment_vars: Option, path_vars: Option, - nurc_env_vars: HashMap>, + nurc_env_keys: HashMap>, //Directory -> Env key. If an environment var has been added from a .nurc in a directory, we track it here so we can remove it when the user leaves the directory. } impl Environment { @@ -48,7 +46,7 @@ impl Environment { Environment { environment_vars: None, path_vars: None, - nurc_env_vars: HashMap::new(), + nurc_env_keys: HashMap::new(), } } @@ -59,7 +57,7 @@ impl Environment { Environment { environment_vars: env, path_vars: path, - nurc_env_vars: HashMap::new(), + nurc_env_keys: HashMap::new(), } } @@ -90,7 +88,7 @@ impl Environment { self.add_env(k, v.as_str().unwrap()); }); - self.nurc_env_vars.insert( + self.nurc_env_keys.insert( std::env::current_dir()?, nurc_vars.keys().map(|k| k.clone()).collect(), ); //Maybe could do without clone here, but leave for now @@ -99,17 +97,16 @@ impl Environment { //If the user has left directories which added env vars through .nurc, we clear those vars //For each directory d in nurc_env_vars: - //if current_dir does not have d as a parent (possibly recursive), - //the vars set by d should be removed + //if current_dir does not have d as a parent (possibly recursive), the vars set by d should be removed pub fn clear_vars_from_unvisited_dirs(&mut self) -> std::io::Result<()> { let current_dir = std::env::current_dir()?; - let mut vars_to_keep = HashMap::new(); - for (d, v) in self.nurc_env_vars.iter() { + let mut new_nurc_env_vars = HashMap::new(); + for (d, v) in self.nurc_env_keys.iter() { let mut working_dir = Some(current_dir.as_path()); while working_dir.is_some() { if working_dir.unwrap() == d { - vars_to_keep.insert(d.clone(), v.clone()); + new_nurc_env_vars.insert(d.clone(), v.clone()); break; } else { working_dir = working_dir.unwrap().parent(); @@ -118,33 +115,25 @@ impl Environment { } let mut vars_to_delete = vec![]; - for (path, vals) in self.nurc_env_vars.iter() { - if !vars_to_keep.contains_key(path) { + for (path, vals) in self.nurc_env_keys.iter() { + if !new_nurc_env_vars.contains_key(path) { vars_to_delete.extend(vals.clone()); } } - vars_to_delete.iter().for_each(|var| self.remove_env(var)); + vars_to_delete.iter().for_each(|env_var| self.remove_env(env_var)); - self.nurc_env_vars = vars_to_keep; + self.nurc_env_keys = new_nurc_env_vars; Ok(()) } - // environment_vars: Option, - // pub fn remove_env(&mut self, key: &str) { if let Some(Value { value: UntaggedValue::Row(envs), tag: _, }) = &mut self.environment_vars { - - let mut file = File::create("env.txt").unwrap(); - write!(&mut file, "env: {:?}", envs.entries).unwrap(); - - let mut file = File::create("removenv.txt").unwrap(); - let removed = envs.remove_key(key); - write!(&mut file, "removed {:?}", (key, removed)).unwrap(); + envs.remove_key(key); } } From fe4a51eef429f3351e891a14ac42397d4a6bb276 Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Thu, 4 Jun 2020 05:31:37 +0200 Subject: [PATCH 08/57] Debugging --- crates/nu-cli/src/env/environment.rs | 2 +- crates/nu-protocol/src/value/dict.rs | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/crates/nu-cli/src/env/environment.rs b/crates/nu-cli/src/env/environment.rs index 5b3c585bf8..b037b8bf42 100644 --- a/crates/nu-cli/src/env/environment.rs +++ b/crates/nu-cli/src/env/environment.rs @@ -133,7 +133,7 @@ impl Environment { tag: _, }) = &mut self.environment_vars { - envs.remove_key(key); + envs.entries.remove(key); } } diff --git a/crates/nu-protocol/src/value/dict.rs b/crates/nu-protocol/src/value/dict.rs index 70873f7983..b66307decf 100644 --- a/crates/nu-protocol/src/value/dict.rs +++ b/crates/nu-protocol/src/value/dict.rs @@ -159,10 +159,6 @@ impl Dictionary { self.entries.contains_key(key) } - pub fn remove_key(&mut self, key: &str) -> Option { - self.entries.remove(key) - } - /// Find the matching Value for a key, if possible pub fn get_data_by_key(&self, name: Spanned<&str>) -> Option { let result = self From 24d2e88e0f1c21fe64528e3cbc235f116190adaf Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Thu, 4 Jun 2020 23:17:55 +0200 Subject: [PATCH 09/57] Debug by logging to file --- crates/nu-cli/src/env/environment.rs | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/crates/nu-cli/src/env/environment.rs b/crates/nu-cli/src/env/environment.rs index b037b8bf42..a30224e7b9 100644 --- a/crates/nu-cli/src/env/environment.rs +++ b/crates/nu-cli/src/env/environment.rs @@ -4,9 +4,11 @@ use nu_protocol::{UntaggedValue, Value}; use std::collections::{HashMap}; use std::ffi::OsString; use std::fmt::Debug; +use std::fs::OpenOptions; use std::fs::File; use std::io::prelude::*; use std::path::PathBuf; +use std::io::Write; pub trait Env: Debug + Send { fn env(&self) -> Option; @@ -68,10 +70,11 @@ impl Environment { //TODO: handle errors pub fn maintain_nurc_environment_vars(&mut self) { - match self.clear_vars_from_unvisited_dirs() { + match self.add_nurc() { _ => {} }; - match self.add_nurc() { + + match self.clear_vars_from_unvisited_dirs() { _ => {} }; } @@ -84,6 +87,14 @@ impl Environment { let toml_doc = contents.parse::().unwrap(); let nurc_vars = toml_doc.get("env").unwrap().as_table().unwrap(); + let mut file = OpenOptions::new() + .write(true) + .append(true) + .create(true) + .open("env.txt").unwrap(); + + write!(&mut file, "keys to add: {:?}\n", nurc_vars).unwrap(); + nurc_vars.iter().for_each(|(k, v)| { self.add_env(k, v.as_str().unwrap()); }); @@ -98,6 +109,7 @@ impl Environment { //If the user has left directories which added env vars through .nurc, we clear those vars //For each directory d in nurc_env_vars: //if current_dir does not have d as a parent (possibly recursive), the vars set by d should be removed + //TODO: Seems like vars are re-added immediately after being removed pub fn clear_vars_from_unvisited_dirs(&mut self) -> std::io::Result<()> { let current_dir = std::env::current_dir()?; @@ -121,6 +133,13 @@ impl Environment { } } + let mut file = OpenOptions::new() + .write(true) + .append(true) + .create(true) + .open("env.txt")?; + + write!(&mut file, "keys to remove: {:?}\n", vars_to_delete).unwrap(); vars_to_delete.iter().for_each(|env_var| self.remove_env(env_var)); self.nurc_env_keys = new_nurc_env_vars; @@ -133,6 +152,7 @@ impl Environment { tag: _, }) = &mut self.environment_vars { + envs.entries.remove(key); } } From 3aeddee2fe7628f60ce57618ae88f1b1ff141d95 Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Thu, 4 Jun 2020 23:43:26 +0200 Subject: [PATCH 10/57] Add and remove env var behavior appears correct However, it does not use existing code that well. --- crates/nu-cli/src/env/environment.rs | 18 +----------------- crates/nu-cli/src/env/environment_syncer.rs | 1 + 2 files changed, 2 insertions(+), 17 deletions(-) diff --git a/crates/nu-cli/src/env/environment.rs b/crates/nu-cli/src/env/environment.rs index a30224e7b9..18347ec8a4 100644 --- a/crates/nu-cli/src/env/environment.rs +++ b/crates/nu-cli/src/env/environment.rs @@ -4,11 +4,9 @@ use nu_protocol::{UntaggedValue, Value}; use std::collections::{HashMap}; use std::ffi::OsString; use std::fmt::Debug; -use std::fs::OpenOptions; use std::fs::File; use std::io::prelude::*; use std::path::PathBuf; -use std::io::Write; pub trait Env: Debug + Send { fn env(&self) -> Option; @@ -87,14 +85,6 @@ impl Environment { let toml_doc = contents.parse::().unwrap(); let nurc_vars = toml_doc.get("env").unwrap().as_table().unwrap(); - let mut file = OpenOptions::new() - .write(true) - .append(true) - .create(true) - .open("env.txt").unwrap(); - - write!(&mut file, "keys to add: {:?}\n", nurc_vars).unwrap(); - nurc_vars.iter().for_each(|(k, v)| { self.add_env(k, v.as_str().unwrap()); }); @@ -133,13 +123,6 @@ impl Environment { } } - let mut file = OpenOptions::new() - .write(true) - .append(true) - .create(true) - .open("env.txt")?; - - write!(&mut file, "keys to remove: {:?}\n", vars_to_delete).unwrap(); vars_to_delete.iter().for_each(|env_var| self.remove_env(env_var)); self.nurc_env_keys = new_nurc_env_vars; @@ -154,6 +137,7 @@ impl Environment { { envs.entries.remove(key); + std::env::remove_var(key); } } diff --git a/crates/nu-cli/src/env/environment_syncer.rs b/crates/nu-cli/src/env/environment_syncer.rs index 3570896b29..f23a5fc757 100644 --- a/crates/nu-cli/src/env/environment_syncer.rs +++ b/crates/nu-cli/src/env/environment_syncer.rs @@ -71,6 +71,7 @@ impl EnvironmentSyncer { } } } + } pub fn sync_path_vars(&mut self, ctx: &mut Context) { From e97e883d1f28647da9e7163c1d8c5627e35844ad Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Fri, 5 Jun 2020 01:23:55 +0200 Subject: [PATCH 11/57] Move work to cli.rs --- crates/nu-cli/src/cli.rs | 23 +++++- crates/nu-cli/src/env/environment.rs | 83 --------------------- crates/nu-cli/src/env/environment_syncer.rs | 1 - 3 files changed, 21 insertions(+), 86 deletions(-) diff --git a/crates/nu-cli/src/cli.rs b/crates/nu-cli/src/cli.rs index bd5097812e..2715396fd4 100644 --- a/crates/nu-cli/src/cli.rs +++ b/crates/nu-cli/src/cli.rs @@ -22,7 +22,8 @@ use rustyline::{ KeyPress, Movement, Word, }; use std::error::Error; -use std::io::{BufRead, BufReader, Write}; +use std::fs::File; +use std::io::{BufRead, BufReader, Write, Read}; use std::iter::Iterator; use std::path::{Path, PathBuf}; use std::sync::atomic::Ordering; @@ -740,6 +741,23 @@ enum LineResult { Break, } +//TODO: Add authentication by saving the path to the .nurc file in some variable? +//For directory wd in whitelisted directories +//if current directory is wd or subdir to wd, add env vars from .nurc in wd +pub fn add_nurc(env: &mut IndexMap) -> std::io::Result<()> { + let mut file = File::open(".nurc")?; + let mut contents = String::new(); + file.read_to_string(&mut contents)?; + + let toml_doc = contents.parse::().unwrap(); + let nurc_vars = toml_doc.get("env").unwrap().as_table().unwrap(); + + nurc_vars.iter().for_each(|(k, v)| { + env.insert(k.clone(), v.as_str().unwrap().to_string()); + }); + + Ok(()) +} /// Process the line by parsing the text to turn it into commands, classify those commands so that we understand what is being called in the pipeline, and then run this pipeline async fn process_line( readline: Result, @@ -888,7 +906,8 @@ async fn process_line( classified_block.block.expand_it_usage(); trace!("{:#?}", classified_block); - let env = ctx.get_env(); + let mut env = ctx.get_env(); + add_nurc(&mut env); match run_block( &classified_block.block, ctx, diff --git a/crates/nu-cli/src/env/environment.rs b/crates/nu-cli/src/env/environment.rs index 18347ec8a4..0f0cbc234e 100644 --- a/crates/nu-cli/src/env/environment.rs +++ b/crates/nu-cli/src/env/environment.rs @@ -38,7 +38,6 @@ impl Env for Box { pub struct Environment { environment_vars: Option, path_vars: Option, - nurc_env_keys: HashMap>, //Directory -> Env key. If an environment var has been added from a .nurc in a directory, we track it here so we can remove it when the user leaves the directory. } impl Environment { @@ -46,7 +45,6 @@ impl Environment { Environment { environment_vars: None, path_vars: None, - nurc_env_keys: HashMap::new(), } } @@ -57,87 +55,6 @@ impl Environment { Environment { environment_vars: env, path_vars: path, - nurc_env_keys: HashMap::new(), - } - } - - //Add env vars specified in the current dirs .nurc, if it exists. - //TODO: Remove env vars after leaving the directory. Save added vars in env? - //Map directory to vars - //TODO: Add authentication by saving the path to the .nurc file in some variable? - //TODO: handle errors - - pub fn maintain_nurc_environment_vars(&mut self) { - match self.add_nurc() { - _ => {} - }; - - match self.clear_vars_from_unvisited_dirs() { - _ => {} - }; - } - - pub fn add_nurc(&mut self) -> std::io::Result<()> { - let mut file = File::open(".nurc")?; - let mut contents = String::new(); - file.read_to_string(&mut contents)?; - - let toml_doc = contents.parse::().unwrap(); - let nurc_vars = toml_doc.get("env").unwrap().as_table().unwrap(); - - nurc_vars.iter().for_each(|(k, v)| { - self.add_env(k, v.as_str().unwrap()); - }); - - self.nurc_env_keys.insert( - std::env::current_dir()?, - nurc_vars.keys().map(|k| k.clone()).collect(), - ); //Maybe could do without clone here, but leave for now - Ok(()) - } - - //If the user has left directories which added env vars through .nurc, we clear those vars - //For each directory d in nurc_env_vars: - //if current_dir does not have d as a parent (possibly recursive), the vars set by d should be removed - //TODO: Seems like vars are re-added immediately after being removed - pub fn clear_vars_from_unvisited_dirs(&mut self) -> std::io::Result<()> { - let current_dir = std::env::current_dir()?; - - let mut new_nurc_env_vars = HashMap::new(); - for (d, v) in self.nurc_env_keys.iter() { - let mut working_dir = Some(current_dir.as_path()); - while working_dir.is_some() { - if working_dir.unwrap() == d { - new_nurc_env_vars.insert(d.clone(), v.clone()); - break; - } else { - working_dir = working_dir.unwrap().parent(); - } - } - } - - let mut vars_to_delete = vec![]; - for (path, vals) in self.nurc_env_keys.iter() { - if !new_nurc_env_vars.contains_key(path) { - vars_to_delete.extend(vals.clone()); - } - } - - vars_to_delete.iter().for_each(|env_var| self.remove_env(env_var)); - - self.nurc_env_keys = new_nurc_env_vars; - Ok(()) - } - - pub fn remove_env(&mut self, key: &str) { - if let Some(Value { - value: UntaggedValue::Row(envs), - tag: _, - }) = &mut self.environment_vars - { - - envs.entries.remove(key); - std::env::remove_var(key); } } diff --git a/crates/nu-cli/src/env/environment_syncer.rs b/crates/nu-cli/src/env/environment_syncer.rs index f23a5fc757..2fb0f87a12 100644 --- a/crates/nu-cli/src/env/environment_syncer.rs +++ b/crates/nu-cli/src/env/environment_syncer.rs @@ -44,7 +44,6 @@ impl EnvironmentSyncer { pub fn sync_env_vars(&mut self, ctx: &mut Context) { let mut environment = self.env.lock(); - environment.maintain_nurc_environment_vars(); if environment.env().is_some() { for (name, value) in ctx.with_host(|host| host.vars()) { if name != "path" && name != "PATH" { From 0f0485957af9ff619b1c6caf3e106b0f65ed570f Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Fri, 5 Jun 2020 04:00:52 +0200 Subject: [PATCH 12/57] Parse config directories --- crates/nu-cli/src/cli.rs | 25 ++------- crates/nu-cli/src/data/config/conf.rs | 6 ++- crates/nu-cli/src/data/config/nuconfig.rs | 12 +++++ crates/nu-cli/src/env/environment.rs | 4 -- crates/nu-cli/src/env/environment_syncer.rs | 56 +++++++++++++++++++-- 5 files changed, 74 insertions(+), 29 deletions(-) diff --git a/crates/nu-cli/src/cli.rs b/crates/nu-cli/src/cli.rs index 2715396fd4..c0d8a2901d 100644 --- a/crates/nu-cli/src/cli.rs +++ b/crates/nu-cli/src/cli.rs @@ -22,8 +22,8 @@ use rustyline::{ KeyPress, Movement, Word, }; use std::error::Error; -use std::fs::File; -use std::io::{BufRead, BufReader, Write, Read}; +use std::fs::{File, OpenOptions}; +use std::io::{BufRead, BufReader, Read, Write}; use std::iter::Iterator; use std::path::{Path, PathBuf}; use std::sync::atomic::Ordering; @@ -741,23 +741,6 @@ enum LineResult { Break, } -//TODO: Add authentication by saving the path to the .nurc file in some variable? -//For directory wd in whitelisted directories -//if current directory is wd or subdir to wd, add env vars from .nurc in wd -pub fn add_nurc(env: &mut IndexMap) -> std::io::Result<()> { - let mut file = File::open(".nurc")?; - let mut contents = String::new(); - file.read_to_string(&mut contents)?; - - let toml_doc = contents.parse::().unwrap(); - let nurc_vars = toml_doc.get("env").unwrap().as_table().unwrap(); - - nurc_vars.iter().for_each(|(k, v)| { - env.insert(k.clone(), v.as_str().unwrap().to_string()); - }); - - Ok(()) -} /// Process the line by parsing the text to turn it into commands, classify those commands so that we understand what is being called in the pipeline, and then run this pipeline async fn process_line( readline: Result, @@ -906,8 +889,8 @@ async fn process_line( classified_block.block.expand_it_usage(); trace!("{:#?}", classified_block); - let mut env = ctx.get_env(); - add_nurc(&mut env); + let env = ctx.get_env(); + match run_block( &classified_block.block, ctx, diff --git a/crates/nu-cli/src/data/config/conf.rs b/crates/nu-cli/src/data/config/conf.rs index b3504441e2..efd44d396c 100644 --- a/crates/nu-cli/src/data/config/conf.rs +++ b/crates/nu-cli/src/data/config/conf.rs @@ -4,7 +4,7 @@ use std::fmt::Debug; pub trait Conf: Debug + Send { fn env(&self) -> Option; fn path(&self) -> Option; - + fn direnv_whitelist(&self) -> Option; fn reload(&self); } @@ -13,6 +13,10 @@ impl Conf for Box { (**self).env() } + fn direnv_whitelist(&self) -> Option { + (**self).direnv_whitelist() + } + fn path(&self) -> Option { (**self).path() } diff --git a/crates/nu-cli/src/data/config/nuconfig.rs b/crates/nu-cli/src/data/config/nuconfig.rs index 3444e1f7a9..97e486fb70 100644 --- a/crates/nu-cli/src/data/config/nuconfig.rs +++ b/crates/nu-cli/src/data/config/nuconfig.rs @@ -20,6 +20,10 @@ impl Conf for NuConfig { self.path() } + fn direnv_whitelist(&self) -> Option { + self.direnv_whitelist() + } + fn reload(&self) { let mut vars = self.vars.lock(); @@ -52,6 +56,14 @@ impl NuConfig { None } + pub fn direnv_whitelist(&self) -> Option { + let vars = self.vars.lock(); + if let Some(dirs) = vars.get("nurc_dirs") { + return Some(dirs.clone()); + } + None + } + pub fn path(&self) -> Option { let vars = self.vars.lock(); diff --git a/crates/nu-cli/src/env/environment.rs b/crates/nu-cli/src/env/environment.rs index 0f0cbc234e..9b586eded2 100644 --- a/crates/nu-cli/src/env/environment.rs +++ b/crates/nu-cli/src/env/environment.rs @@ -1,12 +1,8 @@ use crate::data::config::Conf; use indexmap::{indexmap, IndexSet}; use nu_protocol::{UntaggedValue, Value}; -use std::collections::{HashMap}; use std::ffi::OsString; use std::fmt::Debug; -use std::fs::File; -use std::io::prelude::*; -use std::path::PathBuf; pub trait Env: Debug + Send { fn env(&self) -> Option; diff --git a/crates/nu-cli/src/env/environment_syncer.rs b/crates/nu-cli/src/env/environment_syncer.rs index 2fb0f87a12..31f72c2c3b 100644 --- a/crates/nu-cli/src/env/environment_syncer.rs +++ b/crates/nu-cli/src/env/environment_syncer.rs @@ -1,8 +1,14 @@ use crate::context::Context; use crate::data::config::{Conf, NuConfig}; use crate::env::environment::{Env, Environment}; +use nu_protocol::{UntaggedValue, Value, Primitive}; use parking_lot::Mutex; -use std::sync::Arc; +use std::io::Read; +use std::io::Write; +use std::{ + fs::{File, OpenOptions}, + sync::Arc, path::PathBuf, +}; pub struct EnvironmentSyncer { pub env: Arc>>, @@ -41,9 +47,54 @@ impl EnvironmentSyncer { environment.morph(&*self.config); } + //TODO: Add authentication by saving the path to the .nurc file in some variable? + //For directory wd in whitelisted directories + //if current directory is wd or subdir to wd, add env vars from .nurc in wd + pub fn add_nurc(&self) -> std::io::Result<()> { + let mut file = OpenOptions::new() + .write(true) + .append(true) + .create(true) + .open("vars.txt")?; + + let conf = Arc::clone(&self.config); + + let mut directories = vec![]; + if let Some(Value { + value: UntaggedValue::Table(ref directories_as_values), + tag: _, + }) = conf.direnv_whitelist() + { + for dirval in directories_as_values { + if let Value { + value: UntaggedValue::Primitive(Primitive::String(ref dir)), + tag: _, + } = dirval { + let path = PathBuf::from(dir); + directories.push(path); + } + } + }; + + write!(&mut file, "variables so far: {:?}\n", directories).unwrap(); + + // let mut file = File::open(".nurc")?; + // let mut contents = String::new(); + // file.read_to_string(&mut contents)?; + + // let toml_doc = contents.parse::().unwrap(); + // let nurc_vars = toml_doc.get("env").unwrap().as_table().unwrap(); + + // nurc_vars.iter().for_each(|(k, v)| { + // env.insert(k.clone(), v.as_str().unwrap().to_string()); + // }); + + Ok(()) + } + pub fn sync_env_vars(&mut self, ctx: &mut Context) { let mut environment = self.env.lock(); - + self.add_nurc(); if environment.env().is_some() { for (name, value) in ctx.with_host(|host| host.vars()) { if name != "path" && name != "PATH" { @@ -70,7 +121,6 @@ impl EnvironmentSyncer { } } } - } pub fn sync_path_vars(&mut self, ctx: &mut Context) { From da2751da5440f5b8acfc18dab3ee20b1e6c8e133 Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Fri, 5 Jun 2020 04:58:50 +0200 Subject: [PATCH 13/57] I am in a state of distress --- crates/nu-cli/src/env/environment_syncer.rs | 53 ++++++++++++++------- 1 file changed, 35 insertions(+), 18 deletions(-) diff --git a/crates/nu-cli/src/env/environment_syncer.rs b/crates/nu-cli/src/env/environment_syncer.rs index 31f72c2c3b..e284085c04 100644 --- a/crates/nu-cli/src/env/environment_syncer.rs +++ b/crates/nu-cli/src/env/environment_syncer.rs @@ -1,13 +1,14 @@ use crate::context::Context; use crate::data::config::{Conf, NuConfig}; use crate::env::environment::{Env, Environment}; -use nu_protocol::{UntaggedValue, Value, Primitive}; +use nu_protocol::{Primitive, UntaggedValue, Value}; use parking_lot::Mutex; use std::io::Read; use std::io::Write; use std::{ fs::{File, OpenOptions}, - sync::Arc, path::PathBuf, + path::{Path, PathBuf}, + sync::Arc, }; pub struct EnvironmentSyncer { @@ -47,19 +48,20 @@ impl EnvironmentSyncer { environment.morph(&*self.config); } - //TODO: Add authentication by saving the path to the .nurc file in some variable? //For directory wd in whitelisted directories //if current directory is wd or subdir to wd, add env vars from .nurc in wd + //vars: envtest, moretest, anothertest pub fn add_nurc(&self) -> std::io::Result<()> { + let mut environment = self.env.lock(); let mut file = OpenOptions::new() .write(true) .append(true) .create(true) - .open("vars.txt")?; + .open("dirs.txt")?; let conf = Arc::clone(&self.config); - let mut directories = vec![]; + let mut directories = vec![]; //TODO: sort the directories to achieve desired functionality about overwrites if let Some(Value { value: UntaggedValue::Table(ref directories_as_values), tag: _, @@ -69,32 +71,47 @@ impl EnvironmentSyncer { if let Value { value: UntaggedValue::Primitive(Primitive::String(ref dir)), tag: _, - } = dirval { - let path = PathBuf::from(dir); - directories.push(path); + } = dirval + { + directories.push(PathBuf::from(&dir)); } } }; + directories.sort(); - write!(&mut file, "variables so far: {:?}\n", directories).unwrap(); + write!(&mut file, "sorted dirs: {:?}\n", directories).unwrap(); - // let mut file = File::open(".nurc")?; - // let mut contents = String::new(); - // file.read_to_string(&mut contents)?; + let current_dir = std::env::current_dir()?; - // let toml_doc = contents.parse::().unwrap(); - // let nurc_vars = toml_doc.get("env").unwrap().as_table().unwrap(); + for mut dir in directories { + let mut working_dir = Some(current_dir.as_path()); - // nurc_vars.iter().for_each(|(k, v)| { - // env.insert(k.clone(), v.as_str().unwrap().to_string()); - // }); + while working_dir.is_some() { + if working_dir.unwrap() == dir.as_path() { + dir.push(".nurc"); + let mut file = File::open(dir.as_path())?; + let mut contents = String::new(); + file.read_to_string(&mut contents)?; + let toml_doc = contents.parse::().unwrap(); + let nurc_vars = toml_doc.get("env").unwrap().as_table().unwrap(); + + nurc_vars.iter().for_each(|(k, v)| { + environment.add_env(&k, &v.as_str().unwrap().to_string()); + }); + break; + } else { + working_dir = working_dir.unwrap().parent(); + } + } + } + environment.add_env("envtest", "I overwrote successfully"); Ok(()) } pub fn sync_env_vars(&mut self, ctx: &mut Context) { - let mut environment = self.env.lock(); self.add_nurc(); + 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" { From 0ee54a341842ae70cc3c631b51117a973bfec65c Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Fri, 5 Jun 2020 05:31:52 +0200 Subject: [PATCH 14/57] Rename .nurc to .nu --- crates/nu-cli/src/data/config/nuconfig.rs | 2 +- crates/nu-cli/src/env/environment.rs | 28 +++++++++++++++++++++ crates/nu-cli/src/env/environment_syncer.rs | 5 ++-- 3 files changed, 31 insertions(+), 4 deletions(-) diff --git a/crates/nu-cli/src/data/config/nuconfig.rs b/crates/nu-cli/src/data/config/nuconfig.rs index 97e486fb70..9630e18355 100644 --- a/crates/nu-cli/src/data/config/nuconfig.rs +++ b/crates/nu-cli/src/data/config/nuconfig.rs @@ -58,7 +58,7 @@ impl NuConfig { pub fn direnv_whitelist(&self) -> Option { let vars = self.vars.lock(); - if let Some(dirs) = vars.get("nurc_dirs") { + if let Some(dirs) = vars.get("nu_env_dirs") { return Some(dirs.clone()); } None diff --git a/crates/nu-cli/src/env/environment.rs b/crates/nu-cli/src/env/environment.rs index 9b586eded2..91c47db9e1 100644 --- a/crates/nu-cli/src/env/environment.rs +++ b/crates/nu-cli/src/env/environment.rs @@ -54,6 +54,32 @@ impl Environment { } } + + pub fn add_env_force(&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); + } + pub fn morph(&mut self, configuration: &T) { self.environment_vars = configuration.env(); self.path_vars = configuration.path(); @@ -77,6 +103,8 @@ impl Env for Environment { None } + + fn add_env(&mut self, key: &str, value: &str) { let value = UntaggedValue::string(value); diff --git a/crates/nu-cli/src/env/environment_syncer.rs b/crates/nu-cli/src/env/environment_syncer.rs index e284085c04..a8654a5e59 100644 --- a/crates/nu-cli/src/env/environment_syncer.rs +++ b/crates/nu-cli/src/env/environment_syncer.rs @@ -88,7 +88,7 @@ impl EnvironmentSyncer { while working_dir.is_some() { if working_dir.unwrap() == dir.as_path() { - dir.push(".nurc"); + dir.push(".nu"); let mut file = File::open(dir.as_path())?; let mut contents = String::new(); file.read_to_string(&mut contents)?; @@ -97,7 +97,7 @@ impl EnvironmentSyncer { let nurc_vars = toml_doc.get("env").unwrap().as_table().unwrap(); nurc_vars.iter().for_each(|(k, v)| { - environment.add_env(&k, &v.as_str().unwrap().to_string()); + environment.add_env_force(&k, &v.as_str().unwrap().to_string()); }); break; } else { @@ -105,7 +105,6 @@ impl EnvironmentSyncer { } } } - environment.add_env("envtest", "I overwrote successfully"); Ok(()) } From 66977bebd2e95257a8171bf1ec6cac9710757b4a Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Fri, 5 Jun 2020 05:37:11 +0200 Subject: [PATCH 15/57] Some notes for me --- post.org | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 post.org diff --git a/post.org b/post.org new file mode 100644 index 0000000000..f2eb879153 --- /dev/null +++ b/post.org @@ -0,0 +1,35 @@ +# WIP: Per directory env-variables +For #86 + +Environment variables are added if you have created a file called .nu inside a whitelisted directory, formatted as shown below. (I am, of course, open to change everything about this) +``` +#inside a .nu-file in a whitelisted directory +[env] +var = "value" +anothervar = "anothervalue" +``` + +In order for a .nu-file to be read, the directory it is in must be listed in the `nu_env_dirs` variable in nushell's `config.toml`. +``` +nu_env_dirs = ["/home/sam", "/home/sam/github", "/home/sam/github/test"] +``` + +# The way it works now is that whenever you run the function `maintain_nurc_environment_vars`, the current directory is checked for a `.nu` file and if it exists the variables in it are added. This works. +# Of course, when you leave a directory the variables should be unset. I track this by having a map between directory and a vector of environment variables. If the user is not in the directory or one of its subdirectories, its environment variables are removed... And then for some reason they are re-added again? + +Behavior: +- If you are in a subdirectory to a directory with a .nu-file, the vars in that .nu-file are applied. +- If you leave a directory which set some variables, the variables are unset. +- If a directory contains a .nu with an environment variable already set, the old value will be overwritten with the value from the .nu. This holds even if the old value was set by a .nu in a parent directory. + - The overwritten value is restored when you leave the directory. + - TODO: What happens if you overwrite twice? + +Questions: +- ´add_env´ does not overwrite variables. Need ´add_env_force´? +- `ctx.get_env()` in `cli.rs` lacks access to the config, which is required. Is it ok to do it through the sync call instead? + +TODO: take care of situation where a directory overwrites an existing .nu conf. + +---- + +# \ No newline at end of file From 8498c673bd74e6903aacb801fb3d9b5f270d9448 Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Sat, 6 Jun 2020 01:47:09 +0200 Subject: [PATCH 16/57] Refactoring --- crates/nu-cli/src/env/environment.rs | 119 ++++++++++++++------ crates/nu-cli/src/env/environment_syncer.rs | 68 ++--------- post.org | 1 - 3 files changed, 93 insertions(+), 95 deletions(-) diff --git a/crates/nu-cli/src/env/environment.rs b/crates/nu-cli/src/env/environment.rs index 91c47db9e1..a7f63ca622 100644 --- a/crates/nu-cli/src/env/environment.rs +++ b/crates/nu-cli/src/env/environment.rs @@ -1,14 +1,14 @@ use crate::data::config::Conf; use indexmap::{indexmap, IndexSet}; -use nu_protocol::{UntaggedValue, Value}; +use nu_protocol::{Primitive, UntaggedValue, Value}; use std::ffi::OsString; -use std::fmt::Debug; +use std::{collections::HashMap, fmt::Debug, fs::File, io::Read, path::PathBuf}; pub trait Env: Debug + Send { fn env(&self) -> Option; fn path(&self) -> Option; - fn add_env(&mut self, key: &str, value: &str); + fn add_env(&mut self, key: &str, value: &str, overwrite_existing: bool); fn add_path(&mut self, new_path: OsString); } @@ -21,8 +21,8 @@ impl Env for Box { (**self).path() } - fn add_env(&mut self, key: &str, value: &str) { - (**self).add_env(key, value); + fn add_env(&mut self, key: &str, value: &str, overwrite_existing: bool) { + (**self).add_env(key, value, overwrite_existing); } fn add_path(&mut self, new_path: OsString) { @@ -30,10 +30,62 @@ impl Env for Box { } } +#[derive(Debug, Default)] +struct DirectorySpecificEnvironment { + pub whitelisted_directories: Vec, + pub added_env_vars: HashMap>, //Directory -> Env key. If an environment var has been added from a .nu in a directory, we track it here so we can remove it when the user leaves the directory. + pub overwritten_env_values: HashMap>, //Directory -> (env_key, value). If a .nu overwrites some existing environment variables, they are added here so that they can be restored later. +} + +impl DirectorySpecificEnvironment { + pub fn new(whitelisted_directories: Vec) -> DirectorySpecificEnvironment { + DirectorySpecificEnvironment { + whitelisted_directories, + added_env_vars: HashMap::new(), + overwritten_env_values: HashMap::new(), + } + } + + pub fn env_vars_to_add(&mut self) -> std::io::Result> { + let current_dir = std::env::current_dir()?; + + let mut vars_to_add = HashMap::new(); + for dir in &self.whitelisted_directories { + + //Start in the current directory, then traverse towards the root directory with working_dir to check for .nu files + let mut working_dir = Some(current_dir.as_path()); + + while let Some(wdir) = working_dir { + if wdir == dir.as_path() { + let mut dir = dir.clone(); + dir.push(".nu"); + + //Read the .nu file and parse it into a nice map + let mut file = File::open(dir.as_path())?; + let mut contents = String::new(); + file.read_to_string(&mut contents)?; + let toml_doc = contents.parse::().unwrap(); + let vars_in_current_file = toml_doc.get("env").unwrap().as_table().unwrap(); + + for (k, v) in vars_in_current_file { + vars_to_add.insert(k.clone(), v.as_str().unwrap().to_string()); + } + break; + } else { + working_dir = working_dir.unwrap().parent(); + } + } + } + + Ok(vars_to_add) + } +} + #[derive(Debug, Default)] pub struct Environment { environment_vars: Option, path_vars: Option, + direnv: DirectorySpecificEnvironment, } impl Environment { @@ -41,43 +93,46 @@ impl Environment { Environment { environment_vars: None, path_vars: None, + direnv: DirectorySpecificEnvironment::new(vec![]), } } + pub fn from_config(configuration: &T) -> Environment { let env = configuration.env(); let path = configuration.path(); + let mut directories = vec![]; + if let Some(Value { + value: UntaggedValue::Table(ref directories_as_values), + tag: _, + }) = configuration.direnv_whitelist() + { + for dirval in directories_as_values { + if let Value { + value: UntaggedValue::Primitive(Primitive::String(ref dir)), + tag: _, + } = dirval + { + directories.push(PathBuf::from(&dir)); + } + } + }; + directories.sort(); + Environment { environment_vars: env, path_vars: path, + direnv: DirectorySpecificEnvironment::new(directories), } } - - pub fn add_env_force(&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); + pub fn maintain_directory_environment(&mut self) -> std::io::Result<()> { + let vars_to_add = self.direnv.env_vars_to_add()?; + vars_to_add.iter().for_each(|(k, v)| { + self.add_env(&k, &v, true); + }); + Ok(()) } pub fn morph(&mut self, configuration: &T) { @@ -103,9 +158,7 @@ impl Env for Environment { None } - - - fn add_env(&mut self, key: &str, value: &str) { + fn add_env(&mut self, key: &str, value: &str, overwrite_existing: bool) { let value = UntaggedValue::string(value); let new_envs = { @@ -116,7 +169,7 @@ impl Env for Environment { { let mut new_envs = envs.clone(); - if !new_envs.contains_key(key) { + if !new_envs.contains_key(key) || overwrite_existing { new_envs.insert_data_at_key(key, value.into_value(tag.clone())); } diff --git a/crates/nu-cli/src/env/environment_syncer.rs b/crates/nu-cli/src/env/environment_syncer.rs index a8654a5e59..c29eeaba85 100644 --- a/crates/nu-cli/src/env/environment_syncer.rs +++ b/crates/nu-cli/src/env/environment_syncer.rs @@ -48,75 +48,21 @@ impl EnvironmentSyncer { environment.morph(&*self.config); } - //For directory wd in whitelisted directories - //if current directory is wd or subdir to wd, add env vars from .nurc in wd - //vars: envtest, moretest, anothertest - pub fn add_nurc(&self) -> std::io::Result<()> { - let mut environment = self.env.lock(); - let mut file = OpenOptions::new() - .write(true) - .append(true) - .create(true) - .open("dirs.txt")?; - - let conf = Arc::clone(&self.config); - - let mut directories = vec![]; //TODO: sort the directories to achieve desired functionality about overwrites - if let Some(Value { - value: UntaggedValue::Table(ref directories_as_values), - tag: _, - }) = conf.direnv_whitelist() - { - for dirval in directories_as_values { - if let Value { - value: UntaggedValue::Primitive(Primitive::String(ref dir)), - tag: _, - } = dirval - { - directories.push(PathBuf::from(&dir)); - } - } - }; - directories.sort(); - - write!(&mut file, "sorted dirs: {:?}\n", directories).unwrap(); - - let current_dir = std::env::current_dir()?; - - for mut dir in directories { - let mut working_dir = Some(current_dir.as_path()); - - while working_dir.is_some() { - if working_dir.unwrap() == dir.as_path() { - dir.push(".nu"); - let mut file = File::open(dir.as_path())?; - let mut contents = String::new(); - file.read_to_string(&mut contents)?; - - let toml_doc = contents.parse::().unwrap(); - let nurc_vars = toml_doc.get("env").unwrap().as_table().unwrap(); - - nurc_vars.iter().for_each(|(k, v)| { - environment.add_env_force(&k, &v.as_str().unwrap().to_string()); - }); - break; - } else { - working_dir = working_dir.unwrap().parent(); - } - } - } - Ok(()) - } pub fn sync_env_vars(&mut self, ctx: &mut Context) { - self.add_nurc(); let mut environment = self.env.lock(); + + match environment.maintain_directory_environment() { + Ok(_) => {} + Err(e) => {panic!(e)} + } + 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); + environment.add_env(&name, &value, false); // clear the env var from the session // we are about to replace them diff --git a/post.org b/post.org index f2eb879153..2c304e37b1 100644 --- a/post.org +++ b/post.org @@ -3,7 +3,6 @@ For #86 Environment variables are added if you have created a file called .nu inside a whitelisted directory, formatted as shown below. (I am, of course, open to change everything about this) ``` -#inside a .nu-file in a whitelisted directory [env] var = "value" anothervar = "anothervalue" From 3a278b38dac17886ff6b27196a33dd6f0ec879b8 Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Sat, 6 Jun 2020 02:54:27 +0200 Subject: [PATCH 17/57] Removing vars works, but not done in a very nice fashion --- crates/nu-cli/src/cli.rs | 3 +- crates/nu-cli/src/env/environment.rs | 62 +++++++++++++++++++-- crates/nu-cli/src/env/environment_syncer.rs | 9 +-- 3 files changed, 58 insertions(+), 16 deletions(-) diff --git a/crates/nu-cli/src/cli.rs b/crates/nu-cli/src/cli.rs index c0d8a2901d..cd14373b71 100644 --- a/crates/nu-cli/src/cli.rs +++ b/crates/nu-cli/src/cli.rs @@ -22,8 +22,7 @@ use rustyline::{ KeyPress, Movement, Word, }; use std::error::Error; -use std::fs::{File, OpenOptions}; -use std::io::{BufRead, BufReader, Read, Write}; +use std::io::{BufRead, BufReader, Write}; use std::iter::Iterator; use std::path::{Path, PathBuf}; use std::sync::atomic::Ordering; diff --git a/crates/nu-cli/src/env/environment.rs b/crates/nu-cli/src/env/environment.rs index a7f63ca622..3e8ab969d3 100644 --- a/crates/nu-cli/src/env/environment.rs +++ b/crates/nu-cli/src/env/environment.rs @@ -2,7 +2,14 @@ use crate::data::config::Conf; use indexmap::{indexmap, IndexSet}; use nu_protocol::{Primitive, UntaggedValue, Value}; use std::ffi::OsString; -use std::{collections::HashMap, fmt::Debug, fs::File, io::Read, path::PathBuf}; +use std::io::Write; +use std::{ + collections::HashMap, + fmt::Debug, + fs::{File, OpenOptions}, + io::Read, + path::PathBuf, +}; pub trait Env: Debug + Send { fn env(&self) -> Option; @@ -51,7 +58,6 @@ impl DirectorySpecificEnvironment { let mut vars_to_add = HashMap::new(); for dir in &self.whitelisted_directories { - //Start in the current directory, then traverse towards the root directory with working_dir to check for .nu files let mut working_dir = Some(current_dir.as_path()); @@ -67,9 +73,33 @@ impl DirectorySpecificEnvironment { let toml_doc = contents.parse::().unwrap(); let vars_in_current_file = toml_doc.get("env").unwrap().as_table().unwrap(); + let mut keys_in_file = vec![]; for (k, v) in vars_in_current_file { vars_to_add.insert(k.clone(), v.as_str().unwrap().to_string()); + keys_in_file.push(k.clone()); } + self.added_env_vars.insert(wdir.to_path_buf(), keys_in_file); + break; + } else { + working_dir = working_dir.unwrap().parent(); + } + } + } + Ok(vars_to_add) + } + + //If the user has left directories which added env vars through .nurc, we clear those vars + //For each directory d in nurc_env_vars: + //if current_dir does not have d as a parent (possibly recursive), the vars set by d should be removed + pub fn env_vars_to_delete(&mut self) -> std::io::Result> { + let current_dir = std::env::current_dir()?; + + let mut new_nurc_env_vars = HashMap::new(); + for (d, v) in self.added_env_vars.iter() { + let mut working_dir = Some(current_dir.as_path()); + while working_dir.is_some() { + if working_dir.unwrap() == d { + new_nurc_env_vars.insert(d.clone(), v.clone()); break; } else { working_dir = working_dir.unwrap().parent(); @@ -77,7 +107,14 @@ impl DirectorySpecificEnvironment { } } - Ok(vars_to_add) + let mut vars_to_delete = vec![]; + for (path, vals) in self.added_env_vars.iter() { + if !new_nurc_env_vars.contains_key(path) { + vars_to_delete.extend(vals.clone()); + } + } + + Ok(vars_to_delete) } } @@ -97,7 +134,6 @@ impl Environment { } } - pub fn from_config(configuration: &T) -> Environment { let env = configuration.env(); let path = configuration.path(); @@ -128,13 +164,27 @@ impl Environment { } pub fn maintain_directory_environment(&mut self) -> std::io::Result<()> { - let vars_to_add = self.direnv.env_vars_to_add()?; - vars_to_add.iter().for_each(|(k, v)| { + self.direnv.env_vars_to_add()?.iter().for_each(|(k, v)| { self.add_env(&k, &v, true); }); + + self.direnv.env_vars_to_delete()?.iter().for_each(|v| { + self.remove_env(v); + }); Ok(()) } + fn remove_env(&mut self, key: &str) { + if let Some(Value { + value: UntaggedValue::Row(ref mut envs), + tag: _, + }) = self.environment_vars + { + envs.entries.remove(key); + std::env::remove_var(key); + }; + } + pub fn morph(&mut self, configuration: &T) { self.environment_vars = configuration.env(); self.path_vars = configuration.path(); diff --git a/crates/nu-cli/src/env/environment_syncer.rs b/crates/nu-cli/src/env/environment_syncer.rs index c29eeaba85..e9c99f3be3 100644 --- a/crates/nu-cli/src/env/environment_syncer.rs +++ b/crates/nu-cli/src/env/environment_syncer.rs @@ -1,15 +1,8 @@ use crate::context::Context; use crate::data::config::{Conf, NuConfig}; use crate::env::environment::{Env, Environment}; -use nu_protocol::{Primitive, UntaggedValue, Value}; use parking_lot::Mutex; -use std::io::Read; -use std::io::Write; -use std::{ - fs::{File, OpenOptions}, - path::{Path, PathBuf}, - sync::Arc, -}; +use std::sync::Arc; pub struct EnvironmentSyncer { pub env: Arc>>, From fb6eb1924f8c82db667c260a023801220976befe Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Sat, 6 Jun 2020 03:31:30 +0200 Subject: [PATCH 18/57] Refactor env_vars_to_delete --- crates/nu-cli/src/env/environment.rs | 46 ++++++++++++++-------------- post.org | 13 +++++--- 2 files changed, 31 insertions(+), 28 deletions(-) diff --git a/crates/nu-cli/src/env/environment.rs b/crates/nu-cli/src/env/environment.rs index 3e8ab969d3..beb9141094 100644 --- a/crates/nu-cli/src/env/environment.rs +++ b/crates/nu-cli/src/env/environment.rs @@ -40,8 +40,12 @@ impl Env for Box { #[derive(Debug, Default)] struct DirectorySpecificEnvironment { pub whitelisted_directories: Vec, - pub added_env_vars: HashMap>, //Directory -> Env key. If an environment var has been added from a .nu in a directory, we track it here so we can remove it when the user leaves the directory. - pub overwritten_env_values: HashMap>, //Directory -> (env_key, value). If a .nu overwrites some existing environment variables, they are added here so that they can be restored later. + + //Directory -> Env key. If an environment var has been added from a .nu in a directory, we track it here so we can remove it when the user leaves the directory. + pub added_env_vars: HashMap>, + + //Directory -> (env_key, value). If a .nu overwrites some existing environment variables, they are added here so that they can be restored later. + pub overwritten_env_values: HashMap>, } impl DirectorySpecificEnvironment { @@ -88,31 +92,27 @@ impl DirectorySpecificEnvironment { Ok(vars_to_add) } - //If the user has left directories which added env vars through .nurc, we clear those vars - //For each directory d in nurc_env_vars: - //if current_dir does not have d as a parent (possibly recursive), the vars set by d should be removed + //If the user has left directories which added env vars through .nu, we clear those vars pub fn env_vars_to_delete(&mut self) -> std::io::Result> { let current_dir = std::env::current_dir()?; - let mut new_nurc_env_vars = HashMap::new(); - for (d, v) in self.added_env_vars.iter() { - let mut working_dir = Some(current_dir.as_path()); - while working_dir.is_some() { - if working_dir.unwrap() == d { - new_nurc_env_vars.insert(d.clone(), v.clone()); - break; - } else { - working_dir = working_dir.unwrap().parent(); - } - } - } + let vars_to_delete = self.added_env_vars.iter().fold( + Vec::new(), + |mut vars_to_delete, (directory, env_vars)| { + let mut working_dir = Some(current_dir.as_path()); - let mut vars_to_delete = vec![]; - for (path, vals) in self.added_env_vars.iter() { - if !new_nurc_env_vars.contains_key(path) { - vars_to_delete.extend(vals.clone()); - } - } + while let Some(wdir) = working_dir { + if &wdir == directory { + return vars_to_delete; + } else { + working_dir = working_dir.unwrap().parent(); + } + } + //only delete vars from directories we are not in + vars_to_delete.extend(env_vars.clone()); + vars_to_delete + }, + ); Ok(vars_to_delete) } diff --git a/post.org b/post.org index 2c304e37b1..d3149410ba 100644 --- a/post.org +++ b/post.org @@ -12,9 +12,11 @@ In order for a .nu-file to be read, the directory it is in must be listed in the ``` nu_env_dirs = ["/home/sam", "/home/sam/github", "/home/sam/github/test"] ``` - -# The way it works now is that whenever you run the function `maintain_nurc_environment_vars`, the current directory is checked for a `.nu` file and if it exists the variables in it are added. This works. -# Of course, when you leave a directory the variables should be unset. I track this by having a map between directory and a vector of environment variables. If the user is not in the directory or one of its subdirectories, its environment variables are removed... And then for some reason they are re-added again? +This was implemented for the sake of security. I do not believe that it is appropiate for nushell to pick up random .nu-files unless +the user has explicitly said that it's alright. An adversary could hide a .nu-file in an otherwise unsuspicous folder that you download, +and now you suddenly have nushell picking up whatever environment variables they set. +This meant that the code necessarily become more involved than just looking for a .nu-file in the current directory and applying its variables, +but this extra code also allowed for some other nice features and behavior which is listed below. Behavior: - If you are in a subdirectory to a directory with a .nu-file, the vars in that .nu-file are applied. @@ -24,8 +26,9 @@ Behavior: - TODO: What happens if you overwrite twice? Questions: -- ´add_env´ does not overwrite variables. Need ´add_env_force´? -- `ctx.get_env()` in `cli.rs` lacks access to the config, which is required. Is it ok to do it through the sync call instead? +`remove_env()` accesses the environment directly to remove variables. +Just using what I think was expected, envs.entries.remove(key), does not work. +If this is a problem, how can I make it work better with existing code? TODO: take care of situation where a directory overwrites an existing .nu conf. From 14e12f57b04501e8ecd9ba864effbedca36e403f Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Sat, 6 Jun 2020 05:42:34 +0200 Subject: [PATCH 19/57] Refactor env_vars_to_add() --- crates/nu-cli/src/env/environment.rs | 29 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/crates/nu-cli/src/env/environment.rs b/crates/nu-cli/src/env/environment.rs index beb9141094..aaf451513a 100644 --- a/crates/nu-cli/src/env/environment.rs +++ b/crates/nu-cli/src/env/environment.rs @@ -2,12 +2,9 @@ use crate::data::config::Conf; use indexmap::{indexmap, IndexSet}; use nu_protocol::{Primitive, UntaggedValue, Value}; use std::ffi::OsString; -use std::io::Write; use std::{ collections::HashMap, fmt::Debug, - fs::{File, OpenOptions}, - io::Read, path::PathBuf, }; @@ -62,26 +59,26 @@ impl DirectorySpecificEnvironment { let mut vars_to_add = HashMap::new(); for dir in &self.whitelisted_directories { - //Start in the current directory, then traverse towards the root directory with working_dir to check for .nu files + //Start in the current directory, then traverse towards the root with working_dir to see if we are in a subdirectory of a valid directory. let mut working_dir = Some(current_dir.as_path()); while let Some(wdir) = working_dir { if wdir == dir.as_path() { - let mut dir = dir.clone(); - dir.push(".nu"); //Read the .nu file and parse it into a nice map - let mut file = File::open(dir.as_path())?; - let mut contents = String::new(); - file.read_to_string(&mut contents)?; - let toml_doc = contents.parse::().unwrap(); + let toml_doc = std::fs::read_to_string(wdir.join(".nu").as_path())? + .parse::() + .unwrap(); let vars_in_current_file = toml_doc.get("env").unwrap().as_table().unwrap(); - let mut keys_in_file = vec![]; - for (k, v) in vars_in_current_file { - vars_to_add.insert(k.clone(), v.as_str().unwrap().to_string()); - keys_in_file.push(k.clone()); - } + let keys_in_file = vars_in_current_file + .iter() + .map(|(k, v)| { + vars_to_add.insert(k.clone(), v.as_str().unwrap().to_string()); //This is to add the keys and values to the environment + k.clone() //We add them all to a list to keep track of which keys we have added + }) + .collect(); + self.added_env_vars.insert(wdir.to_path_buf(), keys_in_file); break; } else { @@ -96,6 +93,8 @@ impl DirectorySpecificEnvironment { pub fn env_vars_to_delete(&mut self) -> std::io::Result> { let current_dir = std::env::current_dir()?; + //Gather up all environment variables that should be deleted. + //If we are not in a directory or one of its subdirectories, mark the env_vals it maps to for removal. let vars_to_delete = self.added_env_vars.iter().fold( Vec::new(), |mut vars_to_delete, (directory, env_vars)| { From 6ce5a87c303fb0d20102d165052edf7981cc823c Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Sat, 6 Jun 2020 06:02:15 +0200 Subject: [PATCH 20/57] Move directory environment code to separate file --- crates/nu-cli/src/env.rs | 1 + .../src/env/directory_specific_environment.rs | 90 +++++++++++++++++++ crates/nu-cli/src/env/environment.rs | 90 +------------------ 3 files changed, 93 insertions(+), 88 deletions(-) create mode 100644 crates/nu-cli/src/env/directory_specific_environment.rs diff --git a/crates/nu-cli/src/env.rs b/crates/nu-cli/src/env.rs index 3f1dd1c311..74ae277ab5 100644 --- a/crates/nu-cli/src/env.rs +++ b/crates/nu-cli/src/env.rs @@ -1,3 +1,4 @@ +pub(crate) mod directory_specific_environment; pub(crate) mod environment; pub(crate) mod environment_syncer; pub(crate) mod host; diff --git a/crates/nu-cli/src/env/directory_specific_environment.rs b/crates/nu-cli/src/env/directory_specific_environment.rs new file mode 100644 index 0000000000..0705bb064e --- /dev/null +++ b/crates/nu-cli/src/env/directory_specific_environment.rs @@ -0,0 +1,90 @@ +use indexmap::indexmap; +use std::{ + collections::HashMap, + fmt::Debug, + path::PathBuf, +}; + + +#[derive(Debug, Default)] +pub struct DirectorySpecificEnvironment { + pub whitelisted_directories: Vec, + + //Directory -> Env key. If an environment var has been added from a .nu in a directory, we track it here so we can remove it when the user leaves the directory. + pub added_env_vars: HashMap>, + + //Directory -> (env_key, value). If a .nu overwrites some existing environment variables, they are added here so that they can be restored later. + pub overwritten_env_values: HashMap>, +} + +impl DirectorySpecificEnvironment { + pub fn new(whitelisted_directories: Vec) -> DirectorySpecificEnvironment { + DirectorySpecificEnvironment { + whitelisted_directories, + added_env_vars: HashMap::new(), + overwritten_env_values: HashMap::new(), + } + } + + pub fn env_vars_to_add(&mut self) -> std::io::Result> { + let current_dir = std::env::current_dir()?; + + let mut vars_to_add = HashMap::new(); + for dir in &self.whitelisted_directories { + //Start in the current directory, then traverse towards the root with working_dir to see if we are in a subdirectory of a valid directory. + let mut working_dir = Some(current_dir.as_path()); + + while let Some(wdir) = working_dir { + if wdir == dir.as_path() { + + //Read the .nu file and parse it into a nice map + let toml_doc = std::fs::read_to_string(wdir.join(".nu").as_path())? + .parse::() + .unwrap(); + let vars_in_current_file = toml_doc.get("env").unwrap().as_table().unwrap(); + + let keys_in_file = vars_in_current_file + .iter() + .map(|(k, v)| { + vars_to_add.insert(k.clone(), v.as_str().unwrap().to_string()); //This is to add the keys and values to the environment + k.clone() //We add them all to a list to keep track of which keys we have added + }) + .collect(); + + self.added_env_vars.insert(wdir.to_path_buf(), keys_in_file); + break; + } else { + working_dir = working_dir.unwrap().parent(); + } + } + } + Ok(vars_to_add) + } + + //If the user has left directories which added env vars through .nu, we clear those vars + pub fn env_vars_to_delete(&mut self) -> std::io::Result> { + let current_dir = std::env::current_dir()?; + + //Gather up all environment variables that should be deleted. + //If we are not in a directory or one of its subdirectories, mark the env_vals it maps to for removal. + let vars_to_delete = self.added_env_vars.iter().fold( + Vec::new(), + |mut vars_to_delete, (directory, env_vars)| { + let mut working_dir = Some(current_dir.as_path()); + + while let Some(wdir) = working_dir { + if &wdir == directory { + return vars_to_delete; + } else { + working_dir = working_dir.unwrap().parent(); + } + } + //only delete vars from directories we are not in + vars_to_delete.extend(env_vars.clone()); + vars_to_delete + }, + ); + + Ok(vars_to_delete) + } +} \ No newline at end of file diff --git a/crates/nu-cli/src/env/environment.rs b/crates/nu-cli/src/env/environment.rs index aaf451513a..8106f0e518 100644 --- a/crates/nu-cli/src/env/environment.rs +++ b/crates/nu-cli/src/env/environment.rs @@ -1,12 +1,9 @@ use crate::data::config::Conf; +use crate::env::directory_specific_environment::*; use indexmap::{indexmap, IndexSet}; use nu_protocol::{Primitive, UntaggedValue, Value}; use std::ffi::OsString; -use std::{ - collections::HashMap, - fmt::Debug, - path::PathBuf, -}; +use std::{fmt::Debug, path::PathBuf}; pub trait Env: Debug + Send { fn env(&self) -> Option; @@ -34,89 +31,6 @@ impl Env for Box { } } -#[derive(Debug, Default)] -struct DirectorySpecificEnvironment { - pub whitelisted_directories: Vec, - - //Directory -> Env key. If an environment var has been added from a .nu in a directory, we track it here so we can remove it when the user leaves the directory. - pub added_env_vars: HashMap>, - - //Directory -> (env_key, value). If a .nu overwrites some existing environment variables, they are added here so that they can be restored later. - pub overwritten_env_values: HashMap>, -} - -impl DirectorySpecificEnvironment { - pub fn new(whitelisted_directories: Vec) -> DirectorySpecificEnvironment { - DirectorySpecificEnvironment { - whitelisted_directories, - added_env_vars: HashMap::new(), - overwritten_env_values: HashMap::new(), - } - } - - pub fn env_vars_to_add(&mut self) -> std::io::Result> { - let current_dir = std::env::current_dir()?; - - let mut vars_to_add = HashMap::new(); - for dir in &self.whitelisted_directories { - //Start in the current directory, then traverse towards the root with working_dir to see if we are in a subdirectory of a valid directory. - let mut working_dir = Some(current_dir.as_path()); - - while let Some(wdir) = working_dir { - if wdir == dir.as_path() { - - //Read the .nu file and parse it into a nice map - let toml_doc = std::fs::read_to_string(wdir.join(".nu").as_path())? - .parse::() - .unwrap(); - let vars_in_current_file = toml_doc.get("env").unwrap().as_table().unwrap(); - - let keys_in_file = vars_in_current_file - .iter() - .map(|(k, v)| { - vars_to_add.insert(k.clone(), v.as_str().unwrap().to_string()); //This is to add the keys and values to the environment - k.clone() //We add them all to a list to keep track of which keys we have added - }) - .collect(); - - self.added_env_vars.insert(wdir.to_path_buf(), keys_in_file); - break; - } else { - working_dir = working_dir.unwrap().parent(); - } - } - } - Ok(vars_to_add) - } - - //If the user has left directories which added env vars through .nu, we clear those vars - pub fn env_vars_to_delete(&mut self) -> std::io::Result> { - let current_dir = std::env::current_dir()?; - - //Gather up all environment variables that should be deleted. - //If we are not in a directory or one of its subdirectories, mark the env_vals it maps to for removal. - let vars_to_delete = self.added_env_vars.iter().fold( - Vec::new(), - |mut vars_to_delete, (directory, env_vars)| { - let mut working_dir = Some(current_dir.as_path()); - - while let Some(wdir) = working_dir { - if &wdir == directory { - return vars_to_delete; - } else { - working_dir = working_dir.unwrap().parent(); - } - } - //only delete vars from directories we are not in - vars_to_delete.extend(env_vars.clone()); - vars_to_delete - }, - ); - - Ok(vars_to_delete) - } -} - #[derive(Debug, Default)] pub struct Environment { environment_vars: Option, From a0cedfce8d5829f47b92b192a7f1af990bb55372 Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Sat, 6 Jun 2020 06:13:30 +0200 Subject: [PATCH 21/57] Refactor from_config --- .../src/env/directory_specific_environment.rs | 49 +++++++++++++------ crates/nu-cli/src/env/environment.rs | 26 ++-------- 2 files changed, 37 insertions(+), 38 deletions(-) diff --git a/crates/nu-cli/src/env/directory_specific_environment.rs b/crates/nu-cli/src/env/directory_specific_environment.rs index 0705bb064e..74ea07da17 100644 --- a/crates/nu-cli/src/env/directory_specific_environment.rs +++ b/crates/nu-cli/src/env/directory_specific_environment.rs @@ -1,42 +1,59 @@ -use indexmap::indexmap; -use std::{ - collections::HashMap, - fmt::Debug, - path::PathBuf, -}; - +use indexmap::IndexMap; +use nu_protocol::{Primitive, UntaggedValue, Value}; +use std::{fmt::Debug, path::PathBuf}; #[derive(Debug, Default)] pub struct DirectorySpecificEnvironment { pub whitelisted_directories: Vec, //Directory -> Env key. If an environment var has been added from a .nu in a directory, we track it here so we can remove it when the user leaves the directory. - pub added_env_vars: HashMap>, + pub added_env_vars: IndexMap>, //Directory -> (env_key, value). If a .nu overwrites some existing environment variables, they are added here so that they can be restored later. - pub overwritten_env_values: HashMap>, + pub overwritten_env_values: IndexMap>, } impl DirectorySpecificEnvironment { - pub fn new(whitelisted_directories: Vec) -> DirectorySpecificEnvironment { + pub fn new(whitelisted_directories: Option) -> DirectorySpecificEnvironment { + let mut whitelisted_directories = if let Some(Value { + value: UntaggedValue::Table(ref wrapped_directories), + tag: _, + }) = whitelisted_directories + { + wrapped_directories + .iter() + .fold(vec![], |mut directories, dirval| { + if let Value { + value: UntaggedValue::Primitive(Primitive::String(ref dir)), + tag: _, + } = dirval + { + directories.push(PathBuf::from(&dir)); + } + directories + }) + } else { + vec![] + }; + whitelisted_directories.sort(); + DirectorySpecificEnvironment { whitelisted_directories, - added_env_vars: HashMap::new(), - overwritten_env_values: HashMap::new(), + added_env_vars: IndexMap::new(), + overwritten_env_values: IndexMap::new(), } } - pub fn env_vars_to_add(&mut self) -> std::io::Result> { + pub fn env_vars_to_add(&mut self) -> std::io::Result> { let current_dir = std::env::current_dir()?; - let mut vars_to_add = HashMap::new(); + let mut vars_to_add = IndexMap::new(); for dir in &self.whitelisted_directories { //Start in the current directory, then traverse towards the root with working_dir to see if we are in a subdirectory of a valid directory. let mut working_dir = Some(current_dir.as_path()); while let Some(wdir) = working_dir { if wdir == dir.as_path() { - //Read the .nu file and parse it into a nice map let toml_doc = std::fs::read_to_string(wdir.join(".nu").as_path())? .parse::() @@ -87,4 +104,4 @@ impl DirectorySpecificEnvironment { Ok(vars_to_delete) } -} \ No newline at end of file +} diff --git a/crates/nu-cli/src/env/environment.rs b/crates/nu-cli/src/env/environment.rs index 8106f0e518..0e987e06d1 100644 --- a/crates/nu-cli/src/env/environment.rs +++ b/crates/nu-cli/src/env/environment.rs @@ -1,9 +1,9 @@ use crate::data::config::Conf; use crate::env::directory_specific_environment::*; use indexmap::{indexmap, IndexSet}; -use nu_protocol::{Primitive, UntaggedValue, Value}; +use nu_protocol::{UntaggedValue, Value}; use std::ffi::OsString; -use std::{fmt::Debug, path::PathBuf}; +use std::fmt::Debug; pub trait Env: Debug + Send { fn env(&self) -> Option; @@ -43,7 +43,7 @@ impl Environment { Environment { environment_vars: None, path_vars: None, - direnv: DirectorySpecificEnvironment::new(vec![]), + direnv: DirectorySpecificEnvironment::new(None), } } @@ -51,28 +51,10 @@ impl Environment { let env = configuration.env(); let path = configuration.path(); - let mut directories = vec![]; - if let Some(Value { - value: UntaggedValue::Table(ref directories_as_values), - tag: _, - }) = configuration.direnv_whitelist() - { - for dirval in directories_as_values { - if let Value { - value: UntaggedValue::Primitive(Primitive::String(ref dir)), - tag: _, - } = dirval - { - directories.push(PathBuf::from(&dir)); - } - } - }; - directories.sort(); - Environment { environment_vars: env, path_vars: path, - direnv: DirectorySpecificEnvironment::new(directories), + direnv: DirectorySpecificEnvironment::new(configuration.direnv_whitelist()), } } From 6974eb099499c2c4bdc4040035837d3e0957530b Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Sat, 6 Jun 2020 08:01:50 +0200 Subject: [PATCH 22/57] Restore env values --- .../src/env/directory_specific_environment.rs | 58 ++++++++++++++++++- crates/nu-cli/src/env/environment.rs | 4 ++ 2 files changed, 59 insertions(+), 3 deletions(-) diff --git a/crates/nu-cli/src/env/directory_specific_environment.rs b/crates/nu-cli/src/env/directory_specific_environment.rs index 74ea07da17..bd8f4d88cd 100644 --- a/crates/nu-cli/src/env/directory_specific_environment.rs +++ b/crates/nu-cli/src/env/directory_specific_environment.rs @@ -1,6 +1,7 @@ use indexmap::IndexMap; use nu_protocol::{Primitive, UntaggedValue, Value}; -use std::{fmt::Debug, path::PathBuf}; +use std::io::Write; +use std::{ffi::OsString, fmt::Debug, path::PathBuf, fs::OpenOptions}; #[derive(Debug, Default)] pub struct DirectorySpecificEnvironment { @@ -10,7 +11,7 @@ pub struct DirectorySpecificEnvironment { pub added_env_vars: IndexMap>, //Directory -> (env_key, value). If a .nu overwrites some existing environment variables, they are added here so that they can be restored later. - pub overwritten_env_values: IndexMap>, + pub overwritten_env_values: IndexMap>, } impl DirectorySpecificEnvironment { @@ -44,6 +45,35 @@ impl DirectorySpecificEnvironment { } } + // pub overwritten_env_values: IndexMap>, + //overwritten_env_values maps a directory with a .nu file to some environment variables that it overwrote. + //if we are not in that directory, we re-add those variables. + pub fn overwritten_values_to_restore(&mut self) -> std::io::Result> { + let current_dir = std::env::current_dir()?; + + let mut keyvals_to_restore = IndexMap::new(); + for (directory, keyvals) in &self.overwritten_env_values { + let mut working_dir = Some(current_dir.as_path()); + + let mut readd = true; + while let Some(wdir) = working_dir { + if wdir == directory.as_path() { + readd = false; + break; + } else { + working_dir = working_dir.unwrap().parent(); + } + } + if readd { + for (k, v) in keyvals { + keyvals_to_restore.insert(k.clone(), v.to_str().unwrap().to_string()); + } + } + } + + Ok(keyvals_to_restore) + } + pub fn env_vars_to_add(&mut self) -> std::io::Result> { let current_dir = std::env::current_dir()?; @@ -60,7 +90,7 @@ impl DirectorySpecificEnvironment { .unwrap(); let vars_in_current_file = toml_doc.get("env").unwrap().as_table().unwrap(); - let keys_in_file = vars_in_current_file + let keys_in_file: Vec = vars_in_current_file .iter() .map(|(k, v)| { vars_to_add.insert(k.clone(), v.as_str().unwrap().to_string()); //This is to add the keys and values to the environment @@ -68,7 +98,29 @@ impl DirectorySpecificEnvironment { }) .collect(); + self.overwritten_env_values.insert( + wdir.to_path_buf(), + keys_in_file.iter().fold(vec![], |mut keyvals, key| { + if let Some(val) = std::env::var_os(key) { + + let mut file = OpenOptions::new() + .write(true) + .append(true) + .create(true) + .open("restore.txt").unwrap(); + + write!(&mut file, "about to overwrite: {:?}\n", val).unwrap(); + + keyvals.push((key.clone(), val)); + keyvals + } else { + keyvals + } + }), + ); + self.added_env_vars.insert(wdir.to_path_buf(), keys_in_file); + break; } else { working_dir = working_dir.unwrap().parent(); diff --git a/crates/nu-cli/src/env/environment.rs b/crates/nu-cli/src/env/environment.rs index 0e987e06d1..3d4df5e7fd 100644 --- a/crates/nu-cli/src/env/environment.rs +++ b/crates/nu-cli/src/env/environment.rs @@ -66,6 +66,10 @@ impl Environment { self.direnv.env_vars_to_delete()?.iter().for_each(|v| { self.remove_env(v); }); + + self.direnv.overwritten_values_to_restore()?.iter().for_each(|(k, v)| { + self.add_env(&k, &v, true); + }); Ok(()) } From c618538cf8a5ea6046629882a89d4a528d42f56e Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Sat, 6 Jun 2020 11:45:58 +0200 Subject: [PATCH 23/57] Working? --- .../src/env/directory_specific_environment.rs | 48 +++---------------- crates/nu-cli/src/env/environment.rs | 6 +-- crates/nu-cli/src/env/environment_syncer.rs | 27 +++++++++-- post.org | 6 +-- 4 files changed, 33 insertions(+), 54 deletions(-) diff --git a/crates/nu-cli/src/env/directory_specific_environment.rs b/crates/nu-cli/src/env/directory_specific_environment.rs index bd8f4d88cd..16a5dd605a 100644 --- a/crates/nu-cli/src/env/directory_specific_environment.rs +++ b/crates/nu-cli/src/env/directory_specific_environment.rs @@ -1,7 +1,7 @@ use indexmap::IndexMap; use nu_protocol::{Primitive, UntaggedValue, Value}; use std::io::Write; -use std::{ffi::OsString, fmt::Debug, path::PathBuf, fs::OpenOptions}; +use std::{ffi::OsString, fmt::Debug, fs::OpenOptions, path::PathBuf}; #[derive(Debug, Default)] pub struct DirectorySpecificEnvironment { @@ -52,6 +52,8 @@ impl DirectorySpecificEnvironment { let current_dir = std::env::current_dir()?; let mut keyvals_to_restore = IndexMap::new(); + let mut new_overwritten = IndexMap::new(); + for (directory, keyvals) in &self.overwritten_env_values { let mut working_dir = Some(current_dir.as_path()); @@ -59,6 +61,7 @@ impl DirectorySpecificEnvironment { while let Some(wdir) = working_dir { if wdir == directory.as_path() { readd = false; + new_overwritten.insert(directory.clone(), keyvals.clone()); break; } else { working_dir = working_dir.unwrap().parent(); @@ -71,6 +74,7 @@ impl DirectorySpecificEnvironment { } } + self.overwritten_env_values = new_overwritten; Ok(keyvals_to_restore) } @@ -102,58 +106,20 @@ impl DirectorySpecificEnvironment { wdir.to_path_buf(), keys_in_file.iter().fold(vec![], |mut keyvals, key| { if let Some(val) = std::env::var_os(key) { - - let mut file = OpenOptions::new() - .write(true) - .append(true) - .create(true) - .open("restore.txt").unwrap(); - - write!(&mut file, "about to overwrite: {:?}\n", val).unwrap(); - keyvals.push((key.clone(), val)); - keyvals - } else { - keyvals } + keyvals }), ); self.added_env_vars.insert(wdir.to_path_buf(), keys_in_file); - break; } else { working_dir = working_dir.unwrap().parent(); } } } + Ok(vars_to_add) } - - //If the user has left directories which added env vars through .nu, we clear those vars - pub fn env_vars_to_delete(&mut self) -> std::io::Result> { - let current_dir = std::env::current_dir()?; - - //Gather up all environment variables that should be deleted. - //If we are not in a directory or one of its subdirectories, mark the env_vals it maps to for removal. - let vars_to_delete = self.added_env_vars.iter().fold( - Vec::new(), - |mut vars_to_delete, (directory, env_vars)| { - let mut working_dir = Some(current_dir.as_path()); - - while let Some(wdir) = working_dir { - if &wdir == directory { - return vars_to_delete; - } else { - working_dir = working_dir.unwrap().parent(); - } - } - //only delete vars from directories we are not in - vars_to_delete.extend(env_vars.clone()); - vars_to_delete - }, - ); - - Ok(vars_to_delete) - } } diff --git a/crates/nu-cli/src/env/environment.rs b/crates/nu-cli/src/env/environment.rs index 3d4df5e7fd..e5c88e4572 100644 --- a/crates/nu-cli/src/env/environment.rs +++ b/crates/nu-cli/src/env/environment.rs @@ -35,7 +35,7 @@ impl Env for Box { pub struct Environment { environment_vars: Option, path_vars: Option, - direnv: DirectorySpecificEnvironment, + pub direnv: DirectorySpecificEnvironment, } impl Environment { @@ -63,13 +63,11 @@ impl Environment { self.add_env(&k, &v, true); }); - self.direnv.env_vars_to_delete()?.iter().for_each(|v| { - self.remove_env(v); - }); self.direnv.overwritten_values_to_restore()?.iter().for_each(|(k, v)| { self.add_env(&k, &v, true); }); + Ok(()) } diff --git a/crates/nu-cli/src/env/environment_syncer.rs b/crates/nu-cli/src/env/environment_syncer.rs index e9c99f3be3..c4bf9bc525 100644 --- a/crates/nu-cli/src/env/environment_syncer.rs +++ b/crates/nu-cli/src/env/environment_syncer.rs @@ -41,14 +41,13 @@ impl EnvironmentSyncer { environment.morph(&*self.config); } - pub fn sync_env_vars(&mut self, ctx: &mut Context) { let mut environment = self.env.lock(); - match environment.maintain_directory_environment() { - Ok(_) => {} - Err(e) => {panic!(e)} - } + // match environment.maintain_directory_environment() { + // Ok(_) => {} + // Err(e) => {panic!(e)} + // } if environment.env().is_some() { for (name, value) in ctx.with_host(|host| host.vars()) { @@ -57,6 +56,24 @@ impl EnvironmentSyncer { // that aren't loaded from config. environment.add_env(&name, &value, false); + environment + .direnv + .env_vars_to_add() + .unwrap() + .iter() + .for_each(|(k, v)| { + environment.add_env(&k, &v, true); + }); + + environment + .direnv + .overwritten_values_to_restore() + .unwrap() + .iter() + .for_each(|(k, v)| { + environment.add_env(&k, &v, true); + }); + // 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))); diff --git a/post.org b/post.org index d3149410ba..26c20dab63 100644 --- a/post.org +++ b/post.org @@ -12,8 +12,8 @@ In order for a .nu-file to be read, the directory it is in must be listed in the ``` nu_env_dirs = ["/home/sam", "/home/sam/github", "/home/sam/github/test"] ``` -This was implemented for the sake of security. I do not believe that it is appropiate for nushell to pick up random .nu-files unless -the user has explicitly said that it's alright. An adversary could hide a .nu-file in an otherwise unsuspicous folder that you download, +This was implemented for the sake of security. I do not believe that it is appropiate for nushell to pick up random .nu-files unless +the user has explicitly said that it's alright. An adversary could hide a .nu-file in an otherwise unsuspicous folder that you download, and now you suddenly have nushell picking up whatever environment variables they set. This meant that the code necessarily become more involved than just looking for a .nu-file in the current directory and applying its variables, but this extra code also allowed for some other nice features and behavior which is listed below. @@ -30,8 +30,6 @@ Questions: Just using what I think was expected, envs.entries.remove(key), does not work. If this is a problem, how can I make it work better with existing code? -TODO: take care of situation where a directory overwrites an existing .nu conf. - ---- # \ No newline at end of file From 4fdc3646e8e8e9cc961c067c816777b0594837cb Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Sat, 6 Jun 2020 11:54:33 +0200 Subject: [PATCH 24/57] Working? --- .../src/env/directory_specific_environment.rs | 3 +-- crates/nu-cli/src/env/environment.rs | 11 --------- crates/nu-cli/src/env/environment_syncer.rs | 23 ++----------------- 3 files changed, 3 insertions(+), 34 deletions(-) diff --git a/crates/nu-cli/src/env/directory_specific_environment.rs b/crates/nu-cli/src/env/directory_specific_environment.rs index 16a5dd605a..d2f6295bd2 100644 --- a/crates/nu-cli/src/env/directory_specific_environment.rs +++ b/crates/nu-cli/src/env/directory_specific_environment.rs @@ -1,7 +1,6 @@ use indexmap::IndexMap; use nu_protocol::{Primitive, UntaggedValue, Value}; -use std::io::Write; -use std::{ffi::OsString, fmt::Debug, fs::OpenOptions, path::PathBuf}; +use std::{ffi::OsString, fmt::Debug, path::PathBuf}; #[derive(Debug, Default)] pub struct DirectorySpecificEnvironment { diff --git a/crates/nu-cli/src/env/environment.rs b/crates/nu-cli/src/env/environment.rs index e5c88e4572..99e6f998be 100644 --- a/crates/nu-cli/src/env/environment.rs +++ b/crates/nu-cli/src/env/environment.rs @@ -71,17 +71,6 @@ impl Environment { Ok(()) } - fn remove_env(&mut self, key: &str) { - if let Some(Value { - value: UntaggedValue::Row(ref mut envs), - tag: _, - }) = self.environment_vars - { - envs.entries.remove(key); - std::env::remove_var(key); - }; - } - pub fn morph(&mut self, configuration: &T) { self.environment_vars = configuration.env(); self.path_vars = configuration.path(); diff --git a/crates/nu-cli/src/env/environment_syncer.rs b/crates/nu-cli/src/env/environment_syncer.rs index c4bf9bc525..177f32fb8e 100644 --- a/crates/nu-cli/src/env/environment_syncer.rs +++ b/crates/nu-cli/src/env/environment_syncer.rs @@ -44,11 +44,6 @@ impl EnvironmentSyncer { pub fn sync_env_vars(&mut self, ctx: &mut Context) { let mut environment = self.env.lock(); - // match environment.maintain_directory_environment() { - // Ok(_) => {} - // Err(e) => {panic!(e)} - // } - if environment.env().is_some() { for (name, value) in ctx.with_host(|host| host.vars()) { if name != "path" && name != "PATH" { @@ -57,22 +52,8 @@ impl EnvironmentSyncer { environment.add_env(&name, &value, false); environment - .direnv - .env_vars_to_add() - .unwrap() - .iter() - .for_each(|(k, v)| { - environment.add_env(&k, &v, true); - }); - - environment - .direnv - .overwritten_values_to_restore() - .unwrap() - .iter() - .for_each(|(k, v)| { - environment.add_env(&k, &v, true); - }); + .maintain_directory_environment() + .expect("Could not set directory-based environment variables"); // clear the env var from the session // we are about to replace them From 3c3ee08ffedc6ce8e4dfc1d99fcdd77a4ac953ad Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Sat, 6 Jun 2020 12:17:01 +0200 Subject: [PATCH 25/57] Update comments and change var name --- .../src/env/directory_specific_environment.rs | 29 ++++++++----------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/crates/nu-cli/src/env/directory_specific_environment.rs b/crates/nu-cli/src/env/directory_specific_environment.rs index d2f6295bd2..11d8ee7231 100644 --- a/crates/nu-cli/src/env/directory_specific_environment.rs +++ b/crates/nu-cli/src/env/directory_specific_environment.rs @@ -44,9 +44,6 @@ impl DirectorySpecificEnvironment { } } - // pub overwritten_env_values: IndexMap>, - //overwritten_env_values maps a directory with a .nu file to some environment variables that it overwrote. - //if we are not in that directory, we re-add those variables. pub fn overwritten_values_to_restore(&mut self) -> std::io::Result> { let current_dir = std::env::current_dir()?; @@ -56,17 +53,17 @@ impl DirectorySpecificEnvironment { for (directory, keyvals) in &self.overwritten_env_values { let mut working_dir = Some(current_dir.as_path()); - let mut readd = true; + let mut re_add_keyvals = true; while let Some(wdir) = working_dir { if wdir == directory.as_path() { - readd = false; + re_add_keyvals = false; new_overwritten.insert(directory.clone(), keyvals.clone()); break; } else { working_dir = working_dir.unwrap().parent(); } } - if readd { + if re_add_keyvals { for (k, v) in keyvals { keyvals_to_restore.insert(k.clone(), v.to_str().unwrap().to_string()); } @@ -82,9 +79,9 @@ impl DirectorySpecificEnvironment { let mut vars_to_add = IndexMap::new(); for dir in &self.whitelisted_directories { - //Start in the current directory, then traverse towards the root with working_dir to see if we are in a subdirectory of a valid directory. let mut working_dir = Some(current_dir.as_path()); + //Start in the current directory, then traverse towards the root with working_dir to see if we are in a subdirectory of a valid directory. while let Some(wdir) = working_dir { if wdir == dir.as_path() { //Read the .nu file and parse it into a nice map @@ -93,17 +90,15 @@ impl DirectorySpecificEnvironment { .unwrap(); let vars_in_current_file = toml_doc.get("env").unwrap().as_table().unwrap(); - let keys_in_file: Vec = vars_in_current_file - .iter() - .map(|(k, v)| { - vars_to_add.insert(k.clone(), v.as_str().unwrap().to_string()); //This is to add the keys and values to the environment - k.clone() //We add them all to a list to keep track of which keys we have added - }) - .collect(); + let mut keys_in_current_nufile = vec![]; + for (k, v) in vars_in_current_file { + vars_to_add.insert(k.clone(), v.as_str().unwrap().to_string()); //This is used to add variables to the environment + keys_in_current_nufile.push(k.clone()); //this is used to keep track of which directory added which variables + } - self.overwritten_env_values.insert( + self.overwritten_env_values.insert( //If we are about to overwrite any environment variables, we save them first so they can be restored later. wdir.to_path_buf(), - keys_in_file.iter().fold(vec![], |mut keyvals, key| { + keys_in_current_nufile.iter().fold(vec![], |mut keyvals, key| { if let Some(val) = std::env::var_os(key) { keyvals.push((key.clone(), val)); } @@ -111,7 +106,7 @@ impl DirectorySpecificEnvironment { }), ); - self.added_env_vars.insert(wdir.to_path_buf(), keys_in_file); + self.added_env_vars.insert(wdir.to_path_buf(), keys_in_current_nufile); break; } else { working_dir = working_dir.unwrap().parent(); From 03febb8cab92e69943c4b768cd7731078371cbe7 Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Sat, 6 Jun 2020 12:19:20 +0200 Subject: [PATCH 26/57] Formatting --- .../src/env/directory_specific_environment.rs | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/crates/nu-cli/src/env/directory_specific_environment.rs b/crates/nu-cli/src/env/directory_specific_environment.rs index 11d8ee7231..0a7f054e61 100644 --- a/crates/nu-cli/src/env/directory_specific_environment.rs +++ b/crates/nu-cli/src/env/directory_specific_environment.rs @@ -96,17 +96,21 @@ impl DirectorySpecificEnvironment { keys_in_current_nufile.push(k.clone()); //this is used to keep track of which directory added which variables } - self.overwritten_env_values.insert( //If we are about to overwrite any environment variables, we save them first so they can be restored later. + self.overwritten_env_values.insert( + //If we are about to overwrite any environment variables, we save them first so they can be restored later. wdir.to_path_buf(), - keys_in_current_nufile.iter().fold(vec![], |mut keyvals, key| { - if let Some(val) = std::env::var_os(key) { - keyvals.push((key.clone(), val)); - } - keyvals - }), + keys_in_current_nufile + .iter() + .fold(vec![], |mut keyvals, key| { + if let Some(val) = std::env::var_os(key) { + keyvals.push((key.clone(), val)); + } + keyvals + }), ); - self.added_env_vars.insert(wdir.to_path_buf(), keys_in_current_nufile); + self.added_env_vars + .insert(wdir.to_path_buf(), keys_in_current_nufile); break; } else { working_dir = working_dir.unwrap().parent(); From d6e1a0e6164efe91a93c15e3f33d51228c197eb7 Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Sat, 6 Jun 2020 12:53:45 +0200 Subject: [PATCH 27/57] Remove vars after leaving dir --- crates/nu-cli/src/cli.rs | 1 - .../src/env/directory_specific_environment.rs | 27 +++++++++++++++++++ crates/nu-cli/src/env/environment.rs | 23 +++++++++++++--- post.org | 10 ++----- 4 files changed, 48 insertions(+), 13 deletions(-) diff --git a/crates/nu-cli/src/cli.rs b/crates/nu-cli/src/cli.rs index cd14373b71..bd5097812e 100644 --- a/crates/nu-cli/src/cli.rs +++ b/crates/nu-cli/src/cli.rs @@ -889,7 +889,6 @@ async fn process_line( trace!("{:#?}", classified_block); let env = ctx.get_env(); - match run_block( &classified_block.block, ctx, diff --git a/crates/nu-cli/src/env/directory_specific_environment.rs b/crates/nu-cli/src/env/directory_specific_environment.rs index 0a7f054e61..b213809060 100644 --- a/crates/nu-cli/src/env/directory_specific_environment.rs +++ b/crates/nu-cli/src/env/directory_specific_environment.rs @@ -120,4 +120,31 @@ impl DirectorySpecificEnvironment { Ok(vars_to_add) } + + //If the user has left directories which added env vars through .nu, we clear those vars + pub fn env_vars_to_delete(&mut self) -> std::io::Result> { + let current_dir = std::env::current_dir()?; + + //Gather up all environment variables that should be deleted. + //If we are not in a directory or one of its subdirectories, mark the env_vals it maps to for removal. + let vars_to_delete = self.added_env_vars.iter().fold( + Vec::new(), + |mut vars_to_delete, (directory, env_vars)| { + let mut working_dir = Some(current_dir.as_path()); + + while let Some(wdir) = working_dir { + if &wdir == directory { + return vars_to_delete; + } else { + working_dir = working_dir.unwrap().parent(); + } + } + //only delete vars from directories we are not in + vars_to_delete.extend(env_vars.clone()); + vars_to_delete + }, + ); + + Ok(vars_to_delete) + } } diff --git a/crates/nu-cli/src/env/environment.rs b/crates/nu-cli/src/env/environment.rs index 99e6f998be..a32735cdf4 100644 --- a/crates/nu-cli/src/env/environment.rs +++ b/crates/nu-cli/src/env/environment.rs @@ -59,18 +59,33 @@ impl Environment { } pub fn maintain_directory_environment(&mut self) -> std::io::Result<()> { + self.direnv.env_vars_to_delete()?.iter().for_each(|k| { + self.remove_env(&k); + }); self.direnv.env_vars_to_add()?.iter().for_each(|(k, v)| { self.add_env(&k, &v, true); }); - - self.direnv.overwritten_values_to_restore()?.iter().for_each(|(k, v)| { - self.add_env(&k, &v, true); - }); + self.direnv + .overwritten_values_to_restore()? + .iter() + .for_each(|(k, v)| { + self.add_env(&k, &v, true); + }); Ok(()) } + fn remove_env(&mut self, key: &str) { + if let Some(Value { + value: UntaggedValue::Row(ref mut envs), + tag: _, + }) = self.environment_vars + { + envs.entries.remove(key); + }; + } + pub fn morph(&mut self, configuration: &T) { self.environment_vars = configuration.env(); self.path_vars = configuration.path(); diff --git a/post.org b/post.org index 26c20dab63..96bea39dbc 100644 --- a/post.org +++ b/post.org @@ -13,22 +13,16 @@ In order for a .nu-file to be read, the directory it is in must be listed in the nu_env_dirs = ["/home/sam", "/home/sam/github", "/home/sam/github/test"] ``` This was implemented for the sake of security. I do not believe that it is appropiate for nushell to pick up random .nu-files unless -the user has explicitly said that it's alright. An adversary could hide a .nu-file in an otherwise unsuspicous folder that you download, +the user has explicitly said that it's alright. An adversary could hide a .nu-file in an otherwise unsuspicious folder that you download, and now you suddenly have nushell picking up whatever environment variables they set. This meant that the code necessarily become more involved than just looking for a .nu-file in the current directory and applying its variables, but this extra code also allowed for some other nice features and behavior which is listed below. Behavior: -- If you are in a subdirectory to a directory with a .nu-file, the vars in that .nu-file are applied. +- If you are in a directory, or a subdirectory to a directory with a .nu-file, the vars in that .nu-file are applied. - If you leave a directory which set some variables, the variables are unset. - If a directory contains a .nu with an environment variable already set, the old value will be overwritten with the value from the .nu. This holds even if the old value was set by a .nu in a parent directory. - The overwritten value is restored when you leave the directory. - - TODO: What happens if you overwrite twice? - -Questions: -`remove_env()` accesses the environment directly to remove variables. -Just using what I think was expected, envs.entries.remove(key), does not work. -If this is a problem, how can I make it work better with existing code? ---- From 2770a6f5e47cea7bf0560b024fa9f08cf4dab4c0 Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Sat, 6 Jun 2020 13:01:55 +0200 Subject: [PATCH 28/57] Remove notes I made --- post.org | 29 ----------------------------- 1 file changed, 29 deletions(-) delete mode 100644 post.org diff --git a/post.org b/post.org deleted file mode 100644 index 96bea39dbc..0000000000 --- a/post.org +++ /dev/null @@ -1,29 +0,0 @@ -# WIP: Per directory env-variables -For #86 - -Environment variables are added if you have created a file called .nu inside a whitelisted directory, formatted as shown below. (I am, of course, open to change everything about this) -``` -[env] -var = "value" -anothervar = "anothervalue" -``` - -In order for a .nu-file to be read, the directory it is in must be listed in the `nu_env_dirs` variable in nushell's `config.toml`. -``` -nu_env_dirs = ["/home/sam", "/home/sam/github", "/home/sam/github/test"] -``` -This was implemented for the sake of security. I do not believe that it is appropiate for nushell to pick up random .nu-files unless -the user has explicitly said that it's alright. An adversary could hide a .nu-file in an otherwise unsuspicious folder that you download, -and now you suddenly have nushell picking up whatever environment variables they set. -This meant that the code necessarily become more involved than just looking for a .nu-file in the current directory and applying its variables, -but this extra code also allowed for some other nice features and behavior which is listed below. - -Behavior: -- If you are in a directory, or a subdirectory to a directory with a .nu-file, the vars in that .nu-file are applied. -- If you leave a directory which set some variables, the variables are unset. -- If a directory contains a .nu with an environment variable already set, the old value will be overwritten with the value from the .nu. This holds even if the old value was set by a .nu in a parent directory. - - The overwritten value is restored when you leave the directory. - ----- - -# \ No newline at end of file From e4c951fe9385348827791d58ee23b13cf9d0f5fe Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Sat, 6 Jun 2020 13:26:42 +0200 Subject: [PATCH 29/57] Rename config function --- crates/nu-cli/src/data/config/conf.rs | 6 +++--- crates/nu-cli/src/data/config/nuconfig.rs | 6 +++--- crates/nu-cli/src/env/environment.rs | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/nu-cli/src/data/config/conf.rs b/crates/nu-cli/src/data/config/conf.rs index efd44d396c..1f53f0c7a2 100644 --- a/crates/nu-cli/src/data/config/conf.rs +++ b/crates/nu-cli/src/data/config/conf.rs @@ -4,7 +4,7 @@ use std::fmt::Debug; pub trait Conf: Debug + Send { fn env(&self) -> Option; fn path(&self) -> Option; - fn direnv_whitelist(&self) -> Option; + fn nu_env_dirs(&self) -> Option; fn reload(&self); } @@ -13,8 +13,8 @@ impl Conf for Box { (**self).env() } - fn direnv_whitelist(&self) -> Option { - (**self).direnv_whitelist() + fn nu_env_dirs(&self) -> Option { + (**self).nu_env_dirs() } fn path(&self) -> Option { diff --git a/crates/nu-cli/src/data/config/nuconfig.rs b/crates/nu-cli/src/data/config/nuconfig.rs index 9630e18355..54001026a2 100644 --- a/crates/nu-cli/src/data/config/nuconfig.rs +++ b/crates/nu-cli/src/data/config/nuconfig.rs @@ -20,8 +20,8 @@ impl Conf for NuConfig { self.path() } - fn direnv_whitelist(&self) -> Option { - self.direnv_whitelist() + fn nu_env_dirs(&self) -> Option { + self.nu_env_dirs() } fn reload(&self) { @@ -56,7 +56,7 @@ impl NuConfig { None } - pub fn direnv_whitelist(&self) -> Option { + pub fn nu_env_dirs(&self) -> Option { let vars = self.vars.lock(); if let Some(dirs) = vars.get("nu_env_dirs") { return Some(dirs.clone()); diff --git a/crates/nu-cli/src/env/environment.rs b/crates/nu-cli/src/env/environment.rs index a32735cdf4..53f863f2bb 100644 --- a/crates/nu-cli/src/env/environment.rs +++ b/crates/nu-cli/src/env/environment.rs @@ -54,7 +54,7 @@ impl Environment { Environment { environment_vars: env, path_vars: path, - direnv: DirectorySpecificEnvironment::new(configuration.direnv_whitelist()), + direnv: DirectorySpecificEnvironment::new(configuration.nu_env_dirs()), } } From ff742ed67549d8a2aef26d8d2edfed2b95ad609a Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Sun, 7 Jun 2020 06:55:38 +0200 Subject: [PATCH 30/57] Clippy --- crates/nu-cli/src/data/config/tests.rs | 4 ++++ .../src/env/directory_specific_environment.rs | 23 +++++++++++++------ crates/nu-cli/src/env/environment.rs | 4 ++-- 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/crates/nu-cli/src/data/config/tests.rs b/crates/nu-cli/src/data/config/tests.rs index ecbe0cb7f7..ea8712fe0f 100644 --- a/crates/nu-cli/src/data/config/tests.rs +++ b/crates/nu-cli/src/data/config/tests.rs @@ -16,6 +16,10 @@ impl Conf for FakeConfig { self.config.env() } + fn nu_env_dirs(&self) -> Option { + None + } + fn path(&self) -> Option { self.config.path() } diff --git a/crates/nu-cli/src/env/directory_specific_environment.rs b/crates/nu-cli/src/env/directory_specific_environment.rs index b213809060..ba7e4ee2fd 100644 --- a/crates/nu-cli/src/env/directory_specific_environment.rs +++ b/crates/nu-cli/src/env/directory_specific_environment.rs @@ -60,12 +60,17 @@ impl DirectorySpecificEnvironment { new_overwritten.insert(directory.clone(), keyvals.clone()); break; } else { - working_dir = working_dir.unwrap().parent(); + working_dir = working_dir.expect("Root directory has no parent").parent(); } } if re_add_keyvals { for (k, v) in keyvals { - keyvals_to_restore.insert(k.clone(), v.to_str().unwrap().to_string()); + keyvals_to_restore.insert( + k.clone(), + v.to_str() + .expect("Filepath is not valid unicode") + .to_string(), + ); } } } @@ -86,9 +91,13 @@ impl DirectorySpecificEnvironment { if wdir == dir.as_path() { //Read the .nu file and parse it into a nice map let toml_doc = std::fs::read_to_string(wdir.join(".nu").as_path())? - .parse::() - .unwrap(); - let vars_in_current_file = toml_doc.get("env").unwrap().as_table().unwrap(); + .parse::()?; + + let vars_in_current_file = toml_doc + .get("env") + .ok_or(std::io::Error::new(std::io::ErrorKind::InvalidData, "No [env] section in .nu-file"))? + .as_table() + .ok_or(std::io::Error::new(std::io::ErrorKind::InvalidData, "Malformed [env] section in .nu-file"))?; let mut keys_in_current_nufile = vec![]; for (k, v) in vars_in_current_file { @@ -113,7 +122,7 @@ impl DirectorySpecificEnvironment { .insert(wdir.to_path_buf(), keys_in_current_nufile); break; } else { - working_dir = working_dir.unwrap().parent(); + working_dir = working_dir.expect("Root directory has no parent").parent(); } } } @@ -136,7 +145,7 @@ impl DirectorySpecificEnvironment { if &wdir == directory { return vars_to_delete; } else { - working_dir = working_dir.unwrap().parent(); + working_dir = working_dir.expect("Root directory has no parent").parent(); } } //only delete vars from directories we are not in diff --git a/crates/nu-cli/src/env/environment.rs b/crates/nu-cli/src/env/environment.rs index 53f863f2bb..41e3270a61 100644 --- a/crates/nu-cli/src/env/environment.rs +++ b/crates/nu-cli/src/env/environment.rs @@ -238,7 +238,7 @@ mod tests { let fake_config = FakeConfig::new(&file); let mut actual = Environment::from_config(&fake_config); - actual.add_env("USER", "NUNO"); + actual.add_env("USER", "NUNO", false); assert_eq!( actual.env(), @@ -271,7 +271,7 @@ mod tests { let fake_config = FakeConfig::new(&file); let mut actual = Environment::from_config(&fake_config); - actual.add_env("SHELL", "/usr/bin/sh"); + actual.add_env("SHELL", "/usr/bin/sh", false); assert_eq!( actual.env(), From e1581ec156d831b8745a0b93dcb1af40090c54c2 Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Sun, 7 Jun 2020 07:01:51 +0200 Subject: [PATCH 31/57] Cleanup and handle errors --- .../src/env/directory_specific_environment.rs | 64 +++++++++++++------ crates/nu-cli/src/env/environment_syncer.rs | 3 +- 2 files changed, 44 insertions(+), 23 deletions(-) diff --git a/crates/nu-cli/src/env/directory_specific_environment.rs b/crates/nu-cli/src/env/directory_specific_environment.rs index ba7e4ee2fd..a7c8cac640 100644 --- a/crates/nu-cli/src/env/directory_specific_environment.rs +++ b/crates/nu-cli/src/env/directory_specific_environment.rs @@ -1,16 +1,17 @@ use indexmap::IndexMap; use nu_protocol::{Primitive, UntaggedValue, Value}; +use std::io::{Error, ErrorKind, Result}; use std::{ffi::OsString, fmt::Debug, path::PathBuf}; #[derive(Debug, Default)] pub struct DirectorySpecificEnvironment { - pub whitelisted_directories: Vec, + whitelisted_directories: Vec, //Directory -> Env key. If an environment var has been added from a .nu in a directory, we track it here so we can remove it when the user leaves the directory. - pub added_env_vars: IndexMap>, + added_env_vars: IndexMap>, //Directory -> (env_key, value). If a .nu overwrites some existing environment variables, they are added here so that they can be restored later. - pub overwritten_env_values: IndexMap>, + overwritten_env_values: IndexMap>, } impl DirectorySpecificEnvironment { @@ -22,16 +23,17 @@ impl DirectorySpecificEnvironment { { wrapped_directories .iter() - .fold(vec![], |mut directories, dirval| { + .filter_map(|dirval| { if let Value { value: UntaggedValue::Primitive(Primitive::String(ref dir)), tag: _, } = dirval { - directories.push(PathBuf::from(&dir)); + return Some(PathBuf::from(&dir)); } - directories + None }) + .collect() } else { vec![] }; @@ -44,7 +46,7 @@ impl DirectorySpecificEnvironment { } } - pub fn overwritten_values_to_restore(&mut self) -> std::io::Result> { + pub fn overwritten_values_to_restore(&mut self) -> Result> { let current_dir = std::env::current_dir()?; let mut keyvals_to_restore = IndexMap::new(); @@ -60,7 +62,11 @@ impl DirectorySpecificEnvironment { new_overwritten.insert(directory.clone(), keyvals.clone()); break; } else { - working_dir = working_dir.expect("Root directory has no parent").parent(); + working_dir = working_dir //Keep going up in the directory structure with .parent() + .ok_or_else(|| { + Error::new(ErrorKind::NotFound, "Root directory has no parent") + })? + .parent(); } } if re_add_keyvals { @@ -68,7 +74,9 @@ impl DirectorySpecificEnvironment { keyvals_to_restore.insert( k.clone(), v.to_str() - .expect("Filepath is not valid unicode") + .ok_or_else(|| { + Error::new(ErrorKind::Other, "Filepath is not valid unicode") + })? .to_string(), ); } @@ -79,7 +87,7 @@ impl DirectorySpecificEnvironment { Ok(keyvals_to_restore) } - pub fn env_vars_to_add(&mut self) -> std::io::Result> { + pub fn env_vars_to_add(&mut self) -> Result> { let current_dir = std::env::current_dir()?; let mut vars_to_add = IndexMap::new(); @@ -89,15 +97,24 @@ impl DirectorySpecificEnvironment { //Start in the current directory, then traverse towards the root with working_dir to see if we are in a subdirectory of a valid directory. while let Some(wdir) = working_dir { if wdir == dir.as_path() { - //Read the .nu file and parse it into a nice map let toml_doc = std::fs::read_to_string(wdir.join(".nu").as_path())? .parse::()?; let vars_in_current_file = toml_doc .get("env") - .ok_or(std::io::Error::new(std::io::ErrorKind::InvalidData, "No [env] section in .nu-file"))? + .ok_or_else(|| { + std::io::Error::new( + std::io::ErrorKind::InvalidData, + "No [env] section in .nu-file", + ) + })? .as_table() - .ok_or(std::io::Error::new(std::io::ErrorKind::InvalidData, "Malformed [env] section in .nu-file"))?; + .ok_or_else(|| { + Error::new( + ErrorKind::InvalidData, + "Malformed [env] section in .nu-file", + ) + })?; let mut keys_in_current_nufile = vec![]; for (k, v) in vars_in_current_file { @@ -105,24 +122,29 @@ impl DirectorySpecificEnvironment { keys_in_current_nufile.push(k.clone()); //this is used to keep track of which directory added which variables } + //If we are about to overwrite any environment variables, we save them first so they can be restored later. self.overwritten_env_values.insert( - //If we are about to overwrite any environment variables, we save them first so they can be restored later. wdir.to_path_buf(), keys_in_current_nufile .iter() - .fold(vec![], |mut keyvals, key| { + .filter_map(|key| { if let Some(val) = std::env::var_os(key) { - keyvals.push((key.clone(), val)); + return Some((key.clone(), val)); } - keyvals - }), + None + }) + .collect(), ); self.added_env_vars .insert(wdir.to_path_buf(), keys_in_current_nufile); break; } else { - working_dir = working_dir.expect("Root directory has no parent").parent(); + working_dir = working_dir //Keep going up in the directory structure with .parent() + .ok_or_else(|| { + Error::new(ErrorKind::NotFound, "Root directory has no parent") + })? + .parent(); } } } @@ -131,7 +153,7 @@ impl DirectorySpecificEnvironment { } //If the user has left directories which added env vars through .nu, we clear those vars - pub fn env_vars_to_delete(&mut self) -> std::io::Result> { + pub fn env_vars_to_delete(&mut self) -> Result> { let current_dir = std::env::current_dir()?; //Gather up all environment variables that should be deleted. @@ -142,7 +164,7 @@ impl DirectorySpecificEnvironment { let mut working_dir = Some(current_dir.as_path()); while let Some(wdir) = working_dir { - if &wdir == directory { + if wdir == directory { return vars_to_delete; } else { working_dir = working_dir.expect("Root directory has no parent").parent(); diff --git a/crates/nu-cli/src/env/environment_syncer.rs b/crates/nu-cli/src/env/environment_syncer.rs index 177f32fb8e..7c0a17071b 100644 --- a/crates/nu-cli/src/env/environment_syncer.rs +++ b/crates/nu-cli/src/env/environment_syncer.rs @@ -52,8 +52,7 @@ impl EnvironmentSyncer { environment.add_env(&name, &value, false); environment - .maintain_directory_environment() - .expect("Could not set directory-based environment variables"); + .maintain_directory_environment().ok(); // clear the env var from the session // we are about to replace them From c5b8abbcd36eb6bb5975c8cdda3bc38b08f2db9b Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Sun, 7 Jun 2020 08:36:06 +0200 Subject: [PATCH 32/57] cargo fmt --- crates/nu-cli/src/env/directory_specific_environment.rs | 2 +- crates/nu-cli/src/env/environment_syncer.rs | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/nu-cli/src/env/directory_specific_environment.rs b/crates/nu-cli/src/env/directory_specific_environment.rs index a7c8cac640..3630ad1a04 100644 --- a/crates/nu-cli/src/env/directory_specific_environment.rs +++ b/crates/nu-cli/src/env/directory_specific_environment.rs @@ -31,7 +31,7 @@ impl DirectorySpecificEnvironment { { return Some(PathBuf::from(&dir)); } - None + None }) .collect() } else { diff --git a/crates/nu-cli/src/env/environment_syncer.rs b/crates/nu-cli/src/env/environment_syncer.rs index 7c0a17071b..fbcd72e6a6 100644 --- a/crates/nu-cli/src/env/environment_syncer.rs +++ b/crates/nu-cli/src/env/environment_syncer.rs @@ -51,8 +51,7 @@ impl EnvironmentSyncer { // that aren't loaded from config. environment.add_env(&name, &value, false); - environment - .maintain_directory_environment().ok(); + environment.maintain_directory_environment().ok(); // clear the env var from the session // we are about to replace them From aaacf4c336891584bf1362f46dc8b890d3f7cc47 Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Sun, 7 Jun 2020 08:56:22 +0200 Subject: [PATCH 33/57] Better error messages, remove last (?) unwrap --- .../src/env/directory_specific_environment.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/crates/nu-cli/src/env/directory_specific_environment.rs b/crates/nu-cli/src/env/directory_specific_environment.rs index 3630ad1a04..ca701d76d0 100644 --- a/crates/nu-cli/src/env/directory_specific_environment.rs +++ b/crates/nu-cli/src/env/directory_specific_environment.rs @@ -75,7 +75,7 @@ impl DirectorySpecificEnvironment { k.clone(), v.to_str() .ok_or_else(|| { - Error::new(ErrorKind::Other, "Filepath is not valid unicode") + Error::new(ErrorKind::Other, format!("{:?} is not valid unicode", v)) })? .to_string(), ); @@ -118,7 +118,17 @@ impl DirectorySpecificEnvironment { let mut keys_in_current_nufile = vec![]; for (k, v) in vars_in_current_file { - vars_to_add.insert(k.clone(), v.as_str().unwrap().to_string()); //This is used to add variables to the environment + vars_to_add.insert( + k.clone(), + v.as_str() + .ok_or_else(|| { + Error::new( + ErrorKind::InvalidData, + format!("Could not read environment variable: {}", v), + ) + })? + .to_string(), + ); //This is used to add variables to the environment keys_in_current_nufile.push(k.clone()); //this is used to keep track of which directory added which variables } From fe16db6a2fd780799eae101867b23ee396b8ef2b Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Sun, 7 Jun 2020 09:03:04 +0200 Subject: [PATCH 34/57] FORMAT PLZ --- crates/nu-cli/src/env/directory_specific_environment.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/nu-cli/src/env/directory_specific_environment.rs b/crates/nu-cli/src/env/directory_specific_environment.rs index ca701d76d0..e69852f610 100644 --- a/crates/nu-cli/src/env/directory_specific_environment.rs +++ b/crates/nu-cli/src/env/directory_specific_environment.rs @@ -75,7 +75,10 @@ impl DirectorySpecificEnvironment { k.clone(), v.to_str() .ok_or_else(|| { - Error::new(ErrorKind::Other, format!("{:?} is not valid unicode", v)) + Error::new( + ErrorKind::Other, + format!("{:?} is not valid unicode", v), + ) })? .to_string(), ); From 99824d864ccb223508681ef44a33c7113ab8d66c Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Mon, 8 Jun 2020 06:16:44 +0200 Subject: [PATCH 35/57] Rename whitelisted_directories to allowed_directories --- .../src/env/directory_specific_environment.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/nu-cli/src/env/directory_specific_environment.rs b/crates/nu-cli/src/env/directory_specific_environment.rs index e69852f610..aeb8d8179f 100644 --- a/crates/nu-cli/src/env/directory_specific_environment.rs +++ b/crates/nu-cli/src/env/directory_specific_environment.rs @@ -5,7 +5,7 @@ use std::{ffi::OsString, fmt::Debug, path::PathBuf}; #[derive(Debug, Default)] pub struct DirectorySpecificEnvironment { - whitelisted_directories: Vec, + allowed_directories: Vec, //Directory -> Env key. If an environment var has been added from a .nu in a directory, we track it here so we can remove it when the user leaves the directory. added_env_vars: IndexMap>, @@ -15,11 +15,11 @@ pub struct DirectorySpecificEnvironment { } impl DirectorySpecificEnvironment { - pub fn new(whitelisted_directories: Option) -> DirectorySpecificEnvironment { - let mut whitelisted_directories = if let Some(Value { + pub fn new(allowed_directories: Option) -> DirectorySpecificEnvironment { + let mut allowed_directories = if let Some(Value { value: UntaggedValue::Table(ref wrapped_directories), tag: _, - }) = whitelisted_directories + }) = allowed_directories { wrapped_directories .iter() @@ -37,10 +37,10 @@ impl DirectorySpecificEnvironment { } else { vec![] }; - whitelisted_directories.sort(); + allowed_directories.sort(); DirectorySpecificEnvironment { - whitelisted_directories, + allowed_directories, added_env_vars: IndexMap::new(), overwritten_env_values: IndexMap::new(), } @@ -94,7 +94,7 @@ impl DirectorySpecificEnvironment { let current_dir = std::env::current_dir()?; let mut vars_to_add = IndexMap::new(); - for dir in &self.whitelisted_directories { + for dir in &self.allowed_directories { let mut working_dir = Some(current_dir.as_path()); //Start in the current directory, then traverse towards the root with working_dir to see if we are in a subdirectory of a valid directory. From 5a85a3448e91ebc73db62133b369c7836bf9fd96 Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Mon, 8 Jun 2020 07:05:12 +0200 Subject: [PATCH 36/57] Add comment to clarify how overwritten values are restored. --- crates/nu-cli/src/env/directory_specific_environment.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/nu-cli/src/env/directory_specific_environment.rs b/crates/nu-cli/src/env/directory_specific_environment.rs index aeb8d8179f..ca201aed0f 100644 --- a/crates/nu-cli/src/env/directory_specific_environment.rs +++ b/crates/nu-cli/src/env/directory_specific_environment.rs @@ -46,6 +46,7 @@ impl DirectorySpecificEnvironment { } } + //If we are no longer in a directory, we restore the values it overwrote. pub fn overwritten_values_to_restore(&mut self) -> Result> { let current_dir = std::env::current_dir()?; From a40ed5fef2767fdb7fd7b7637159b2453826d4c1 Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Tue, 9 Jun 2020 07:00:22 +0200 Subject: [PATCH 37/57] Change list of allowed dirs to indexmap --- .../src/env/directory_specific_environment.rs | 104 +++++++++--------- 1 file changed, 54 insertions(+), 50 deletions(-) diff --git a/crates/nu-cli/src/env/directory_specific_environment.rs b/crates/nu-cli/src/env/directory_specific_environment.rs index ca201aed0f..30a2f54806 100644 --- a/crates/nu-cli/src/env/directory_specific_environment.rs +++ b/crates/nu-cli/src/env/directory_specific_environment.rs @@ -1,11 +1,11 @@ -use indexmap::IndexMap; +use indexmap::{IndexMap, IndexSet}; use nu_protocol::{Primitive, UntaggedValue, Value}; use std::io::{Error, ErrorKind, Result}; use std::{ffi::OsString, fmt::Debug, path::PathBuf}; #[derive(Debug, Default)] pub struct DirectorySpecificEnvironment { - allowed_directories: Vec, + allowed_directories: IndexSet, //Directory -> Env key. If an environment var has been added from a .nu in a directory, we track it here so we can remove it when the user leaves the directory. added_env_vars: IndexMap>, @@ -38,9 +38,13 @@ impl DirectorySpecificEnvironment { vec![] }; allowed_directories.sort(); + let mut allowed = IndexSet::new(); + for d in allowed_directories { + allowed.insert(d); + } DirectorySpecificEnvironment { - allowed_directories, + allowed_directories: allowed, added_env_vars: IndexMap::new(), overwritten_env_values: IndexMap::new(), } @@ -93,74 +97,74 @@ impl DirectorySpecificEnvironment { pub fn env_vars_to_add(&mut self) -> Result> { let current_dir = std::env::current_dir()?; - let mut vars_to_add = IndexMap::new(); - for dir in &self.allowed_directories { - let mut working_dir = Some(current_dir.as_path()); + let mut working_dir = Some(current_dir.as_path()); - //Start in the current directory, then traverse towards the root with working_dir to see if we are in a subdirectory of a valid directory. - while let Some(wdir) = working_dir { - if wdir == dir.as_path() { - let toml_doc = std::fs::read_to_string(wdir.join(".nu").as_path())? - .parse::()?; + //Start in the current directory, then traverse towards the root with working_dir to see if we are in a subdirectory of a valid directory. + while let Some(wdir) = working_dir { + if self.allowed_directories.contains(wdir) { + let toml_doc = match std::fs::read_to_string(wdir.join(".nu").as_path()) { + Ok(doc) => doc.parse::()?, + Err(_) => return Ok(vars_to_add), + }; - let vars_in_current_file = toml_doc - .get("env") - .ok_or_else(|| { - std::io::Error::new( - std::io::ErrorKind::InvalidData, - "No [env] section in .nu-file", - ) - })? - .as_table() - .ok_or_else(|| { - Error::new( - ErrorKind::InvalidData, - "Malformed [env] section in .nu-file", - ) - })?; + let vars_in_current_file = toml_doc + .get("env") + .ok_or_else(|| { + std::io::Error::new( + std::io::ErrorKind::InvalidData, + "No [env] section in .nu-file", + ) + })? + .as_table() + .ok_or_else(|| { + Error::new( + ErrorKind::InvalidData, + "Malformed [env] section in .nu-file", + ) + })?; - let mut keys_in_current_nufile = vec![]; - for (k, v) in vars_in_current_file { + let mut keys_in_current_nufile = vec![]; + for (k, v) in vars_in_current_file { + if !vars_to_add.contains_key(k) { vars_to_add.insert( k.clone(), v.as_str() .ok_or_else(|| { Error::new( ErrorKind::InvalidData, - format!("Could not read environment variable: {}", v), + format!("Could not read environment variable: {}\n", v), ) })? .to_string(), ); //This is used to add variables to the environment - keys_in_current_nufile.push(k.clone()); //this is used to keep track of which directory added which variables } + keys_in_current_nufile.push(k.clone()); //this is used to keep track of which directory added which variables + } - //If we are about to overwrite any environment variables, we save them first so they can be restored later. - self.overwritten_env_values.insert( - wdir.to_path_buf(), - keys_in_current_nufile - .iter() - .filter_map(|key| { - if let Some(val) = std::env::var_os(key) { - return Some((key.clone(), val)); - } - None - }) - .collect(), - ); + //If we are about to overwrite any environment variables, we save them first so they can be restored later. + self.overwritten_env_values.insert( + wdir.to_path_buf(), + keys_in_current_nufile + .iter() + .filter_map(|key| { + if let Some(val) = std::env::var_os(key) { + return Some((key.clone(), val)); + } + None + }) + .collect(), + ); - self.added_env_vars - .insert(wdir.to_path_buf(), keys_in_current_nufile); - break; - } else { - working_dir = working_dir //Keep going up in the directory structure with .parent() + self.added_env_vars + .insert(wdir.to_path_buf(), keys_in_current_nufile); + } + working_dir = + working_dir //Keep going up in the directory structure with .parent() .ok_or_else(|| { Error::new(ErrorKind::NotFound, "Root directory has no parent") })? .parent(); - } - } } Ok(vars_to_add) From 2fcde8daa9940cb912e01c38c717b5c15b33bd36 Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Tue, 9 Jun 2020 10:15:31 +0200 Subject: [PATCH 38/57] Rewrite starting --- .../src/env/directory_specific_environment.rs | 82 +++++++------------ 1 file changed, 29 insertions(+), 53 deletions(-) diff --git a/crates/nu-cli/src/env/directory_specific_environment.rs b/crates/nu-cli/src/env/directory_specific_environment.rs index 30a2f54806..4a3c9e5111 100644 --- a/crates/nu-cli/src/env/directory_specific_environment.rs +++ b/crates/nu-cli/src/env/directory_specific_environment.rs @@ -16,7 +16,7 @@ pub struct DirectorySpecificEnvironment { impl DirectorySpecificEnvironment { pub fn new(allowed_directories: Option) -> DirectorySpecificEnvironment { - let mut allowed_directories = if let Some(Value { + let allowed_directories = if let Some(Value { value: UntaggedValue::Table(ref wrapped_directories), tag: _, }) = allowed_directories @@ -35,16 +35,11 @@ impl DirectorySpecificEnvironment { }) .collect() } else { - vec![] + IndexSet::new() }; - allowed_directories.sort(); - let mut allowed = IndexSet::new(); - for d in allowed_directories { - allowed.insert(d); - } DirectorySpecificEnvironment { - allowed_directories: allowed, + allowed_directories, added_env_vars: IndexMap::new(), overwritten_env_values: IndexMap::new(), } @@ -97,67 +92,48 @@ impl DirectorySpecificEnvironment { pub fn env_vars_to_add(&mut self) -> Result> { let current_dir = std::env::current_dir()?; - let mut vars_to_add = IndexMap::new(); let mut working_dir = Some(current_dir.as_path()); + let mut vars_to_add = IndexMap::new(); + //Start in the current directory, then traverse towards the root with working_dir to see if we are in a subdirectory of a valid directory. while let Some(wdir) = working_dir { if self.allowed_directories.contains(wdir) { - let toml_doc = match std::fs::read_to_string(wdir.join(".nu").as_path()) { - Ok(doc) => doc.parse::()?, - Err(_) => return Ok(vars_to_add), - }; + let toml_doc = std::fs::read_to_string(wdir.join(".nu").as_path()) + .unwrap_or_else(|_| "[env]".to_string()) + .parse::()?; - let vars_in_current_file = toml_doc + toml_doc .get("env") - .ok_or_else(|| { - std::io::Error::new( - std::io::ErrorKind::InvalidData, - "No [env] section in .nu-file", - ) - })? + .expect("No env section in file") .as_table() .ok_or_else(|| { Error::new( ErrorKind::InvalidData, "Malformed [env] section in .nu-file", ) - })?; + })? + .iter() + .for_each(|(k, v)| { + if !vars_to_add.contains_key(k) { + vars_to_add.insert(k.clone(), v.as_str().unwrap().to_string()); - let mut keys_in_current_nufile = vec![]; - for (k, v) in vars_in_current_file { - if !vars_to_add.contains_key(k) { - vars_to_add.insert( - k.clone(), - v.as_str() - .ok_or_else(|| { - Error::new( - ErrorKind::InvalidData, - format!("Could not read environment variable: {}\n", v), - ) - })? - .to_string(), - ); //This is used to add variables to the environment - } - keys_in_current_nufile.push(k.clone()); //this is used to keep track of which directory added which variables - } - - //If we are about to overwrite any environment variables, we save them first so they can be restored later. - self.overwritten_env_values.insert( - wdir.to_path_buf(), - keys_in_current_nufile - .iter() - .filter_map(|key| { - if let Some(val) = std::env::var_os(key) { - return Some((key.clone(), val)); + //If we are about to overwrite any environment variables, we save them first so they can be restored later. + if let Some(val) = std::env::var_os(k) { + let entry = self + .overwritten_env_values + .entry(wdir.to_path_buf()) + .or_insert(vec![]); + entry.push((k.clone(), val)); } - None - }) - .collect(), - ); - self.added_env_vars - .insert(wdir.to_path_buf(), keys_in_current_nufile); + let entry = self + .added_env_vars + .entry(wdir.to_path_buf()) + .or_insert(vec![]); + entry.push(k.clone()); + } + }); } working_dir = working_dir //Keep going up in the directory structure with .parent() From 862ff60b3ef50796d405269f5adaa0bf82861099 Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Wed, 10 Jun 2020 08:33:04 +0200 Subject: [PATCH 39/57] rewrite everything --- .../src/env/directory_specific_environment.rs | 92 +++++++------------ crates/nu-cli/src/env/environment.rs | 13 +-- 2 files changed, 38 insertions(+), 67 deletions(-) diff --git a/crates/nu-cli/src/env/directory_specific_environment.rs b/crates/nu-cli/src/env/directory_specific_environment.rs index 4a3c9e5111..05a08ab016 100644 --- a/crates/nu-cli/src/env/directory_specific_environment.rs +++ b/crates/nu-cli/src/env/directory_specific_environment.rs @@ -1,7 +1,8 @@ use indexmap::{IndexMap, IndexSet}; use nu_protocol::{Primitive, UntaggedValue, Value}; +use std::io::Write; use std::io::{Error, ErrorKind, Result}; -use std::{ffi::OsString, fmt::Debug, path::PathBuf}; +use std::{ffi::OsString, fmt::Debug, fs::OpenOptions, path::PathBuf}; #[derive(Debug, Default)] pub struct DirectorySpecificEnvironment { @@ -48,45 +49,23 @@ impl DirectorySpecificEnvironment { //If we are no longer in a directory, we restore the values it overwrote. pub fn overwritten_values_to_restore(&mut self) -> Result> { let current_dir = std::env::current_dir()?; + let working_dir = Some(current_dir.as_path()); - let mut keyvals_to_restore = IndexMap::new(); - let mut new_overwritten = IndexMap::new(); - - for (directory, keyvals) in &self.overwritten_env_values { - let mut working_dir = Some(current_dir.as_path()); - - let mut re_add_keyvals = true; - while let Some(wdir) = working_dir { - if wdir == directory.as_path() { - re_add_keyvals = false; - new_overwritten.insert(directory.clone(), keyvals.clone()); - break; - } else { - working_dir = working_dir //Keep going up in the directory structure with .parent() - .ok_or_else(|| { - Error::new(ErrorKind::NotFound, "Root directory has no parent") - })? - .parent(); + let keyvals_to_restore = self + .overwritten_env_values + .iter() + .filter_map(|(directory, keyvals)| { + while let Some(wdir) = working_dir { + if &wdir == directory { + return false; + } else { + working_dir = working_dir.expect("This directory has no parent").parent(); + } } - } - if re_add_keyvals { - for (k, v) in keyvals { - keyvals_to_restore.insert( - k.clone(), - v.to_str() - .ok_or_else(|| { - Error::new( - ErrorKind::Other, - format!("{:?} is not valid unicode", v), - ) - })? - .to_string(), - ); - } - } - } + true + }) + .collect(); - self.overwritten_env_values = new_overwritten; Ok(keyvals_to_restore) } @@ -94,25 +73,21 @@ impl DirectorySpecificEnvironment { let current_dir = std::env::current_dir()?; let mut working_dir = Some(current_dir.as_path()); + let empty = toml::value::Table::new(); let mut vars_to_add = IndexMap::new(); //Start in the current directory, then traverse towards the root with working_dir to see if we are in a subdirectory of a valid directory. while let Some(wdir) = working_dir { if self.allowed_directories.contains(wdir) { let toml_doc = std::fs::read_to_string(wdir.join(".nu").as_path()) - .unwrap_or_else(|_| "[env]".to_string()) + .unwrap_or_else(|_| r#"[env]"#.to_string()) .parse::()?; toml_doc .get("env") - .expect("No env section in file") + .unwrap() .as_table() - .ok_or_else(|| { - Error::new( - ErrorKind::InvalidData, - "Malformed [env] section in .nu-file", - ) - })? + .unwrap_or_else(|| &empty) .iter() .for_each(|(k, v)| { if !vars_to_add.contains_key(k) { @@ -120,27 +95,22 @@ impl DirectorySpecificEnvironment { //If we are about to overwrite any environment variables, we save them first so they can be restored later. if let Some(val) = std::env::var_os(k) { - let entry = self - .overwritten_env_values + self.overwritten_env_values .entry(wdir.to_path_buf()) - .or_insert(vec![]); - entry.push((k.clone(), val)); + .or_insert(vec![]) + .push((k.clone(), val)); + } else { + self.added_env_vars + .entry(wdir.to_path_buf()) + .or_insert(vec![]) + .push(k.clone()); } - - let entry = self - .added_env_vars - .entry(wdir.to_path_buf()) - .or_insert(vec![]); - entry.push(k.clone()); } }); } - working_dir = - working_dir //Keep going up in the directory structure with .parent() - .ok_or_else(|| { - Error::new(ErrorKind::NotFound, "Root directory has no parent") - })? - .parent(); + working_dir = working_dir //Keep going up in the directory structure with .parent() + .expect("This directory has no parent") + .parent(); } Ok(vars_to_add) diff --git a/crates/nu-cli/src/env/environment.rs b/crates/nu-cli/src/env/environment.rs index 41e3270a61..a291a32650 100644 --- a/crates/nu-cli/src/env/environment.rs +++ b/crates/nu-cli/src/env/environment.rs @@ -59,12 +59,9 @@ impl Environment { } pub fn maintain_directory_environment(&mut self) -> std::io::Result<()> { - self.direnv.env_vars_to_delete()?.iter().for_each(|k| { - self.remove_env(&k); - }); - self.direnv.env_vars_to_add()?.iter().for_each(|(k, v)| { - self.add_env(&k, &v, true); - }); + // self.direnv.env_vars_to_delete()?.iter().for_each(|k| { + // self.remove_env(&k); + // }); self.direnv .overwritten_values_to_restore()? @@ -73,6 +70,10 @@ impl Environment { self.add_env(&k, &v, true); }); + self.direnv.env_vars_to_add()?.iter().for_each(|(k, v)| { + self.add_env(&k, &v, true); + }); + Ok(()) } From 0beb32de52a06eda5693ef2d35b418a4dc597743 Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Sun, 14 Jun 2020 10:48:08 +0200 Subject: [PATCH 40/57] Overwritten env values tracks an indexmap instead of vector --- .../src/env/directory_specific_environment.rs | 57 ++++++++++++------- crates/nu-cli/src/env/environment.rs | 18 +++--- 2 files changed, 47 insertions(+), 28 deletions(-) diff --git a/crates/nu-cli/src/env/directory_specific_environment.rs b/crates/nu-cli/src/env/directory_specific_environment.rs index 16dcf2b9f0..9edc2f41c9 100644 --- a/crates/nu-cli/src/env/directory_specific_environment.rs +++ b/crates/nu-cli/src/env/directory_specific_environment.rs @@ -12,7 +12,7 @@ pub struct DirectorySpecificEnvironment { added_env_vars: IndexMap>, //Directory -> (env_key, value). If a .nu overwrites some existing environment variables, they are added here so that they can be restored later. - overwritten_env_values: IndexMap>, + overwritten_env_values: IndexMap>, } impl DirectorySpecificEnvironment { @@ -49,25 +49,35 @@ impl DirectorySpecificEnvironment { //If we are no longer in a directory, we restore the values it overwrote. pub fn overwritten_values_to_restore(&mut self) -> Result> { let current_dir = std::env::current_dir()?; - let working_dir = Some(current_dir.as_path()); + let mut working_dir = Some(current_dir.as_path()); - self - .overwritten_env_values - .iter() - .filter(|(directory, keyvals)| { - while let Some(wdir) = working_dir { - if &wdir == directory { - return false; - } else { - working_dir = working_dir.expect("This directory has no parent").parent(); - } - } - true - }) - .collect(); + let mut file = OpenOptions::new() + .write(true) + .append(true) + .create(true) + .open("restore.txt").unwrap(); + + write!(&mut file, "about to restore: {:?}\n", self.overwritten_env_values).unwrap(); let mut keyvals_to_restore = IndexMap::new(); + let mut new_overwritten_env_values = IndexMap::new(); + //If we are not in wdir or its subdir, remove its vals + self.overwritten_env_values + .iter() + .for_each(|(directory, keyvals)| { + while let Some(wdir) = working_dir { + if &wdir == directory { + keyvals.iter().for_each(|(k, v)| { + keyvals_to_restore.insert(k.clone(), v.to_str().unwrap().to_string()); + }); + } + working_dir = working_dir.expect("This directory has no parent").parent(); + } + new_overwritten_env_values.insert(directory.clone(), keyvals.clone()); + }); + + self.overwritten_env_values = new_overwritten_env_values; Ok(keyvals_to_restore) } @@ -81,7 +91,7 @@ impl DirectorySpecificEnvironment { //Start in the current directory, then traverse towards the root with working_dir to see if we are in a subdirectory of a valid directory. while let Some(wdir) = working_dir { if self.allowed_directories.contains(wdir) { - let toml_doc = std::fs::read_to_string(wdir.join(".nu").as_path()) + let toml_doc = std::fs::read_to_string(wdir.join(".nu-env").as_path()) .unwrap_or_else(|_| r#"[env]"#.to_string()) .parse::()?; @@ -99,8 +109,8 @@ impl DirectorySpecificEnvironment { if let Some(val) = std::env::var_os(k) { self.overwritten_env_values .entry(wdir.to_path_buf()) - .or_insert(vec![]) - .push((k.clone(), val)); + .or_insert(IndexMap::new()) + .insert(k.clone(), val); } else { self.added_env_vars .entry(wdir.to_path_buf()) @@ -110,6 +120,15 @@ impl DirectorySpecificEnvironment { } }); } + + let mut file = OpenOptions::new() + .write(true) + .append(true) + .create(true) + .open("restore.txt").unwrap(); + + write!(&mut file, "overwritten: {:?}\n\n", self.overwritten_env_values).unwrap(); + working_dir = working_dir //Keep going up in the directory structure with .parent() .expect("This directory has no parent") .parent(); diff --git a/crates/nu-cli/src/env/environment.rs b/crates/nu-cli/src/env/environment.rs index a291a32650..d2b1520531 100644 --- a/crates/nu-cli/src/env/environment.rs +++ b/crates/nu-cli/src/env/environment.rs @@ -59,16 +59,16 @@ impl Environment { } pub fn maintain_directory_environment(&mut self) -> std::io::Result<()> { - // self.direnv.env_vars_to_delete()?.iter().for_each(|k| { - // self.remove_env(&k); - // }); + self.direnv.env_vars_to_delete()?.iter().for_each(|k| { + self.remove_env(&k); + }); - self.direnv - .overwritten_values_to_restore()? - .iter() - .for_each(|(k, v)| { - self.add_env(&k, &v, true); - }); + // self.direnv + // .overwritten_values_to_restore()? + // .iter() + // .for_each(|(k, v)| { + // self.add_env(&k, &v, true); + // }); self.direnv.env_vars_to_add()?.iter().for_each(|(k, v)| { self.add_env(&k, &v, true); From 3fbbe6e322d6c5ae5dae2e8b9c35787cd4038a82 Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Sun, 14 Jun 2020 10:57:16 +0200 Subject: [PATCH 41/57] Refactor restore function --- .../src/env/directory_specific_environment.rs | 50 +++++++------------ crates/nu-cli/src/env/environment.rs | 23 ++++++--- 2 files changed, 34 insertions(+), 39 deletions(-) diff --git a/crates/nu-cli/src/env/directory_specific_environment.rs b/crates/nu-cli/src/env/directory_specific_environment.rs index 9edc2f41c9..c8513d7ddc 100644 --- a/crates/nu-cli/src/env/directory_specific_environment.rs +++ b/crates/nu-cli/src/env/directory_specific_environment.rs @@ -51,31 +51,25 @@ impl DirectorySpecificEnvironment { let current_dir = std::env::current_dir()?; let mut working_dir = Some(current_dir.as_path()); - - let mut file = OpenOptions::new() - .write(true) - .append(true) - .create(true) - .open("restore.txt").unwrap(); - - write!(&mut file, "about to restore: {:?}\n", self.overwritten_env_values).unwrap(); - + let mut new_overwritten_env_values: IndexMap> = + IndexMap::new(); let mut keyvals_to_restore = IndexMap::new(); - let mut new_overwritten_env_values = IndexMap::new(); - //If we are not in wdir or its subdir, remove its vals - self.overwritten_env_values - .iter() - .for_each(|(directory, keyvals)| { - while let Some(wdir) = working_dir { - if &wdir == directory { - keyvals.iter().for_each(|(k, v)| { - keyvals_to_restore.insert(k.clone(), v.to_str().unwrap().to_string()); - }); - } - working_dir = working_dir.expect("This directory has no parent").parent(); - } - new_overwritten_env_values.insert(directory.clone(), keyvals.clone()); - }); + + while let Some(wdir) = working_dir { + if let Some(val) = self.overwritten_env_values.get(wdir) { + new_overwritten_env_values.insert(wdir.to_path_buf(), val.clone()); + } + working_dir = working_dir.unwrap().parent(); + } + + for (dir, keyvals) in &self.overwritten_env_values { + if !new_overwritten_env_values.contains_key(dir) { + keyvals.iter().for_each(|(k, v)| { + keyvals_to_restore.insert(k.clone(), v.to_str().unwrap().to_string()); + }); + self.added_env_vars.remove(dir); + } + } self.overwritten_env_values = new_overwritten_env_values; Ok(keyvals_to_restore) @@ -121,14 +115,6 @@ impl DirectorySpecificEnvironment { }); } - let mut file = OpenOptions::new() - .write(true) - .append(true) - .create(true) - .open("restore.txt").unwrap(); - - write!(&mut file, "overwritten: {:?}\n\n", self.overwritten_env_values).unwrap(); - working_dir = working_dir //Keep going up in the directory structure with .parent() .expect("This directory has no parent") .parent(); diff --git a/crates/nu-cli/src/env/environment.rs b/crates/nu-cli/src/env/environment.rs index d2b1520531..b84cdcd93d 100644 --- a/crates/nu-cli/src/env/environment.rs +++ b/crates/nu-cli/src/env/environment.rs @@ -2,8 +2,9 @@ use crate::data::config::Conf; use crate::env::directory_specific_environment::*; use indexmap::{indexmap, IndexSet}; use nu_protocol::{UntaggedValue, Value}; +use std::io::Write; use std::ffi::OsString; -use std::fmt::Debug; +use std::{fs::OpenOptions, fmt::Debug}; pub trait Env: Debug + Send { fn env(&self) -> Option; @@ -63,12 +64,20 @@ impl Environment { self.remove_env(&k); }); - // self.direnv - // .overwritten_values_to_restore()? - // .iter() - // .for_each(|(k, v)| { - // self.add_env(&k, &v, true); - // }); + // let mut file = OpenOptions::new() + // .write(true) + // .append(true) + // .create(true) + // .open("restore.txt") + // .unwrap(); + + self.direnv + .overwritten_values_to_restore()? + .iter() + .for_each(|(k, v)| { + // write!(&mut file, "restoring: {:?}\n", (k, v)).unwrap(); + self.add_env(&k, &v, true); + }); self.direnv.env_vars_to_add()?.iter().for_each(|(k, v)| { self.add_env(&k, &v, true); From 97bad0e4deb931a9b1f124f7d527e601f429fd1a Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Sun, 14 Jun 2020 12:13:12 +0200 Subject: [PATCH 42/57] Untrack removed vars properly --- .../src/env/directory_specific_environment.rs | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/crates/nu-cli/src/env/directory_specific_environment.rs b/crates/nu-cli/src/env/directory_specific_environment.rs index c8513d7ddc..32e86f1041 100644 --- a/crates/nu-cli/src/env/directory_specific_environment.rs +++ b/crates/nu-cli/src/env/directory_specific_environment.rs @@ -9,7 +9,7 @@ pub struct DirectorySpecificEnvironment { allowed_directories: IndexSet, //Directory -> Env key. If an environment var has been added from a .nu in a directory, we track it here so we can remove it when the user leaves the directory. - added_env_vars: IndexMap>, + added_env_vars: IndexMap>, //Directory -> (env_key, value). If a .nu overwrites some existing environment variables, they are added here so that they can be restored later. overwritten_env_values: IndexMap>, @@ -51,8 +51,7 @@ impl DirectorySpecificEnvironment { let current_dir = std::env::current_dir()?; let mut working_dir = Some(current_dir.as_path()); - let mut new_overwritten_env_values: IndexMap> = - IndexMap::new(); + let mut new_overwritten_env_values = IndexMap::new(); let mut keyvals_to_restore = IndexMap::new(); while let Some(wdir) = working_dir { @@ -67,7 +66,6 @@ impl DirectorySpecificEnvironment { keyvals.iter().for_each(|(k, v)| { keyvals_to_restore.insert(k.clone(), v.to_str().unwrap().to_string()); }); - self.added_env_vars.remove(dir); } } @@ -105,11 +103,11 @@ impl DirectorySpecificEnvironment { .entry(wdir.to_path_buf()) .or_insert(IndexMap::new()) .insert(k.clone(), val); - } else { + } else { //Otherwise, we just track that we added it here self.added_env_vars .entry(wdir.to_path_buf()) - .or_insert(vec![]) - .push(k.clone()); + .or_insert(IndexSet::new()) + .insert(k.clone()); } } }); @@ -124,9 +122,11 @@ impl DirectorySpecificEnvironment { } //If the user has left directories which added env vars through .nu, we clear those vars + //once they are marked for deletion, remove them from added_env_vars pub fn env_vars_to_delete(&mut self) -> Result> { let current_dir = std::env::current_dir()?; + let mut new_added_env_vars: IndexMap> = IndexMap::new(); //Gather up all environment variables that should be deleted. //If we are not in a directory or one of its subdirectories, mark the env_vals it maps to for removal. let vars_to_delete = self.added_env_vars.iter().fold( @@ -136,6 +136,9 @@ impl DirectorySpecificEnvironment { while let Some(wdir) = working_dir { if wdir == directory { + if let Some(vars_added_by_this_directory) = self.added_env_vars.get(wdir) { //If we are still in a directory, we should continue to track the vars it added. + new_added_env_vars.insert(wdir.to_path_buf(), vars_added_by_this_directory.clone()); + } return vars_to_delete; } else { working_dir = working_dir.expect("Root directory has no parent").parent(); @@ -146,7 +149,7 @@ impl DirectorySpecificEnvironment { vars_to_delete }, ); - + self.added_env_vars = new_added_env_vars; Ok(vars_to_delete) } } From b5923d0cd3b822c381f22bc9043be76dd5527828 Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Sun, 14 Jun 2020 14:56:11 +0200 Subject: [PATCH 43/57] Performance concerns --- .../src/env/directory_specific_environment.rs | 48 +++++++++---------- 1 file changed, 23 insertions(+), 25 deletions(-) diff --git a/crates/nu-cli/src/env/directory_specific_environment.rs b/crates/nu-cli/src/env/directory_specific_environment.rs index 32e86f1041..5e7cc6638f 100644 --- a/crates/nu-cli/src/env/directory_specific_environment.rs +++ b/crates/nu-cli/src/env/directory_specific_environment.rs @@ -84,7 +84,7 @@ impl DirectorySpecificEnvironment { while let Some(wdir) = working_dir { if self.allowed_directories.contains(wdir) { let toml_doc = std::fs::read_to_string(wdir.join(".nu-env").as_path()) - .unwrap_or_else(|_| r#"[env]"#.to_string()) + .unwrap_or_else(|_| "[env]".to_string()) .parse::()?; toml_doc @@ -101,12 +101,13 @@ impl DirectorySpecificEnvironment { if let Some(val) = std::env::var_os(k) { self.overwritten_env_values .entry(wdir.to_path_buf()) - .or_insert(IndexMap::new()) + .or_insert_with(|| IndexMap::new()) .insert(k.clone(), val); - } else { //Otherwise, we just track that we added it here + } else { + //Otherwise, we just track that we added it here self.added_env_vars .entry(wdir.to_path_buf()) - .or_insert(IndexSet::new()) + .or_insert_with(|| IndexSet::new()) .insert(k.clone()); } } @@ -126,29 +127,26 @@ impl DirectorySpecificEnvironment { pub fn env_vars_to_delete(&mut self) -> Result> { let current_dir = std::env::current_dir()?; - let mut new_added_env_vars: IndexMap> = IndexMap::new(); - //Gather up all environment variables that should be deleted. - //If we are not in a directory or one of its subdirectories, mark the env_vals it maps to for removal. - let vars_to_delete = self.added_env_vars.iter().fold( - Vec::new(), - |mut vars_to_delete, (directory, env_vars)| { - let mut working_dir = Some(current_dir.as_path()); + let mut new_added_env_vars = IndexMap::new(); + let mut working_dir = Some(current_dir.as_path()); - while let Some(wdir) = working_dir { - if wdir == directory { - if let Some(vars_added_by_this_directory) = self.added_env_vars.get(wdir) { //If we are still in a directory, we should continue to track the vars it added. - new_added_env_vars.insert(wdir.to_path_buf(), vars_added_by_this_directory.clone()); - } - return vars_to_delete; - } else { - working_dir = working_dir.expect("Root directory has no parent").parent(); - } + while let Some(wdir) = working_dir { + if let Some(vars_added_by_this_directory) = self.added_env_vars.get(wdir) { + //If we are still in a directory, we should continue to track the vars it added. + new_added_env_vars.insert(wdir.to_path_buf(), vars_added_by_this_directory.clone()); + } + working_dir = working_dir.expect("Root directory has no parent").parent(); + } + + //Gather up all environment variables that should be deleted. + let mut vars_to_delete = vec![]; + self.added_env_vars.iter().for_each(|(dir, added_keys)| { + if !new_added_env_vars.contains_key(dir) { + for k in added_keys { + vars_to_delete.push(k.clone()); } - //only delete vars from directories we are not in - vars_to_delete.extend(env_vars.clone()); - vars_to_delete - }, - ); + } + }); self.added_env_vars = new_added_env_vars; Ok(vars_to_delete) } From 309f5e02a02c2214a09fa274c558cf3b59bae044 Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Sun, 14 Jun 2020 14:56:11 +0200 Subject: [PATCH 44/57] Performance concerns --- .../src/env/directory_specific_environment.rs | 52 +++++++++---------- 1 file changed, 24 insertions(+), 28 deletions(-) diff --git a/crates/nu-cli/src/env/directory_specific_environment.rs b/crates/nu-cli/src/env/directory_specific_environment.rs index 32e86f1041..26a0059b2f 100644 --- a/crates/nu-cli/src/env/directory_specific_environment.rs +++ b/crates/nu-cli/src/env/directory_specific_environment.rs @@ -9,7 +9,7 @@ pub struct DirectorySpecificEnvironment { allowed_directories: IndexSet, //Directory -> Env key. If an environment var has been added from a .nu in a directory, we track it here so we can remove it when the user leaves the directory. - added_env_vars: IndexMap>, + added_env_vars: IndexMap>, //Directory -> (env_key, value). If a .nu overwrites some existing environment variables, they are added here so that they can be restored later. overwritten_env_values: IndexMap>, @@ -84,7 +84,7 @@ impl DirectorySpecificEnvironment { while let Some(wdir) = working_dir { if self.allowed_directories.contains(wdir) { let toml_doc = std::fs::read_to_string(wdir.join(".nu-env").as_path()) - .unwrap_or_else(|_| r#"[env]"#.to_string()) + .unwrap_or_else(|_| "[env]".to_string()) .parse::()?; toml_doc @@ -101,13 +101,14 @@ impl DirectorySpecificEnvironment { if let Some(val) = std::env::var_os(k) { self.overwritten_env_values .entry(wdir.to_path_buf()) - .or_insert(IndexMap::new()) + .or_insert_with(|| IndexMap::new()) .insert(k.clone(), val); - } else { //Otherwise, we just track that we added it here + } else { + //Otherwise, we just track that we added it here self.added_env_vars .entry(wdir.to_path_buf()) - .or_insert(IndexSet::new()) - .insert(k.clone()); + .or_insert_with(|| vec![]) + .push(k.clone()); } } }); @@ -126,29 +127,24 @@ impl DirectorySpecificEnvironment { pub fn env_vars_to_delete(&mut self) -> Result> { let current_dir = std::env::current_dir()?; - let mut new_added_env_vars: IndexMap> = IndexMap::new(); - //Gather up all environment variables that should be deleted. - //If we are not in a directory or one of its subdirectories, mark the env_vals it maps to for removal. - let vars_to_delete = self.added_env_vars.iter().fold( - Vec::new(), - |mut vars_to_delete, (directory, env_vars)| { - let mut working_dir = Some(current_dir.as_path()); + let mut new_added_env_vars = IndexMap::new(); + let mut working_dir = Some(current_dir.as_path()); - while let Some(wdir) = working_dir { - if wdir == directory { - if let Some(vars_added_by_this_directory) = self.added_env_vars.get(wdir) { //If we are still in a directory, we should continue to track the vars it added. - new_added_env_vars.insert(wdir.to_path_buf(), vars_added_by_this_directory.clone()); - } - return vars_to_delete; - } else { - working_dir = working_dir.expect("Root directory has no parent").parent(); - } - } - //only delete vars from directories we are not in - vars_to_delete.extend(env_vars.clone()); - vars_to_delete - }, - ); + while let Some(wdir) = working_dir { + if let Some(vars_added_by_this_directory) = self.added_env_vars.get(wdir) { + new_added_env_vars.insert(wdir.to_path_buf(), vars_added_by_this_directory.clone()); + //If we are still in a directory, we should continue to track the vars it added. + } + working_dir = working_dir.expect("Root directory has no parent").parent(); + } + + //Gather up all environment variables that should be deleted. + let mut vars_to_delete = vec![]; + for (dir, added_keys) in &self.added_env_vars { + if !new_added_env_vars.contains_key(dir) { + vars_to_delete.extend(added_keys.clone()); + } + } self.added_env_vars = new_added_env_vars; Ok(vars_to_delete) } From 730f9ca30a0434ea0d43083a9a67e511db808e14 Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Mon, 15 Jun 2020 15:42:09 +0200 Subject: [PATCH 45/57] Error handling --- .../src/env/directory_specific_environment.rs | 39 +++++++++++-------- crates/nu-cli/src/env/environment.rs | 11 +----- 2 files changed, 23 insertions(+), 27 deletions(-) diff --git a/crates/nu-cli/src/env/directory_specific_environment.rs b/crates/nu-cli/src/env/directory_specific_environment.rs index 26a0059b2f..bbacc0bc0f 100644 --- a/crates/nu-cli/src/env/directory_specific_environment.rs +++ b/crates/nu-cli/src/env/directory_specific_environment.rs @@ -1,8 +1,11 @@ use indexmap::{IndexMap, IndexSet}; use nu_protocol::{Primitive, UntaggedValue, Value}; -use std::io::Write; -use std::io::{Error, ErrorKind, Result}; -use std::{ffi::OsString, fmt::Debug, fs::OpenOptions, path::PathBuf}; +use std::{ + ffi::OsString, + fmt::Debug, + io::{Error, ErrorKind, Result}, + path::PathBuf, +}; #[derive(Debug, Default)] pub struct DirectorySpecificEnvironment { @@ -12,7 +15,7 @@ pub struct DirectorySpecificEnvironment { added_env_vars: IndexMap>, //Directory -> (env_key, value). If a .nu overwrites some existing environment variables, they are added here so that they can be restored later. - overwritten_env_values: IndexMap>, + overwritten_env_vars: IndexMap>, } impl DirectorySpecificEnvironment { @@ -42,7 +45,7 @@ impl DirectorySpecificEnvironment { DirectorySpecificEnvironment { allowed_directories, added_env_vars: IndexMap::new(), - overwritten_env_values: IndexMap::new(), + overwritten_env_vars: IndexMap::new(), } } @@ -55,13 +58,15 @@ impl DirectorySpecificEnvironment { let mut keyvals_to_restore = IndexMap::new(); while let Some(wdir) = working_dir { - if let Some(val) = self.overwritten_env_values.get(wdir) { + if let Some(val) = self.overwritten_env_vars.get(wdir) { new_overwritten_env_values.insert(wdir.to_path_buf(), val.clone()); } - working_dir = working_dir.unwrap().parent(); + working_dir = working_dir + .expect("This should not be None because of the while condition") + .parent(); } - for (dir, keyvals) in &self.overwritten_env_values { + for (dir, keyvals) in &self.overwritten_env_vars { if !new_overwritten_env_values.contains_key(dir) { keyvals.iter().for_each(|(k, v)| { keyvals_to_restore.insert(k.clone(), v.to_str().unwrap().to_string()); @@ -69,7 +74,7 @@ impl DirectorySpecificEnvironment { } } - self.overwritten_env_values = new_overwritten_env_values; + self.overwritten_env_vars = new_overwritten_env_values; Ok(keyvals_to_restore) } @@ -77,7 +82,6 @@ impl DirectorySpecificEnvironment { let current_dir = std::env::current_dir()?; let mut working_dir = Some(current_dir.as_path()); - let empty = toml::value::Table::new(); let mut vars_to_add = IndexMap::new(); //Start in the current directory, then traverse towards the root with working_dir to see if we are in a subdirectory of a valid directory. @@ -89,9 +93,9 @@ impl DirectorySpecificEnvironment { toml_doc .get("env") - .unwrap() + .ok_or_else(|| Error::new(ErrorKind::InvalidData, "env section missing"))? .as_table() - .unwrap_or_else(|| &empty) + .ok_or_else(|| Error::new(ErrorKind::InvalidData, "env section malformed"))? .iter() .for_each(|(k, v)| { if !vars_to_add.contains_key(k) { @@ -99,7 +103,7 @@ impl DirectorySpecificEnvironment { //If we are about to overwrite any environment variables, we save them first so they can be restored later. if let Some(val) = std::env::var_os(k) { - self.overwritten_env_values + self.overwritten_env_vars .entry(wdir.to_path_buf()) .or_insert_with(|| IndexMap::new()) .insert(k.clone(), val); @@ -113,9 +117,8 @@ impl DirectorySpecificEnvironment { } }); } - working_dir = working_dir //Keep going up in the directory structure with .parent() - .expect("This directory has no parent") + .expect("This should not be None because of the while condition") .parent(); } @@ -132,10 +135,12 @@ impl DirectorySpecificEnvironment { while let Some(wdir) = working_dir { if let Some(vars_added_by_this_directory) = self.added_env_vars.get(wdir) { - new_added_env_vars.insert(wdir.to_path_buf(), vars_added_by_this_directory.clone()); //If we are still in a directory, we should continue to track the vars it added. + new_added_env_vars.insert(wdir.to_path_buf(), vars_added_by_this_directory.clone()); } - working_dir = working_dir.expect("Root directory has no parent").parent(); + working_dir = working_dir + .expect("This should not be None because of the while condition") + .parent(); } //Gather up all environment variables that should be deleted. diff --git a/crates/nu-cli/src/env/environment.rs b/crates/nu-cli/src/env/environment.rs index b84cdcd93d..86b6b82d13 100644 --- a/crates/nu-cli/src/env/environment.rs +++ b/crates/nu-cli/src/env/environment.rs @@ -2,9 +2,8 @@ use crate::data::config::Conf; use crate::env::directory_specific_environment::*; use indexmap::{indexmap, IndexSet}; use nu_protocol::{UntaggedValue, Value}; -use std::io::Write; use std::ffi::OsString; -use std::{fs::OpenOptions, fmt::Debug}; +use std::fmt::Debug; pub trait Env: Debug + Send { fn env(&self) -> Option; @@ -64,18 +63,10 @@ impl Environment { self.remove_env(&k); }); - // let mut file = OpenOptions::new() - // .write(true) - // .append(true) - // .create(true) - // .open("restore.txt") - // .unwrap(); - self.direnv .overwritten_values_to_restore()? .iter() .for_each(|(k, v)| { - // write!(&mut file, "restoring: {:?}\n", (k, v)).unwrap(); self.add_env(&k, &v, true); }); From 26ec9cf4322b0e542db20bb27b1ec25b542c205c Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Mon, 15 Jun 2020 15:48:14 +0200 Subject: [PATCH 46/57] Clippy --- crates/nu-cli/src/env/directory_specific_environment.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/nu-cli/src/env/directory_specific_environment.rs b/crates/nu-cli/src/env/directory_specific_environment.rs index bbacc0bc0f..b11bca1841 100644 --- a/crates/nu-cli/src/env/directory_specific_environment.rs +++ b/crates/nu-cli/src/env/directory_specific_environment.rs @@ -105,13 +105,13 @@ impl DirectorySpecificEnvironment { if let Some(val) = std::env::var_os(k) { self.overwritten_env_vars .entry(wdir.to_path_buf()) - .or_insert_with(|| IndexMap::new()) + .or_insert(IndexMap::new()) .insert(k.clone(), val); } else { //Otherwise, we just track that we added it here self.added_env_vars .entry(wdir.to_path_buf()) - .or_insert_with(|| vec![]) + .or_insert(vec![]) .push(k.clone()); } } From 6a0b4d112255322c86f35fac406565c58f7b1272 Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Mon, 15 Jun 2020 16:24:29 +0200 Subject: [PATCH 47/57] Add type aliases for String and OsString --- .../src/env/directory_specific_environment.rs | 45 ++++++++++++------- crates/nu-cli/src/env/environment.rs | 27 +++++++---- 2 files changed, 47 insertions(+), 25 deletions(-) diff --git a/crates/nu-cli/src/env/directory_specific_environment.rs b/crates/nu-cli/src/env/directory_specific_environment.rs index b11bca1841..a0e677cc0d 100644 --- a/crates/nu-cli/src/env/directory_specific_environment.rs +++ b/crates/nu-cli/src/env/directory_specific_environment.rs @@ -1,21 +1,25 @@ use indexmap::{IndexMap, IndexSet}; use nu_protocol::{Primitive, UntaggedValue, Value}; +use std::io::Write; use std::{ ffi::OsString, fmt::Debug, + fs::OpenOptions, io::{Error, ErrorKind, Result}, path::PathBuf, }; +type EnvKey = String; +type EnvVal = OsString; #[derive(Debug, Default)] pub struct DirectorySpecificEnvironment { allowed_directories: IndexSet, //Directory -> Env key. If an environment var has been added from a .nu in a directory, we track it here so we can remove it when the user leaves the directory. - added_env_vars: IndexMap>, + added_env_vars: IndexMap>, //Directory -> (env_key, value). If a .nu overwrites some existing environment variables, they are added here so that they can be restored later. - overwritten_env_vars: IndexMap>, + overwritten_env_vars: IndexMap>, } impl DirectorySpecificEnvironment { @@ -50,7 +54,7 @@ impl DirectorySpecificEnvironment { } //If we are no longer in a directory, we restore the values it overwrote. - pub fn overwritten_values_to_restore(&mut self) -> Result> { + pub fn overwritten_values_to_restore(&mut self) -> Result> { let current_dir = std::env::current_dir()?; let mut working_dir = Some(current_dir.as_path()); @@ -69,7 +73,7 @@ impl DirectorySpecificEnvironment { for (dir, keyvals) in &self.overwritten_env_vars { if !new_overwritten_env_values.contains_key(dir) { keyvals.iter().for_each(|(k, v)| { - keyvals_to_restore.insert(k.clone(), v.to_str().unwrap().to_string()); + keyvals_to_restore.insert(k.clone(), v.clone()); }); } } @@ -78,7 +82,7 @@ impl DirectorySpecificEnvironment { Ok(keyvals_to_restore) } - pub fn env_vars_to_add(&mut self) -> Result> { + pub fn env_vars_to_add(&mut self) -> Result> { let current_dir = std::env::current_dir()?; let mut working_dir = Some(current_dir.as_path()); @@ -97,22 +101,29 @@ impl DirectorySpecificEnvironment { .as_table() .ok_or_else(|| Error::new(ErrorKind::InvalidData, "env section malformed"))? .iter() - .for_each(|(k, v)| { - if !vars_to_add.contains_key(k) { - vars_to_add.insert(k.clone(), v.as_str().unwrap().to_string()); + .for_each(|(env_key, directory_env_val)| { + if !vars_to_add.contains_key(env_key) { + let directory_env_val: EnvVal = + directory_env_val.as_str().unwrap().into(); //If we are about to overwrite any environment variables, we save them first so they can be restored later. - if let Some(val) = std::env::var_os(k) { - self.overwritten_env_vars - .entry(wdir.to_path_buf()) - .or_insert(IndexMap::new()) - .insert(k.clone(), val); + if let Some(existing_val) = std::env::var_os(env_key) { + if existing_val != directory_env_val { + self.overwritten_env_vars + .entry(wdir.to_path_buf()) + .or_insert(IndexMap::new()) + .insert(env_key.clone(), existing_val); + + vars_to_add.insert(env_key.clone(), directory_env_val); + } } else { //Otherwise, we just track that we added it here self.added_env_vars .entry(wdir.to_path_buf()) .or_insert(vec![]) - .push(k.clone()); + .push(env_key.clone()); + + vars_to_add.insert(env_key.clone(), directory_env_val); } } }); @@ -127,9 +138,8 @@ impl DirectorySpecificEnvironment { //If the user has left directories which added env vars through .nu, we clear those vars //once they are marked for deletion, remove them from added_env_vars - pub fn env_vars_to_delete(&mut self) -> Result> { + pub fn env_vars_to_delete(&mut self) -> Result> { let current_dir = std::env::current_dir()?; - let mut new_added_env_vars = IndexMap::new(); let mut working_dir = Some(current_dir.as_path()); @@ -143,7 +153,7 @@ impl DirectorySpecificEnvironment { .parent(); } - //Gather up all environment variables that should be deleted. + // Gather up all environment variables that should be deleted. let mut vars_to_delete = vec![]; for (dir, added_keys) in &self.added_env_vars { if !new_added_env_vars.contains_key(dir) { @@ -151,6 +161,7 @@ impl DirectorySpecificEnvironment { } } self.added_env_vars = new_added_env_vars; + Ok(vars_to_delete) } } diff --git a/crates/nu-cli/src/env/environment.rs b/crates/nu-cli/src/env/environment.rs index 86b6b82d13..e7a60368e9 100644 --- a/crates/nu-cli/src/env/environment.rs +++ b/crates/nu-cli/src/env/environment.rs @@ -1,9 +1,10 @@ use crate::data::config::Conf; +use std::io::Write; use crate::env::directory_specific_environment::*; use indexmap::{indexmap, IndexSet}; use nu_protocol::{UntaggedValue, Value}; use std::ffi::OsString; -use std::fmt::Debug; +use std::{fs::OpenOptions, fmt::Debug}; pub trait Env: Debug + Send { fn env(&self) -> Option; @@ -63,15 +64,15 @@ impl Environment { self.remove_env(&k); }); - self.direnv - .overwritten_values_to_restore()? - .iter() - .for_each(|(k, v)| { - self.add_env(&k, &v, true); - }); + // self.direnv + // .overwritten_values_to_restore()? + // .iter() + // .for_each(|(k, v)| { + // self.add_env(&k, &v.to_string_lossy(), true); + // }); self.direnv.env_vars_to_add()?.iter().for_each(|(k, v)| { - self.add_env(&k, &v, true); + self.add_env(&k, &v.to_string_lossy(), true); }); Ok(()) @@ -83,7 +84,17 @@ impl Environment { tag: _, }) = self.environment_vars { + + let mut file = OpenOptions::new() + .write(true) + .append(true) + .create(true) + .open("delete.txt") + .unwrap(); + + write!(&mut file, "deleting: {:?}\n", key).unwrap(); envs.entries.remove(key); + std::env::remove_var(key); }; } From 28f01e92b79f13ac0970ca8bb4254454d006e052 Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Mon, 15 Jun 2020 18:21:27 +0200 Subject: [PATCH 48/57] Deletion almost works --- crates/nu-cli/src/env/environment.rs | 20 ++++++-------------- crates/nu-cli/src/env/environment_syncer.rs | 2 +- 2 files changed, 7 insertions(+), 15 deletions(-) diff --git a/crates/nu-cli/src/env/environment.rs b/crates/nu-cli/src/env/environment.rs index e7a60368e9..15f39a6755 100644 --- a/crates/nu-cli/src/env/environment.rs +++ b/crates/nu-cli/src/env/environment.rs @@ -64,12 +64,12 @@ impl Environment { self.remove_env(&k); }); - // self.direnv - // .overwritten_values_to_restore()? - // .iter() - // .for_each(|(k, v)| { - // self.add_env(&k, &v.to_string_lossy(), true); - // }); + self.direnv + .overwritten_values_to_restore()? + .iter() + .for_each(|(k, v)| { + self.add_env(&k, &v.to_string_lossy(), true); + }); self.direnv.env_vars_to_add()?.iter().for_each(|(k, v)| { self.add_env(&k, &v.to_string_lossy(), true); @@ -85,14 +85,6 @@ impl Environment { }) = self.environment_vars { - let mut file = OpenOptions::new() - .write(true) - .append(true) - .create(true) - .open("delete.txt") - .unwrap(); - - write!(&mut file, "deleting: {:?}\n", key).unwrap(); envs.entries.remove(key); std::env::remove_var(key); }; diff --git a/crates/nu-cli/src/env/environment_syncer.rs b/crates/nu-cli/src/env/environment_syncer.rs index fbcd72e6a6..7c378ea47e 100644 --- a/crates/nu-cli/src/env/environment_syncer.rs +++ b/crates/nu-cli/src/env/environment_syncer.rs @@ -44,6 +44,7 @@ impl EnvironmentSyncer { pub fn sync_env_vars(&mut self, ctx: &mut Context) { let mut environment = self.env.lock(); + environment.maintain_directory_environment().ok(); if environment.env().is_some() { for (name, value) in ctx.with_host(|host| host.vars()) { if name != "path" && name != "PATH" { @@ -51,7 +52,6 @@ impl EnvironmentSyncer { // that aren't loaded from config. environment.add_env(&name, &value, false); - environment.maintain_directory_environment().ok(); // clear the env var from the session // we are about to replace them From 06b11225641a911095166d291b43842a3c5c7ab3 Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Mon, 15 Jun 2020 18:31:04 +0200 Subject: [PATCH 49/57] Working? --- crates/nu-cli/src/env/environment.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/nu-cli/src/env/environment.rs b/crates/nu-cli/src/env/environment.rs index 15f39a6755..0cca777255 100644 --- a/crates/nu-cli/src/env/environment.rs +++ b/crates/nu-cli/src/env/environment.rs @@ -84,7 +84,6 @@ impl Environment { tag: _, }) = self.environment_vars { - envs.entries.remove(key); std::env::remove_var(key); }; From 4e0a0df8a89e37c94b4ee2e04dcda8ac74589c75 Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Mon, 15 Jun 2020 21:38:33 +0200 Subject: [PATCH 50/57] Error handling and refactoring --- .../src/env/directory_specific_environment.rs | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/crates/nu-cli/src/env/directory_specific_environment.rs b/crates/nu-cli/src/env/directory_specific_environment.rs index a0e677cc0d..fd25a284a0 100644 --- a/crates/nu-cli/src/env/directory_specific_environment.rs +++ b/crates/nu-cli/src/env/directory_specific_environment.rs @@ -16,7 +16,7 @@ pub struct DirectorySpecificEnvironment { allowed_directories: IndexSet, //Directory -> Env key. If an environment var has been added from a .nu in a directory, we track it here so we can remove it when the user leaves the directory. - added_env_vars: IndexMap>, + added_env_vars: IndexMap>, //Directory -> (env_key, value). If a .nu overwrites some existing environment variables, they are added here so that they can be restored later. overwritten_env_vars: IndexMap>, @@ -91,8 +91,7 @@ impl DirectorySpecificEnvironment { //Start in the current directory, then traverse towards the root with working_dir to see if we are in a subdirectory of a valid directory. while let Some(wdir) = working_dir { if self.allowed_directories.contains(wdir) { - let toml_doc = std::fs::read_to_string(wdir.join(".nu-env").as_path()) - .unwrap_or_else(|_| "[env]".to_string()) + let toml_doc = std::fs::read_to_string(wdir.join(".nu-env").as_path())? .parse::()?; toml_doc @@ -101,29 +100,29 @@ impl DirectorySpecificEnvironment { .as_table() .ok_or_else(|| Error::new(ErrorKind::InvalidData, "env section malformed"))? .iter() - .for_each(|(env_key, directory_env_val)| { - if !vars_to_add.contains_key(env_key) { + .for_each(|(directory_env_key, directory_env_val)| { + if !vars_to_add.contains_key(directory_env_key) { let directory_env_val: EnvVal = directory_env_val.as_str().unwrap().into(); //If we are about to overwrite any environment variables, we save them first so they can be restored later. - if let Some(existing_val) = std::env::var_os(env_key) { + if let Some(existing_val) = std::env::var_os(directory_env_key) { if existing_val != directory_env_val { self.overwritten_env_vars .entry(wdir.to_path_buf()) .or_insert(IndexMap::new()) - .insert(env_key.clone(), existing_val); + .insert(directory_env_key.clone(), existing_val); - vars_to_add.insert(env_key.clone(), directory_env_val); + vars_to_add.insert(directory_env_key.clone(), directory_env_val); } } else { //Otherwise, we just track that we added it here self.added_env_vars .entry(wdir.to_path_buf()) - .or_insert(vec![]) - .push(env_key.clone()); + .or_insert(IndexSet::new()) + .insert(directory_env_key.clone()); - vars_to_add.insert(env_key.clone(), directory_env_val); + vars_to_add.insert(directory_env_key.clone(), directory_env_val); } } }); @@ -138,7 +137,7 @@ impl DirectorySpecificEnvironment { //If the user has left directories which added env vars through .nu, we clear those vars //once they are marked for deletion, remove them from added_env_vars - pub fn env_vars_to_delete(&mut self) -> Result> { + pub fn env_vars_to_delete(&mut self) -> Result> { let current_dir = std::env::current_dir()?; let mut new_added_env_vars = IndexMap::new(); let mut working_dir = Some(current_dir.as_path()); @@ -154,12 +153,15 @@ impl DirectorySpecificEnvironment { } // Gather up all environment variables that should be deleted. - let mut vars_to_delete = vec![]; + let mut vars_to_delete = IndexSet::new(); for (dir, added_keys) in &self.added_env_vars { if !new_added_env_vars.contains_key(dir) { - vars_to_delete.extend(added_keys.clone()); + for k in added_keys { + vars_to_delete.insert(k.clone()); + } } } + self.added_env_vars = new_added_env_vars; Ok(vars_to_delete) From 2f4f8632ded0a5f71b7304845496d9d3ad5bbe70 Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Tue, 16 Jun 2020 11:01:11 +0200 Subject: [PATCH 51/57] nicer errors --- .../src/env/directory_specific_environment.rs | 18 ++++++++++--- crates/nu-cli/src/env/environment.rs | 25 ++++++++++--------- crates/nu-cli/src/env/environment_syncer.rs | 6 +++-- 3 files changed, 32 insertions(+), 17 deletions(-) diff --git a/crates/nu-cli/src/env/directory_specific_environment.rs b/crates/nu-cli/src/env/directory_specific_environment.rs index fd25a284a0..2b4a6b01b0 100644 --- a/crates/nu-cli/src/env/directory_specific_environment.rs +++ b/crates/nu-cli/src/env/directory_specific_environment.rs @@ -8,6 +8,7 @@ use std::{ io::{Error, ErrorKind, Result}, path::PathBuf, }; +use nu_errors::ShellError; type EnvKey = String; type EnvVal = OsString; @@ -82,12 +83,23 @@ impl DirectorySpecificEnvironment { Ok(keyvals_to_restore) } - pub fn env_vars_to_add(&mut self) -> Result> { + pub fn env_vars_to_add(&mut self) -> std::result::Result, ShellError> { let current_dir = std::env::current_dir()?; let mut working_dir = Some(current_dir.as_path()); let mut vars_to_add = IndexMap::new(); + // let mut file = OpenOptions::new() + // .write(true) + // .append(true) + // .create(true) + // .open("toadd.txt") + // .unwrap( + // ); + + // write!(&mut file, "1: {:?}\n", vars_to_add).unwrap(); + //WE CRASHING SOMEWHERE HERE + //Start in the current directory, then traverse towards the root with working_dir to see if we are in a subdirectory of a valid directory. while let Some(wdir) = working_dir { if self.allowed_directories.contains(wdir) { @@ -96,9 +108,9 @@ impl DirectorySpecificEnvironment { toml_doc .get("env") - .ok_or_else(|| Error::new(ErrorKind::InvalidData, "env section missing"))? + .ok_or_else(|| Err(ShellError::untagged_runtime_error("env section missing")))? .as_table() - .ok_or_else(|| Error::new(ErrorKind::InvalidData, "env section malformed"))? + .ok_or_else(|| Err(ShellError::untagged_runtime_error("env section malformed")))? .iter() .for_each(|(directory_env_key, directory_env_val)| { if !vars_to_add.contains_key(directory_env_key) { diff --git a/crates/nu-cli/src/env/environment.rs b/crates/nu-cli/src/env/environment.rs index 0cca777255..d1fb49fb72 100644 --- a/crates/nu-cli/src/env/environment.rs +++ b/crates/nu-cli/src/env/environment.rs @@ -60,20 +60,21 @@ impl Environment { } pub fn maintain_directory_environment(&mut self) -> std::io::Result<()> { - self.direnv.env_vars_to_delete()?.iter().for_each(|k| { - self.remove_env(&k); - }); + // self.direnv.env_vars_to_delete()?.iter().for_each(|k| { + // self.remove_env(&k); + // }); - self.direnv - .overwritten_values_to_restore()? - .iter() - .for_each(|(k, v)| { - self.add_env(&k, &v.to_string_lossy(), true); - }); + // self.direnv + // .overwritten_values_to_restore()? + // .iter() + // .for_each(|(k, v)| { + // self.add_env(&k, &v.to_string_lossy(), true); + // }); - self.direnv.env_vars_to_add()?.iter().for_each(|(k, v)| { - self.add_env(&k, &v.to_string_lossy(), true); - }); + // self.direnv.env_vars_to_add()?.iter().for_each(|(k, v)| { + // std::env::set_var(k, v); + // self.add_env(&k, &v.to_string_lossy(), true); + // }); Ok(()) } diff --git a/crates/nu-cli/src/env/environment_syncer.rs b/crates/nu-cli/src/env/environment_syncer.rs index 7c378ea47e..195a733c4a 100644 --- a/crates/nu-cli/src/env/environment_syncer.rs +++ b/crates/nu-cli/src/env/environment_syncer.rs @@ -3,6 +3,7 @@ use crate::data::config::{Conf, NuConfig}; use crate::env::environment::{Env, Environment}; use parking_lot::Mutex; use std::sync::Arc; +use nu_errors::ShellError; pub struct EnvironmentSyncer { pub env: Arc>>, @@ -41,10 +42,9 @@ impl EnvironmentSyncer { environment.morph(&*self.config); } - pub fn sync_env_vars(&mut self, ctx: &mut Context) { + pub fn sync_env_vars(&mut self, ctx: &mut Context) -> Result<(), ShellError> { let mut environment = self.env.lock(); - environment.maintain_directory_environment().ok(); if environment.env().is_some() { for (name, value) in ctx.with_host(|host| host.vars()) { if name != "path" && name != "PATH" { @@ -52,6 +52,7 @@ impl EnvironmentSyncer { // that aren't loaded from config. environment.add_env(&name, &value, false); + environment.maintain_directory_environment()?; // clear the env var from the session // we are about to replace them @@ -72,6 +73,7 @@ impl EnvironmentSyncer { } } } + Ok(()) } pub fn sync_path_vars(&mut self, ctx: &mut Context) { From fb1839971a2cd0a82a72fdb52d8023a8d94aa777 Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Tue, 16 Jun 2020 19:20:04 +0200 Subject: [PATCH 52/57] Add TODO file --- crates/nu-cli/src/env/TODO.org | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) create mode 100644 crates/nu-cli/src/env/TODO.org diff --git a/crates/nu-cli/src/env/TODO.org b/crates/nu-cli/src/env/TODO.org new file mode 100644 index 0000000000..1b42c89e7d --- /dev/null +++ b/crates/nu-cli/src/env/TODO.org @@ -0,0 +1,22 @@ +* TODO +** Run even if =env()= returns None +It happens if there is no env-section in .config/nu/config.toml. +This results in +#+begin_src rust +impl Env for Environment { + fn env(&self) -> Option { + if let Some(vars) = &self.environment_vars { + return Some(vars.clone()); + } + + None + } +#+end_src +returning =None=, which completely skips running the code for dealing with directory specific environment variables. +** Confirm intended behavior + - If you are in a directory, or a subdirectory to a directory with a .nu-file, the vars in that .nu-file are applied. + - If you leave a directory which set some variables, the variables are unset. + - If a directory contains a .nu with an environment variable that was already set, the old value will be overwritten. + - This holds even if the old value was set by a .nu in a parent directory. The overwritten value is restored when you leave the directory. +** Security + https://github.com/nushell/nushell/issues/1965 \ No newline at end of file From 688df20a30cdb0e57d9c2736792671d085b45260 Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Wed, 17 Jun 2020 00:02:21 +0200 Subject: [PATCH 53/57] Error handling --- crates/nu-cli/src/cli.rs | 4 +- crates/nu-cli/src/env/TODO.org | 3 +- .../src/env/directory_specific_environment.rs | 69 +++++++++---------- crates/nu-cli/src/env/environment.rs | 9 +-- crates/nu-cli/src/env/environment_syncer.rs | 3 +- 5 files changed, 43 insertions(+), 45 deletions(-) diff --git a/crates/nu-cli/src/cli.rs b/crates/nu-cli/src/cli.rs index bd5097812e..07ecbe620c 100644 --- a/crates/nu-cli/src/cli.rs +++ b/crates/nu-cli/src/cli.rs @@ -237,7 +237,7 @@ pub fn create_default_context( syncer.load_environment(); let mut context = Context::basic()?; - syncer.sync_env_vars(&mut context); + syncer.sync_env_vars(&mut context)?; syncer.sync_path_vars(&mut context); { @@ -668,7 +668,7 @@ pub async fn cli( // 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_env_vars(&mut context)?; syncer.sync_path_vars(&mut context); match line { diff --git a/crates/nu-cli/src/env/TODO.org b/crates/nu-cli/src/env/TODO.org index 1b42c89e7d..3bd9614728 100644 --- a/crates/nu-cli/src/env/TODO.org +++ b/crates/nu-cli/src/env/TODO.org @@ -19,4 +19,5 @@ returning =None=, which completely skips running the code for dealing with direc - If a directory contains a .nu with an environment variable that was already set, the old value will be overwritten. - This holds even if the old value was set by a .nu in a parent directory. The overwritten value is restored when you leave the directory. ** Security - https://github.com/nushell/nushell/issues/1965 \ No newline at end of file + https://github.com/nushell/nushell/issues/1965 +** Nice errors \ No newline at end of file diff --git a/crates/nu-cli/src/env/directory_specific_environment.rs b/crates/nu-cli/src/env/directory_specific_environment.rs index 2b4a6b01b0..be1535edc1 100644 --- a/crates/nu-cli/src/env/directory_specific_environment.rs +++ b/crates/nu-cli/src/env/directory_specific_environment.rs @@ -1,4 +1,5 @@ use indexmap::{IndexMap, IndexSet}; +use nu_errors::ShellError; use nu_protocol::{Primitive, UntaggedValue, Value}; use std::io::Write; use std::{ @@ -8,7 +9,6 @@ use std::{ io::{Error, ErrorKind, Result}, path::PathBuf, }; -use nu_errors::ShellError; type EnvKey = String; type EnvVal = OsString; @@ -83,22 +83,12 @@ impl DirectorySpecificEnvironment { Ok(keyvals_to_restore) } - pub fn env_vars_to_add(&mut self) -> std::result::Result, ShellError> { + pub fn env_vars_to_add(&mut self) -> std::io::Result> { let current_dir = std::env::current_dir()?; let mut working_dir = Some(current_dir.as_path()); - let mut vars_to_add = IndexMap::new(); - // let mut file = OpenOptions::new() - // .write(true) - // .append(true) - // .create(true) - // .open("toadd.txt") - // .unwrap( - // ); - - // write!(&mut file, "1: {:?}\n", vars_to_add).unwrap(); - //WE CRASHING SOMEWHERE HERE + // //WE CRASHING SOMEWHERE HERE //Start in the current directory, then traverse towards the root with working_dir to see if we are in a subdirectory of a valid directory. while let Some(wdir) = working_dir { @@ -108,42 +98,49 @@ impl DirectorySpecificEnvironment { toml_doc .get("env") - .ok_or_else(|| Err(ShellError::untagged_runtime_error("env section missing")))? + .ok_or_else(|| Error::new(ErrorKind::InvalidData, "env section missing"))? .as_table() - .ok_or_else(|| Err(ShellError::untagged_runtime_error("env section malformed")))? + .ok_or_else(|| Error::new(ErrorKind::InvalidData, "env section malformed"))? .iter() - .for_each(|(directory_env_key, directory_env_val)| { - if !vars_to_add.contains_key(directory_env_key) { - let directory_env_val: EnvVal = - directory_env_val.as_str().unwrap().into(); + .for_each(|(dir_env_key, dir_env_val)| { + let dir_env_val: EnvVal = dir_env_val.as_str().unwrap().into(); - //If we are about to overwrite any environment variables, we save them first so they can be restored later. - if let Some(existing_val) = std::env::var_os(directory_env_key) { - if existing_val != directory_env_val { - self.overwritten_env_vars - .entry(wdir.to_path_buf()) - .or_insert(IndexMap::new()) - .insert(directory_env_key.clone(), existing_val); + //If we are about to overwrite any environment variables, we save them first so they can be restored later. + if let Some(existing_val) = std::env::var_os(dir_env_key) { + // if existing_val != dir_env_val { + // self.overwritten_env_vars + // .entry(wdir.to_path_buf()) + // .or_insert(IndexMap::new()) + // .insert(dir_env_key.clone(), existing_val); - vars_to_add.insert(directory_env_key.clone(), directory_env_val); - } - } else { - //Otherwise, we just track that we added it here - self.added_env_vars - .entry(wdir.to_path_buf()) - .or_insert(IndexSet::new()) - .insert(directory_env_key.clone()); + // vars_to_add.insert(dir_env_key.clone(), dir_env_val); + // } + } else { + //Otherwise, we just track that we added it here + self.added_env_vars + .entry(wdir.to_path_buf()) + .or_insert(IndexSet::new()) + .insert(dir_env_key.clone()); - vars_to_add.insert(directory_env_key.clone(), directory_env_val); - } + vars_to_add.insert(dir_env_key.clone(), dir_env_val); } }); } + working_dir = working_dir //Keep going up in the directory structure with .parent() .expect("This should not be None because of the while condition") .parent(); } + let mut file = OpenOptions::new() + .write(true) + .append(true) + .create(true) + .open("toadd.txt") + .unwrap(); + + write!(&mut file, "adding: {:?}\n", vars_to_add).unwrap(); + Ok(vars_to_add) } diff --git a/crates/nu-cli/src/env/environment.rs b/crates/nu-cli/src/env/environment.rs index d1fb49fb72..29a2796f59 100644 --- a/crates/nu-cli/src/env/environment.rs +++ b/crates/nu-cli/src/env/environment.rs @@ -5,6 +5,7 @@ use indexmap::{indexmap, IndexSet}; use nu_protocol::{UntaggedValue, Value}; use std::ffi::OsString; use std::{fs::OpenOptions, fmt::Debug}; +use nu_errors::ShellError; pub trait Env: Debug + Send { fn env(&self) -> Option; @@ -71,10 +72,10 @@ impl Environment { // self.add_env(&k, &v.to_string_lossy(), true); // }); - // self.direnv.env_vars_to_add()?.iter().for_each(|(k, v)| { - // std::env::set_var(k, v); - // self.add_env(&k, &v.to_string_lossy(), true); - // }); + self.direnv.env_vars_to_add()?.iter().for_each(|(k, v)| { + // std::env::set_var(k, v); + self.add_env(&k, &v.to_string_lossy(), true); + }); Ok(()) } diff --git a/crates/nu-cli/src/env/environment_syncer.rs b/crates/nu-cli/src/env/environment_syncer.rs index 195a733c4a..a37a0dc1ea 100644 --- a/crates/nu-cli/src/env/environment_syncer.rs +++ b/crates/nu-cli/src/env/environment_syncer.rs @@ -45,6 +45,7 @@ impl EnvironmentSyncer { pub fn sync_env_vars(&mut self, ctx: &mut Context) -> Result<(), ShellError> { let mut environment = self.env.lock(); + environment.maintain_directory_environment().ok(); if environment.env().is_some() { for (name, value) in ctx.with_host(|host| host.vars()) { if name != "path" && name != "PATH" { @@ -52,8 +53,6 @@ impl EnvironmentSyncer { // that aren't loaded from config. environment.add_env(&name, &value, false); - environment.maintain_directory_environment()?; - // 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))); From dbfd0979a6b04c6c77b6ce75bfe85b2e690a7c52 Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Wed, 17 Jun 2020 00:57:50 +0200 Subject: [PATCH 54/57] Reworking adding of vars --- .../src/env/directory_specific_environment.rs | 39 +++++++++++-------- crates/nu-cli/src/env/environment.rs | 2 - 2 files changed, 22 insertions(+), 19 deletions(-) diff --git a/crates/nu-cli/src/env/directory_specific_environment.rs b/crates/nu-cli/src/env/directory_specific_environment.rs index be1535edc1..3f1a7aceaa 100644 --- a/crates/nu-cli/src/env/directory_specific_environment.rs +++ b/crates/nu-cli/src/env/directory_specific_environment.rs @@ -1,5 +1,4 @@ use indexmap::{IndexMap, IndexSet}; -use nu_errors::ShellError; use nu_protocol::{Primitive, UntaggedValue, Value}; use std::io::Write; use std::{ @@ -90,6 +89,7 @@ impl DirectorySpecificEnvironment { // //WE CRASHING SOMEWHERE HERE + let mut seen_vars = IndexSet::new(); //Start in the current directory, then traverse towards the root with working_dir to see if we are in a subdirectory of a valid directory. while let Some(wdir) = working_dir { if self.allowed_directories.contains(wdir) { @@ -107,23 +107,28 @@ impl DirectorySpecificEnvironment { //If we are about to overwrite any environment variables, we save them first so they can be restored later. if let Some(existing_val) = std::env::var_os(dir_env_key) { - // if existing_val != dir_env_val { - // self.overwritten_env_vars - // .entry(wdir.to_path_buf()) - // .or_insert(IndexMap::new()) - // .insert(dir_env_key.clone(), existing_val); + if existing_val == dir_env_val { + seen_vars.insert(dir_env_key.clone()); + } else if !seen_vars.contains(dir_env_key) { + // self.overwritten_env_vars + // .entry(wdir.to_path_buf()) + // .or_insert(IndexMap::new()) + // .insert(dir_env_key.clone(), existing_val); - // vars_to_add.insert(dir_env_key.clone(), dir_env_val); - // } - } else { - //Otherwise, we just track that we added it here - self.added_env_vars - .entry(wdir.to_path_buf()) - .or_insert(IndexSet::new()) - .insert(dir_env_key.clone()); - - vars_to_add.insert(dir_env_key.clone(), dir_env_val); - } + std::env::set_var(dir_env_key, dir_env_val.clone()); + seen_vars.insert(dir_env_key.clone()); + vars_to_add.insert(dir_env_key.clone(), dir_env_val); + } + }// else { + // //Otherwise, we just track that we added it here + // // self.added_env_vars + // // .entry(wdir.to_path_buf()) + // // .or_insert(IndexSet::new()) + // // .insert(dir_env_key.clone()); + // std::env::set_var(dir_env_key, dir_env_val.clone()); + // vars_to_add.insert(dir_env_key.clone(), dir_env_val); + // seen_vars.insert(dir_env_key); + // } }); } diff --git a/crates/nu-cli/src/env/environment.rs b/crates/nu-cli/src/env/environment.rs index 29a2796f59..09475ed173 100644 --- a/crates/nu-cli/src/env/environment.rs +++ b/crates/nu-cli/src/env/environment.rs @@ -5,7 +5,6 @@ use indexmap::{indexmap, IndexSet}; use nu_protocol::{UntaggedValue, Value}; use std::ffi::OsString; use std::{fs::OpenOptions, fmt::Debug}; -use nu_errors::ShellError; pub trait Env: Debug + Send { fn env(&self) -> Option; @@ -76,7 +75,6 @@ impl Environment { // std::env::set_var(k, v); self.add_env(&k, &v.to_string_lossy(), true); }); - Ok(()) } From b6a7f56a1ea1f88d0a9091c3fc231ccb19b2fa50 Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Wed, 17 Jun 2020 00:57:50 +0200 Subject: [PATCH 55/57] Reworking adding of vars --- .../src/env/directory_specific_environment.rs | 35 ++++++++++--------- crates/nu-cli/src/env/environment.rs | 2 -- 2 files changed, 19 insertions(+), 18 deletions(-) diff --git a/crates/nu-cli/src/env/directory_specific_environment.rs b/crates/nu-cli/src/env/directory_specific_environment.rs index be1535edc1..d3758dc186 100644 --- a/crates/nu-cli/src/env/directory_specific_environment.rs +++ b/crates/nu-cli/src/env/directory_specific_environment.rs @@ -1,5 +1,4 @@ use indexmap::{IndexMap, IndexSet}; -use nu_errors::ShellError; use nu_protocol::{Primitive, UntaggedValue, Value}; use std::io::Write; use std::{ @@ -90,6 +89,7 @@ impl DirectorySpecificEnvironment { // //WE CRASHING SOMEWHERE HERE + let mut seen_vars = IndexSet::new(); //Start in the current directory, then traverse towards the root with working_dir to see if we are in a subdirectory of a valid directory. while let Some(wdir) = working_dir { if self.allowed_directories.contains(wdir) { @@ -107,23 +107,26 @@ impl DirectorySpecificEnvironment { //If we are about to overwrite any environment variables, we save them first so they can be restored later. if let Some(existing_val) = std::env::var_os(dir_env_key) { - // if existing_val != dir_env_val { - // self.overwritten_env_vars - // .entry(wdir.to_path_buf()) - // .or_insert(IndexMap::new()) - // .insert(dir_env_key.clone(), existing_val); + if !seen_vars.contains(dir_env_key) { + // self.overwritten_env_vars + // .entry(wdir.to_path_buf()) + // .or_insert(IndexMap::new()) + // .insert(dir_env_key.clone(), existing_val); - // vars_to_add.insert(dir_env_key.clone(), dir_env_val); - // } - } else { + std::env::set_var(dir_env_key, dir_env_val.clone()); + seen_vars.insert(dir_env_key.clone()); + vars_to_add.insert(dir_env_key.clone(), dir_env_val); + } + } //else { //Otherwise, we just track that we added it here - self.added_env_vars - .entry(wdir.to_path_buf()) - .or_insert(IndexSet::new()) - .insert(dir_env_key.clone()); - - vars_to_add.insert(dir_env_key.clone(), dir_env_val); - } + // self.added_env_vars + // .entry(wdir.to_path_buf()) + // .or_insert(IndexSet::new()) + // .insert(dir_env_key.clone()); + // std::env::set_var(dir_env_key, dir_env_val.clone()); + // vars_to_add.insert(dir_env_key.clone(), dir_env_val); + // seen_vars.insert(dir_env_key.clone()); + //} }); } diff --git a/crates/nu-cli/src/env/environment.rs b/crates/nu-cli/src/env/environment.rs index 29a2796f59..09475ed173 100644 --- a/crates/nu-cli/src/env/environment.rs +++ b/crates/nu-cli/src/env/environment.rs @@ -5,7 +5,6 @@ use indexmap::{indexmap, IndexSet}; use nu_protocol::{UntaggedValue, Value}; use std::ffi::OsString; use std::{fs::OpenOptions, fmt::Debug}; -use nu_errors::ShellError; pub trait Env: Debug + Send { fn env(&self) -> Option; @@ -76,7 +75,6 @@ impl Environment { // std::env::set_var(k, v); self.add_env(&k, &v.to_string_lossy(), true); }); - Ok(()) } From 542d7ac2a18a078780791383a10817e845b86998 Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Wed, 17 Jun 2020 13:27:37 +0200 Subject: [PATCH 56/57] Ready for testing --- crates/nu-cli/src/env/TODO.org | 5 ++- .../src/env/directory_specific_environment.rs | 37 ++++++------------- crates/nu-cli/src/env/environment.rs | 6 +-- 3 files changed, 19 insertions(+), 29 deletions(-) diff --git a/crates/nu-cli/src/env/TODO.org b/crates/nu-cli/src/env/TODO.org index 3bd9614728..bc0a9f9e0f 100644 --- a/crates/nu-cli/src/env/TODO.org +++ b/crates/nu-cli/src/env/TODO.org @@ -20,4 +20,7 @@ returning =None=, which completely skips running the code for dealing with direc - This holds even if the old value was set by a .nu in a parent directory. The overwritten value is restored when you leave the directory. ** Security https://github.com/nushell/nushell/issues/1965 -** Nice errors \ No newline at end of file +** Nice errors +** Potential issues + - Currently the functionality to restore environment variables is disabled, as it is handled by other parts of nushell just fine. + - This functionality might need to be re-added if \ No newline at end of file diff --git a/crates/nu-cli/src/env/directory_specific_environment.rs b/crates/nu-cli/src/env/directory_specific_environment.rs index d3758dc186..8446d8ef02 100644 --- a/crates/nu-cli/src/env/directory_specific_environment.rs +++ b/crates/nu-cli/src/env/directory_specific_environment.rs @@ -1,10 +1,8 @@ use indexmap::{IndexMap, IndexSet}; use nu_protocol::{Primitive, UntaggedValue, Value}; -use std::io::Write; use std::{ ffi::OsString, fmt::Debug, - fs::OpenOptions, io::{Error, ErrorKind, Result}, path::PathBuf, }; @@ -108,25 +106,23 @@ impl DirectorySpecificEnvironment { //If we are about to overwrite any environment variables, we save them first so they can be restored later. if let Some(existing_val) = std::env::var_os(dir_env_key) { if !seen_vars.contains(dir_env_key) { - // self.overwritten_env_vars - // .entry(wdir.to_path_buf()) - // .or_insert(IndexMap::new()) - // .insert(dir_env_key.clone(), existing_val); + self.overwritten_env_vars + .entry(wdir.to_path_buf()) + .or_insert(IndexMap::new()) + .insert(dir_env_key.clone(), existing_val); - std::env::set_var(dir_env_key, dir_env_val.clone()); seen_vars.insert(dir_env_key.clone()); vars_to_add.insert(dir_env_key.clone(), dir_env_val); } - } //else { + } else { //Otherwise, we just track that we added it here - // self.added_env_vars - // .entry(wdir.to_path_buf()) - // .or_insert(IndexSet::new()) - // .insert(dir_env_key.clone()); - // std::env::set_var(dir_env_key, dir_env_val.clone()); - // vars_to_add.insert(dir_env_key.clone(), dir_env_val); - // seen_vars.insert(dir_env_key.clone()); - //} + self.added_env_vars + .entry(wdir.to_path_buf()) + .or_insert(IndexSet::new()) + .insert(dir_env_key.clone()); + vars_to_add.insert(dir_env_key.clone(), dir_env_val); + seen_vars.insert(dir_env_key.clone()); + } }); } @@ -135,15 +131,6 @@ impl DirectorySpecificEnvironment { .parent(); } - let mut file = OpenOptions::new() - .write(true) - .append(true) - .create(true) - .open("toadd.txt") - .unwrap(); - - write!(&mut file, "adding: {:?}\n", vars_to_add).unwrap(); - Ok(vars_to_add) } diff --git a/crates/nu-cli/src/env/environment.rs b/crates/nu-cli/src/env/environment.rs index 09475ed173..78128e171e 100644 --- a/crates/nu-cli/src/env/environment.rs +++ b/crates/nu-cli/src/env/environment.rs @@ -60,9 +60,9 @@ impl Environment { } pub fn maintain_directory_environment(&mut self) -> std::io::Result<()> { - // self.direnv.env_vars_to_delete()?.iter().for_each(|k| { - // self.remove_env(&k); - // }); + self.direnv.env_vars_to_delete()?.iter().for_each(|k| { + self.remove_env(&k); + }); // self.direnv // .overwritten_values_to_restore()? From 727af8ca4ef016a478262679242982953ba7dacf Mon Sep 17 00:00:00 2001 From: Sam Hedin Date: Wed, 17 Jun 2020 13:44:25 +0200 Subject: [PATCH 57/57] Refactoring --- .../src/env/directory_specific_environment.rs | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/crates/nu-cli/src/env/directory_specific_environment.rs b/crates/nu-cli/src/env/directory_specific_environment.rs index 8446d8ef02..870bd079ca 100644 --- a/crates/nu-cli/src/env/directory_specific_environment.rs +++ b/crates/nu-cli/src/env/directory_specific_environment.rs @@ -57,8 +57,6 @@ impl DirectorySpecificEnvironment { let mut working_dir = Some(current_dir.as_path()); let mut new_overwritten_env_values = IndexMap::new(); - let mut keyvals_to_restore = IndexMap::new(); - while let Some(wdir) = working_dir { if let Some(val) = self.overwritten_env_vars.get(wdir) { new_overwritten_env_values.insert(wdir.to_path_buf(), val.clone()); @@ -68,11 +66,12 @@ impl DirectorySpecificEnvironment { .parent(); } + let mut keyvals_to_restore = IndexMap::new(); for (dir, keyvals) in &self.overwritten_env_vars { if !new_overwritten_env_values.contains_key(dir) { - keyvals.iter().for_each(|(k, v)| { + for (k, v) in keyvals { keyvals_to_restore.insert(k.clone(), v.clone()); - }); + } } } @@ -85,9 +84,6 @@ impl DirectorySpecificEnvironment { let mut working_dir = Some(current_dir.as_path()); let mut vars_to_add = IndexMap::new(); - // //WE CRASHING SOMEWHERE HERE - - let mut seen_vars = IndexSet::new(); //Start in the current directory, then traverse towards the root with working_dir to see if we are in a subdirectory of a valid directory. while let Some(wdir) = working_dir { if self.allowed_directories.contains(wdir) { @@ -105,13 +101,13 @@ impl DirectorySpecificEnvironment { //If we are about to overwrite any environment variables, we save them first so they can be restored later. if let Some(existing_val) = std::env::var_os(dir_env_key) { - if !seen_vars.contains(dir_env_key) { + //This condition is to make sure variables in parent directories don't overwrite variables set by subdirectories. + if !vars_to_add.contains_key(dir_env_key) { self.overwritten_env_vars .entry(wdir.to_path_buf()) .or_insert(IndexMap::new()) .insert(dir_env_key.clone(), existing_val); - seen_vars.insert(dir_env_key.clone()); vars_to_add.insert(dir_env_key.clone(), dir_env_val); } } else { @@ -121,7 +117,6 @@ impl DirectorySpecificEnvironment { .or_insert(IndexSet::new()) .insert(dir_env_key.clone()); vars_to_add.insert(dir_env_key.clone(), dir_env_val); - seen_vars.insert(dir_env_key.clone()); } }); } @@ -138,9 +133,10 @@ impl DirectorySpecificEnvironment { //once they are marked for deletion, remove them from added_env_vars pub fn env_vars_to_delete(&mut self) -> Result> { let current_dir = std::env::current_dir()?; - let mut new_added_env_vars = IndexMap::new(); let mut working_dir = Some(current_dir.as_path()); + //We start from the current directory and go towards the root. We retain the variables set by directories we are in. + let mut new_added_env_vars = IndexMap::new(); while let Some(wdir) = working_dir { if let Some(vars_added_by_this_directory) = self.added_env_vars.get(wdir) { //If we are still in a directory, we should continue to track the vars it added. @@ -160,7 +156,6 @@ impl DirectorySpecificEnvironment { } } } - self.added_env_vars = new_added_env_vars; Ok(vars_to_delete)