From d318e38b76c8174d48fddfb99064401050cd8333 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= Date: Mon, 30 Jan 2023 16:22:17 +0100 Subject: [PATCH] Revert "fix(watch): preserve `ProcState::file_fetcher` between restarts (#15466) (#17591) This reverts commit 3545bff678f20c3fdf17fe6b26f96cf1b74f917c. --- cli/cache/mod.rs | 4 ++- cli/cache/node.rs | 1 - cli/cache/parsed_source.rs | 8 ----- cli/graph_util.rs | 8 ++--- cli/module_loader.rs | 1 + cli/proc_state.rs | 50 +++++++------------------- cli/tests/integration/watcher_tests.rs | 40 +-------------------- cli/tools/bench.rs | 12 +++---- cli/tools/run.rs | 16 +++++---- cli/tools/test.rs | 13 +++---- cli/util/file_watcher.rs | 2 +- runtime/ops/signal.rs | 4 +-- test_util/src/lib.rs | 11 ------ 13 files changed, 43 insertions(+), 127 deletions(-) diff --git a/cli/cache/mod.rs b/cli/cache/mod.rs index c8fcaa22343965..a52fe5f4860b57 100644 --- a/cli/cache/mod.rs +++ b/cli/cache/mod.rs @@ -50,10 +50,12 @@ pub struct FetchCacher { impl FetchCacher { pub fn new( emit_cache: EmitCache, - file_fetcher: Arc, + file_fetcher: FileFetcher, root_permissions: PermissionsContainer, dynamic_permissions: PermissionsContainer, ) -> Self { + let file_fetcher = Arc::new(file_fetcher); + Self { emit_cache, dynamic_permissions, diff --git a/cli/cache/node.rs b/cli/cache/node.rs index b1977222911a72..da17633aa385f6 100644 --- a/cli/cache/node.rs +++ b/cli/cache/node.rs @@ -24,7 +24,6 @@ struct CjsAnalysisData { pub reexports: Vec, } -#[derive(Clone)] pub struct NodeAnalysisCache { db_file_path: Option, inner: Arc>>>, diff --git a/cli/cache/parsed_source.rs b/cli/cache/parsed_source.rs index 6385e7384410c7..30eecf7029c7df 100644 --- a/cli/cache/parsed_source.rs +++ b/cli/cache/parsed_source.rs @@ -67,14 +67,6 @@ impl ParsedSourceCache { } } - pub fn reset_for_file_watcher(&self) -> Self { - Self { - db_cache_path: self.db_cache_path.clone(), - cli_version: self.cli_version.clone(), - sources: Default::default(), - } - } - pub fn get_parsed_source_from_module( &self, module: &deno_graph::Module, diff --git a/cli/graph_util.rs b/cli/graph_util.rs index 3c5545a503a988..30b426667b7fe8 100644 --- a/cli/graph_util.rs +++ b/cli/graph_util.rs @@ -71,7 +71,7 @@ pub struct GraphData { impl GraphData { /// Store data from `graph` into `self`. - pub fn add_graph(&mut self, graph: &ModuleGraph) { + pub fn add_graph(&mut self, graph: &ModuleGraph, reload: bool) { for graph_import in &graph.imports { for dep in graph_import.dependencies.values() { for resolved in [&dep.maybe_code, &dep.maybe_type] { @@ -90,7 +90,7 @@ impl GraphData { let mut has_npm_specifier_in_graph = false; for (specifier, result) in graph.specifiers() { - if self.modules.contains_key(specifier) { + if !reload && self.modules.contains_key(specifier) { continue; } @@ -477,7 +477,7 @@ impl GraphData { impl From<&ModuleGraph> for GraphData { fn from(graph: &ModuleGraph) -> Self { let mut graph_data = GraphData::default(); - graph_data.add_graph(graph); + graph_data.add_graph(graph, false); graph_data } } @@ -549,7 +549,7 @@ pub async fn create_graph_and_maybe_check( let check_js = ps.options.check_js(); let mut graph_data = GraphData::default(); - graph_data.add_graph(&graph); + graph_data.add_graph(&graph, false); graph_data .check( &graph.roots, diff --git a/cli/module_loader.rs b/cli/module_loader.rs index c7872988b1b018..8958a9aa6da0f6 100644 --- a/cli/module_loader.rs +++ b/cli/module_loader.rs @@ -277,6 +277,7 @@ impl ModuleLoader for CliModuleLoader { lib, root_permissions, dynamic_permissions, + false, ) .await } diff --git a/cli/proc_state.rs b/cli/proc_state.rs index 2612af75b4bc90..d161a3334b8566 100644 --- a/cli/proc_state.rs +++ b/cli/proc_state.rs @@ -75,7 +75,7 @@ pub struct ProcState(Arc); pub struct Inner { pub dir: DenoDir, - pub file_fetcher: Arc, + pub file_fetcher: FileFetcher, pub http_client: HttpClient, pub options: Arc, pub emit_cache: EmitCache, @@ -145,38 +145,6 @@ impl ProcState { Ok(ps) } - /// Reset all runtime state to its default. This should be used on file - /// watcher restarts. - pub fn reset_for_file_watcher(&mut self) { - self.0 = Arc::new(Inner { - dir: self.dir.clone(), - options: self.options.clone(), - emit_cache: self.emit_cache.clone(), - emit_options_hash: self.emit_options_hash, - emit_options: self.emit_options.clone(), - file_fetcher: self.file_fetcher.clone(), - http_client: self.http_client.clone(), - graph_data: Default::default(), - lockfile: self.lockfile.clone(), - maybe_import_map: self.maybe_import_map.clone(), - maybe_inspector_server: self.maybe_inspector_server.clone(), - root_cert_store: self.root_cert_store.clone(), - blob_store: Default::default(), - broadcast_channel: Default::default(), - shared_array_buffer_store: Default::default(), - compiled_wasm_module_store: Default::default(), - parsed_source_cache: self.parsed_source_cache.reset_for_file_watcher(), - maybe_resolver: self.maybe_resolver.clone(), - maybe_file_watcher_reporter: self.maybe_file_watcher_reporter.clone(), - node_analysis_cache: self.node_analysis_cache.clone(), - npm_cache: self.npm_cache.clone(), - npm_resolver: self.npm_resolver.clone(), - cjs_resolutions: Default::default(), - progress_bar: self.progress_bar.clone(), - node_std_graph_prepared: AtomicBool::new(false), - }); - } - async fn build_with_sender( cli_options: Arc, maybe_sender: Option>>, @@ -268,7 +236,7 @@ impl ProcState { .write_hashable(&emit_options) .finish(), emit_options, - file_fetcher: Arc::new(file_fetcher), + file_fetcher, http_client, graph_data: Default::default(), lockfile, @@ -303,6 +271,7 @@ impl ProcState { lib: TsTypeLib, root_permissions: PermissionsContainer, dynamic_permissions: PermissionsContainer, + reload_on_watch: bool, ) -> Result<(), AnyError> { log::debug!("Preparing module load."); let _pb_clear_guard = self.progress_bar.clear_guard(); @@ -311,7 +280,7 @@ impl ProcState { r.scheme() == "npm" && NpmPackageReference::from_specifier(r).is_ok() }); - if !has_root_npm_specifier { + if !reload_on_watch && !has_root_npm_specifier { let graph_data = self.graph_data.read(); if self.options.type_check_mode() == TypeCheckMode::None || graph_data.is_type_checked(&roots, &lib) @@ -345,6 +314,7 @@ impl ProcState { struct ProcStateLoader<'a> { inner: &'a mut cache::FetchCacher, graph_data: Arc>, + reload: bool, } impl Loader for ProcStateLoader<'_> { fn get_cache_info( @@ -361,7 +331,9 @@ impl ProcState { let graph_data = self.graph_data.read(); let found_specifier = graph_data.follow_redirect(specifier); match graph_data.get(&found_specifier) { - Some(_) => Box::pin(futures::future::ready(Err(anyhow!("")))), + Some(_) if !self.reload => { + Box::pin(futures::future::ready(Err(anyhow!("")))) + } _ => self.inner.load(specifier, is_dynamic), } } @@ -369,6 +341,7 @@ impl ProcState { let mut loader = ProcStateLoader { inner: &mut cache, graph_data: self.graph_data.clone(), + reload: reload_on_watch, }; let maybe_file_watcher_reporter: Option<&dyn deno_graph::source::Reporter> = @@ -407,7 +380,7 @@ impl ProcState { let (npm_package_reqs, has_node_builtin_specifier) = { let mut graph_data = self.graph_data.write(); - graph_data.add_graph(&graph); + graph_data.add_graph(&graph, reload_on_watch); let check_js = self.options.check_js(); graph_data .check( @@ -506,6 +479,7 @@ impl ProcState { lib, PermissionsContainer::allow_all(), PermissionsContainer::allow_all(), + false, ) .await } @@ -519,7 +493,7 @@ impl ProcState { let node_std_graph = self .create_graph(vec![node::MODULE_ALL_URL.clone()]) .await?; - self.graph_data.write().add_graph(&node_std_graph); + self.graph_data.write().add_graph(&node_std_graph, false); self.node_std_graph_prepared.store(true, Ordering::Relaxed); Ok(()) } diff --git a/cli/tests/integration/watcher_tests.rs b/cli/tests/integration/watcher_tests.rs index 3f586d812299b4..113067122f70e0 100644 --- a/cli/tests/integration/watcher_tests.rs +++ b/cli/tests/integration/watcher_tests.rs @@ -1086,44 +1086,6 @@ fn test_watch_unload_handler_error_on_drop() { check_alive_then_kill(child); } -// Regression test for https://github.com/denoland/deno/issues/15465. -#[test] -fn run_watch_reload_once() { - let _g = util::http_server(); - let t = TempDir::new(); - let file_to_watch = t.path().join("file_to_watch.js"); - let file_content = r#" - import { time } from "http://localhost:4545/dynamic_module.ts"; - console.log(time); - "#; - write(&file_to_watch, file_content).unwrap(); - - let mut child = util::deno_cmd() - .current_dir(util::testdata_path()) - .arg("run") - .arg("--watch") - .arg("--reload") - .arg(&file_to_watch) - .env("NO_COLOR", "1") - .stdout(std::process::Stdio::piped()) - .stderr(std::process::Stdio::piped()) - .spawn() - .unwrap(); - let (mut stdout_lines, mut stderr_lines) = child_lines(&mut child); - - wait_contains("finished", &mut stderr_lines); - let first_output = stdout_lines.next().unwrap(); - - write(&file_to_watch, file_content).unwrap(); - // The remote dynamic module should not have been reloaded again. - - wait_contains("finished", &mut stderr_lines); - let second_output = stdout_lines.next().unwrap(); - assert_eq!(second_output, first_output); - - check_alive_then_kill(child); -} - #[test] fn run_watch_dynamic_imports() { let t = TempDir::new(); @@ -1185,11 +1147,11 @@ fn run_watch_dynamic_imports() { &mut stdout_lines, ); + wait_contains("finished", &mut stderr_lines); wait_for( |m| m.contains("Watching paths") && m.contains("imported2.js"), &mut stderr_lines, ); - wait_contains("finished", &mut stderr_lines); write( &file_to_watch3, diff --git a/cli/tools/bench.rs b/cli/tools/bench.rs index 419ac2da66cbfe..18ac63f3872cea 100644 --- a/cli/tools/bench.rs +++ b/cli/tools/bench.rs @@ -30,7 +30,6 @@ use indexmap::IndexMap; use log::Level; use serde::Deserialize; use serde::Serialize; -use std::cell::RefCell; use std::collections::HashSet; use std::path::Path; use std::path::PathBuf; @@ -338,6 +337,7 @@ async fn check_specifiers( lib, PermissionsContainer::allow_all(), PermissionsContainer::new(permissions), + true, ) .await?; @@ -529,14 +529,12 @@ pub async fn run_benchmarks_with_watch( Permissions::from_options(&ps.options.permissions_options())?; let no_check = ps.options.type_check_mode() == TypeCheckMode::None; - let ps = RefCell::new(ps); - let resolver = |changed: Option>| { let paths_to_watch = bench_options.files.include.clone(); let paths_to_watch_clone = paths_to_watch.clone(); let files_changed = changed.is_some(); let bench_options = &bench_options; - let ps = ps.borrow().clone(); + let ps = ps.clone(); async move { let bench_modules = @@ -640,8 +638,7 @@ pub async fn run_benchmarks_with_watch( let operation = |modules_to_reload: Vec| { let permissions = &permissions; let bench_options = &bench_options; - ps.borrow_mut().reset_for_file_watcher(); - let ps = ps.borrow().clone(); + let ps = ps.clone(); async move { let specifiers = @@ -666,13 +663,12 @@ pub async fn run_benchmarks_with_watch( } }; - let clear_screen = !ps.borrow().options.no_clear_screen(); file_watcher::watch_func( resolver, operation, file_watcher::PrintConfig { job_name: "Bench".to_string(), - clear_screen, + clear_screen: !ps.options.no_clear_screen(), }, ) .await?; diff --git a/cli/tools/run.rs b/cli/tools/run.rs index 2d001c42bced8a..d5b5eb98141176 100644 --- a/cli/tools/run.rs +++ b/cli/tools/run.rs @@ -1,6 +1,7 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. use std::io::Read; +use std::path::PathBuf; use std::sync::Arc; use deno_ast::MediaType; @@ -102,13 +103,16 @@ async fn run_with_watch(flags: Flags, script: String) -> Result { let flags = Arc::new(flags); let main_module = resolve_url_or_path(&script)?; let (sender, receiver) = tokio::sync::mpsc::unbounded_channel(); - let mut ps = - ProcState::build_for_file_watcher((*flags).clone(), sender.clone()).await?; - let operation = |main_module: ModuleSpecifier| { - ps.reset_for_file_watcher(); - let ps = ps.clone(); + let operation = |(sender, main_module): ( + tokio::sync::mpsc::UnboundedSender>, + ModuleSpecifier, + )| { + let flags = flags.clone(); Ok(async move { + let ps = + ProcState::build_for_file_watcher((*flags).clone(), sender.clone()) + .await?; let permissions = PermissionsContainer::new(Permissions::from_options( &ps.options.permissions_options(), )?); @@ -122,7 +126,7 @@ async fn run_with_watch(flags: Flags, script: String) -> Result { util::file_watcher::watch_func2( receiver, operation, - main_module, + (sender, main_module), util::file_watcher::PrintConfig { job_name: "Process".to_string(), clear_screen: !flags.no_clear_screen, diff --git a/cli/tools/test.rs b/cli/tools/test.rs index e680d571873c98..08000c765c75f8 100644 --- a/cli/tools/test.rs +++ b/cli/tools/test.rs @@ -44,7 +44,6 @@ use rand::seq::SliceRandom; use rand::SeedableRng; use regex::Regex; use serde::Deserialize; -use std::cell::RefCell; use std::collections::BTreeMap; use std::collections::HashSet; use std::fmt::Write as _; @@ -956,6 +955,7 @@ pub async fn check_specifiers( lib, PermissionsContainer::new(Permissions::allow_all()), PermissionsContainer::new(permissions.clone()), + false, ) .await?; } @@ -977,6 +977,7 @@ pub async fn check_specifiers( lib, PermissionsContainer::allow_all(), PermissionsContainer::new(permissions), + true, ) .await?; @@ -1352,14 +1353,12 @@ pub async fn run_tests_with_watch( Permissions::from_options(&ps.options.permissions_options())?; let no_check = ps.options.type_check_mode() == TypeCheckMode::None; - let ps = RefCell::new(ps); - let resolver = |changed: Option>| { let paths_to_watch = test_options.files.include.clone(); let paths_to_watch_clone = paths_to_watch.clone(); let files_changed = changed.is_some(); let test_options = &test_options; - let ps = ps.borrow().clone(); + let ps = ps.clone(); async move { let test_modules = if test_options.doc { @@ -1467,8 +1466,7 @@ pub async fn run_tests_with_watch( let operation = |modules_to_reload: Vec| { let permissions = &permissions; let test_options = &test_options; - ps.borrow_mut().reset_for_file_watcher(); - let ps = ps.borrow().clone(); + let ps = ps.clone(); async move { let specifiers_with_mode = fetch_specifiers_with_test_mode( @@ -1504,13 +1502,12 @@ pub async fn run_tests_with_watch( } }; - let clear_screen = !ps.borrow().options.no_clear_screen(); file_watcher::watch_func( resolver, operation, file_watcher::PrintConfig { job_name: "Test".to_string(), - clear_screen, + clear_screen: !ps.options.no_clear_screen(), }, ) .await?; diff --git a/cli/util/file_watcher.rs b/cli/util/file_watcher.rs index 05415f2a63edf2..cb8ee419a6b269 100644 --- a/cli/util/file_watcher.rs +++ b/cli/util/file_watcher.rs @@ -322,13 +322,13 @@ where continue; }, _ = operation_future => { - consume_paths_to_watch(&mut watcher, &mut paths_to_watch_receiver); // TODO(bartlomieju): print exit code here? info!( "{} {} finished. Restarting on file change...", colors::intense_blue("Watcher"), job_name, ); + consume_paths_to_watch(&mut watcher, &mut paths_to_watch_receiver); }, }; diff --git a/runtime/ops/signal.rs b/runtime/ops/signal.rs index f88d870582f001..39458bad98fc27 100644 --- a/runtime/ops/signal.rs +++ b/runtime/ops/signal.rs @@ -298,7 +298,7 @@ pub fn signal_str_to_int(s: &str) -> Result { "SIGINFO" => Ok(29), "SIGUSR1" => Ok(30), "SIGUSR2" => Ok(31), - _ => Err(type_error(format!("Invalid signal: {}", s))), + _ => Err(type_error(format!("Invalid signal: {s}"))), } } @@ -336,7 +336,7 @@ pub fn signal_int_to_str(s: libc::c_int) -> Result<&'static str, AnyError> { 29 => Ok("SIGINFO"), 30 => Ok("SIGUSR1"), 31 => Ok("SIGUSR2"), - _ => Err(type_error(format!("Invalid signal: {}", s))), + _ => Err(type_error(format!("Invalid signal: {s}"))), } } diff --git a/test_util/src/lib.rs b/test_util/src/lib.rs index 45008cf1fb1d7c..0b98f094c0a64b 100644 --- a/test_util/src/lib.rs +++ b/test_util/src/lib.rs @@ -971,17 +971,6 @@ async fn main_server( ); Ok(res) } - (_, "/dynamic_module.ts") => { - let mut res = Response::new(Body::from(format!( - r#"export const time = {};"#, - std::time::SystemTime::now().elapsed().unwrap().as_nanos() - ))); - res.headers_mut().insert( - "Content-type", - HeaderValue::from_static("application/typescript"), - ); - Ok(res) - } (_, "/echo_accept") => { let accept = req.headers().get("accept").map(|v| v.to_str().unwrap()); let res = Response::new(Body::from(