Skip to content
This repository has been archived by the owner on Sep 28, 2022. It is now read-only.

Commit

Permalink
[guppy] fix cross-drive path diffs on Windows
Browse files Browse the repository at this point in the history
Fixes #642.
  • Loading branch information
sunshowers committed May 29, 2022
1 parent b2c01f6 commit 69b42d8
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 6 deletions.
6 changes: 4 additions & 2 deletions guppy-summaries/src/summary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,11 @@ pub enum SummarySource {

/// A non-workspace path.
///
/// The path is expected to be relative to the workspace root.
/// The path is usually relative to the workspace root, but on Windows a path that spans drives
/// (e.g. a path on D:\ when the workspace root is on C:\) cannot be relative. In those cases,
/// this will be the absolute path of the package.
Path {
/// The path of this package, relative to the workspace root.
/// The path of this package.
#[serde(serialize_with = "serialize_forward_slashes")]
path: Utf8PathBuf,
},
Expand Down
47 changes: 44 additions & 3 deletions guppy/src/graph/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,9 +214,7 @@ impl<'a> GraphBuildState<'a> {
)));
}
};
let rel_path = pathdiff::diff_utf8_paths(dirname, self.workspace_root)
.expect("workspace root is absolute");
PackageSourceImpl::Path(convert_forward_slashes(rel_path).into_boxed_path())
PackageSourceImpl::create_path(dirname, self.workspace_root)
};

let mut build_targets = BuildTargets::new(&package_id);
Expand Down Expand Up @@ -384,6 +382,21 @@ impl<'a> GraphBuildState<'a> {
}
}

impl PackageSourceImpl {
fn create_path(path: &Utf8Path, workspace_root: &Utf8Path) -> Self {
let path_diff =
pathdiff::diff_utf8_paths(path, workspace_root).expect("workspace root is absolute");
// On Windows, the directory name and the workspace root might be on different drives,
// in which case the path can't be relative.
let path_diff = if path_diff.is_absolute() {
path_diff
} else {
convert_forward_slashes(path_diff)
};
Self::Path(path_diff.into_boxed_path())
}
}

impl NamedFeatureDep {
fn from_cargo_string(input: impl Into<String>) -> Self {
let input = input.into();
Expand Down Expand Up @@ -806,6 +819,7 @@ impl PackagePublishImpl {
}

/// Replace backslashes in a relative path with forward slashes on Windows.
#[track_caller]
fn convert_forward_slashes<'a>(rel_path: impl Into<Cow<'a, Utf8Path>>) -> Utf8PathBuf {
let rel_path = rel_path.into();
debug_assert!(
Expand Down Expand Up @@ -847,6 +861,33 @@ mod tests {
);
}

#[test]
fn test_create_path() {
assert_eq!(
PackageSourceImpl::create_path("/data/foo".as_ref(), "/data/bar".as_ref()),
PackageSourceImpl::Path("../foo".into())
);
assert_eq!(
PackageSourceImpl::create_path("/tmp/foo".as_ref(), "/data/bar".as_ref()),
PackageSourceImpl::Path("../../tmp/foo".into())
);
}

#[cfg(windows)]
#[test]
fn test_create_path_windows() {
// Ensure that relative paths are stored with forward slashes.
assert_eq!(
PackageSourceImpl::create_path("C:\\data\\foo".as_ref(), "C:\\data\\bar".as_ref()),
PackageSourceImpl::Path("../foo".into())
);
// Paths that span drives cannot be stored as relative.
assert_eq!(
PackageSourceImpl::create_path("D:\\tmp\\foo".as_ref(), "C:\\data\\bar".as_ref()),
PackageSourceImpl::Path("D:\\tmp\\foo".into())
);
}

#[test]
fn test_convert_forward_slashes() {
let components = vec!["..", "..", "foo", "bar", "baz.txt"];
Expand Down
2 changes: 1 addition & 1 deletion guppy/src/graph/graph_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1594,7 +1594,7 @@ pub enum GitReq<'g> {
}

/// Internal representation of the source of a package.
#[derive(Clone, Debug)]
#[derive(Clone, Debug, PartialEq, Eq)]
pub(super) enum PackageSourceImpl {
Workspace(Box<Utf8Path>),
Path(Box<Utf8Path>),
Expand Down

0 comments on commit 69b42d8

Please sign in to comment.