Rework with new changes

denoland · Jun 29, 2024 · 87d8ecc · 87d8ecc
1 parent 2cca787
commit 87d8ecc
Show file tree

Hide file tree

Showing 15 changed files with 57 additions and 89 deletions.
diff --git a/cli/args/lockfile.rs b/cli/args/lockfile.rs
@@ -23,6 23,7 @@ use deno_lockfile::Lockfile;
 pub struct CliLockfile {
  lockfile: Mutex<Lockfile>,
  pub filename: PathBuf,
  pub frozen: bool,
 }
 
 pub struct Guard<'a, T> {
@@ -44,11 45,12 @@ impl<'a, T> std::ops::DerefMut for Guard<'a, T> {
 }
 
 impl CliLockfile {
- pub fn new(lockfile: Lockfile) -> Self {
  pub fn new(lockfile: Lockfile, frozen: bool) -> Self {
  let filename = lockfile.filename.clone();
  Self {
  lockfile: Mutex::new(lockfile),
  filename,
  frozen,
  }
  }
 
@@ -71,6 73,7 @@ impl CliLockfile {
  }
 
  pub fn write_if_changed(&self) -> Result<(), AnyError> {
  self.error_if_changed()?;
  let mut lockfile = self.lockfile.lock();
  let Some(bytes) = lockfile.resolve_write_bytes() else {
  return Ok(()); // nothing to do
@@ -127,48 130,52 @@ impl CliLockfile {
  };
 
  let lockfile = if flags.lock_write {
- CliLockfile::new(Lockfile::new_empty(filename, true))
  CliLockfile::new(
  Lockfile::new_empty(filename, true),
  flags.frozen_lockfile,
  )
  } else {
- Self::read_from_path(filename)?
  Self::read_from_path(filename, flags.frozen_lockfile)?
  };
  Ok(Some(lockfile))
  }
- pub fn read_from_path(filename: PathBuf) -> Result<CliLockfile, AnyError> {
  pub fn read_from_path(
  filename: PathBuf,
  frozen: bool,
  ) -> Result<CliLockfile, AnyError> {
  match std::fs::read_to_string(&filename) {
- Ok(text) => Ok(CliLockfile::new(Lockfile::with_lockfile_content(
- filename, &text, false,
- )?)),
- Err(err) if err.kind() == std::io::ErrorKind::NotFound => {
- Ok(CliLockfile::new(Lockfile::new_empty(filename, false)))
- }
  Ok(text) => Ok(CliLockfile::new(
  Lockfile::with_lockfile_content(filename, &text, false)?,
  frozen,
  )),
  Err(err) if err.kind() == std::io::ErrorKind::NotFound => Ok(
  CliLockfile::new(Lockfile::new_empty(filename, false), frozen),
  ),
  Err(err) => Err(err).with_context(|| {
  format!("Failed reading lockfile '{}'", filename.display())
  }),
  }
  }
-}
-
-pub fn error_if_lockfile_has_changes(
- lockfile: &Lockfile,
- frozen: bool,
-) -> Result<(), AnyError> {
- if frozen && lockfile.has_content_changed {
- let suggested = if *super::DENO_FUTURE {
- "`deno cache`, `deno install`,"
  pub fn error_if_changed(&self) -> Result<(), AnyError> {
  let lockfile = self.lockfile.lock();
  if self.frozen && lockfile.has_content_changed {
  let suggested = if *super::DENO_FUTURE {
  "`deno cache`, `deno install`,"
  } else {
  "`deno cache`"
  };
 
  let contents =
  std::fs::read_to_string(&lockfile.filename).unwrap_or_default();
  let new_contents = lockfile.as_json_string();
  let diff = crate::util::diff::diff(&contents, &new_contents);
  // has an extra newline at the end
  let diff = diff.trim_end();
  Err(deno_core::anyhow::anyhow!(
  "The lockfile is out of date. Run {suggested} or rerun without the `--frozen` flag to update it.\nchanges:\n{diff}"
  ))
  } else {
- "`deno cache`"
- };
-
- let contents =
- std::fs::read_to_string(&lockfile.filename).unwrap_or_default();
- let new_contents = lockfile.as_json_string();
- let diff = crate::util::diff::diff(&contents, &new_contents);
- // has an extra newline at the end
- let diff = diff.trim_end();
- Err(deno_core::anyhow::anyhow!(
- "The lockfile is out of date. Run {suggested} or rerun without the `--frozen` flag to update it.\nchanges:\n{diff}"
- ))
- } else {
- Ok(())
  Ok(())
  }
  }
 }
diff --git a/cli/args/mod.rs b/cli/args/mod.rs
@@ -34,7 34,6 @@ pub use deno_config::TsConfigType;
 pub use deno_config::TsTypeLib;
 pub use deno_config::WorkspaceConfig;
 pub use flags::*;
-pub use lockfile::error_if_lockfile_has_changes;
 pub use lockfile::CliLockfile;
 pub use package_json::PackageJsonDepsProvider;
 
@@ -972,10 971,6 @@ impl CliOptions {
  )
  }
 
- pub fn frozen_lockfile(&self) -> bool {
- self.flags.frozen_lockfile
- }
-
  #[inline(always)]
  pub fn initial_cwd(&self) -> &Path {
  &self.initial_cwd

diff --git a/cli/factory.rs b/cli/factory.rs
@@ -502,7 502,6 @@ impl CliFactory {
  bare_node_builtins_enabled: self
  .options
  .unstable_bare_node_builtins(),
- frozen_lockfile: self.options.frozen_lockfile(),
  })))
  }
  .boxed_local(),
@@ -889,7 888,6 @@ impl CliFactory {
  maybe_root_package_json_deps: self.options.maybe_package_json_deps(),
  create_hmr_runner,
  create_coverage_collector,
- frozen_lockfile: self.options.frozen_lockfile(),
  })
  }
 }
diff --git a/cli/graph_util.rs b/cli/graph_util.rs
@@ -528,9 528,7 @@ impl ModuleGraphBuilder {
  // opted into using a node_modules directory
  if self.options.node_modules_dir_enablement() == Some(true) {
  if let Some(npm_resolver) = self.npm_resolver.as_managed() {
- npm_resolver
- .ensure_top_level_package_json_install(self.options.frozen_lockfile())
- .await?;
  npm_resolver.ensure_top_level_package_json_install().await?;
  }
  }
 

diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs
@@ -1825,7 1825,7 @@ fn resolve_node_modules_dir(
 }
 
 fn resolve_lockfile_from_path(lockfile_path: PathBuf) -> Option<CliLockfile> {
- match CliLockfile::read_from_path(lockfile_path) {
  match CliLockfile::read_from_path(lockfile_path, false) {
  Ok(value) => {
  if value.filename.exists() {
  if let Ok(specifier) = ModuleSpecifier::from_file_path(&value.filename)

diff --git a/cli/lsp/resolver.rs b/cli/lsp/resolver.rs
@@ -515,7 515,6 @@ fn create_graph_resolver(
  sloppy_imports_resolver: unstable_sloppy_imports.then(|| {
  SloppyImportsResolver::new_without_stat_cache(Arc::new(deno_fs::RealFs))
  }),
- frozen_lockfile: false,
  }))
 }
 

diff --git a/cli/module_loader.rs b/cli/module_loader.rs
@@ -71,16 71,10 @@ use deno_semver::npm::NpmPackageReqReference;
 
 pub async fn load_top_level_deps(factory: &CliFactory) -> Result<(), AnyError> {
  let npm_resolver = factory.npm_resolver().await?;
- let frozen = factory.cli_options().frozen_lockfile();
  if let Some(npm_resolver) = npm_resolver.as_managed() {
- if !npm_resolver
- .ensure_top_level_package_json_install(frozen)
- .await?
- {
- if frozen {
- if let Some(lockfile) = factory.maybe_lockfile() {
- crate::args::error_if_lockfile_has_changes(&lockfile.lock(), frozen)?;
- }
  if !npm_resolver.ensure_top_level_package_json_install().await? {
  if let Some(lockfile) = factory.maybe_lockfile() {
  lockfile.error_if_changed()?;
  }
 
  npm_resolver.cache_packages().await?;

diff --git a/cli/npm/managed/mod.rs b/cli/npm/managed/mod.rs
@@ -365,18 365,16 @@ impl ManagedCliNpmResolver {
  pub async fn add_package_reqs(
  &self,
  packages: &[PackageReq],
- frozen: bool,
  ) -> Result<(), AnyError> {
  self
- .add_package_reqs_raw(packages, frozen)
  .add_package_reqs_raw(packages)
  .await
  .dependencies_result
  }
 
  pub async fn add_package_reqs_raw(
  &self,
  packages: &[PackageReq],
- frozen: bool,
  ) -> AddPkgReqsResult {
  if packages.is_empty() {
  return AddPkgReqsResult {
@@ -387,11 385,9 @@ impl ManagedCliNpmResolver {
 
  let mut result = self.resolution.add_package_reqs(packages).await;
 
- if result.dependencies_result.is_ok() && frozen {
  if result.dependencies_result.is_ok() {
  if let Some(lockfile) = self.maybe_lockfile.as_ref() {
- let lockfile = lockfile.lock();
- result.dependencies_result =
- crate::args::error_if_lockfile_has_changes(&lockfile, frozen);
  result.dependencies_result = lockfile.error_if_changed();
  }
  }
  if result.dependencies_result.is_ok() {
@@ -430,7 426,7 @@ impl ManagedCliNpmResolver {
  ) -> Result<(), AnyError> {
  // add and ensure this isn't added to the lockfile
  self
- .add_package_reqs(&[PackageReq::from_str("@types/node").unwrap()], false)
  .add_package_reqs(&[PackageReq::from_str("@types/node").unwrap()])
  .await?;
 
  Ok(())
@@ -463,7 459,6 @@ impl ManagedCliNpmResolver {
  /// If this returns `false`, `node_modules` has _not_ been set up.
  pub async fn ensure_top_level_package_json_install(
  &self,
- frozen: bool,
  ) -> Result<bool, AnyError> {
  let Some(reqs) = self.package_json_deps_provider.reqs() else {
  return Ok(false);
@@ -484,7 479,7 @@ impl ManagedCliNpmResolver {
  }
 
  let reqs = reqs.into_iter().cloned().collect::<Vec<_>>();
- self.add_package_reqs(&reqs, frozen).await.map(|_| true)
  self.add_package_reqs(&reqs).await.map(|_| true)
  }
 
  pub async fn cache_package_info(

diff --git a/cli/resolver.rs b/cli/resolver.rs
@@ -433,7 433,6 @@ pub struct CliGraphResolver {
  npm_resolver: Option<Arc<dyn CliNpmResolver>>,
  found_package_json_dep_flag: AtomicFlag,
  bare_node_builtins_enabled: bool,
- frozen_lockfile: bool,
 }
 
 pub struct CliGraphResolverOptions<'a> {
@@ -445,7 444,6 @@ pub struct CliGraphResolverOptions<'a> {
  pub maybe_import_map: Option<Arc<ImportMap>>,
  pub maybe_vendor_dir: Option<&'a PathBuf>,
  pub bare_node_builtins_enabled: bool,
- pub frozen_lockfile: bool,
 }
 
 impl CliGraphResolver {
@@ -484,7 482,6 @@ impl CliGraphResolver {
  npm_resolver: options.npm_resolver,
  found_package_json_dep_flag: Default::default(),
  bare_node_builtins_enabled: options.bare_node_builtins_enabled,
- frozen_lockfile: options.frozen_lockfile,
  }
  }
 
@@ -497,7 494,6 @@ impl CliGraphResolver {
  npm_resolver: self.npm_resolver.as_ref(),
  found_package_json_dep_flag: &self.found_package_json_dep_flag,
  bare_node_builtins_enabled: self.bare_node_builtins_enabled,
- frozen_lockfile: self.frozen_lockfile,
  }
  }
 
@@ -764,7 760,6 @@ pub struct WorkerCliNpmGraphResolver<'a> {
  npm_resolver: Option<&'a Arc<dyn CliNpmResolver>>,
  found_package_json_dep_flag: &'a AtomicFlag,
  bare_node_builtins_enabled: bool,
- frozen_lockfile: bool,
 }
 
 #[async_trait(?Send)]
@@ -830,16 825,14 @@ impl<'a> deno_graph::source::NpmResolver for WorkerCliNpmGraphResolver<'a> {
 
  let top_level_result = if self.found_package_json_dep_flag.is_raised() {
  npm_resolver
- .ensure_top_level_package_json_install(self.frozen_lockfile)
  .ensure_top_level_package_json_install()
  .await
  .map(|_| ())
  } else {
  Ok(())
  };
 
- let result = npm_resolver
- .add_package_reqs_raw(package_reqs, self.frozen_lockfile)
- .await;
  let result = npm_resolver.add_package_reqs_raw(package_reqs).await;
  NpmResolvePkgReqsResult {
  results: result
  .results

diff --git a/cli/standalone/mod.rs b/cli/standalone/mod.rs
@@ -556,7 556,6 @@ pub async fn run(
  hmr: false,
  inspect_brk: false,
  inspect_wait: false,
- frozen_lockfile: true,
  strace_ops: None,
  is_inspecting: false,
  is_npm_main: main_module.scheme() == "npm",

diff --git a/cli/tools/repl/session.rs b/cli/tools/repl/session.rs
@@ -718,7 718,7 @@ impl ReplSession {
  let has_node_specifier =
  resolved_imports.iter().any(|url| url.scheme() == "node");
  if !npm_imports.is_empty() || has_node_specifier {
- npm_resolver.add_package_reqs(&npm_imports, false).await?;
  npm_resolver.add_package_reqs(&npm_imports).await?;
 
  // prevent messages in the repl about @types/node not being cached
  if has_node_specifier {

diff --git a/cli/tools/run/mod.rs b/cli/tools/run/mod.rs
@@ -195,12 195,9 @@ pub async fn eval_command(
 async fn maybe_npm_install(factory: &CliFactory) -> Result<(), AnyError> {
  // ensure an "npm install" is done if the user has explicitly
  // opted into using a managed node_modules directory
- let options = factory.cli_options();
- if options.node_modules_dir_enablement() == Some(true) {
  if factory.cli_options().node_modules_dir_enablement() == Some(true) {
  if let Some(npm_resolver) = factory.npm_resolver().await?.as_managed() {
- npm_resolver
- .ensure_top_level_package_json_install(options.frozen_lockfile())
- .await?;
  npm_resolver.ensure_top_level_package_json_install().await?;
  }
  }
  Ok(())

diff --git a/cli/tools/task.rs b/cli/tools/task.rs
@@ -121,9 121,7 @@ pub async fn execute_script(
  // directory and managed resolver
  if cli_options.has_node_modules_dir() {
  if let Some(npm_resolver) = npm_resolver.as_managed() {
- npm_resolver
- .ensure_top_level_package_json_install(cli_options.frozen_lockfile())
- .await?;
  npm_resolver.ensure_top_level_package_json_install().await?;
  }
  }
 

diff --git a/cli/tools/vendor/test.rs b/cli/tools/vendor/test.rs
@@ -298,7 298,6 @@ fn build_resolver(
  maybe_import_map: original_import_map.map(Arc::new),
  maybe_vendor_dir: None,
  bare_node_builtins_enabled: false,
- frozen_lockfile: false,
  })
 }
 

diff --git a/cli/worker.rs b/cli/worker.rs
@@ -120,7 120,6 @@ pub struct CliMainWorkerOptions {
  pub maybe_root_package_json_deps: Option<PackageJsonDeps>,
  pub create_hmr_runner: Option<CreateHmrRunnerCb>,
  pub create_coverage_collector: Option<CreateCoverageCollectorCb>,
- pub frozen_lockfile: bool,
 }
 
 struct SharedWorkerState {
@@ -505,10 504,7 @@ impl CliMainWorkerFactory {
  };
  if let Some(npm_resolver) = shared.npm_resolver.as_managed() {
  npm_resolver
- .add_package_reqs(
- &[package_ref.req().clone()],
- shared.options.frozen_lockfile,
- )
  .add_package_reqs(&[package_ref.req().clone()])
  .await?;
  }