From fad9ff5ea4b3551ed19734012b0295a0b2a8d402 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 28 Mar 2022 14:38:15 -0400 Subject: [PATCH] chore: fix compile_windows_ext test (#14142) --- cli/main.rs | 24 +----- cli/tests/integration/compile_tests.rs | 30 ------- cli/tools/standalone.rs | 112 ++++++++++++++++++------- 3 files changed, 88 insertions(+), 78 deletions(-) diff --git a/cli/main.rs b/cli/main.rs index 214b7c2fe1..49ad7f5195 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -70,7 +70,6 @@ use crate::proc_state::ProcState; use crate::resolver::ImportMapResolver; use crate::resolver::JsxResolver; use crate::source_maps::apply_source_map; -use crate::tools::installer::infer_name_from_url; use deno_ast::MediaType; use deno_core::error::generic_error; use deno_core::error::AnyError; @@ -423,18 +422,8 @@ async fn compile_command( let ps = ProcState::build(Arc::new(flags)).await?; let deno_dir = &ps.dir; - let output = compile_flags.output.as_ref().and_then(|output| { - if fs_util::path_has_trailing_slash(output) { - let infer_file_name = infer_name_from_url(&module_specifier).map(PathBuf::from)?; - Some(output.join(infer_file_name)) - } else { - Some(output.to_path_buf()) - } - }).or_else(|| { - infer_name_from_url(&module_specifier).map(PathBuf::from) - }).ok_or_else(|| generic_error( - "An executable name was not provided. One could not be inferred from the URL. Aborting.", - ))?; + let output_path = + tools::standalone::resolve_compile_executable_output_path(&compile_flags)?; let graph = Arc::try_unwrap( create_graph_and_maybe_check(module_specifier.clone(), &ps, debug).await?, @@ -467,14 +456,9 @@ async fn compile_command( ) .await?; - info!("{} {}", colors::green("Emit"), output.display()); + info!("{} {}", colors::green("Emit"), output_path.display()); - tools::standalone::write_standalone_binary( - output.clone(), - compile_flags.target, - final_bin, - ) - .await?; + tools::standalone::write_standalone_binary(output_path, final_bin).await?; Ok(0) } diff --git a/cli/tests/integration/compile_tests.rs b/cli/tests/integration/compile_tests.rs index c1f42632bf..9d1001ed66 100644 --- a/cli/tests/integration/compile_tests.rs +++ b/cli/tests/integration/compile_tests.rs @@ -36,36 +36,6 @@ fn compile() { assert_eq!(output.stdout, "Welcome to Deno!\n".as_bytes()); } -// this is ignored, because when building on a release build, the test attempts -// to download a binary of a yet to be published version. -// TODO(@kitsonk) https://github.com/denoland/deno/issues/14103 -#[ignore] -#[test] -#[cfg(windows)] -// https://github.com/denoland/deno/issues/9667 -fn compile_windows_ext() { - let dir = TempDir::new().unwrap(); - let exe = dir.path().join("welcome_9667"); - let output = util::deno_cmd() - .current_dir(util::root_path()) - .arg("compile") - .arg("--unstable") - .arg("--output") - .arg(&exe) - .arg("--target") - .arg("x86_64-unknown-linux-gnu") - .arg("./test_util/std/examples/welcome.ts") - // TODO(kt3k): Prints command output to the test log for debugging purpose. - // Uncomment this line when this test become stable. - //.stdout(std::process::Stdio::piped()) - .spawn() - .unwrap() - .wait_with_output() - .unwrap(); - assert!(output.status.success()); - assert!(std::path::Path::new(&exe).exists()); -} - #[test] fn standalone_args() { let dir = TempDir::new().unwrap(); diff --git a/cli/tools/standalone.rs b/cli/tools/standalone.rs index dc9ca45085..3d88d543bb 100644 --- a/cli/tools/standalone.rs +++ b/cli/tools/standalone.rs @@ -2,15 +2,19 @@ use crate::deno_dir::DenoDir; use crate::flags::CheckFlag; +use crate::flags::CompileFlags; use crate::flags::DenoSubcommand; use crate::flags::Flags; use crate::flags::RunFlags; +use crate::fs_util; use crate::standalone::Metadata; use crate::standalone::MAGIC_TRAILER; use crate::ProcState; use deno_core::anyhow::bail; use deno_core::anyhow::Context; +use deno_core::error::generic_error; use deno_core::error::AnyError; +use deno_core::resolve_url_or_path; use deno_core::serde_json; use deno_core::url::Url; use deno_graph::ModuleSpecifier; @@ -26,6 +30,8 @@ use std::io::Write; use std::path::Path; use std::path::PathBuf; +use super::installer::infer_name_from_url; + pub async fn get_base_binary( deno_dir: &DenoDir, target: Option, @@ -159,44 +165,26 @@ pub async fn create_standalone_binary( /// This function writes out a final binary to specified path. If output path /// is not already standalone binary it will return error instead. pub async fn write_standalone_binary( - output: PathBuf, - target: Option, + output_path: PathBuf, final_bin: Vec, ) -> Result<(), AnyError> { - let output = match target { - Some(target) => { - if target.contains("windows") { - output.with_extension("exe") - } else { - output - } - } - None => { - if cfg!(windows) && output.extension().unwrap_or_default() != "exe" { - output.with_extension("exe") - } else { - output - } - } - }; - - if output.exists() { + if output_path.exists() { // If the output is a directory, throw error - if output.is_dir() { + if output_path.is_dir() { bail!( concat!( "Could not compile to file '{}' because a directory exists with ", "the same name. You can use the `--output ` flag to ", "provide an alternative name." ), - output.display() + output_path.display() ); } // Make sure we don't overwrite any file not created by Deno compiler. // Check for magic trailer in last 24 bytes. let mut has_trailer = false; - let mut output_file = File::open(&output)?; + let mut output_file = File::open(&output_path)?; // This seek may fail because the file is too small to possibly be // `deno compile` output. if output_file.seek(SeekFrom::End(-24)).is_ok() { @@ -212,15 +200,15 @@ pub async fn write_standalone_binary( "and cannot be overwritten. Please delete the existing file or ", "use the `--output Result { + let module_specifier = resolve_url_or_path(&compile_flags.source_file)?; + compile_flags.output.as_ref().and_then(|output| { + if fs_util::path_has_trailing_slash(output) { + let infer_file_name = infer_name_from_url(&module_specifier).map(PathBuf::from)?; + Some(output.join(infer_file_name)) + } else { + Some(output.to_path_buf()) + } + }).or_else(|| { + infer_name_from_url(&module_specifier).map(PathBuf::from) + }).ok_or_else(|| generic_error( + "An executable name was not provided. One could not be inferred from the URL. Aborting.", + )).map(|output| { + match &compile_flags.target { + Some(target) => { + if target.contains("windows") { + output.with_extension("exe") + } else { + output + } + } + None => { + if cfg!(windows) && output.extension().unwrap_or_default() != "exe" { + output.with_extension("exe") + } else { + output + } + } + } + }) +} + +#[cfg(test)] +mod test { + pub use super::*; + + #[test] + fn resolve_compile_executable_output_path_target_linux() { + let path = resolve_compile_executable_output_path(&CompileFlags { + source_file: "mod.ts".to_string(), + output: Some(PathBuf::from("./file")), + args: Vec::new(), + target: Some("x86_64-unknown-linux-gnu".to_string()), + }) + .unwrap(); + + // no extension, no matter what the operating system is + // because the target was specified as linux + // https://github.com/denoland/deno/issues/9667 + assert_eq!(path.file_name().unwrap(), "file"); + } + + #[test] + fn resolve_compile_executable_output_path_target_windows() { + let path = resolve_compile_executable_output_path(&CompileFlags { + source_file: "mod.ts".to_string(), + output: Some(PathBuf::from("./file")), + args: Vec::new(), + target: Some("x86_64-pc-windows-msvc".to_string()), + }) + .unwrap(); + assert_eq!(path.file_name().unwrap(), "file.exe"); + } +}