add skip_write option to lockfile

denoland · dsherret · Oct 2, 2024 · Oct 1, 2024 · Oct 1, 2024 · Oct 1, 2024
commit 911325998d2f148c1f7df404286bc11dec2f63b6
diff --git a/cli/args/flags.rs b/cli/args/flags.rs
@@ -580,6 +580,15 @@ pub struct UnstableConfig {
   pub features: Vec<String>, // --unstabe-kv --unstable-cron
 }
 
+#[derive(Clone, Debug, Eq, PartialEq, Default)]
+pub struct InternalFlags {
+  /// Used when the language server is configured with an
+  /// explicit cache option.
+  pub cache_path: Option<PathBuf>,
+  /// Only reads to the lockfile instead of writing to it.
+  pub lockfile_skip_write: bool,
+}
+
 #[derive(Clone, Debug, Eq, PartialEq, Default)]
 pub struct Flags {
   /// Vector of CLI arguments - these are user script arguments, all Deno
@@ -591,9 +600,6 @@ pub struct Flags {
   pub ca_stores: Option<Vec<String>>,
   pub ca_data: Option<CaData>,
   pub cache_blocklist: Vec<String>,
-  /// This is not exposed as an option in the CLI, it is used internally when
-  /// the language server is configured with an explicit cache option.
-  pub cache_path: Option<PathBuf>,
   pub cached_only: bool,
   pub type_check_mode: TypeCheckMode,
   pub config_flag: ConfigFlag,
@@ -602,6 +608,8 @@ pub struct Flags {
   pub enable_op_summary_metrics: bool,
   pub enable_testing_features: bool,
   pub ext: Option<String>,
+  /// Flags that aren't exposed in the CLI, but are used internally.
+  pub internal: InternalFlags,
   pub ignore: Vec<String>,
   pub import_map_path: Option<String>,
   pub env_file: Option<String>,

diff --git a/cli/args/lockfile.rs b/cli/args/lockfile.rs
@@ -24,11 +24,20 @@ use crate::args::InstallKind;
 
 use deno_lockfile::Lockfile;
 
+#[derive(Debug)]
+pub struct CliLockfileReadFromPathOptions {
+  pub file_path: PathBuf,
+  pub frozen: bool,
+  /// Causes the lockfile to only be read from, but not written to.
+  pub skip_write: bool,
+}
+
 #[derive(Debug)]
 pub struct CliLockfile {
   lockfile: Mutex<Lockfile>,
   pub filename: PathBuf,
-  pub frozen: bool,
+  frozen: bool,
+  skip_write: bool,
 }
 
 pub struct Guard<'a, T> {
@@ -50,15 +59,6 @@ impl<'a, T> std::ops::DerefMut for Guard<'a, T> {
 }
 
 impl CliLockfile {
-  pub fn new(lockfile: Lockfile, frozen: bool) -> Self {
-    let filename = lockfile.filename.clone();
-    Self {
-      lockfile: Mutex::new(lockfile),
-      filename,
-      frozen,
-    }
-  }
-
   /// Get the inner deno_lockfile::Lockfile.
   pub fn lock(&self) -> Guard<Lockfile> {
     Guard {
@@ -78,6 +78,10 @@ impl CliLockfile {
   }
 
   pub fn write_if_changed(&self) -> Result<(), AnyError> {
+    if self.skip_write {
+      return Ok(());
+    }
+
     self.error_if_changed()?;
     let mut lockfile = self.lockfile.lock();
     let Some(bytes) = lockfile.resolve_write_bytes() else {
@@ -142,7 +146,7 @@ impl CliLockfile {
       return Ok(None);
     }
 
-    let filename = match flags.lock {
+    let file_path = match flags.lock {
       Some(ref lock) => PathBuf::from(lock),
       None => match workspace.resolve_lockfile_path()? {
         Some(path) => path,
@@ -160,7 +164,11 @@ impl CliLockfile {
         .unwrap_or(false)
     });
 
-    let lockfile = Self::read_from_path(filename, frozen)?;
+    let lockfile = Self::read_from_path(CliLockfileReadFromPathOptions {
+      file_path,
+      frozen,
+      skip_write: flags.internal.lockfile_skip_write,
+    })?;
 
     // initialize the lockfile with the workspace's configuration
     let root_url = workspace.root_dir();
@@ -212,25 +220,29 @@ impl CliLockfile {
   }
 
   pub fn read_from_path(
-    file_path: PathBuf,
-    frozen: bool,
+    opts: CliLockfileReadFromPathOptions,
   ) -> Result<CliLockfile, AnyError> {
-    match std::fs::read_to_string(&file_path) {
-      Ok(text) => Ok(CliLockfile::new(
-        Lockfile::new(deno_lockfile::NewLockfileOptions {
-          file_path,
-          content: &text,
-          overwrite: false,
-        })?,
-        frozen,
-      )),
-      Err(err) if err.kind() == std::io::ErrorKind::NotFound => Ok(
-        CliLockfile::new(Lockfile::new_empty(file_path, false), frozen),
-      ),
-      Err(err) => Err(err).with_context(|| {
-        format!("Failed reading lockfile '{}'", file_path.display())
-      }),
-    }
+    let lockfile = match std::fs::read_to_string(&opts.file_path) {
+      Ok(text) => Lockfile::new(deno_lockfile::NewLockfileOptions {
+        file_path: opts.file_path,
+        content: &text,
+        overwrite: false,
+      })?,
+      Err(err) if err.kind() == std::io::ErrorKind::NotFound => {
+        Lockfile::new_empty(opts.file_path, false)
+      }
+      Err(err) => {
+        return Err(err).with_context(|| {
+          format!("Failed reading lockfile '{}'", opts.file_path.display())
+        });
+      }
+    };
+    Ok(CliLockfile {
+      filename: lockfile.filename.clone(),
+      lockfile: Mutex::new(lockfile),
+      frozen: opts.frozen,
+      skip_write: opts.skip_write,
+    })
   }
 
   pub fn error_if_changed(&self) -> Result<(), AnyError> {

diff --git a/cli/args/mod.rs b/cli/args/mod.rs
@@ -44,6 +44,7 @@ pub use deno_config::glob::FilePatterns;
 pub use deno_json::check_warn_tsconfig;
 pub use flags::*;
 pub use lockfile::CliLockfile;
+pub use lockfile::CliLockfileReadFromPathOptions;
 pub use package_json::NpmInstallDepsProvider;
 
 use deno_ast::ModuleSpecifier;
@@ -828,7 +829,7 @@ impl CliOptions {
 
     let maybe_lockfile = maybe_lockfile.filter(|_| !force_global_cache);
     let deno_dir_provider =
-      Arc::new(DenoDirProvider::new(flags.cache_path.clone()));
+      Arc::new(DenoDirProvider::new(flags.internal.cache_path.clone()));
     let maybe_node_modules_folder = resolve_node_modules_folder(
       &initial_cwd,
       &flags,

diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs
@@ -53,6 +53,7 @@ use super::logging::lsp_log;
 use crate::args::discover_npmrc_from_workspace;
 use crate::args::has_flag_env_var;
 use crate::args::CliLockfile;
+use crate::args::CliLockfileReadFromPathOptions;
 use crate::args::ConfigFile;
 use crate::args::LintFlags;
 use crate::args::LintOptions;
@@ -1931,7 +1932,11 @@ fn resolve_lockfile_from_path(
   lockfile_path: PathBuf,
   frozen: bool,
 ) -> Option<CliLockfile> {
-  match CliLockfile::read_from_path(lockfile_path, frozen) {
+  match CliLockfile::read_from_path(CliLockfileReadFromPathOptions {
+    file_path: lockfile_path,
+    frozen,
+    skip_write: false,
+  }) {
     Ok(value) => {
       if value.filename.exists() {
         if let Ok(specifier) = ModuleSpecifier::from_file_path(&value.filename)

diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs
@@ -96,6 +96,7 @@ use crate::args::CaData;
 use crate::args::CacheSetting;
 use crate::args::CliOptions;
 use crate::args::Flags;
+use crate::args::InternalFlags;
 use crate::args::UnstableFmtOptions;
 use crate::factory::CliFactory;
 use crate::file_fetcher::FileFetcher;
@@ -3600,7 +3601,10 @@ impl Inner {
     };
     let cli_options = CliOptions::new(
       Arc::new(Flags {
-        cache_path: Some(self.cache.deno_dir().root.clone()),
+        internal: InternalFlags {
+          cache_path: Some(self.cache.deno_dir().root.clone()),
+          ..Default::default()
+        },
         ca_stores: workspace_settings.certificate_stores.clone(),
         ca_data: workspace_settings.tls_certificate.clone().map(CaData::File),
         unsafely_ignore_certificate_errors: workspace_settings

diff --git a/cli/main.rs b/cli/main.rs
@@ -193,7 +193,9 @@ async fn run_subcommand(flags: Arc<Flags>) -> Result<i32, AnyError> {
                 };
                 flags.node_modules_dir = Some(deno_config::deno_json::NodeModulesDirMode::None);
                 // use the current lockfile, but don't write it out
-                // todo(THIS PR): add something to prevent this from writing to to the lockfile
+                if flags.frozen_lockfile.is_none() {
+                  flags.internal.lockfile_skip_write = true;
+                }
                 return tools::run::run_script(WorkerExecutionMode::Run, Arc::new(flags), watch).await;
               }
             }