fix(cli): local sources are not cached in memory (denoland#8328)

Fixes denoland#4743
Closes denoland#5253
Fixes denoland#5631
Fixes denoland#6116

Co-authored-by: Bartek Iwańczuk <[email protected]>
Co-authored-by: Luca Casonato <[email protected]>

cli/file_fetcher.rs

@@ -451,36 +451,50 @@ impl FileFetcher {
       Ok(file)
     } else {
       let is_local = scheme == "file";
-
-      let result = if is_local {
+      if is_local {
         fetch_local(specifier)
       } else if !self.allow_remote {
         Err(custom_error(
           "NoRemote",
           format!("A remote specifier was requested: \"{}\", but --no-remote is specified.", specifier),
         ))
       } else {
-        self.fetch_remote(specifier, permissions, 10).await
-      };
-
-      if let Ok(file) = &result {
-        self.cache.insert(specifier.clone(), file.clone());
+        let result = self.fetch_remote(specifier, permissions, 10).await;
+        // only cache remote resources, as they are the only things that would
+        // be "expensive" to fetch multiple times during an invocation, and it
+        // also allows local file sources to be changed, enabling things like
+        // dynamic import and workers to be updated while Deno is running.
+        if let Ok(file) = &result {
+          self.cache.insert(specifier.clone(), file.clone());
+        }
+        result
       }
-
-      result
     }
   }
 
-  /// Get a previously in memory cached file.
-  pub fn get_cached(&self, specifier: &ModuleSpecifier) -> Option<File> {
-    self.cache.get(specifier)
-  }
-
   /// Get the location of the current HTTP cache associated with the fetcher.
   pub fn get_http_cache_location(&self) -> PathBuf {
     self.http_cache.location.clone()
   }
 
+  /// A synchronous way to retrieve a source file, where if the file has already
+  /// been cached in memory it will be returned, otherwise for local files will
+  /// be read from disk.
+  pub fn get_source(&self, specifier: &ModuleSpecifier) -> Option<File> {
+    let maybe_file = self.cache.get(specifier);
+    if maybe_file.is_none() {
+      let is_local = specifier.as_url().scheme() == "file";
+      if is_local {
+        if let Ok(file) = fetch_local(specifier) {
+          return Some(file);
+        }
+      }
+      None
+    } else {
+      maybe_file
+    }
+  }
+
   /// Insert a temporary module into the in memory cache for the file fetcher.
   pub fn insert_cached(&self, file: File) -> Option<File> {
     self.cache.insert(file.specifier.clone(), file)
@@ -778,7 +792,7 @@ mod tests {
   }
 
   #[tokio::test]
-  async fn test_get_cached() {
+  async fn test_get_source() {
     let _http_server_guard = test_util::http_server();
     let (file_fetcher, _) = setup(CacheSetting::Use, None);
     let specifier = ModuleSpecifier::resolve_url(
@@ -791,7 +805,7 @@ mod tests {
       .await;
     assert!(result.is_ok());
 
-    let maybe_file = file_fetcher.get_cached(&specifier);
+    let maybe_file = file_fetcher.get_source(&specifier);
     assert!(maybe_file.is_some());
     let file = maybe_file.unwrap();
     assert_eq!(file.source, "export const redirect = 1;\n");
@@ -1297,6 +1311,32 @@ mod tests {
     let _ = fs::remove_dir_all(temp_dir);
   }
 
+  #[tokio::test]
+  async fn test_fetch_local_bypasses_file_cache() {
+    let (file_fetcher, temp_dir) = setup(CacheSetting::Use, None);
+    let fixture_path = temp_dir.path().join("mod.ts");
+    let specifier =
+      ModuleSpecifier::resolve_url_or_path(&fixture_path.to_string_lossy())
+        .unwrap();
+    fs::write(fixture_path.clone(), r#"console.log("hello deno");"#)
+      .expect("could not write file");
+    let result = file_fetcher
+      .fetch(&specifier, &Permissions::allow_all())
+      .await;
+    assert!(result.is_ok());
+    let file = result.unwrap();
+    assert_eq!(file.source, r#"console.log("hello deno");"#);
+
+    fs::write(fixture_path, r#"console.log("goodbye deno");"#)
+      .expect("could not write file");
+    let result = file_fetcher
+      .fetch(&specifier, &Permissions::allow_all())
+      .await;
+    assert!(result.is_ok());
+    let file = result.unwrap();
+    assert_eq!(file.source, r#"console.log("goodbye deno");"#);
+  }
+
   #[tokio::test]
   async fn test_fetch_local_utf_16be() {
     let expected = String::from_utf8(

cli/program_state.rs

@@ -188,7 +188,7 @@ impl ProgramState {
     // really should just be getting this from the module graph.
     let out = self
       .file_fetcher
-      .get_cached(&module_specifier)
+      .get_source(&module_specifier)
       .expect("Cached source file doesn't exist");
 
     let specifier = out.specifier.clone();
@@ -323,7 +323,7 @@ impl SourceMapGetter for ProgramState {
     line_number: usize,
   ) -> Option<String> {
     if let Ok(specifier) = ModuleSpecifier::resolve_url(file_name) {
-      self.file_fetcher.get_cached(&specifier).map(|out| {
+      self.file_fetcher.get_source(&specifier).map(|out| {
         // Do NOT use .lines(): it skips the terminating empty line.
         // (due to internally using .split_terminator() instead of .split())
         let lines: Vec<&str> = out.source.split('\n').collect();

test_util/src/lib.rs

@@ -860,18 +860,35 @@ impl CheckOutputIntegrationTest {
     reader.read_to_string(&mut actual).unwrap();
 
     let status = process.wait().expect("failed to finish process");
-    let exit_code = status.code().unwrap();
 
-    actual = strip_ansi_codes(&actual).to_string();
-
-    if self.exit_code != exit_code {
-      println!("OUTPUT\n{}\nOUTPUT", actual);
-      panic!(
-        "bad exit code, expected: {:?}, actual: {:?}",
-        self.exit_code, exit_code
-      );
+    if let Some(exit_code) = status.code() {
+      if self.exit_code != exit_code {
+        println!("OUTPUT\n{}\nOUTPUT", actual);
+        panic!(
+          "bad exit code, expected: {:?}, actual: {:?}",
+          self.exit_code, exit_code
+        );
+      }
+    } else {
+      #[cfg(unix)]
+      {
+        use std::os::unix::process::ExitStatusExt;
+        let signal = status.signal().unwrap();
+        println!("OUTPUT\n{}\nOUTPUT", actual);
+        panic!(
+          "process terminated by signal, expected exit code: {:?}, actual signal: {:?}",
+          self.exit_code, signal
+        );
+      }
+      #[cfg(not(unix))]
+      {
+        println!("OUTPUT\n{}\nOUTPUT", actual);
+        panic!("process terminated without status code on non unix platform, expected exit code: {:?}", self.exit_code);
+      }
     }
 
+    actual = strip_ansi_codes(&actual).to_string();
+
     let expected = if let Some(s) = self.output_str {
       s.to_owned()
     } else {