From 91443bbc0b3e5420d8a0b3506f1b18a88d48560a Mon Sep 17 00:00:00 2001 From: David Sherret Date: Thu, 8 Dec 2022 11:50:09 -0500 Subject: [PATCH] fix(compile): ensure import map is used when specified in deno config file (#16990) Closes #14246 --- cli/args/flags.rs | 21 +-- cli/args/mod.rs | 50 +++++-- cli/main.rs | 59 ++------ cli/proc_state.rs | 4 +- cli/tests/compile_tests.rs | 34 +++++ .../compile/standalone_import_map_config.json | 3 + cli/tools/doc.rs | 4 +- cli/tools/info.rs | 6 +- cli/tools/installer.rs | 3 +- cli/tools/standalone.rs | 132 +++++------------- cli/tsc/mod.rs | 25 ++++ cli/worker.rs | 6 +- 12 files changed, 163 insertions(+), 184 deletions(-) create mode 100644 cli/tests/testdata/compile/standalone_import_map_config.json diff --git a/cli/args/flags.rs b/cli/args/flags.rs index f1acbf48dc..f4bc0c862f 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -10,7 +10,6 @@ use deno_core::serde::Deserialize; use deno_core::serde::Serialize; use deno_core::url::Url; use deno_runtime::permissions::parse_sys_kind; -use deno_runtime::permissions::PermissionsOptions; use log::debug; use log::Level; use once_cell::sync::Lazy; @@ -482,20 +481,6 @@ impl Flags { } } - pub fn permissions_options(&self) -> PermissionsOptions { - PermissionsOptions { - allow_env: self.allow_env.clone(), - allow_hrtime: self.allow_hrtime, - allow_net: self.allow_net.clone(), - allow_ffi: self.allow_ffi.clone(), - allow_read: self.allow_read.clone(), - allow_run: self.allow_run.clone(), - allow_sys: self.allow_sys.clone(), - allow_write: self.allow_write.clone(), - prompt: !self.no_prompt, - } - } - pub fn has_permission(&self) -> bool { self.allow_all || self.allow_hrtime @@ -2997,11 +2982,7 @@ fn permission_args_parse(flags: &mut Flags, matches: &clap::ArgMatches) { flags.allow_ffi = Some(vec![]); flags.allow_hrtime = true; } - #[cfg(not(test))] - let has_no_prompt_env = env::var("DENO_NO_PROMPT") == Ok("1".to_string()); - #[cfg(test)] - let has_no_prompt_env = false; - if has_no_prompt_env || matches.is_present("no-prompt") { + if matches.is_present("no-prompt") { flags.no_prompt = true; } } diff --git a/cli/args/mod.rs b/cli/args/mod.rs index 38e0abea0d..3f3f53d183 100644 --- a/cli/args/mod.rs +++ b/cli/args/mod.rs @@ -176,7 +176,7 @@ struct CliOptionOverrides { import_map_specifier: Option>, } -/// Holds the common options used by many sub commands +/// Holds the resolved options of many sources used by sub commands /// and provides some helper function for creating common objects. pub struct CliOptions { // the source of the options is a detail the rest of the @@ -414,6 +414,14 @@ impl CliOptions { &self.flags.argv } + pub fn ca_file(&self) -> &Option { + &self.flags.ca_file + } + + pub fn ca_stores(&self) -> &Option> { + &self.flags.ca_stores + } + pub fn check_js(&self) -> bool { self .maybe_config_file @@ -467,8 +475,8 @@ impl CliOptions { .unwrap_or(false) } - pub fn location_flag(&self) -> Option<&Url> { - self.flags.location.as_ref() + pub fn location_flag(&self) -> &Option { + &self.flags.location } pub fn maybe_custom_root(&self) -> Option { @@ -483,6 +491,10 @@ impl CliOptions { self.flags.no_clear_screen } + pub fn no_prompt(&self) -> bool { + resolve_no_prompt(&self.flags) + } + pub fn no_remote(&self) -> bool { self.flags.no_remote } @@ -492,7 +504,17 @@ impl CliOptions { } pub fn permissions_options(&self) -> PermissionsOptions { - self.flags.permissions_options() + PermissionsOptions { + allow_env: self.flags.allow_env.clone(), + allow_hrtime: self.flags.allow_hrtime, + allow_net: self.flags.allow_net.clone(), + allow_ffi: self.flags.allow_ffi.clone(), + allow_read: self.flags.allow_read.clone(), + allow_run: self.flags.allow_run.clone(), + allow_sys: self.flags.allow_sys.clone(), + allow_write: self.flags.allow_write.clone(), + prompt: !self.no_prompt(), + } } pub fn reload_flag(&self) -> bool { @@ -525,16 +547,20 @@ impl CliOptions { self.flags.type_check_mode } - pub fn unsafely_ignore_certificate_errors(&self) -> Option<&Vec> { - self.flags.unsafely_ignore_certificate_errors.as_ref() + pub fn unsafely_ignore_certificate_errors(&self) -> &Option> { + &self.flags.unsafely_ignore_certificate_errors } pub fn unstable(&self) -> bool { self.flags.unstable } - pub fn watch_paths(&self) -> Option<&Vec> { - self.flags.watch.as_ref() + pub fn v8_flags(&self) -> &Vec { + &self.flags.v8_flags + } + + pub fn watch_paths(&self) -> &Option> { + &self.flags.watch } } @@ -590,6 +616,14 @@ fn resolve_import_map_specifier( Ok(None) } +/// Resolves the no_prompt value based on the cli flags and environment. +pub fn resolve_no_prompt(flags: &Flags) -> bool { + flags.no_prompt || { + let value = env::var("DENO_NO_PROMPT"); + matches!(value.as_ref().map(|s| s.as_str()), Ok("1")) + } +} + #[cfg(test)] mod test { use super::*; diff --git a/cli/main.rs b/cli/main.rs index 98700c2b7a..05dc768f7d 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -88,60 +88,26 @@ use std::pin::Pin; use std::sync::Arc; use worker::create_main_worker; -pub fn get_types(unstable: bool) -> String { - let mut types = vec![ - tsc::DENO_NS_LIB, - tsc::DENO_CONSOLE_LIB, - tsc::DENO_URL_LIB, - tsc::DENO_WEB_LIB, - tsc::DENO_FETCH_LIB, - tsc::DENO_WEBGPU_LIB, - tsc::DENO_WEBSOCKET_LIB, - tsc::DENO_WEBSTORAGE_LIB, - tsc::DENO_CRYPTO_LIB, - tsc::DENO_BROADCAST_CHANNEL_LIB, - tsc::DENO_NET_LIB, - tsc::SHARED_GLOBALS_LIB, - tsc::DENO_CACHE_LIB, - tsc::WINDOW_LIB, - ]; - - if unstable { - types.push(tsc::UNSTABLE_NS_LIB); - } - - types.join("\n") -} - async fn compile_command( flags: Flags, compile_flags: CompileFlags, ) -> Result { - let debug = flags.log_level == Some(log::Level::Debug); - - let run_flags = tools::standalone::compile_to_runtime_flags( - &flags, - compile_flags.args.clone(), - )?; - + let ps = ProcState::build(flags.clone()).await?; let module_specifier = resolve_url_or_path(&compile_flags.source_file)?; - let ps = ProcState::build(flags).await?; let deno_dir = &ps.dir; 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?, + create_graph_and_maybe_check(module_specifier.clone(), &ps).await?, ) - .map_err(|_| { - generic_error("There should only be one reference to ModuleGraph") - })?; + .unwrap(); // at the moment, we don't support npm specifiers in deno_compile, so show an error error_for_any_npm_specifier(&graph)?; - graph.valid().unwrap(); + graph.valid()?; let parser = ps.parsed_source_cache.as_capturing_parser(); let eszip = eszip::EszipV2::from_graph(graph, &parser, Default::default())?; @@ -161,7 +127,7 @@ async fn compile_command( original_binary, eszip, module_specifier.clone(), - run_flags, + &compile_flags, ps, ) .await?; @@ -275,8 +241,9 @@ async fn eval_command( // type, and so our "fake" specifier needs to have the proper extension. let main_module = resolve_url_or_path(&format!("./$deno$eval.{}", eval_flags.ext))?; - let permissions = Permissions::from_options(&flags.permissions_options())?; let ps = ProcState::build(flags).await?; + let permissions = + Permissions::from_options(&ps.options.permissions_options())?; let mut worker = create_main_worker(&ps, main_module.clone(), permissions).await?; // Create a dummy source file. @@ -306,7 +273,6 @@ async fn eval_command( async fn create_graph_and_maybe_check( root: ModuleSpecifier, ps: &ProcState, - debug: bool, ) -> Result, AnyError> { let mut cache = cache::FetchCacher::new( ps.emit_cache.clone(), @@ -371,7 +337,7 @@ async fn create_graph_and_maybe_check( ps.npm_resolver.clone(), check::CheckOptions { type_check_mode: ps.options.type_check_mode(), - debug, + debug: ps.options.log_level() == Some(log::Level::Debug), maybe_config_specifier, ts_config: ts_config_result.ts_config, log_checks: true, @@ -416,7 +382,6 @@ async fn bundle_command( flags: Flags, bundle_flags: BundleFlags, ) -> Result { - let debug = flags.log_level == Some(log::Level::Debug); let cli_options = Arc::new(CliOptions::from_flags(flags)?); let resolver = |_| { let cli_options = cli_options.clone(); @@ -427,8 +392,7 @@ async fn bundle_command( debug!(">>>>> bundle START"); let ps = ProcState::from_options(cli_options).await?; - let graph = - create_graph_and_maybe_check(module_specifier, &ps, debug).await?; + let graph = create_graph_and_maybe_check(module_specifier, &ps).await?; let mut paths_to_watch: Vec = graph .specifiers() @@ -629,11 +593,12 @@ async fn run_with_watch(flags: Flags, script: String) -> Result { ModuleSpecifier, )| { let flags = flags.clone(); - let permissions = Permissions::from_options(&flags.permissions_options())?; Ok(async move { let ps = ProcState::build_for_file_watcher((*flags).clone(), sender.clone()) .await?; + let permissions = + Permissions::from_options(&ps.options.permissions_options())?; let worker = create_main_worker(&ps, main_module.clone(), permissions).await?; worker.run_for_watcher().await?; @@ -766,7 +731,7 @@ async fn completions_command( } async fn types_command(flags: Flags) -> Result { - let types = get_types(flags.unstable); + let types = tsc::get_types_declaration_file_text(flags.unstable); display::write_to_stdout_ignore_sigpipe(types.as_bytes())?; Ok(0) } diff --git a/cli/proc_state.rs b/cli/proc_state.rs index aae606c806..07f8a0860b 100644 --- a/cli/proc_state.rs +++ b/cli/proc_state.rs @@ -160,9 +160,7 @@ impl ProcState { let progress_bar = ProgressBar::default(); let http_client = HttpClient::new( Some(root_cert_store.clone()), - cli_options - .unsafely_ignore_certificate_errors() - .map(ToOwned::to_owned), + cli_options.unsafely_ignore_certificate_errors().clone(), )?; let file_fetcher = FileFetcher::new( http_cache, diff --git a/cli/tests/compile_tests.rs b/cli/tests/compile_tests.rs index 869a8780ed..970ef5ee39 100644 --- a/cli/tests/compile_tests.rs +++ b/cli/tests/compile_tests.rs @@ -448,6 +448,40 @@ mod compile { assert!(output.status.success()); } + #[test] + fn standalone_import_map_config_file() { + let dir = TempDir::new(); + let exe = if cfg!(windows) { + dir.path().join("import_map.exe") + } else { + dir.path().join("import_map") + }; + let output = util::deno_cmd() + .current_dir(util::testdata_path()) + .arg("compile") + .arg("--unstable") + .arg("--allow-read") + .arg("--config") + .arg("compile/standalone_import_map_config.json") + .arg("--output") + .arg(&exe) + .arg("./compile/standalone_import_map.ts") + .stdout(std::process::Stdio::piped()) + .spawn() + .unwrap() + .wait_with_output() + .unwrap(); + assert!(output.status.success()); + let output = Command::new(exe) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .spawn() + .unwrap() + .wait_with_output() + .unwrap(); + assert!(output.status.success()); + } + #[test] // https://github.com/denoland/deno/issues/12670 fn skip_rebundle() { diff --git a/cli/tests/testdata/compile/standalone_import_map_config.json b/cli/tests/testdata/compile/standalone_import_map_config.json new file mode 100644 index 0000000000..4959827f40 --- /dev/null +++ b/cli/tests/testdata/compile/standalone_import_map_config.json @@ -0,0 +1,3 @@ +{ + "importMap": "./standalone_import_map.json" +} diff --git a/cli/tools/doc.rs b/cli/tools/doc.rs index 30211d3793..b01e968470 100644 --- a/cli/tools/doc.rs +++ b/cli/tools/doc.rs @@ -6,8 +6,8 @@ use crate::colors; use crate::display::write_json_to_stdout; use crate::display::write_to_stdout_ignore_sigpipe; use crate::file_fetcher::File; -use crate::get_types; use crate::proc_state::ProcState; +use crate::tsc::get_types_declaration_file_text; use deno_ast::MediaType; use deno_core::anyhow::bail; use deno_core::error::AnyError; @@ -29,7 +29,7 @@ pub async fn print_docs( let mut doc_nodes = if source_file == "--builtin" { let source_file_specifier = ModuleSpecifier::parse("deno://lib.deno.d.ts").unwrap(); - let content = get_types(ps.options.unstable()); + let content = get_types_declaration_file_text(ps.options.unstable()); let mut loader = deno_graph::source::MemoryLoader::new( vec![( source_file_specifier.to_string(), diff --git a/cli/tools/info.rs b/cli/tools/info.rs index f494c8d009..6ab74360bf 100644 --- a/cli/tools/info.rs +++ b/cli/tools/info.rs @@ -47,7 +47,11 @@ pub async fn info(flags: Flags, info_flags: InfoFlags) -> Result<(), AnyError> { } } else { // If it was just "deno info" print location of caches and exit - print_cache_info(&ps, info_flags.json, ps.options.location_flag())?; + print_cache_info( + &ps, + info_flags.json, + ps.options.location_flag().as_ref(), + )?; } Ok(()) } diff --git a/cli/tools/installer.rs b/cli/tools/installer.rs index 6914c99193..0e915677a0 100644 --- a/cli/tools/installer.rs +++ b/cli/tools/installer.rs @@ -1,5 +1,6 @@ // Copyright 2018-2022 the Deno authors. All rights reserved. MIT license. +use crate::args::resolve_no_prompt; use crate::args::ConfigFlag; use crate::args::Flags; use crate::args::InstallFlags; @@ -357,7 +358,7 @@ fn resolve_shim_data( executable_args.push("--cached-only".to_string()); } - if flags.no_prompt { + if resolve_no_prompt(flags) { executable_args.push("--no-prompt".to_string()); } diff --git a/cli/tools/standalone.rs b/cli/tools/standalone.rs index 4436aaa282..5e9867b12d 100644 --- a/cli/tools/standalone.rs +++ b/cli/tools/standalone.rs @@ -1,10 +1,6 @@ // Copyright 2018-2022 the Deno authors. All rights reserved. MIT license. use crate::args::CompileFlags; -use crate::args::DenoSubcommand; -use crate::args::Flags; -use crate::args::RunFlags; -use crate::args::TypeCheckMode; use crate::cache::DenoDir; use crate::standalone::Metadata; use crate::standalone::MAGIC_TRAILER; @@ -21,7 +17,7 @@ use deno_graph::ModuleSpecifier; use deno_runtime::deno_fetch::reqwest::Client; use deno_runtime::permissions::Permissions; use std::env; -use std::fs::read; +use std::fs; use std::fs::File; use std::io::Read; use std::io::Seek; @@ -98,47 +94,46 @@ pub async fn create_standalone_binary( mut original_bin: Vec, eszip: eszip::EszipV2, entrypoint: ModuleSpecifier, - flags: Flags, + compile_flags: &CompileFlags, ps: ProcState, ) -> Result, AnyError> { let mut eszip_archive = eszip.into_bytes(); - let ca_data = match &flags.ca_file { - Some(ca_file) => Some(read(ca_file)?), - None => None, - }; - let maybe_import_map: Option<(Url, String)> = match flags - .import_map_path - .as_ref() - { - None => None, - Some(import_map_url) => { - let import_map_specifier = deno_core::resolve_url_or_path(import_map_url) - .context(format!("Bad URL (\"{}\") for import map.", import_map_url))?; - let file = ps - .file_fetcher - .fetch(&import_map_specifier, &mut Permissions::allow_all()) - .await - .context(format!( - "Unable to load '{}' import map", - import_map_specifier - ))?; - - Some((import_map_specifier, file.source.to_string())) + let ca_data = match ps.options.ca_file() { + Some(ca_file) => { + Some(fs::read(ca_file).with_context(|| format!("Reading: {}", ca_file))?) } + None => None, }; + let maybe_import_map: Option<(Url, String)> = + match ps.options.resolve_import_map_specifier()? { + None => None, + Some(import_map_specifier) => { + let file = ps + .file_fetcher + .fetch(&import_map_specifier, &mut Permissions::allow_all()) + .await + .context(format!( + "Unable to load '{}' import map", + import_map_specifier + ))?; + + Some((import_map_specifier, file.source.to_string())) + } + }; let metadata = Metadata { - argv: flags.argv.clone(), - unstable: flags.unstable, - seed: flags.seed, - location: flags.location.clone(), - permissions: flags.permissions_options(), - v8_flags: flags.v8_flags.clone(), - unsafely_ignore_certificate_errors: flags - .unsafely_ignore_certificate_errors + argv: compile_flags.args.clone(), + unstable: ps.options.unstable(), + seed: ps.options.seed(), + location: ps.options.location_flag().clone(), + permissions: ps.options.permissions_options(), + v8_flags: ps.options.v8_flags().clone(), + unsafely_ignore_certificate_errors: ps + .options + .unsafely_ignore_certificate_errors() .clone(), - log_level: flags.log_level, - ca_stores: flags.ca_stores, + log_level: ps.options.log_level(), + ca_stores: ps.options.ca_stores().clone(), ca_data, entrypoint, maybe_import_map, @@ -233,67 +228,6 @@ pub async fn write_standalone_binary( Ok(()) } -/// Transform the flags passed to `deno compile` to flags that would be used at -/// runtime, as if `deno run` were used. -/// - Flags that affect module resolution, loading, type checking, etc. aren't -/// applicable at runtime so are set to their defaults like `false`. -/// - Other flags are inherited. -pub fn compile_to_runtime_flags( - flags: &Flags, - baked_args: Vec, -) -> Result { - // IMPORTANT: Don't abbreviate any of this to `..flags` or - // `..Default::default()`. That forces us to explicitly consider how any - // change to `Flags` should be reflected here. - Ok(Flags { - argv: baked_args, - subcommand: DenoSubcommand::Run(RunFlags { - script: "placeholder".to_string(), - }), - allow_all: flags.allow_all, - allow_env: flags.allow_env.clone(), - allow_hrtime: flags.allow_hrtime, - allow_net: flags.allow_net.clone(), - allow_ffi: flags.allow_ffi.clone(), - allow_read: flags.allow_read.clone(), - allow_run: flags.allow_run.clone(), - allow_sys: flags.allow_sys.clone(), - allow_write: flags.allow_write.clone(), - ca_stores: flags.ca_stores.clone(), - ca_file: flags.ca_file.clone(), - cache_blocklist: vec![], - cache_path: None, - cached_only: false, - config_flag: Default::default(), - coverage_dir: flags.coverage_dir.clone(), - enable_testing_features: false, - ignore: vec![], - import_map_path: flags.import_map_path.clone(), - inspect_brk: None, - inspect: None, - node_modules_dir: false, - location: flags.location.clone(), - lock_write: false, - lock: None, - log_level: flags.log_level, - type_check_mode: TypeCheckMode::Local, - unsafely_ignore_certificate_errors: flags - .unsafely_ignore_certificate_errors - .clone(), - no_remote: false, - no_lock: false, - no_npm: false, - no_prompt: flags.no_prompt, - reload: false, - seed: flags.seed, - unstable: flags.unstable, - v8_flags: flags.v8_flags.clone(), - version: false, - watch: None, - no_clear_screen: false, - }) -} - pub fn resolve_compile_executable_output_path( compile_flags: &CompileFlags, ) -> Result { diff --git a/cli/tsc/mod.rs b/cli/tsc/mod.rs index c22640748b..18d72e413d 100644 --- a/cli/tsc/mod.rs +++ b/cli/tsc/mod.rs @@ -87,6 +87,31 @@ pub static COMPILER_SNAPSHOT: Lazy> = Lazy::new( }, ); +pub fn get_types_declaration_file_text(unstable: bool) -> String { + let mut types = vec![ + DENO_NS_LIB, + DENO_CONSOLE_LIB, + DENO_URL_LIB, + DENO_WEB_LIB, + DENO_FETCH_LIB, + DENO_WEBGPU_LIB, + DENO_WEBSOCKET_LIB, + DENO_WEBSTORAGE_LIB, + DENO_CRYPTO_LIB, + DENO_BROADCAST_CHANNEL_LIB, + DENO_NET_LIB, + SHARED_GLOBALS_LIB, + DENO_CACHE_LIB, + WINDOW_LIB, + ]; + + if unstable { + types.push(UNSTABLE_NS_LIB); + } + + types.join("\n") +} + pub fn compiler_snapshot() -> Snapshot { Snapshot::Static(&COMPILER_SNAPSHOT) } diff --git a/cli/worker.rs b/cli/worker.rs index 4762bab1f2..27cefc1b8d 100644 --- a/cli/worker.rs +++ b/cli/worker.rs @@ -510,7 +510,7 @@ async fn create_main_worker_internal( .map_or(false, |l| l == log::Level::Debug), enable_testing_features: ps.options.enable_testing_features(), locale: deno_core::v8::icu::get_language_tag(), - location: ps.options.location_flag().map(ToOwned::to_owned), + location: ps.options.location_flag().clone(), no_color: !colors::use_color(), is_tty: colors::is_tty(), runtime_version: version::deno(), @@ -524,7 +524,7 @@ async fn create_main_worker_internal( unsafely_ignore_certificate_errors: ps .options .unsafely_ignore_certificate_errors() - .map(ToOwned::to_owned), + .clone(), root_cert_store: Some(ps.root_cert_store.clone()), seed: ps.options.seed(), source_map_getter: Some(Box::new(module_loader.clone())), @@ -685,7 +685,7 @@ fn create_web_worker_callback( unsafely_ignore_certificate_errors: ps .options .unsafely_ignore_certificate_errors() - .map(ToOwned::to_owned), + .clone(), root_cert_store: Some(ps.root_cert_store.clone()), seed: ps.options.seed(), create_web_worker_cb,