From 54982e948e882fbf413e06319f711d85b232026b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Mon, 12 Aug 2019 01:12:09 +0200 Subject: [PATCH] fix: cache paths on Windows are broken (#2760) --- cli/disk_cache.rs | 100 +++++++++++++++++++++++++++++++++++----------- 1 file changed, 77 insertions(+), 23 deletions(-) diff --git a/cli/disk_cache.rs b/cli/disk_cache.rs index fdbe2cbd549077..975a31f45683d5 100644 --- a/cli/disk_cache.rs +++ b/cli/disk_cache.rs @@ -1,8 +1,11 @@ use crate::fs as deno_fs; use std::ffi::OsStr; use std::fs; +use std::path::Component; use std::path::Path; use std::path::PathBuf; +use std::path::Prefix; +use std::str; use url::Url; #[derive(Clone)] @@ -18,15 +21,12 @@ impl DiskCache { } } - // TODO(bartlomieju) this method is not working properly for Windows paths, - // Example: file:///C:/deno/js/unit_test_runner.ts - // would produce: C:deno\\js\\unit_test_runner.ts - // it should produce: file\deno\js\unit_test_runner.ts pub fn get_cache_filename(self: &Self, url: &Url) -> PathBuf { let mut out = PathBuf::new(); let scheme = url.scheme(); out.push(scheme); + match scheme { "http" | "https" => { let host = url.host_str().unwrap(); @@ -37,13 +37,48 @@ impl DiskCache { None => host.to_string(), }; out.push(host_port); + + for path_seg in url.path_segments().unwrap() { + out.push(path_seg); + } + } + "file" => { + let path = url.to_file_path().unwrap(); + let mut path_components = path.components(); + + if cfg!(target_os = "windows") { + if let Some(Component::Prefix(prefix_component)) = + path_components.next() + { + // Windows doesn't support ":" in filenames, so we need to extract disk prefix + // Example: file:///C:/deno/js/unit_test_runner.ts + // it should produce: file\c\deno\js\unit_test_runner.ts + match prefix_component.kind() { + Prefix::Disk(disk_byte) | Prefix::VerbatimDisk(disk_byte) => { + let disk = (disk_byte as char).to_string(); + out.push(disk); + } + _ => unreachable!(), + } + } + } + + // Must be relative, so strip forward slash + let mut remaining_components = path_components.as_path(); + if let Ok(stripped) = remaining_components.strip_prefix("/") { + remaining_components = stripped; + }; + + out = out.join(remaining_components); + } + scheme => { + unimplemented!( + "Don't know how to create cache name for scheme: {}", + scheme + ); } - _ => {} }; - for path_seg in url.path_segments().unwrap() { - out.push(path_seg); - } out } @@ -90,9 +125,15 @@ mod tests { #[test] fn test_get_cache_filename() { - let cache = DiskCache::new(&PathBuf::from("foo")); + let cache_location = if cfg!(target_os = "windows") { + PathBuf::from(r"C:\deno_dir\") + } else { + PathBuf::from("/deno_dir/") + }; + + let cache = DiskCache::new(&cache_location); - let test_cases = [ + let mut test_cases = vec![ ( "http://deno.land/std/http/file_server.ts", "http/deno.land/std/http/file_server.ts", @@ -105,17 +146,21 @@ mod tests { "https://deno.land/std/http/file_server.ts", "https/deno.land/std/http/file_server.ts", ), - ( + ]; + + if cfg!(target_os = "windows") { + test_cases.push(("file:///D:/a/1/s/format.ts", "file/D/a/1/s/format.ts")); + } else { + test_cases.push(( "file:///std/http/file_server.ts", "file/std/http/file_server.ts", - ), - ]; + )); + } for test_case in &test_cases { - assert_eq!( - cache.get_cache_filename(&Url::parse(test_case.0).unwrap()), - PathBuf::from(test_case.1) - ) + let cache_filename = + cache.get_cache_filename(&Url::parse(test_case.0).unwrap()); + assert_eq!(cache_filename, PathBuf::from(test_case.1)); } } @@ -123,17 +168,12 @@ mod tests { fn test_get_cache_filename_with_extension() { let cache = DiskCache::new(&PathBuf::from("foo")); - let test_cases = [ + let mut test_cases = vec![ ( "http://deno.land/std/http/file_server.ts", "js", "http/deno.land/std/http/file_server.ts.js", ), - ( - "file:///std/http/file_server", - "js", - "file/std/http/file_server.js", - ), ( "http://deno.land/std/http/file_server.ts", "js.map", @@ -141,6 +181,20 @@ mod tests { ), ]; + if cfg!(target_os = "windows") { + test_cases.push(( + "file:///D:/std/http/file_server", + "js", + "file/D/std/http/file_server.js", + )); + } else { + test_cases.push(( + "file:///std/http/file_server", + "js", + "file/std/http/file_server.js", + )); + } + for test_case in &test_cases { assert_eq!( cache.get_cache_filename_with_extension(