fix Windows-specific path normalization using GetLongPathNameW

crates/pet-fs/Cargo.toml

@@ -6,6 +6,7 @@ license = "MIT"
 
 [target.'cfg(target_os = "windows")'.dependencies]
 msvc_spectre_libs = { version = "0.1.1", features = ["error"] }
+windows-sys = { version = "0.59", features = ["Win32_Storage_FileSystem", "Win32_Foundation"] }
 
 [dependencies]
 log = "0.4.21"

crates/pet-fs/src/path.rs

@@ -6,9 +6,10 @@ use std::{
     path::{Path, PathBuf},
 };
 
-// Similar to fs::canonicalize, but ignores UNC paths and returns the path as is (for windows).
-// Usefulfor windows to ensure we have the paths in the right casing.
+// Normalizes the case of a path on Windows without resolving junctions/symlinks.
+// Uses GetLongPathNameW which normalizes case but preserves junction paths.
 // For unix, this is a noop.
+// See: https://github.com/microsoft/python-environment-tools/issues/186
 pub fn norm_case<P: AsRef<Path>>(path: P) -> PathBuf {
     // On unix do not use canonicalize, results in weird issues with homebrew paths
     // Even readlink does the same thing
@@ -18,25 +19,79 @@ pub fn norm_case<P: AsRef<Path>>(path: P) -> PathBuf {
     return path.as_ref().to_path_buf();
 
     #[cfg(windows)]
-    use std::fs;
-
-    #[cfg(windows)]
-    if let Ok(resolved) = fs::canonicalize(&path) {
-        if cfg!(unix) {
-            return resolved;
-        }
-        // Windows specific handling, https://github.com/rust-lang/rust/issues/42869
-        let has_unc_prefix = path.as_ref().to_string_lossy().starts_with(r"\\?\");
-        if resolved.to_string_lossy().starts_with(r"\\?\") && !has_unc_prefix {
-            // If the resolved path has a UNC prefix, but the original path did not,
-            // we need to remove the UNC prefix.
-            PathBuf::from(resolved.to_string_lossy().trim_start_matches(r"\\?\"))
+    {
+        // First, convert to absolute path if relative, without resolving symlinks/junctions
+        let absolute_path = if path.as_ref().is_absolute() {
+            path.as_ref().to_path_buf()
+        } else if let Ok(abs) = std::env::current_dir() {
+            abs.join(path.as_ref())
         } else {
-            resolved
-        }
-    } else {
-        path.as_ref().to_path_buf()
+            path.as_ref().to_path_buf()
+        };
+
+        // Use GetLongPathNameW to normalize case without resolving junctions
+        normalize_case_windows(&absolute_path).unwrap_or_else(|| path.as_ref().to_path_buf())
+    }
+}
+
+/// Windows-specific path case normalization using GetLongPathNameW.
+/// This normalizes the case of path components but does NOT resolve junctions or symlinks.
+#[cfg(windows)]
+fn normalize_case_windows(path: &Path) -> Option<PathBuf> {
+    use std::ffi::OsString;
+    use std::os::windows::ffi::{OsStrExt, OsStringExt};
+    use windows_sys::Win32::Storage::FileSystem::GetLongPathNameW;
+
+    // Normalize forward slashes to backslashes (canonicalize did this)
+    let path_str = path.to_string_lossy().replace('/', "\\");
+    let normalized_path = PathBuf::from(&path_str);
+
+    // Convert path to wide string (UTF-16) with null terminator
+    let wide_path: Vec<u16> = normalized_path
+        .as_os_str()
+        .encode_wide()
+        .chain(std::iter::once(0))
+        .collect();
+
+    // First call to get required buffer size
+    let required_len = unsafe { GetLongPathNameW(wide_path.as_ptr(), std::ptr::null_mut(), 0) };
+
+    if required_len == 0 {
+        // GetLongPathNameW failed, return None
+        return None;
+    }
+
+    // Allocate buffer and get the normalized path
+    let mut buffer: Vec<u16> = vec![0; required_len as usize];
+    let actual_len =
+        unsafe { GetLongPathNameW(wide_path.as_ptr(), buffer.as_mut_ptr(), required_len) };
+
+    if actual_len == 0 || actual_len > required_len {
+        // Call failed or buffer too small
+        return None;
+    }
+
+    // Truncate buffer to actual length (excluding null terminator)
+    buffer.truncate(actual_len as usize);
+
+    // Convert back to PathBuf
+    let os_string = OsString::from_wide(&buffer);
+    let mut result_str = os_string.to_string_lossy().to_string();
+
+    // Remove UNC prefix if original path didn't have it
+    // GetLongPathNameW may add \\?\ prefix in some cases
+    let original_has_unc = path_str.starts_with(r"\\?\");
+    if result_str.starts_with(r"\\?\") && !original_has_unc {
+        result_str = result_str.trim_start_matches(r"\\?\").to_string();
     }
+
+    // Strip trailing path separators to match canonicalize behavior
+    // (but keep the root like "C:\")
+    while result_str.len() > 3 && (result_str.ends_with('\\') || result_str.ends_with('/')) {
+        result_str.pop();
+    }
+
+    Some(PathBuf::from(result_str))
 }
 
 // Resolves symlinks to the real file.
@@ -107,3 +162,181 @@ fn get_user_home() -> Option<PathBuf> {
         Err(_) => None,
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_norm_case_returns_path_for_nonexistent() {
+        // norm_case should return the original path if it doesn't exist
+        let nonexistent = PathBuf::from("/this/path/does/not/exist/anywhere");
+        let result = norm_case(&nonexistent);
+        assert_eq!(result, nonexistent);
+    }
+
+    #[test]
+    fn test_norm_case_existing_path() {
+        // norm_case should work on existing paths
+        let temp_dir = std::env::temp_dir();
+        let result = norm_case(&temp_dir);
+        // On unix, should return unchanged; on Windows, should normalize case
+        assert!(result.exists());
+    }
+
+    #[test]
+    #[cfg(unix)]
+    fn test_norm_case_unix_noop() {
+        // On unix, norm_case should return the path unchanged
+        let path = PathBuf::from("/Some/Path/With/Mixed/Case");
+        let result = norm_case(&path);
+        assert_eq!(result, path);
+    }
+
+    #[test]
+    #[cfg(windows)]
+    fn test_norm_case_windows_case_normalization() {
+        // On Windows, norm_case should normalize the case of existing paths
+        // Use the Windows directory which always exists
+        let path = PathBuf::from("c:\\windows\\system32");
+        let result = norm_case(&path);
+        // The result should have proper casing (C:\Windows\System32)
+        assert!(result.to_string_lossy().contains("Windows"));
+        assert!(result.to_string_lossy().contains("System32"));
+    }
+
+    #[test]
+    #[cfg(windows)]
+    fn test_norm_case_windows_preserves_junction() {
+        // This is the key test for issue #186:
+        // norm_case should NOT resolve junctions to their target
+        use std::fs;
+        use std::process::Command;
+
+        let temp_dir = std::env::temp_dir();
+        let target_dir = temp_dir.join("pet_test_junction_target");
+        let junction_dir = temp_dir.join("pet_test_junction_link");
+
+        // Clean up any existing test directories
+        let _ = fs::remove_dir_all(&target_dir);
+        let _ = fs::remove_dir_all(&junction_dir);
+
+        // Create target directory
+        fs::create_dir_all(&target_dir).expect("Failed to create target directory");
+
+        // Create a junction using mklink /J (requires no special privileges)
+        let output = Command::new("cmd")
+            .args([
+                "/C",
+                "mklink",
+                "/J",
+                &junction_dir.to_string_lossy(),
+                &target_dir.to_string_lossy(),
+            ])
+            .output()
+            .expect("Failed to create junction");
+
+        if !output.status.success() {
+            // Clean up and skip test if junction creation failed
+            let _ = fs::remove_dir_all(&target_dir);
+            eprintln!(
+                "Skipping junction test - failed to create junction: {}",
+                String::from_utf8_lossy(&output.stderr)
+            );
+            return;
+        }
+
+        // Verify junction was created
+        assert!(junction_dir.exists(), "Junction should exist");
+
+        // The key assertion: norm_case should return the junction path, NOT the target path
+        let result = norm_case(&junction_dir);
+
+        // The result should still be the junction path, not resolved to target
+        // Compare the path names (case-insensitive on Windows)
+        assert!(
+            result
+                .to_string_lossy()
+                .to_lowercase()
+                .contains("pet_test_junction_link"),
+            "norm_case should preserve junction path, got: {:?}",
+            result
+        );
+        assert!(
+            !result
+                .to_string_lossy()
+                .to_lowercase()
+                .contains("pet_test_junction_target"),
+            "norm_case should NOT resolve to target path, got: {:?}",
+            result
+        );
+
+        // Clean up
+        // Remove junction first (using rmdir, not remove_dir_all, to not follow the junction)
+        let _ = Command::new("cmd")
+            .args(["/C", "rmdir", &junction_dir.to_string_lossy()])
+            .output();
+        let _ = fs::remove_dir_all(&target_dir);
+    }
+
+    #[test]
+    #[cfg(windows)]
+    fn test_norm_case_windows_relative_path() {
+        // Test that relative paths are converted to absolute
+        let relative = PathBuf::from(".");
+        let result = norm_case(&relative);
+        assert!(result.is_absolute(), "Result should be absolute path");
+    }
+
+    #[test]
+    #[cfg(windows)]
+    fn test_norm_case_windows_no_unc_prefix_added() {
+        // Ensure we don't add UNC prefix to paths that didn't have it
+        let path = PathBuf::from("C:\\Windows");
+        let result = norm_case(&path);
+        assert!(
+            !result.to_string_lossy().starts_with(r"\\?\"),
+            "Should not add UNC prefix"
+        );
+    }
+
+    #[test]
+    #[cfg(windows)]
+    fn test_norm_case_windows_strips_trailing_slash() {
+        // norm_case should strip trailing slashes to match canonicalize behavior
+        let path_with_slash = PathBuf::from("C:\\Windows\\");
+        let result = norm_case(&path_with_slash);
+        assert!(
+            !result.to_string_lossy().ends_with('\\'),
+            "Should strip trailing backslash, got: {:?}",
+            result
+        );
+
+        // But root paths like C:\ should keep their slash
+        let root_path = PathBuf::from("C:\\");
+        let root_result = norm_case(&root_path);
+        assert!(
+            root_result.to_string_lossy().ends_with('\\'),
+            "Root path should keep trailing backslash, got: {:?}",
+            root_result
+        );
+    }
+
+    #[test]
+    #[cfg(windows)]
+    fn test_norm_case_windows_normalizes_slashes() {
+        // norm_case should convert forward slashes to backslashes (like canonicalize did)
+        let path_with_forward_slashes = PathBuf::from("C:/Windows/System32");
+        let result = norm_case(&path_with_forward_slashes);
+        assert!(
+            !result.to_string_lossy().contains('/'),
+            "Should convert forward slashes to backslashes, got: {:?}",
+            result
+        );
+        assert!(
+            result.to_string_lossy().contains('\\'),
+            "Should have backslashes, got: {:?}",
+            result
+        );
+    }
+}