From 9496e5512975825efebe0db86335d0d2dc8c9095 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 20 Jun 2022 09:51:44 +0800 Subject: [PATCH 1/5] remove canonicalized-path abstraction --- git-path/tests/path.rs | 2 + git-path/tests/realpath/mod.rs | 76 +++++++++++++++------------------- 2 files changed, 36 insertions(+), 42 deletions(-) diff --git a/git-path/tests/path.rs b/git-path/tests/path.rs index beba1271cef..3815f0c04fa 100644 --- a/git-path/tests/path.rs +++ b/git-path/tests/path.rs @@ -1,2 +1,4 @@ +pub type Result = std::result::Result>; + mod convert; mod realpath; diff --git a/git-path/tests/realpath/mod.rs b/git-path/tests/realpath/mod.rs index b6160924067..3f7ec019477 100644 --- a/git-path/tests/realpath/mod.rs +++ b/git-path/tests/realpath/mod.rs @@ -58,11 +58,12 @@ fn assorted() { #[test] fn link_cycle_is_detected() { - let tmp_dir = CanonicalizedTempDir::new(); + let tmp_dir = canonicalized_tempdir().unwrap(); + let dir = tmp_dir.path(); let link_name = "link"; - let link_destination = tmp_dir.join(link_name); - let link_path = tmp_dir.join(link_name); - create_symlink(&link_path, &link_destination); + let link_destination = dir.join(link_name); + let link_path = dir.join(link_name); + create_symlink(&link_path, &link_destination).unwrap(); let max_symlinks = 8; assert!( @@ -76,10 +77,11 @@ fn link_cycle_is_detected() { #[test] fn symlink_with_absolute_path_gets_expanded() { - let tmp_dir = CanonicalizedTempDir::new(); - let link_from = tmp_dir.join("a").join("b").join("tmp_p_q_link"); - let link_to = tmp_dir.join("p").join("q"); - create_symlink(&link_from, &link_to); + let tmp_dir = canonicalized_tempdir().unwrap(); + let dir = tmp_dir.path(); + let link_from = dir.join("a").join("b").join("tmp_p_q_link"); + let link_to = dir.join("p").join("q"); + create_symlink(&link_from, &link_to).unwrap(); let max_symlinks = 8; assert_eq!( realpath_opts(link_from.join(".git"), tmp_dir, max_symlinks).unwrap(), @@ -90,21 +92,26 @@ fn symlink_with_absolute_path_gets_expanded() { #[test] fn symlink_to_relative_path_gets_expanded_into_absolute_path() { - let cwd = CanonicalizedTempDir::new(); + let cwd = canonicalized_tempdir().unwrap(); + let dir = cwd.path(); let link_name = "pq_link"; - create_symlink(&cwd.join("r").join(link_name), &Path::new("p").join("q")); + create_symlink(&dir.join("r").join(link_name), &Path::new("p").join("q")).unwrap(); assert_eq!( - realpath_opts(Path::new(link_name).join(".git"), cwd.join("r"), 8).unwrap(), - cwd.join("r").join("p").join("q").join(".git"), + realpath_opts(Path::new(link_name).join(".git"), dir.join("r"), 8).unwrap(), + dir.join("r").join("p").join("q").join(".git"), "symlink to relative path gets expanded into absolute path" ); } #[test] fn symlink_processing_is_disabled_if_the_value_is_zero() { - let cwd = CanonicalizedTempDir::new(); + let cwd = canonicalized_tempdir().unwrap(); let link_name = "x_link"; - create_symlink(&cwd.join(link_name), Path::new("link destination does not exist")); + create_symlink( + &cwd.path().join(link_name), + Path::new("link destination does not exist"), + ) + .unwrap(); assert!( matches!( realpath_opts(&Path::new(link_name).join(".git"), &cwd, 0), @@ -114,39 +121,24 @@ fn symlink_processing_is_disabled_if_the_value_is_zero() { ); } -pub struct CanonicalizedTempDir { - pub dir: tempfile::TempDir, -} +pub fn create_symlink(from: impl AsRef, to: impl AsRef) -> std::io::Result<()> { + std::fs::create_dir_all(from.as_ref().parent().unwrap())?; -pub fn create_symlink(from: &Path, to: &Path) { - std::fs::create_dir_all(from.parent().unwrap()).unwrap(); #[cfg(not(target_os = "windows"))] - std::os::unix::fs::symlink(to, &from).unwrap(); - #[cfg(target_os = "windows")] - std::os::windows::fs::symlink_file(to, &from).unwrap(); -} - -impl CanonicalizedTempDir { - pub fn new() -> Self { - #[cfg(windows)] - let canonicalized_tempdir = std::env::temp_dir(); - #[cfg(not(windows))] - let canonicalized_tempdir = std::env::temp_dir().canonicalize().unwrap(); - let dir = tempfile::tempdir_in(canonicalized_tempdir).unwrap(); - Self { dir } + { + std::os::unix::fs::symlink(to, from) } -} -impl AsRef for CanonicalizedTempDir { - fn as_ref(&self) -> &Path { - self - } + #[cfg(target_os = "windows")] + std::os::windows::fs::symlink_file(to, from) } -impl std::ops::Deref for CanonicalizedTempDir { - type Target = Path; +fn canonicalized_tempdir() -> crate::Result { + #[cfg(windows)] + let canonicalized_tempdir = std::env::temp_dir(); - fn deref(&self) -> &Self::Target { - self.dir.path() - } + #[cfg(not(windows))] + let canonicalized_tempdir = std::env::temp_dir().canonicalize()?; + + Ok(tempfile::tempdir_in(canonicalized_tempdir)?) } From efa14234c352b6b8417f0a42fc946e88f2eb52d3 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Mon, 20 Jun 2022 09:55:57 +0800 Subject: [PATCH 2/5] avoid unwraps in tests as they are now stable --- git-path/tests/convert/absolutize.rs | 7 ++-- git-path/tests/realpath/mod.rs | 53 +++++++++++++++------------- 2 files changed, 33 insertions(+), 27 deletions(-) diff --git a/git-path/tests/convert/absolutize.rs b/git-path/tests/convert/absolutize.rs index 97b0844acb8..aae63762ad7 100644 --- a/git-path/tests/convert/absolutize.rs +++ b/git-path/tests/convert/absolutize.rs @@ -15,8 +15,8 @@ fn no_change_if_there_are_no_trailing_relative_components() { } #[test] -fn special_cases_around_cwd() { - let cwd = std::env::current_dir().unwrap(); +fn special_cases_around_cwd() -> crate::Result { + let cwd = std::env::current_dir()?; assert_eq!( absolutize(p("a/.."), None::<&Path>), p("."), @@ -24,9 +24,10 @@ fn special_cases_around_cwd() { ); assert_eq!( absolutize(p("a/../.."), Some(&cwd)), - cwd.parent().unwrap(), + cwd.parent().expect("parent"), "it automatically extends the poppable items by using the current working dir" ); + Ok(()) } #[test] diff --git a/git-path/tests/realpath/mod.rs b/git-path/tests/realpath/mod.rs index 3f7ec019477..31d34d71547 100644 --- a/git-path/tests/realpath/mod.rs +++ b/git-path/tests/realpath/mod.rs @@ -3,8 +3,8 @@ use std::path::Path; use tempfile::tempdir; #[test] -fn assorted() { - let cwd = tempdir().unwrap(); +fn assorted() -> crate::Result { + let cwd = tempdir()?; let cwd = cwd.path(); let symlinks_disabled = 0; @@ -14,31 +14,31 @@ fn assorted() { ); assert_eq!( - realpath_opts("b/.git", cwd, symlinks_disabled).unwrap(), + realpath_opts("b/.git", cwd, symlinks_disabled)?, cwd.join("b").join(".git"), "relative paths are prefixed with current dir" ); assert_eq!( - realpath_opts("b//.git", cwd, symlinks_disabled).unwrap(), + realpath_opts("b//.git", cwd, symlinks_disabled)?, cwd.join("b").join(".git"), "empty path components are ignored" ); assert_eq!( - realpath_opts("./tmp/.git", cwd, symlinks_disabled).unwrap(), + realpath_opts("./tmp/.git", cwd, symlinks_disabled)?, cwd.join("tmp").join(".git"), "path starting with dot is relative and is prefixed with current dir" ); assert_eq!( - realpath_opts("./tmp/a/./.git", cwd, symlinks_disabled).unwrap(), + realpath_opts("./tmp/a/./.git", cwd, symlinks_disabled)?, cwd.join("tmp").join("a").join(".git"), "all ./ path components are ignored unless they the one at the beginning of the path" ); assert_eq!( - realpath_opts("./b/../tmp/.git", cwd, symlinks_disabled).unwrap(), + realpath_opts("./b/../tmp/.git", cwd, symlinks_disabled)?, cwd.join("tmp").join(".git"), "dot dot goes to parent path component" ); @@ -49,21 +49,23 @@ fn assorted() { #[cfg(target_os = "windows")] let absolute_path = Path::new("C:\\c\\d\\.git"); assert_eq!( - realpath_opts(absolute_path, cwd, symlinks_disabled).unwrap(), + realpath_opts(absolute_path, cwd, symlinks_disabled)?, absolute_path, "absolute path without symlinks has nothing to resolve and remains unchanged" ); } + + Ok(()) } #[test] -fn link_cycle_is_detected() { - let tmp_dir = canonicalized_tempdir().unwrap(); +fn link_cycle_is_detected() -> crate::Result { + let tmp_dir = canonicalized_tempdir()?; let dir = tmp_dir.path(); let link_name = "link"; let link_destination = dir.join(link_name); let link_path = dir.join(link_name); - create_symlink(&link_path, &link_destination).unwrap(); + create_symlink(&link_path, &link_destination)?; let max_symlinks = 8; assert!( @@ -73,45 +75,47 @@ fn link_cycle_is_detected() { ), "link cycle is detected" ); + Ok(()) } #[test] -fn symlink_with_absolute_path_gets_expanded() { - let tmp_dir = canonicalized_tempdir().unwrap(); +fn symlink_with_absolute_path_gets_expanded() -> crate::Result { + let tmp_dir = canonicalized_tempdir()?; let dir = tmp_dir.path(); let link_from = dir.join("a").join("b").join("tmp_p_q_link"); let link_to = dir.join("p").join("q"); - create_symlink(&link_from, &link_to).unwrap(); + create_symlink(&link_from, &link_to)?; let max_symlinks = 8; assert_eq!( - realpath_opts(link_from.join(".git"), tmp_dir, max_symlinks).unwrap(), + realpath_opts(link_from.join(".git"), tmp_dir, max_symlinks)?, link_to.join(".git"), "symlink with absolute path gets expanded" ); + Ok(()) } #[test] -fn symlink_to_relative_path_gets_expanded_into_absolute_path() { - let cwd = canonicalized_tempdir().unwrap(); +fn symlink_to_relative_path_gets_expanded_into_absolute_path() -> crate::Result { + let cwd = canonicalized_tempdir()?; let dir = cwd.path(); let link_name = "pq_link"; - create_symlink(&dir.join("r").join(link_name), &Path::new("p").join("q")).unwrap(); + create_symlink(&dir.join("r").join(link_name), &Path::new("p").join("q"))?; assert_eq!( - realpath_opts(Path::new(link_name).join(".git"), dir.join("r"), 8).unwrap(), + realpath_opts(Path::new(link_name).join(".git"), dir.join("r"), 8)?, dir.join("r").join("p").join("q").join(".git"), "symlink to relative path gets expanded into absolute path" ); + Ok(()) } #[test] -fn symlink_processing_is_disabled_if_the_value_is_zero() { - let cwd = canonicalized_tempdir().unwrap(); +fn symlink_processing_is_disabled_if_the_value_is_zero() -> crate::Result { + let cwd = canonicalized_tempdir()?; let link_name = "x_link"; create_symlink( &cwd.path().join(link_name), Path::new("link destination does not exist"), - ) - .unwrap(); + )?; assert!( matches!( realpath_opts(&Path::new(link_name).join(".git"), &cwd, 0), @@ -119,9 +123,10 @@ fn symlink_processing_is_disabled_if_the_value_is_zero() { ), "symlink processing is disabled if the value is zero" ); + Ok(()) } -pub fn create_symlink(from: impl AsRef, to: impl AsRef) -> std::io::Result<()> { +fn create_symlink(from: impl AsRef, to: impl AsRef) -> std::io::Result<()> { std::fs::create_dir_all(from.as_ref().parent().unwrap())?; #[cfg(not(target_os = "windows"))] From 3d16c36d7288d9a5fae5b9d23715e043d4d8ce76 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 21 Jun 2022 12:09:16 +0800 Subject: [PATCH 3/5] feat: Support for SUDO_UID as fallback for ownership check on unix. --- git-sec/src/identity.rs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/git-sec/src/identity.rs b/git-sec/src/identity.rs index 615c9c52a20..61e1c7cfb85 100644 --- a/git-sec/src/identity.rs +++ b/git-sec/src/identity.rs @@ -34,8 +34,19 @@ mod impl_ { let uid = unsafe { libc::geteuid() }; Ok(uid) } - - Ok(owner_from_path(path)? == owner_of_current_process()?) + use std::str::FromStr; + + let owner_of_path = owner_from_path(path)?; + let owner_of_process = owner_of_current_process()?; + if owner_of_path == owner_of_process { + Ok(true) + } else if let Some(sudo_uid) = + std::env::var_os("SUDO_UID").and_then(|val| val.to_str().and_then(|val_str| u32::from_str(val_str).ok())) + { + Ok(owner_of_path == sudo_uid) + } else { + Ok(false) + } } } From 229dc917fc7d9241b85e5818260a6fbdd3a5daaa Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 21 Jun 2022 17:23:06 +0800 Subject: [PATCH 4/5] Use git_path::realpath in all places that allow it right now It seems better to be able to avoid questionmark errors, even though they might be specific to some ways tests are run as well. Note that test failures can happen with realpath and git-repository equality comparisons for some reason. --- git-discover/src/upwards.rs | 3 ++- git-odb/src/alternate/mod.rs | 7 +++-- git-path/tests/realpath/mod.rs | 7 +---- git-repository/src/repository/location.rs | 32 ++++++++++++----------- src/porcelain/options.rs | 4 +-- 5 files changed, 27 insertions(+), 26 deletions(-) diff --git a/git-discover/src/upwards.rs b/git-discover/src/upwards.rs index 7222aaffde2..71af066a71a 100644 --- a/git-discover/src/upwards.rs +++ b/git-discover/src/upwards.rs @@ -280,7 +280,8 @@ pub(crate) mod function { cursor = if cursor.as_os_str().is_empty() { cwd.clone() } else { - cursor.canonicalize().ok() + // TODO: realpath or absolutize? No test runs into this. + Some(git_path::absolutize(&cursor, cwd.as_deref()).into_owned()) } .ok_or(Error::InaccessibleDirectory { path: cursor })?; } diff --git a/git-odb/src/alternate/mod.rs b/git-odb/src/alternate/mod.rs index ffcbb16a9c2..418124a193e 100644 --- a/git-odb/src/alternate/mod.rs +++ b/git-odb/src/alternate/mod.rs @@ -28,6 +28,8 @@ pub enum Error { #[error(transparent)] Io(#[from] io::Error), #[error(transparent)] + Realpath(#[from] git_path::realpath::Error), + #[error(transparent)] Parse(#[from] parse::Error), #[error("Alternates form a cycle: {} -> {}", .0.iter().map(|p| format!("'{}'", p.display())).collect::>().join(" -> "), .0.first().expect("more than one directories").display())] Cycle(Vec), @@ -42,13 +44,14 @@ pub fn resolve(objects_directory: impl Into) -> Result, Er let relative_base = objects_directory.into(); let mut dirs = vec![(0, relative_base.clone())]; let mut out = Vec::new(); - let mut seen = vec![relative_base.canonicalize()?]; + let cwd = std::env::current_dir()?; + let mut seen = vec![git_path::realpath(&relative_base, &cwd)?]; while let Some((depth, dir)) = dirs.pop() { match fs::read(dir.join("info").join("alternates")) { Ok(input) => { for path in parse::content(&input)?.into_iter() { let path = relative_base.join(path); - let path_canonicalized = path.canonicalize()?; + let path_canonicalized = git_path::realpath(&path, &cwd)?; if seen.contains(&path_canonicalized) { return Err(Error::Cycle(seen)); } diff --git a/git-path/tests/realpath/mod.rs b/git-path/tests/realpath/mod.rs index 31d34d71547..2ff4ec4d55b 100644 --- a/git-path/tests/realpath/mod.rs +++ b/git-path/tests/realpath/mod.rs @@ -139,11 +139,6 @@ fn create_symlink(from: impl AsRef, to: impl AsRef) -> std::io::Resu } fn canonicalized_tempdir() -> crate::Result { - #[cfg(windows)] - let canonicalized_tempdir = std::env::temp_dir(); - - #[cfg(not(windows))] - let canonicalized_tempdir = std::env::temp_dir().canonicalize()?; - + let canonicalized_tempdir = git_path::realpath(std::env::temp_dir(), std::env::current_dir()?)?; Ok(tempfile::tempdir_in(canonicalized_tempdir)?) } diff --git a/git-repository/src/repository/location.rs b/git-repository/src/repository/location.rs index b0b7bb882ae..ab7ef25fc1e 100644 --- a/git-repository/src/repository/location.rs +++ b/git-repository/src/repository/location.rs @@ -26,21 +26,23 @@ impl crate::Repository { // TODO: tests, details - there is a lot about environment variables to change things around. pub fn prefix(&self) -> Option> { self.work_tree.as_ref().map(|root| { - root.canonicalize().and_then(|root| { - std::env::current_dir().and_then(|cwd| { - cwd.strip_prefix(&root) - .map_err(|_| { - std::io::Error::new( - std::io::ErrorKind::Other, - format!( - "CWD '{}' isn't within the work tree '{}'", - cwd.display(), - root.display() - ), - ) - }) - .map(ToOwned::to_owned) - }) + std::env::current_dir().and_then(|cwd| { + git_path::realpath(root, &cwd) + .map_err(|err| std::io::Error::new(std::io::ErrorKind::Other, err)) + .and_then(|root| { + cwd.strip_prefix(&root) + .map_err(|_| { + std::io::Error::new( + std::io::ErrorKind::Other, + format!( + "CWD '{}' isn't within the work tree '{}'", + cwd.display(), + root.display() + ), + ) + }) + .map(ToOwned::to_owned) + }) }) }) } diff --git a/src/porcelain/options.rs b/src/porcelain/options.rs index e6f7894b158..e6ecb8a37cf 100644 --- a/src/porcelain/options.rs +++ b/src/porcelain/options.rs @@ -106,14 +106,14 @@ pub struct EstimateHours { } mod validator { + use git_repository as git; use std::{ffi::OsStr, path::PathBuf}; use anyhow::Context; fn is_repo_inner(dir: &OsStr) -> anyhow::Result<()> { let git_dir = PathBuf::from(dir).join(".git"); - let p = git_dir - .canonicalize() + let p = git::path::realpath(&git_dir, std::env::current_dir()?) .with_context(|| format!("Could not canonicalize git repository at '{}'", git_dir.display()))?; if p.extension().unwrap_or_default() == "git" || p.file_name().unwrap_or_default() == ".git" From 61abb0b006292d2122784b032e198cc716fb7b92 Mon Sep 17 00:00:00 2001 From: Sebastian Thiel Date: Tue, 21 Jun 2022 17:41:35 +0800 Subject: [PATCH 5/5] Remove another special case on windows due to canonicalize() --- git-discover/tests/upwards/mod.rs | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/git-discover/tests/upwards/mod.rs b/git-discover/tests/upwards/mod.rs index e5f32880e0e..7374d0ab822 100644 --- a/git-discover/tests/upwards/mod.rs +++ b/git-discover/tests/upwards/mod.rs @@ -83,11 +83,6 @@ fn from_dir_with_dot_dot() -> crate::Result { let dir = working_dir.join("some/very/deeply/nested/subdir/../../../../../.."); let (path, trust) = git_discover::upwards(&dir)?; assert_eq!(path.kind(), Kind::WorkTree { linked_git_dir: None }); - // On CI on windows we get a cursor like this with a question mark so our prefix check won't work. - // We recover, but that means this assertion will fail. - // &cursor = "\\\\?\\D:\\a\\gitoxide\\gitoxide\\.git" - // &cwd = "D:\\a\\gitoxide\\gitoxide\\git-repository" - #[cfg(not(windows))] assert_eq!( path.as_ref(), std::path::Path::new(".."), @@ -166,18 +161,11 @@ fn from_existing_worktree() -> crate::Result { assert_eq!(trust, expected_trust()); let (git_dir, worktree) = path.into_repository_and_work_tree_directories(); - #[cfg(not(windows))] assert_eq!( - git_dir.strip_prefix(top_level_repo.canonicalize().unwrap()), + git_dir.strip_prefix(git_path::realpath(&top_level_repo, std::env::current_dir()?).unwrap()), Ok(std::path::Path::new(expected_git_dir)), "we don't skip over worktrees and discover their git dir (gitdir is absolute in file)" ); - #[cfg(windows)] - assert_eq!( - git_dir.canonicalize()?, - top_level_repo.join(expected_git_dir).canonicalize()?, - "we don't skip over worktrees and discover their git dir (gitdir is absolute in file)" - ); let worktree = worktree.expect("linked worktree is set"); assert_eq!( worktree.strip_prefix(&top_level_repo),