From e4990804fad279f055decf70c794ea5f22372641 Mon Sep 17 00:00:00 2001 From: Ryan Dahl Date: Wed, 5 Sep 2018 11:19:39 -0400 Subject: [PATCH] Improve module resolution. Windows can't handle ":" in path names, so we use a special directory format .deno/deps/localhost_PORT4545/ to represent hosts with non-default ports. Fixes #645. --- src/deno_dir.rs | 240 ++++++++++++++++++++++++----------- tests/006_url_imports.ts | 2 +- tests/006_url_imports.ts.out | 1 + tests/subdir/mod2.ts | 1 + 4 files changed, 166 insertions(+), 78 deletions(-) create mode 100644 tests/subdir/mod2.ts diff --git a/src/deno_dir.rs b/src/deno_dir.rs index 0a3557d5b3..bc6a1f306b 100644 --- a/src/deno_dir.rs +++ b/src/deno_dir.rs @@ -137,8 +137,8 @@ impl DenoDir { } else if module_name.starts_with(ASSET_PREFIX) { panic!("Asset resolution should be done in JS, not Rust."); } else { - assert!( - module_name == filename, + assert_eq!( + module_name, filename, "if a module isn't remote, it should have the same filename" ); let src = fs::read_to_string(Path::new(filename))?; @@ -151,14 +151,14 @@ impl DenoDir { module_specifier: &str, containing_file: &str, ) -> Result { + debug!( + "code_fetch. module_specifier {} containing_file {}", + module_specifier, containing_file + ); + let (module_name, filename) = self.resolve_module(module_specifier, containing_file)?; - debug!( - "code_fetch. module_name = {} module_specifier = {} containing_file = {} filename = {}", - module_name, module_specifier, containing_file, filename - ); - let out = self .get_source_code(module_name.as_str(), filename.as_str()) .and_then(|source_code| { @@ -190,14 +190,18 @@ impl DenoDir { } // Prototype: https://github.com/denoland/deno/blob/golang/os.go#L56-L68 - #[allow(dead_code)] - fn src_file_to_url>(self: &DenoDir, filename: P) -> String { - let filename = filename.as_ref().to_path_buf(); - if filename.starts_with(&self.deps) { - let rest = filename.strip_prefix(&self.deps).unwrap(); - "http://".to_string() + rest.to_str().unwrap() + fn src_file_to_url(self: &DenoDir, filename: &str) -> String { + let filename_path = Path::new(filename); + if filename_path.starts_with(&self.deps) { + let rest = filename_path.strip_prefix(&self.deps).unwrap(); + // Windows doesn't support ":" in filenames, so we represent port using a + // special string. + // TODO(ry) This current implementation will break on a URL that has + // the default port but contains "_PORT" in the path. + let rest = rest.to_str().unwrap().replacen("_PORT", ":", 1); + "http://".to_string() + &rest } else { - String::from(filename.to_str().unwrap()) + String::from(filename) } } @@ -211,39 +215,31 @@ impl DenoDir { let module_name; let filename; + let module_specifier = self.src_file_to_url(module_specifier); + let containing_file = self.src_file_to_url(containing_file); + debug!( - "resolve_module before module_specifier {} containing_file {}", + "resolve_module module_specifier {} containing_file {}", module_specifier, containing_file ); - //let module_specifier = src_file_to_url(module_specifier); - //let containing_file = src_file_to_url(containing_file); - //let base_url = Url::parse(&containing_file)?; - - let j: Url = - if containing_file == "." || Path::new(module_specifier).is_absolute() { - if module_specifier.starts_with("http://") { - Url::parse(module_specifier)? - } else { - Url::from_file_path(module_specifier).unwrap() - } - } else if containing_file.ends_with("/") { - let r = Url::from_directory_path(&containing_file); - // TODO(ry) Properly handle error. - if r.is_err() { - error!("Url::from_directory_path error {}", containing_file); - } - let base = r.unwrap(); - base.join(module_specifier)? - } else { - let r = Url::from_file_path(&containing_file); - // TODO(ry) Properly handle error. - if r.is_err() { - error!("Url::from_file_path error {}", containing_file); - } - let base = r.unwrap(); - base.join(module_specifier)? - }; + let j: Url = if containing_file == "." + || is_remote(&module_specifier) + || Path::new(&module_specifier).is_absolute() + { + parse_local_or_remote(&module_specifier)? + } else if containing_file.ends_with("/") { + let r = Url::from_directory_path(&containing_file); + // TODO(ry) Properly handle error. + if r.is_err() { + error!("Url::from_directory_path error {}", containing_file); + } + let base = r.unwrap(); + base.join(module_specifier.as_ref())? + } else { + let base = parse_local_or_remote(&containing_file)?; + base.join(module_specifier.as_ref())? + }; match j.scheme() { "file" => { @@ -252,7 +248,7 @@ impl DenoDir { filename = p; } _ => { - module_name = module_specifier.to_string(); + module_name = j.to_string(); filename = deno_fs::normalize_path( get_cache_filename(self.deps.as_path(), j).as_ref(), ) @@ -265,8 +261,16 @@ impl DenoDir { } fn get_cache_filename(basedir: &Path, url: Url) -> PathBuf { + let host = url.host_str().unwrap(); + let host_port = match url.port() { + // Windows doesn't support ":" in filenames, so we represent port using a + // special string. + Some(port) => format!("{}_PORT{}", host, port), + None => host.to_string(), + }; + let mut out = basedir.to_path_buf(); - out.push(url.host_str().unwrap()); + out.push(host_port); for path_seg in url.path_segments().unwrap() { out.push(path_seg); } @@ -280,7 +284,7 @@ fn test_get_cache_filename() { let cache_file = get_cache_filename(&basedir, url); assert_eq!( cache_file, - Path::new("/cache/dir/example.com/path/to/file.ts") + Path::new("/cache/dir/example.com_PORT1234/path/to/file.ts") ); } @@ -398,26 +402,32 @@ fn test_code_fetch() { } #[test] -fn test_src_file_to_url() { +fn test_src_file_to_url_1() { let (_temp_dir, deno_dir) = test_setup(); assert_eq!("hello", deno_dir.src_file_to_url("hello")); assert_eq!("/hello", deno_dir.src_file_to_url("/hello")); - let x = String::from(deno_dir.deps.join("hello/world.txt").to_str().unwrap()); - assert_eq!("http://hello/world.txt", deno_dir.src_file_to_url(x)); + let x = deno_dir.deps.join("hello/world.txt"); + assert_eq!( + "http://hello/world.txt", + deno_dir.src_file_to_url(x.to_str().unwrap()) + ); +} + +#[test] +fn test_src_file_to_url_2() { + let (_temp_dir, deno_dir) = test_setup(); + let x = deno_dir.deps.join("localhost_PORT4545/world.txt"); + assert_eq!( + "http://localhost:4545/world.txt", + deno_dir.src_file_to_url(x.to_str().unwrap()) + ); } // https://github.com/denoland/deno/blob/golang/os_test.go#L16-L87 #[test] -fn test_resolve_module() { +fn test_resolve_module_1() { let (_temp_dir, deno_dir) = test_setup(); - let d = deno_fs::normalize_path( - deno_dir - .deps - .join("localhost/testdata/subdir/print_hello.ts") - .as_ref(), - ); - let test_cases = [ ( "./subdir/print_hello.ts", @@ -449,26 +459,6 @@ fn test_resolve_module() { add_root!("/this/module/got/imported.js"), add_root!("/this/module/got/imported.js"), ), - ( - "http://localhost:4545/testdata/subdir/print_hello.ts", - add_root!("/Users/rld/go/src/github.com/denoland/deno/testdata/006_url_imports.ts"), - "http://localhost:4545/testdata/subdir/print_hello.ts", - d.as_ref(), - ), - /* - ( - path.Join(SrcDir, "unpkg.com/liltest@0.0.5/index.ts"), - ".", - "http://unpkg.com/liltest@0.0.5/index.ts", - path.Join(SrcDir, "unpkg.com/liltest@0.0.5/index.ts"), - ), - ( - "./util", - path.Join(SrcDir, "unpkg.com/liltest@0.0.5/index.ts"), - "http://unpkg.com/liltest@0.0.5/util", - path.Join(SrcDir, "unpkg.com/liltest@0.0.5/util"), - ), - */ ]; for &test in test_cases.iter() { let module_specifier = String::from(test.0); @@ -481,8 +471,104 @@ fn test_resolve_module() { } } +#[test] +fn test_resolve_module_2() { + let (_temp_dir, deno_dir) = test_setup(); + + let module_specifier = "http://localhost:4545/testdata/subdir/print_hello.ts"; + let containing_file = add_root!("/deno/testdata/006_url_imports.ts"); + + let expected_module_name = + "http://localhost:4545/testdata/subdir/print_hello.ts"; + let expected_filename = deno_fs::normalize_path( + deno_dir + .deps + .join("localhost_PORT4545/testdata/subdir/print_hello.ts") + .as_ref(), + ); + + let (module_name, filename) = deno_dir + .resolve_module(module_specifier, containing_file) + .unwrap(); + assert_eq!(module_name, expected_module_name); + assert_eq!(filename, expected_filename); +} + +#[test] +fn test_resolve_module_3() { + let (_temp_dir, deno_dir) = test_setup(); + + let module_specifier_ = + deno_dir.deps.join("unpkg.com/liltest@0.0.5/index.ts"); + let module_specifier = module_specifier_.to_str().unwrap(); + let containing_file = "."; + + let expected_module_name = "http://unpkg.com/liltest@0.0.5/index.ts"; + let expected_filename = deno_fs::normalize_path( + deno_dir + .deps + .join("unpkg.com/liltest@0.0.5/index.ts") + .as_ref(), + ); + + let (module_name, filename) = deno_dir + .resolve_module(module_specifier, containing_file) + .unwrap(); + assert_eq!(module_name, expected_module_name); + assert_eq!(filename, expected_filename); +} + +#[test] +fn test_resolve_module_4() { + let (_temp_dir, deno_dir) = test_setup(); + + let module_specifier = "./util"; + let containing_file_ = deno_dir.deps.join("unpkg.com/liltest@0.0.5/index.ts"); + let containing_file = containing_file_.to_str().unwrap(); + + let expected_module_name = "http://unpkg.com/liltest@0.0.5/util"; + let expected_filename = deno_fs::normalize_path( + deno_dir.deps.join("unpkg.com/liltest@0.0.5/util").as_ref(), + ); + + let (module_name, filename) = deno_dir + .resolve_module(module_specifier, containing_file) + .unwrap(); + assert_eq!(module_name, expected_module_name); + assert_eq!(filename, expected_filename); +} + +#[test] +fn test_resolve_module_5() { + let (_temp_dir, deno_dir) = test_setup(); + + let module_specifier = "http://localhost:4545/tests/subdir/mod2.ts"; + let containing_file = add_root!("/deno/tests/006_url_imports.ts"); + let expected_module_name = "http://localhost:4545/tests/subdir/mod2.ts"; + let expected_filename = deno_fs::normalize_path( + deno_dir + .deps + .join("localhost_PORT4545/tests/subdir/mod2.ts") + .as_ref(), + ); + + let (module_name, filename) = deno_dir + .resolve_module(module_specifier, containing_file) + .unwrap(); + assert_eq!(module_name, expected_module_name); + assert_eq!(filename, expected_filename); +} + const ASSET_PREFIX: &str = "/$asset$/"; fn is_remote(module_name: &str) -> bool { module_name.starts_with("http") } + +fn parse_local_or_remote(p: &str) -> Result { + if is_remote(p) { + Url::parse(p) + } else { + Url::from_file_path(p).map_err(|_err| url::ParseError::IdnaError) + } +} diff --git a/tests/006_url_imports.ts b/tests/006_url_imports.ts index 53dd8b8765..b2c7db2704 100644 --- a/tests/006_url_imports.ts +++ b/tests/006_url_imports.ts @@ -1,3 +1,3 @@ -import { printHello } from "http://localhost:4545/tests/subdir/print_hello.ts"; +import { printHello } from "http://localhost:4545/tests/subdir/mod2.ts"; printHello(); console.log("success"); diff --git a/tests/006_url_imports.ts.out b/tests/006_url_imports.ts.out index d2a3863c59..f481ff137b 100644 --- a/tests/006_url_imports.ts.out +++ b/tests/006_url_imports.ts.out @@ -1,3 +1,4 @@ +Downloading http://localhost:4545/tests/subdir/mod2.ts Downloading http://localhost:4545/tests/subdir/print_hello.ts Hello success diff --git a/tests/subdir/mod2.ts b/tests/subdir/mod2.ts new file mode 100644 index 0000000000..ce1adc0e81 --- /dev/null +++ b/tests/subdir/mod2.ts @@ -0,0 +1 @@ +export { printHello } from "./print_hello.ts";