From 7b851fc30e109213e911eec38460315872f1ae59 Mon Sep 17 00:00:00 2001 From: Ohad Lutzky Date: Thu, 2 Nov 2023 08:01:09 +0000 Subject: [PATCH] feat(scanner): add option not to follow symlinks (#5325) Add follow_symlinks option Settings this to false can fix hanging on symlinks to slow/inaccessible filesystems. --- .github/config-schema.json | 4 ++ docs/config/README.md | 8 +++ src/configs/starship_root.rs | 2 + src/context.rs | 96 +++++++++++++++++++++++++++++++----- 4 files changed, 97 insertions(+), 13 deletions(-) diff --git a/.github/config-schema.json b/.github/config-schema.json index f999866b..1f091efb 100644 --- a/.github/config-schema.json +++ b/.github/config-schema.json @@ -1794,6 +1794,10 @@ "default": true, "type": "boolean" }, + "follow_symlinks": { + "default": true, + "type": "boolean" + }, "palette": { "type": [ "string", diff --git a/docs/config/README.md b/docs/config/README.md index 5dc22dd2..55315d93 100644 --- a/docs/config/README.md +++ b/docs/config/README.md @@ -219,6 +219,14 @@ This is the list of prompt-wide configuration options. | `add_newline` | `true` | Inserts blank line between shell prompts. | | `palette` | `''` | Sets which color palette from `palettes` to use. | | `palettes` | `{}` | Collection of color palettes that assign [colors](/advanced-config/#style-strings) to user-defined names. Note that color palettes cannot reference their own color definitions. | +| `follow_symlinks` | `true` | Follows symlinks to check if they're directories; used in modules such as git. | + +::: tip + +If you have symlinks to networked filesystems, consider setting +`follow_symlinks` to `false`. + +::: ### Example diff --git a/src/configs/starship_root.rs b/src/configs/starship_root.rs index aa3d65f9..6308d6d9 100644 --- a/src/configs/starship_root.rs +++ b/src/configs/starship_root.rs @@ -18,6 +18,7 @@ pub struct StarshipRootConfig { pub scan_timeout: u64, pub command_timeout: u64, pub add_newline: bool, + pub follow_symlinks: bool, #[serde(skip_serializing_if = "Option::is_none")] pub palette: Option, pub palettes: HashMap, @@ -134,6 +135,7 @@ impl Default for StarshipRootConfig { scan_timeout: 30, command_timeout: 500, add_newline: true, + follow_symlinks: true, palette: None, palettes: HashMap::default(), } diff --git a/src/context.rs b/src/context.rs index a0753009..83af209a 100644 --- a/src/context.rs +++ b/src/context.rs @@ -358,7 +358,11 @@ impl<'a> Context<'a> { pub fn dir_contents(&self) -> Result<&DirContents, std::io::Error> { self.dir_contents.get_or_try_init(|| { let timeout = self.root_config.scan_timeout; - DirContents::from_path_with_timeout(&self.current_dir, Duration::from_millis(timeout)) + DirContents::from_path_with_timeout( + &self.current_dir, + Duration::from_millis(timeout), + self.root_config.follow_symlinks, + ) }) } @@ -479,11 +483,15 @@ pub struct DirContents { impl DirContents { #[cfg(test)] - fn from_path(base: &Path) -> Result { - Self::from_path_with_timeout(base, Duration::from_secs(30)) + fn from_path(base: &Path, follow_symlinks: bool) -> Result { + Self::from_path_with_timeout(base, Duration::from_secs(30), follow_symlinks) } - fn from_path_with_timeout(base: &Path, timeout: Duration) -> Result { + fn from_path_with_timeout( + base: &Path, + timeout: Duration, + follow_symlinks: bool, + ) -> Result { let start = Instant::now(); let mut folders: HashSet = HashSet::new(); @@ -501,7 +509,15 @@ impl DirContents { .filter_map(|(_, entry)| entry.ok()) .for_each(|entry| { let path = PathBuf::from(entry.path().strip_prefix(base).unwrap()); - if entry.path().is_dir() { + + let is_dir = match follow_symlinks { + true => entry.path().is_dir(), + false => fs::symlink_metadata(entry.path()) + .map(|m| m.is_dir()) + .unwrap_or(false), + }; + + if is_dir { folders.insert(path); } else { if !path.to_string_lossy().starts_with('.') { @@ -897,10 +913,63 @@ mod tests { Ok(dir) } + #[test] + fn test_scan_dir_no_symlinks() -> Result<(), Box> { + #[cfg(not(target_os = "windows"))] + use std::os::unix::fs::symlink; + #[cfg(target_os = "windows")] + use std::os::windows::fs::symlink_dir as symlink; + + let d = testdir(&["file"])?; + fs::create_dir(d.path().join("folder"))?; + + symlink(d.path().join("folder"), d.path().join("link_to_folder"))?; + symlink(d.path().join("file"), d.path().join("link_to_file"))?; + + let dc_following_symlinks = DirContents::from_path(d.path(), true)?; + + assert!(ScanDir { + dir_contents: &dc_following_symlinks, + files: &["link_to_file"], + extensions: &[], + folders: &[], + } + .is_match()); + + assert!(ScanDir { + dir_contents: &dc_following_symlinks, + files: &[], + extensions: &[], + folders: &["link_to_folder"], + } + .is_match()); + + let dc_not_following_symlinks = DirContents::from_path(d.path(), false)?; + + assert!(ScanDir { + dir_contents: &dc_not_following_symlinks, + files: &["link_to_file"], + extensions: &[], + folders: &[], + } + .is_match()); + + assert!(!ScanDir { + dir_contents: &dc_not_following_symlinks, + files: &[], + extensions: &[], + folders: &["link_to_folder"], + } + .is_match()); + + Ok(()) + } + #[test] fn test_scan_dir() -> Result<(), Box> { let empty = testdir(&[])?; - let empty_dc = DirContents::from_path(empty.path())?; + let follow_symlinks = true; + let empty_dc = DirContents::from_path(empty.path(), follow_symlinks)?; assert!(!ScanDir { dir_contents: &empty_dc, @@ -912,7 +981,7 @@ mod tests { empty.close()?; let rust = testdir(&["README.md", "Cargo.toml", "src/main.rs"])?; - let rust_dc = DirContents::from_path(rust.path())?; + let rust_dc = DirContents::from_path(rust.path(), follow_symlinks)?; assert!(!ScanDir { dir_contents: &rust_dc, files: &["package.json"], @@ -923,7 +992,7 @@ mod tests { rust.close()?; let java = testdir(&["README.md", "src/com/test/Main.java", "pom.xml"])?; - let java_dc = DirContents::from_path(java.path())?; + let java_dc = DirContents::from_path(java.path(), follow_symlinks)?; assert!(!ScanDir { dir_contents: &java_dc, files: &["package.json"], @@ -934,7 +1003,7 @@ mod tests { java.close()?; let node = testdir(&["README.md", "node_modules/lodash/main.js", "package.json"])?; - let node_dc = DirContents::from_path(node.path())?; + let node_dc = DirContents::from_path(node.path(), follow_symlinks)?; assert!(ScanDir { dir_contents: &node_dc, files: &["package.json"], @@ -945,7 +1014,7 @@ mod tests { node.close()?; let tarballs = testdir(&["foo.tgz", "foo.tar.gz"])?; - let tarballs_dc = DirContents::from_path(tarballs.path())?; + let tarballs_dc = DirContents::from_path(tarballs.path(), follow_symlinks)?; assert!(ScanDir { dir_contents: &tarballs_dc, files: &[], @@ -956,7 +1025,7 @@ mod tests { tarballs.close()?; let dont_match_ext = testdir(&["foo.js", "foo.ts"])?; - let dont_match_ext_dc = DirContents::from_path(dont_match_ext.path())?; + let dont_match_ext_dc = DirContents::from_path(dont_match_ext.path(), follow_symlinks)?; assert!(!ScanDir { dir_contents: &dont_match_ext_dc, files: &[], @@ -967,7 +1036,7 @@ mod tests { dont_match_ext.close()?; let dont_match_file = testdir(&["goodfile", "evilfile"])?; - let dont_match_file_dc = DirContents::from_path(dont_match_file.path())?; + let dont_match_file_dc = DirContents::from_path(dont_match_file.path(), follow_symlinks)?; assert!(!ScanDir { dir_contents: &dont_match_file_dc, files: &["goodfile", "!notfound", "!evilfile"], @@ -978,7 +1047,8 @@ mod tests { dont_match_file.close()?; let dont_match_folder = testdir(&["gooddir/somefile", "evildir/somefile"])?; - let dont_match_folder_dc = DirContents::from_path(dont_match_folder.path())?; + let dont_match_folder_dc = + DirContents::from_path(dont_match_folder.path(), follow_symlinks)?; assert!(!ScanDir { dir_contents: &dont_match_folder_dc, files: &[],