From 686ac0b23a89f9ec299d1d5a47ff009c69232865 Mon Sep 17 00:00:00 2001 From: ejortega <24722023+ejortega@users.noreply.github.com> Date: Mon, 7 Aug 2023 19:40:23 -0500 Subject: [PATCH] Update relative path handling --- cli/src/commands/parse.rs | 136 +++++++++++++++++++++++++++++++------- 1 file changed, 113 insertions(+), 23 deletions(-) diff --git a/cli/src/commands/parse.rs b/cli/src/commands/parse.rs index 96eb5e96c..bc1119496 100644 --- a/cli/src/commands/parse.rs +++ b/cli/src/commands/parse.rs @@ -82,13 +82,10 @@ pub fn parse_lockfile( let format = find_lockfile_format(&path, lockfile_type); let project_root = project_root.to_owned(); - // Attempt to strip root path - let path = strip_root_path(path, &project_root); - // Attempt to parse with all known parsers as fallback. let (format, lockfile) = match format { Some(format) => format, - None => return try_get_packages(path), + None => return try_get_packages(path, &project_root), }; // Parse with the identified parser. @@ -97,13 +94,13 @@ pub fn parse_lockfile( // Attempt to parse the identified lockfile. let mut lockfile_error = None; if let Some(lockfile) = lockfile { - // Attempt to strip root path for identified lockfile - let lockfile = strip_root_path(lockfile, &project_root); - // Parse lockfile content. let content = fs::read_to_string(&lockfile).map_err(Into::into); let packages = content.and_then(|content| parse_lockfile_content(&content, parser)); + // Attempt to strip root path for identified lockfile + let lockfile = strip_root_path(lockfile, &project_root); + match packages { Ok(packages) => return Ok(ParsedLockfile { path: lockfile, format, packages }), // Store error on failure. @@ -207,8 +204,9 @@ fn find_manifest_format(path: &Path) -> Option<(LockfileFormat, Option) } /// Attempt to get packages from an unknown lockfile type -fn try_get_packages(path: PathBuf) -> Result { +fn try_get_packages(path: PathBuf, project_root: &Option) -> Result { let data = fs::read_to_string(&path)?; + let lockfile = strip_root_path(path, project_root); for format in LockfileFormat::iter() { let parser = format.parser(); @@ -217,11 +215,11 @@ fn try_get_packages(path: PathBuf) -> Result { let packages = filter_packages(packages); - return Ok(ParsedLockfile { path, packages, format }); + return Ok(ParsedLockfile { path: lockfile, packages, format }); } } - Err(anyhow!("Failed to identify type for lockfile {path:?}")) + Err(anyhow!("Failed to identify type for lockfile {lockfile:?}")) } /// Filter packages for submission. @@ -285,21 +283,52 @@ where None } -/// Strip root prefix from lockfile paths +/// Modify the provided path to be relative to a given project root or the +/// current directory. +/// +/// If a project root is provided and the path starts with this root, the +/// function will return a path relative to this root. If no project root is +/// provided, or if the path doesn't start with the project root, the function +/// will return a path relative to the current directory. fn strip_root_path(path: PathBuf, project_root: &Option) -> PathBuf { - match (project_root, path.is_absolute()) { - // Strip project root path when set - (Some(base), _) => path.strip_prefix(base).unwrap_or(&path), - - // Strip current directory if we have an absolute path but not a project root - (None, true) => { - let curr_dir = env::current_dir().unwrap_or_else(|_| path.clone()); - path.strip_prefix(&curr_dir).unwrap_or(&path) - }, + let canonical_path = path.canonicalize().unwrap_or(path.clone()); + let canonical_project_root = + project_root.as_ref().map(|p| p.canonicalize().unwrap_or(p.clone())); + + // If the canonical path starts with the provided project root, return the + // relative path. + if let Some(base) = &canonical_project_root { + return relative_from_to(base, &canonical_path); + } - (None, false) => &path, + // If there's no project root, return the path relative to the current + // directory. + let curr_dir = env::current_dir().unwrap_or_default(); + relative_from_to(&curr_dir, &canonical_path) +} + +/// Calculate a relative path from a starting directory (`from`) to a target +/// path (`to`). +/// +/// This function iterates through the components of both paths in tandem. It +/// skips the common prefix and then, for each remaining component in the +/// starting directory, it adds a `..` to the result. Finally, it appends the +/// unique components of the target path. +fn relative_from_to(from: &Path, to: &Path) -> PathBuf { + let mut from_components = from.components().peekable(); + let mut to_components = to.components().peekable(); + + while from_components.peek() == to_components.peek() { + from_components.next(); + to_components.next(); + } + + let mut result = PathBuf::new(); + while from_components.next().is_some() { + result.push(".."); } - .to_path_buf() + result.extend(to_components); + result } #[cfg(test)] @@ -337,7 +366,7 @@ mod tests { ]; for (file, expected_format) in test_cases { - let parsed = try_get_packages(PathBuf::from(file)).unwrap(); + let parsed = try_get_packages(PathBuf::from(file), &None).unwrap(); assert_eq!(parsed.format, expected_format, "{}", file); } } @@ -384,4 +413,65 @@ mod tests { assert_eq!(path, Some(sibling_lockfile_path)); } + + #[test] + fn test_relative_path() { + // Set up a temporary directory for testing. + let tempdir = tempfile::tempdir().unwrap(); + let tempdir = tempdir.path().canonicalize().unwrap(); + let curr_dir = env::current_dir().unwrap(); + + // Create a parent directory named "projects" within the temp directory. + let all_projects = tempdir.join("projects"); + fs::create_dir_all(&all_projects).unwrap(); + + // Create a sample project directory named "sample" inside the "projects" + // directory. Also create a "Cargo.lock" file inside the "sample" + // directory. + let sample_dir = all_projects.join("sample"); + let lockfile_path = sample_dir.join("Cargo.lock"); + fs::create_dir_all(&sample_dir).unwrap(); + File::create(&lockfile_path).unwrap(); + + // Change the current directory to the "sample" project directory. + env::set_current_dir(&sample_dir).unwrap(); + let path = strip_root_path(lockfile_path.clone(), &Some(sample_dir.clone())); + // The path to the lockfile should now be just the filename since it's in the + // current directory. + assert_eq!(path.as_os_str(), "Cargo.lock"); + + // Create a subdirectory named "sub" within the "sample" project directory. + let sub_dir = sample_dir.join("sub"); + fs::create_dir_all(&sub_dir).unwrap(); + + // Change the current directory to the new "sub" directory. + env::set_current_dir(&sub_dir).unwrap(); + let mut rel_lockfile_path = sub_dir.clone(); + rel_lockfile_path.push(".."); + rel_lockfile_path.push("Cargo.lock"); + + // Get the relative path from the sub directory to the lockfile in the sample + // directory. + let path = strip_root_path(rel_lockfile_path.clone(), &Some(sample_dir.clone())); + // The path to the lockfile should be the same as before since we are looking + // relative to the sample directory. + assert_eq!(path.as_os_str(), "Cargo.lock"); + + // Create another "Cargo.lock" file one level above the "sample" directory. + let above_lockfile_path = all_projects.join("Cargo.lock"); + File::create(above_lockfile_path).unwrap(); + let mut rel_lockfile_path = sub_dir; + rel_lockfile_path.push(".."); + rel_lockfile_path.push(".."); + rel_lockfile_path.push("Cargo.lock"); + + // Although the current directory is still "sub", get the relative path to the + // lockfile above the "sample" directory. + let path = strip_root_path(rel_lockfile_path.clone(), &Some(sample_dir.clone())); + // The path to the lockfile should be relative to the "sample" directory. + assert_eq!(path.as_os_str(), "../Cargo.lock"); + + // Restore the original working directory after the test. + env::set_current_dir(curr_dir).unwrap(); + } }