diff --git a/Cargo.lock b/Cargo.lock index 6d26a98c76a7e1..a476b9ebab5097 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1476,9 +1476,9 @@ dependencies = [ [[package]] name = "deno_npm" -version = "0.15.2" +version = "0.15.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "210f62105862f1ff371e278c623c7ed73d62b0efece4d417c15663d37b730098" +checksum = "718b0b55031643de7808f8b426661b22a685820f1f459e028776bcc49e07b881" dependencies = [ "anyhow", "async-trait", diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 95b221e14c8ccd..0c9d228c3c25e8 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -65,7 +65,7 @@ deno_emit = "=0.31.5" deno_graph = "=0.61.5" deno_lint = { version = "=0.52.2", features = ["docs"] } deno_lockfile.workspace = true -deno_npm = "0.15.2" +deno_npm = "0.15.3" deno_runtime = { workspace = true, features = ["dont_create_runtime_snapshot", "include_js_files_for_snapshotting"] } deno_semver = "0.5.1" deno_task_shell = "=0.14.0" diff --git a/cli/standalone/binary.rs b/cli/standalone/binary.rs index d1a5863eed3d74..de7b3f6ec1a69b 100644 --- a/cli/standalone/binary.rs +++ b/cli/standalone/binary.rs @@ -568,9 +568,16 @@ impl<'a> DenoCompileBinaryWriter<'a> { } fn build_vfs(&self) -> Result { + fn maybe_warn_different_system(system_info: &NpmSystemInfo) { + if system_info != &NpmSystemInfo::default() { + log::warn!("{} The node_modules directory may be incompatible with the target system.", crate::colors::yellow("Warning")); + } + } + match self.npm_resolver.as_inner() { InnerCliNpmResolverRef::Managed(npm_resolver) => { if let Some(node_modules_path) = npm_resolver.root_node_modules_path() { + maybe_warn_different_system(&self.npm_system_info); let mut builder = VfsBuilder::new(node_modules_path.clone())?; builder.add_dir_recursive(node_modules_path)?; Ok(builder) @@ -593,6 +600,7 @@ impl<'a> DenoCompileBinaryWriter<'a> { } } InnerCliNpmResolverRef::Byonm(npm_resolver) => { + maybe_warn_different_system(&self.npm_system_info); // the root_node_modules directory will always exist for byonm let node_modules_path = npm_resolver.root_node_modules_path().unwrap(); let parent_path = node_modules_path.parent().unwrap(); diff --git a/cli/standalone/virtual_fs.rs b/cli/standalone/virtual_fs.rs index ee870611b224da..fe79477d3f5ad2 100644 --- a/cli/standalone/virtual_fs.rs +++ b/cli/standalone/virtual_fs.rs @@ -94,27 +94,40 @@ impl VfsBuilder { } else if file_type.is_file() { self.add_file_at_path(&path)?; } else if file_type.is_symlink() { - let target = util::fs::canonicalize_path(&path) - .with_context(|| format!("Reading symlink {}", path.display()))?; - if let Err(StripRootError { .. }) = self.add_symlink(&path, &target) { - if target.is_file() { - // this may change behavior, so warn the user about it - log::warn!( - "Symlink target is outside '{}'. Inlining symlink at '{}' to '{}' as file.", - self.root_path.display(), - path.display(), - target.display(), - ); - // inline the symlink and make the target file - let file_bytes = std::fs::read(&target) - .with_context(|| format!("Reading {}", path.display()))?; - self.add_file(&path, file_bytes)?; - } else { + match util::fs::canonicalize_path(&path) { + Ok(target) => { + if let Err(StripRootError { .. }) = self.add_symlink(&path, &target) + { + if target.is_file() { + // this may change behavior, so warn the user about it + log::warn!( + "{} Symlink target is outside '{}'. Inlining symlink at '{}' to '{}' as file.", + crate::colors::yellow("Warning"), + self.root_path.display(), + path.display(), + target.display(), + ); + // inline the symlink and make the target file + let file_bytes = std::fs::read(&target) + .with_context(|| format!("Reading {}", path.display()))?; + self.add_file(&path, file_bytes)?; + } else { + log::warn!( + "{} Symlink target is outside '{}'. Excluding symlink at '{}' with target '{}'.", + crate::colors::yellow("Warning"), + self.root_path.display(), + path.display(), + target.display(), + ); + } + } + } + Err(err) => { log::warn!( - "Symlink target is outside '{}'. Excluding symlink at '{}' with target '{}'.", - self.root_path.display(), + "{} Failed resolving symlink. Ignoring.\n Path: {}\n Message: {:#}", + crate::colors::yellow("Warning"), path.display(), - target.display(), + err ); } } diff --git a/cli/tests/integration/compile_tests.rs b/cli/tests/integration/compile_tests.rs index 9963a044b34f20..95d214429b843b 100644 --- a/cli/tests/integration/compile_tests.rs +++ b/cli/tests/integration/compile_tests.rs @@ -1062,6 +1062,44 @@ fn compile_node_modules_symlink_outside() { output.assert_matches_file("compile/node_modules_symlink_outside/main.out"); } +#[test] +fn compile_node_modules_symlink_non_existent() { + let context = TestContextBuilder::for_npm().use_temp_cwd().build(); + let temp_dir = context.temp_dir().path(); + temp_dir.join("main.ts").write( + r#"import { getValue, setValue } from "npm:@denotest/esm-basic"; +setValue(4); +console.log(getValue());"#, + ); + let node_modules_dir = temp_dir.join("node_modules"); + node_modules_dir.create_dir_all(); + // create a symlink that points to a non_existent file + node_modules_dir.symlink_dir("non_existent", "folder"); + // compile folder + let output = context + .new_command() + .args("compile --allow-read --node-modules-dir --output bin main.ts") + .run(); + output.assert_exit_code(0); + output.assert_matches_text( + r#"Download http://localhost:4545/npm/registry/@denotest/esm-basic +Download http://localhost:4545/npm/registry/@denotest/esm-basic/1.0.0.tgz +Initialize @denotest/esm-basic@1.0.0 +Check file:///[WILDCARD]/main.ts +Compile file:///[WILDCARD]/main.ts to [WILDCARD] +Warning Failed resolving symlink. Ignoring. + Path: [WILDCARD] + Message: [WILDCARD]) +"#, + ); + + // run + let binary_path = + temp_dir.join(if cfg!(windows) { "bin.exe" } else { "bin" }); + let output = context.new_command().name(binary_path).run(); + output.assert_matches_text("4\n"); +} + #[test] fn dynamic_imports_tmp_lit() { let context = TestContextBuilder::new().build(); diff --git a/cli/tests/testdata/compile/node_modules_symlink_outside/main_compile_file.out b/cli/tests/testdata/compile/node_modules_symlink_outside/main_compile_file.out index 7602a40028c1b9..1154c3256b0019 100644 --- a/cli/tests/testdata/compile/node_modules_symlink_outside/main_compile_file.out +++ b/cli/tests/testdata/compile/node_modules_symlink_outside/main_compile_file.out @@ -1,2 +1,2 @@ Compile file:///[WILDCARD]/node_modules_symlink_outside/main.ts to [WILDCARD] -Symlink target is outside '[WILDCARD]node_modules_symlink_outside[WILDCARD]node_modules'. Inlining symlink at '[WILDCARD]node_modules_symlink_outside[WILDCARD]node_modules[WILDCARD]test.txt' to '[WILDCARD]node_modules_symlink_outside[WILDCARD]test.txt' as file. +Warning Symlink target is outside '[WILDCARD]node_modules_symlink_outside[WILDCARD]node_modules'. Inlining symlink at '[WILDCARD]node_modules_symlink_outside[WILDCARD]node_modules[WILDCARD]test.txt' to '[WILDCARD]node_modules_symlink_outside[WILDCARD]test.txt' as file. diff --git a/cli/tests/testdata/compile/node_modules_symlink_outside/main_compile_folder.out b/cli/tests/testdata/compile/node_modules_symlink_outside/main_compile_folder.out index 883a3f262f898f..e88b9583494f2e 100644 --- a/cli/tests/testdata/compile/node_modules_symlink_outside/main_compile_folder.out +++ b/cli/tests/testdata/compile/node_modules_symlink_outside/main_compile_folder.out @@ -3,4 +3,4 @@ Download http://localhost:4545/npm/registry/@denotest/esm-basic/1.0.0.tgz Initialize @denotest/esm-basic@1.0.0 Check file:///[WILDCARD]/node_modules_symlink_outside/main.ts Compile file:///[WILDCARD]/node_modules_symlink_outside/main.ts to [WILDCARD] -Symlink target is outside '[WILDCARD]node_modules_symlink_outside[WILDCARD]node_modules'. Excluding symlink at '[WILDCARD]node_modules_symlink_outside[WILDCARD]node_modules[WILDCARD]some_folder' with target '[WILDCARD]node_modules_symlink_outside[WILDCARD]some_folder'. +Warning Symlink target is outside '[WILDCARD]node_modules_symlink_outside[WILDCARD]node_modules'. Excluding symlink at '[WILDCARD]node_modules_symlink_outside[WILDCARD]node_modules[WILDCARD]some_folder' with target '[WILDCARD]node_modules_symlink_outside[WILDCARD]some_folder'. diff --git a/cli/tools/compile.rs b/cli/tools/compile.rs index ebba884cd6b932..b36ee94bd12e20 100644 --- a/cli/tools/compile.rs +++ b/cli/tools/compile.rs @@ -71,8 +71,9 @@ pub async fn compile( ); validate_output_path(&output_path)?; - let mut file = std::fs::File::create(&output_path)?; - binary_writer + let mut file = std::fs::File::create(&output_path) + .with_context(|| format!("Opening file '{}'", output_path.display()))?; + let write_result = binary_writer .write_bin( &mut file, eszip, @@ -81,8 +82,13 @@ pub async fn compile( cli_options, ) .await - .with_context(|| format!("Writing {}", output_path.display()))?; + .with_context(|| format!("Writing {}", output_path.display())); drop(file); + if let Err(err) = write_result { + // errored, so attempt to remove the output path + let _ = std::fs::remove_file(output_path); + return Err(err); + } // set it as executable #[cfg(unix)]