Fix autoenv executing scripts multiple times (#2171)

* Fix autoenv executing scripts multiple times

Previously, if the user had only specified entry or exitscripts the scripts
would execute many times. This should be fixed now

* Add tests

* Run exitscripts

* More tests and fixes to existing tests

* Test solution with visited dirs

* Track visited directories

* Comments and fmt

crates/nu-cli/src/env/directory_specific_environment.rs

@@ -20,6 +20,9 @@ pub struct DirectorySpecificEnvironment {
     //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.
     //If setting the var overwrote some value, we save the old value in an option so we can restore it later.
     added_vars: IndexMap<PathBuf, IndexMap<EnvKey, Option<EnvVal>>>,
+
+    //We track directories that we have read .nu-env from. This is different from the keys in added_vars since sometimes a file only wants to run scripts.
+    visited_dirs: IndexSet<PathBuf>,
     exitscripts: IndexMap<PathBuf, Vec<String>>,
 }
 
@@ -42,6 +45,7 @@ impl DirectorySpecificEnvironment {
         DirectorySpecificEnvironment {
             last_seen_directory: root_dir,
             added_vars: IndexMap::new(),
+            visited_dirs: IndexSet::new(),
             exitscripts: IndexMap::new(),
         }
     }
@@ -75,13 +79,15 @@ impl DirectorySpecificEnvironment {
             return Ok(());
         }
 
+        //We track which keys we set as we go up the directory hierarchy, so that we don't overwrite a value we set in a subdir.
         let mut added_keys = IndexSet::new();
+
         //We note which directories we pass so we can clear unvisited dirs later.
         let mut seen_directories = IndexSet::new();
 
         //Add all .nu-envs until we reach a dir which we have already added, or we reached the root.
         let mut popped = true;
-        while !self.added_vars.contains_key(&dir) && popped {
+        while popped && !self.visited_dirs.contains(&dir) {
             let nu_env_file = dir.join(".nu-env");
             if nu_env_file.exists() {
                 let nu_env_doc = self.toml_if_trusted(&nu_env_file)?;
@@ -92,6 +98,7 @@ impl DirectorySpecificEnvironment {
                         self.maybe_add_key(&mut added_keys, &dir, &env_key, &env_val);
                     }
                 }
+                self.visited_dirs.insert(dir.clone());
 
                 //Add variables that need to evaluate scripts to run, from [scriptvars] section
                 if let Some(sv) = nu_env_doc.scriptvars {
@@ -132,13 +139,30 @@ impl DirectorySpecificEnvironment {
                         std::env::remove_var(k);
                     }
                 }
-                if let Some(es) = self.exitscripts.get(&dir) {
-                    for s in es {
-                        run(s.as_str())?;
-                    }
+            }
+        }
+
+        let mut new_visited = IndexSet::new();
+        for dir in self.visited_dirs.drain(..) {
+            if seen_directories.contains(&dir) {
+                new_visited.insert(dir);
+            }
+        }
+
+        //Run exitscripts, can not be done in same loop as new vars as some files can contain only exitscripts
+        let mut new_exitscripts = IndexMap::new();
+        for (dir, scripts) in self.exitscripts.drain(..) {
+            if seen_directories.contains(&dir) {
+                new_exitscripts.insert(dir, scripts);
+            } else {
+                for s in scripts {
+                    run(s.as_str())?;
                 }
             }
         }
+
+        self.visited_dirs = new_visited;
+        self.exitscripts = new_exitscripts;
         self.added_vars = new_vars;
         self.last_seen_directory = current_dir()?;
         Ok(())

tests/shell/pipeline/commands/internal.rs

@@ -41,6 +41,7 @@ fn takes_rows_of_nu_value_strings_and_pipes_it_to_stdin_of_external() {
 fn autoenv() {
     Playground::setup("autoenv_test", |dirs, sandbox| {
         sandbox.mkdir("foo/bar");
+        sandbox.mkdir("bizz/buzz");
         sandbox.mkdir("foob");
 
         let scriptfile = if cfg!(target_os = "windows") {
@@ -77,15 +78,53 @@ fn autoenv() {
                 "foo/.nu-env",
                 r#"[env]
                     overwrite_me = "set_in_foo"
-                    fookey = "fooval""#,
+                    fookey = "fooval" "#,
             ),
             FileWithContent(
                 "foo/bar/.nu-env",
                 r#"[env]
                     overwrite_me = "set_in_bar""#,
             ),
+            FileWithContent(
+                "bizz/.nu-env",
+                r#"[scripts]
+                    entryscripts = ["touch hello.txt"]
+                    exitscripts = ["touch bye.txt"]"#,
+            ),
         ]);
 
+        // Make sure entry scripts are run
+        let actual = nu!(
+            cwd: dirs.test(),
+            r#"cd ..
+               autoenv trust autoenv_test
+               cd autoenv_test
+               ls | where name == "hello.txt" | get name"#
+        );
+        assert!(actual.out.contains("hello.txt"));
+
+        // Make sure entry scripts are run when re-visiting a directory
+        let actual = nu!(
+            cwd: dirs.test(),
+            r#"autoenv trust bizz
+               cd bizz
+               rm hello.txt
+               cd ..
+               cd bizz
+               ls | where name == "hello.txt" | get name"#
+        );
+        assert!(actual.out.contains("hello.txt"));
+
+        // Entryscripts should not run after changing to a subdirectory.
+        let actual = nu!(
+            cwd: dirs.test(),
+            r#"autoenv trust bizz
+               cd bizz
+               cd buzz
+               ls | where name == hello.txt | get name"#
+        );
+        assert!(!actual.out.ends_with("hello.txt"));
+
         //Make sure basic keys are set
         let actual = nu!(
             cwd: dirs.test(),
@@ -145,16 +184,6 @@ fn autoenv() {
         );
         assert!(actual.out.contains("bye.txt"));
 
-        //Subdirectories should overwrite the values of parent directories.
-        let actual = nu!(
-            cwd: dirs.test(),
-            r#"autoenv trust foo
-                   cd foo/bar
-                   autoenv trust
-                   echo $nu.env.overwrite_me"#
-        );
-        assert!(actual.out.ends_with("set_in_bar"));
-
         //Variables set in parent directories should be set even if you directly cd to a subdir
         let actual = nu!(
             cwd: dirs.test(),
@@ -165,6 +194,16 @@ fn autoenv() {
         );
         assert!(actual.out.ends_with("fooval"));
 
+        //Subdirectories should overwrite the values of parent directories.
+        let actual = nu!(
+            cwd: dirs.test(),
+            r#"autoenv trust foo
+                   cd foo/bar
+                   autoenv trust
+                   echo $nu.env.overwrite_me"#
+        );
+        assert!(actual.out.ends_with("set_in_bar"));
+
         //Make sure that overwritten values are restored.
         //By deleting foo/.nu-env, we make sure that the value is actually restored and not just set again by autoenv when we re-visit foo.
         let actual = nu!(