From 65de5fb4658e92f0730901b8b083af375050bd64 Mon Sep 17 00:00:00 2001 From: Maxim <59533214+biryukovmaxim@users.noreply.github.com> Date: Fri, 11 Feb 2022 22:04:31 +0300 Subject: [PATCH] refactor: use `Arc` instead of making copies of `Flags` struct (#13610) --- cli/flags.rs | 22 +++++++-------- cli/lsp/cache.rs | 4 +-- cli/main.rs | 59 +++++++++++++++++++-------------------- cli/proc_state.rs | 4 +-- cli/standalone.rs | 2 +- cli/tools/coverage/mod.rs | 3 +- cli/tools/doc.rs | 4 +-- cli/tools/fmt.rs | 2 +- cli/tools/lint.rs | 1 + cli/tools/standalone.rs | 29 +++++++++---------- cli/tools/test.rs | 34 +++++++++++----------- 11 files changed, 81 insertions(+), 83 deletions(-) diff --git a/cli/flags.rs b/cli/flags.rs index d4ba685fb8..7255502fec 100644 --- a/cli/flags.rs +++ b/cli/flags.rs @@ -384,19 +384,17 @@ impl Flags { vec![] } } -} -impl From for PermissionsOptions { - fn from(flags: Flags) -> Self { - Self { - allow_env: flags.allow_env, - allow_hrtime: flags.allow_hrtime, - allow_net: flags.allow_net, - allow_ffi: flags.allow_ffi, - allow_read: flags.allow_read, - allow_run: flags.allow_run, - allow_write: flags.allow_write, - prompt: flags.prompt, + 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_write: self.allow_write.clone(), + prompt: self.prompt, } } } diff --git a/cli/lsp/cache.rs b/cli/lsp/cache.rs index d174907d5f..8b2c85dec7 100644 --- a/cli/lsp/cache.rs +++ b/cli/lsp/cache.rs @@ -50,13 +50,13 @@ impl CacheServer { let _join_handle = thread::spawn(move || { let runtime = create_basic_runtime(); runtime.block_on(async { - let ps = ProcState::build(Flags { + let ps = ProcState::build(Arc::new(Flags { cache_path: maybe_cache_path, ca_stores: maybe_ca_stores, ca_file: maybe_ca_file, unsafely_ignore_certificate_errors, ..Default::default() - }) + })) .await .unwrap(); let maybe_import_map_resolver = diff --git a/cli/main.rs b/cli/main.rs index acf49bb3f1..f8c8e695ac 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -395,13 +395,11 @@ async fn compile_command( ) -> Result { let debug = flags.log_level == Some(log::Level::Debug); - let run_flags = tools::standalone::compile_to_runtime_flags( - flags.clone(), - compile_flags.args, - )?; + let run_flags = + tools::standalone::compile_to_runtime_flags(&flags, compile_flags.args)?; let module_specifier = resolve_url_or_path(&compile_flags.source_file)?; - let ps = ProcState::build(flags.clone()).await?; + let ps = ProcState::build(Arc::new(flags)).await?; let deno_dir = &ps.dir; let output = compile_flags.output.and_then(|output| { @@ -432,7 +430,7 @@ async fn compile_command( }) .flatten() .unwrap_or_else(|| { - bundle_module_graph(graph.as_ref(), &ps, &flags).map(|r| r.0) + bundle_module_graph(graph.as_ref(), &ps, &ps.flags).map(|r| r.0) })?; info!( @@ -468,7 +466,7 @@ async fn info_command( flags: Flags, info_flags: InfoFlags, ) -> Result { - let ps = ProcState::build(flags).await?; + let ps = ProcState::build(Arc::new(flags)).await?; if let Some(specifier) = info_flags.file { let specifier = resolve_url_or_path(&specifier)?; let mut cache = cache::FetchCacher::new( @@ -526,8 +524,9 @@ async fn install_command( let mut preload_flags = flags.clone(); preload_flags.inspect = None; preload_flags.inspect_brk = None; - let permissions = Permissions::from_options(&preload_flags.clone().into()); - let ps = ProcState::build(preload_flags).await?; + let permissions = + Permissions::from_options(&preload_flags.permissions_options()); + let ps = ProcState::build(Arc::new(preload_flags)).await?; let main_module = resolve_url_or_path(&install_flags.module_url)?; let mut worker = create_main_worker(&ps, main_module.clone(), permissions, vec![]); @@ -571,7 +570,7 @@ async fn cache_command( } else { emit::TypeLib::DenoWindow }; - let ps = ProcState::build(flags).await?; + let ps = ProcState::build(Arc::new(flags)).await?; for file in cache_flags.files { let specifier = resolve_url_or_path(&file)?; @@ -597,8 +596,8 @@ 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)).unwrap(); - let permissions = Permissions::from_options(&flags.clone().into()); - let ps = ProcState::build(flags.clone()).await?; + let permissions = Permissions::from_options(&flags.permissions_options()); + let ps = ProcState::build(Arc::new(flags)).await?; let mut worker = create_main_worker(&ps, main_module.clone(), permissions, vec![]); // Create a dummy source file. @@ -622,7 +621,7 @@ async fn eval_command( // to allow module access by TS compiler. ps.file_fetcher.insert_cached(file); debug!("main_module {}", &main_module); - if flags.compat { + if ps.flags.compat { worker.execute_side_module(&compat::GLOBAL_URL).await?; } worker.execute_main_module(&main_module).await?; @@ -786,7 +785,7 @@ async fn bundle_command( bundle_flags: BundleFlags, ) -> Result { let debug = flags.log_level == Some(log::Level::Debug); - + let flags = Arc::new(flags); let resolver = |_| { let flags = flags.clone(); let source_file1 = bundle_flags.source_file.clone(); @@ -795,7 +794,7 @@ async fn bundle_command( let module_specifier = resolve_url_or_path(&source_file1)?; debug!(">>>>> bundle START"); - let ps = ProcState::build(flags.clone()).await?; + let ps = ProcState::build(flags).await?; let graph = create_graph_and_maybe_check(module_specifier, &ps, debug).await?; @@ -831,11 +830,10 @@ async fn bundle_command( }; let operation = |(ps, graph): (ProcState, Arc)| { - let flags = flags.clone(); let out_file = bundle_flags.out_file.clone(); async move { let (bundle_emit, maybe_bundle_map) = - bundle_module_graph(graph.as_ref(), &ps, &flags)?; + bundle_module_graph(graph.as_ref(), &ps, &ps.flags)?; debug!(">>>>> bundle END"); if let Some(out_file) = out_file.as_ref() { @@ -908,7 +906,7 @@ async fn format_command( flags: Flags, fmt_flags: FmtFlags, ) -> Result { - let ps = ProcState::build(flags.clone()).await?; + let ps = ProcState::build(Arc::new(flags)).await?; let maybe_fmt_config = if let Some(config_file) = &ps.maybe_config_file { config_file.to_fmt_config()? } else { @@ -923,7 +921,7 @@ async fn format_command( return Ok(0); } - tools::fmt::format(flags, fmt_flags, maybe_fmt_config).await?; + tools::fmt::format(ps.flags.as_ref(), fmt_flags, maybe_fmt_config).await?; Ok(0) } @@ -932,11 +930,11 @@ async fn repl_command( repl_flags: ReplFlags, ) -> Result { let main_module = resolve_url_or_path("./$deno$repl.ts").unwrap(); - let permissions = Permissions::from_options(&flags.clone().into()); - let ps = ProcState::build(flags.clone()).await?; + let permissions = Permissions::from_options(&flags.permissions_options()); + let ps = ProcState::build(Arc::new(flags)).await?; let mut worker = create_main_worker(&ps, main_module.clone(), permissions, vec![]); - if flags.compat { + if ps.flags.compat { worker.execute_side_module(&compat::GLOBAL_URL).await?; compat::add_global_require(&mut worker.js_runtime, main_module.as_str())?; worker.run_event_loop(false).await?; @@ -948,8 +946,8 @@ async fn repl_command( } async fn run_from_stdin(flags: Flags) -> Result { - let ps = ProcState::build(flags.clone()).await?; - let permissions = Permissions::from_options(&flags.clone().into()); + let ps = ProcState::build(Arc::new(flags)).await?; + let permissions = Permissions::from_options(&ps.flags.permissions_options()); let main_module = resolve_url_or_path("./$deno$stdin.ts").unwrap(); let mut worker = create_main_worker(&ps.clone(), main_module.clone(), permissions, vec![]); @@ -970,7 +968,7 @@ async fn run_from_stdin(flags: Flags) -> Result { ps.file_fetcher.insert_cached(source_file); debug!("main_module {}", main_module); - if flags.compat { + if ps.flags.compat { worker.execute_side_module(&compat::GLOBAL_URL).await?; } worker.execute_main_module(&main_module).await?; @@ -983,6 +981,7 @@ async fn run_from_stdin(flags: Flags) -> Result { // TODO(bartlomieju): this function is not handling `exit_code` set by the runtime // code properly. async fn run_with_watch(flags: Flags, script: String) -> Result { + let flags = Arc::new(flags); let resolver = |_| { let script1 = script.clone(); let script2 = script.clone(); @@ -1131,7 +1130,7 @@ async fn run_with_watch(flags: Flags, script: String) -> Result { let operation = |(ps, main_module): (ProcState, ModuleSpecifier)| { let flags = flags.clone(); - let permissions = Permissions::from_options(&flags.clone().into()); + let permissions = Permissions::from_options(&flags.permissions_options()); async move { // We make use an module executor guard to ensure that unload is always fired when an // operation is called. @@ -1177,8 +1176,8 @@ async fn run_command( // map specified and bare specifier is used on the command line - this should // probably call `ProcState::resolve` instead let main_module = resolve_url_or_path(&run_flags.script)?; - let ps = ProcState::build(flags.clone()).await?; - let permissions = Permissions::from_options(&flags.clone().into()); + let ps = ProcState::build(Arc::new(flags)).await?; + let permissions = Permissions::from_options(&ps.flags.permissions_options()); let mut worker = create_main_worker(&ps, main_module.clone(), permissions, vec![]); @@ -1199,7 +1198,7 @@ async fn run_command( debug!("main_module {}", main_module); - if flags.compat { + if ps.flags.compat { // TODO(bartlomieju): fix me assert_eq!(main_module.scheme(), "file"); @@ -1326,7 +1325,7 @@ fn init_v8_flags(v8_flags: &[String]) { fn get_subcommand( flags: Flags, ) -> Pin>>> { - match flags.clone().subcommand { + match flags.subcommand.clone() { DenoSubcommand::Bundle(bundle_flags) => { bundle_command(flags, bundle_flags).boxed_local() } diff --git a/cli/proc_state.rs b/cli/proc_state.rs index 5b347d169d..12bdd3149c 100644 --- a/cli/proc_state.rs +++ b/cli/proc_state.rs @@ -65,7 +65,7 @@ pub struct ProcState(Arc); pub struct Inner { /// Flags parsed from `argv` contents. - pub flags: flags::Flags, + pub flags: Arc, pub dir: deno_dir::DenoDir, pub coverage_dir: Option, pub file_fetcher: FileFetcher, @@ -90,7 +90,7 @@ impl Deref for ProcState { } impl ProcState { - pub async fn build(flags: flags::Flags) -> Result { + pub async fn build(flags: Arc) -> Result { let maybe_custom_root = flags .cache_path .clone() diff --git a/cli/standalone.rs b/cli/standalone.rs index a50e7d80d4..c47ec57366 100644 --- a/cli/standalone.rs +++ b/cli/standalone.rs @@ -200,7 +200,7 @@ pub async fn run( ) -> Result<(), AnyError> { let flags = metadata_to_flags(&metadata); let main_module = resolve_url(SPECIFIER)?; - let ps = ProcState::build(flags).await?; + let ps = ProcState::build(Arc::new(flags)).await?; let permissions = Permissions::from_options(&metadata.permissions); let blob_store = BlobStore::default(); let broadcast_channel = InMemoryBroadcastChannel::default(); diff --git a/cli/tools/coverage/mod.rs b/cli/tools/coverage/mod.rs index 6177295a66..2a64677833 100644 --- a/cli/tools/coverage/mod.rs +++ b/cli/tools/coverage/mod.rs @@ -23,6 +23,7 @@ use std::fs::File; use std::io::BufWriter; use std::io::Write; use std::path::PathBuf; +use std::sync::Arc; use text_lines::TextLines; use uuid::Uuid; @@ -559,7 +560,7 @@ pub async fn cover_files( flags: Flags, coverage_flags: CoverageFlags, ) -> Result<(), AnyError> { - let ps = ProcState::build(flags).await?; + let ps = ProcState::build(Arc::new(flags)).await?; let script_coverages = collect_coverages(coverage_flags.files, coverage_flags.ignore)?; diff --git a/cli/tools/doc.rs b/cli/tools/doc.rs index 6e17c9fca2..38f8c29188 100644 --- a/cli/tools/doc.rs +++ b/cli/tools/doc.rs @@ -96,7 +96,7 @@ pub async fn print_docs( flags: Flags, doc_flags: DocFlags, ) -> Result<(), AnyError> { - let ps = ProcState::build(flags.clone()).await?; + let ps = ProcState::build(Arc::new(flags)).await?; let source_file = doc_flags .source_file .unwrap_or_else(|| "--builtin".to_string()); @@ -122,7 +122,7 @@ pub async fn print_docs( doc_parser.parse_source( &source_file_specifier, MediaType::Dts, - Arc::new(get_types(flags.unstable)), + Arc::new(get_types(ps.flags.unstable)), ) } else { let module_specifier = resolve_url_or_path(&source_file)?; diff --git a/cli/tools/fmt.rs b/cli/tools/fmt.rs index a0ab3ea568..45cf69b864 100644 --- a/cli/tools/fmt.rs +++ b/cli/tools/fmt.rs @@ -39,7 +39,7 @@ use std::sync::{Arc, Mutex}; /// Format JavaScript/TypeScript files. pub async fn format( - flags: Flags, + flags: &Flags, fmt_flags: FmtFlags, maybe_fmt_config: Option, ) -> Result<(), AnyError> { diff --git a/cli/tools/lint.rs b/cli/tools/lint.rs index 74c5540fcd..1dfda8adc3 100644 --- a/cli/tools/lint.rs +++ b/cli/tools/lint.rs @@ -50,6 +50,7 @@ fn create_reporter(kind: LintReporterKind) -> Box { } pub async fn lint(flags: Flags, lint_flags: LintFlags) -> Result<(), AnyError> { + let flags = Arc::new(flags); let LintFlags { maybe_rules_tags, maybe_rules_include, diff --git a/cli/tools/standalone.rs b/cli/tools/standalone.rs index ec957e195e..84a1f00fbb 100644 --- a/cli/tools/standalone.rs +++ b/cli/tools/standalone.rs @@ -99,7 +99,7 @@ pub fn create_standalone_binary( unstable: flags.unstable, seed: flags.seed, location: flags.location.clone(), - permissions: flags.clone().into(), + permissions: flags.permissions_options(), v8_flags: flags.v8_flags.clone(), unsafely_ignore_certificate_errors: flags .unsafely_ignore_certificate_errors @@ -200,7 +200,7 @@ pub async fn write_standalone_binary( /// applicable at runtime so are set to their defaults like `false`. /// - Other flags are inherited. pub fn compile_to_runtime_flags( - flags: Flags, + flags: &Flags, baked_args: Vec, ) -> Result { // IMPORTANT: Don't abbreviate any of this to `..flags` or @@ -212,40 +212,41 @@ pub fn compile_to_runtime_flags( script: "placeholder".to_string(), }), allow_all: flags.allow_all, - allow_env: flags.allow_env, + allow_env: flags.allow_env.clone(), allow_hrtime: flags.allow_hrtime, - allow_net: flags.allow_net, - allow_ffi: flags.allow_ffi, - allow_read: flags.allow_read, - allow_run: flags.allow_run, - allow_write: flags.allow_write, - ca_stores: flags.ca_stores, - ca_file: flags.ca_file, + 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_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_path: None, - coverage_dir: flags.coverage_dir, + coverage_dir: flags.coverage_dir.clone(), enable_testing_features: false, ignore: vec![], import_map_path: None, inspect_brk: None, inspect: None, - location: flags.location, + location: flags.location.clone(), lock_write: false, lock: None, log_level: flags.log_level, check: CheckFlag::All, compat: flags.compat, unsafely_ignore_certificate_errors: flags - .unsafely_ignore_certificate_errors, + .unsafely_ignore_certificate_errors + .clone(), no_remote: false, prompt: flags.prompt, reload: false, repl: false, seed: flags.seed, unstable: flags.unstable, - v8_flags: flags.v8_flags, + v8_flags: flags.v8_flags.clone(), version: false, watch: None, no_clear_screen: false, diff --git a/cli/tools/test.rs b/cli/tools/test.rs index 2cc1ac808b..5d99677f20 100644 --- a/cli/tools/test.rs +++ b/cli/tools/test.rs @@ -702,7 +702,7 @@ async fn fetch_inline_files( /// Type check a collection of module and document specifiers. async fn check_specifiers( - ps: ProcState, + ps: &ProcState, permissions: Permissions, specifiers: Vec<(ModuleSpecifier, TestMode)>, lib: emit::TypeLib, @@ -972,7 +972,7 @@ fn collect_specifiers_with_test_mode( /// cannot be run, and therefore need to be marked as `TestMode::Documentation` /// as well. async fn fetch_specifiers_with_test_mode( - ps: ProcState, + ps: &ProcState, include: Vec, ignore: Vec, include_inline: bool, @@ -999,10 +999,10 @@ pub async fn run_tests( flags: Flags, test_flags: TestFlags, ) -> Result<(), AnyError> { - let ps = ProcState::build(flags.clone()).await?; - let permissions = Permissions::from_options(&flags.clone().into()); + let ps = ProcState::build(Arc::new(flags)).await?; + let permissions = Permissions::from_options(&ps.flags.permissions_options()); let specifiers_with_mode = fetch_specifiers_with_test_mode( - ps.clone(), + &ps, test_flags.include.unwrap_or_else(|| vec![".".to_string()]), test_flags.ignore.clone(), test_flags.doc, @@ -1013,30 +1013,26 @@ pub async fn run_tests( return Err(generic_error("No test modules found")); } - let lib = if flags.unstable { + let lib = if ps.flags.unstable { emit::TypeLib::UnstableDenoWindow } else { emit::TypeLib::DenoWindow }; - check_specifiers( - ps.clone(), - permissions.clone(), - specifiers_with_mode.clone(), - lib, - ) - .await?; + check_specifiers(&ps, permissions.clone(), specifiers_with_mode.clone(), lib) + .await?; if test_flags.no_run { return Ok(()); } + let compat = ps.flags.compat; test_specifiers( ps, permissions, specifiers_with_mode, TestSpecifierOptions { - compat_mode: flags.compat, + compat_mode: compat, concurrent_jobs: test_flags.concurrent_jobs, fail_fast: test_flags.fail_fast, filter: test_flags.filter, @@ -1052,8 +1048,9 @@ pub async fn run_tests_with_watch( flags: Flags, test_flags: TestFlags, ) -> Result<(), AnyError> { + let flags = Arc::new(flags); let ps = ProcState::build(flags.clone()).await?; - let permissions = Permissions::from_options(&flags.clone().into()); + let permissions = Permissions::from_options(&flags.permissions_options()); let lib = if flags.unstable { emit::TypeLib::UnstableDenoWindow @@ -1233,6 +1230,7 @@ pub async fn run_tests_with_watch( }; let operation = |modules_to_reload: Vec<(ModuleSpecifier, ModuleKind)>| { + let flags = flags.clone(); let filter = test_flags.filter.clone(); let include = include.clone(); let ignore = ignore.clone(); @@ -1242,7 +1240,7 @@ pub async fn run_tests_with_watch( async move { let specifiers_with_mode = fetch_specifiers_with_test_mode( - ps.clone(), + &ps, include.clone(), ignore.clone(), test_flags.doc, @@ -1256,7 +1254,7 @@ pub async fn run_tests_with_watch( .collect::>(); check_specifiers( - ps.clone(), + &ps, permissions.clone(), specifiers_with_mode.clone(), lib, @@ -1268,7 +1266,7 @@ pub async fn run_tests_with_watch( } test_specifiers( - ps.clone(), + ps, permissions.clone(), specifiers_with_mode, TestSpecifierOptions {