denoland · nathanwhit · Jul 2, 2024 · Jun 25, 2024 · Jun 27, 2024 · Jun 27, 2024
diff --git a/cli/args/flags.rs b/cli/args/flags.rs
@@ -498,6 498,7 @@ pub struct Flags {
  pub argv: Vec<String>,
  pub subcommand: DenoSubcommand,
 
  pub frozen_lockfile: bool,
  pub ca_stores: Option<Vec<String>>,
  pub ca_data: Option<CaData>,
  pub cache_blocklist: Vec<String>,
@@ -1487,12 1488,15 @@ Future runs of this module will trigger no downloads or compilation unless
 --reload is specified.",
  )
  .defer(|cmd| {
- compile_args(cmd).arg(check_arg(false)).arg(
- Arg::new("file")
- .num_args(1..)
- .required(true)
- .value_hint(ValueHint::FilePath),
- )
  compile_args(cmd)
  .arg(check_arg(false))
  .arg(
  Arg::new("file")
  .num_args(1..)
  .required(true)
  .value_hint(ValueHint::FilePath),
  )
  .arg(frozen_lockfile_arg())
  })
 }
 
@@ -3271,6 3275,7 @@ fn runtime_args(
  app
  };
  app
  .arg(frozen_lockfile_arg())
  .arg(cached_only_arg())
  .arg(location_arg())
  .arg(v8_flags_arg())
@@ -3384,6 3389,14 @@ fn cached_only_arg() -> Arg {
  .help("Require that remote dependencies are already cached")
 }
 
 fn frozen_lockfile_arg() -> Arg {
  Arg::new("frozen")
  .long("frozen")
  .alias("frozen-lockfile")
  .action(ArgAction::SetTrue)
  .help("Error out if lockfile is out of date")
 }
 
 /// Used for subcommands that operate on executable scripts only.
 /// `deno fmt` has its own `--ext` arg because its possible values differ.
 /// If --ext is not provided and the script doesn't have a file extension,
@@ -3774,6 3787,7 @@ fn bundle_parse(flags: &mut Flags, matches: &mut ArgMatches) {
 
 fn cache_parse(flags: &mut Flags, matches: &mut ArgMatches) {
  compile_args_parse(flags, matches);
  frozen_lockfile_arg_parse(flags, matches);
  let files = matches.remove_many::<String>("file").unwrap().collect();
  flags.subcommand = DenoSubcommand::Cache(CacheFlags { files });
 }
@@ -4576,6 4590,7 @@ fn runtime_args_parse(
 ) {
  compile_args_parse(flags, matches);
  cached_only_arg_parse(flags, matches);
  frozen_lockfile_arg_parse(flags, matches);
  if include_perms {
  permission_args_parse(flags, matches);
  }
@@ -4667,6 4682,12 @@ fn cached_only_arg_parse(flags: &mut Flags, matches: &mut ArgMatches) {
  }
 }
 
 fn frozen_lockfile_arg_parse(flags: &mut Flags, matches: &mut ArgMatches) {
  if matches.get_flag("frozen") {
  flags.frozen_lockfile = true;
  }
 }
 
 fn ext_arg_parse(flags: &mut Flags, matches: &mut ArgMatches) {
  flags.ext = matches.remove_one::<String>("ext");
 }

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,23 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_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 {
  Ok(())
  }
  }
 }
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/module_loader.rs b/cli/module_loader.rs
@@ -72,10 72,13 @@ use deno_semver::npm::NpmPackageReqReference;
 pub async fn load_top_level_deps(factory: &CliFactory) -> Result<(), AnyError> {
  let npm_resolver = factory.npm_resolver().await?;
  if let Some(npm_resolver) = npm_resolver.as_managed() {
- npm_resolver.ensure_top_level_package_json_install().await?;
- // TODO(nathanwhit): we call `cache_packages` if the lockfile is modified,
- // so by calling it here it's possible we end up calling it twice
- npm_resolver.cache_packages().await?;
  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?;
  }
  }
  // cache as many entries in the import map as we can
  if let Some(import_map) = factory.maybe_import_map().await? {

diff --git a/cli/npm/managed/mod.rs b/cli/npm/managed/mod.rs
@@ -361,12 361,15 @@ impl ManagedCliNpmResolver {
  }
 
  /// Adds package requirements to the resolver and ensures everything is setup.
  /// This includes setting up the `node_modules` directory, if applicable.
  pub async fn add_package_reqs(
  &self,
  packages: &[PackageReq],
  ) -> Result<(), AnyError> {
- let result = self.add_package_reqs_raw(packages).await;
- result.dependencies_result
  self
  .add_package_reqs_raw(packages)
  .await
  .dependencies_result
  }
 
  pub async fn add_package_reqs_raw(
@@ -381,6 384,12 @@ impl ManagedCliNpmResolver {
  }
 
  let mut result = self.resolution.add_package_reqs(packages).await;
 
  if result.dependencies_result.is_ok() {
  if let Some(lockfile) = self.maybe_lockfile.as_ref() {
  result.dependencies_result = lockfile.error_if_changed();
  }
  }
  if result.dependencies_result.is_ok() {
  result.dependencies_result =
  self.cache_packages().await.map_err(AnyError::from);
@@ -442,14 451,19 @@ impl ManagedCliNpmResolver {
  self.resolution.resolve_pkg_id_from_pkg_req(req)
  }
 
  /// Ensures that the top level `package.json` dependencies are installed.
  /// This may set up the `node_modules` directory.
  ///
  /// Returns `true` if any changes (such as caching packages) were made.
  /// If this returns `false`, `node_modules` has _not_ been set up.
  pub async fn ensure_top_level_package_json_install(
  &self,
- ) -> Result<(), AnyError> {
  ) -> Result<bool, AnyError> {
  let Some(reqs) = self.package_json_deps_provider.reqs() else {
- return Ok(());
  return Ok(false);
  };
  if !self.top_level_install_flag.raise() {
- return Ok(()); // already did this
  return Ok(false); // already did this
  }
  // check if something needs resolving before bothering to load all
  // the package information (which is slow)
@@ -460,11 474,11 @@ impl ManagedCliNpmResolver {
  log::debug!(
  "All package.json deps resolvable. Skipping top level install."
  );
- return Ok(()); // everything is already resolvable
  return Ok(false); // everything is already resolvable
  }
 
  let reqs = reqs.into_iter().cloned().collect::<Vec<_>>();
- self.add_package_reqs(&reqs).await
  self.add_package_reqs(&reqs).await.map(|_| true)
  }
 
  pub async fn cache_package_info(

diff --git a/cli/resolver.rs b/cli/resolver.rs
@@ -824,7 824,10 @@ 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().await
  npm_resolver
  .ensure_top_level_package_json_install()
  .await
  .map(|_| ())
  } else {
  Ok(())
  };

diff --git a/cli/tools/installer.rs b/cli/tools/installer.rs
@@ -466,6 466,10 @@ async fn resolve_shim_data(
  executable_args.push("--cached-only".to_string());
  }
 
  if flags.frozen_lockfile {
  executable_args.push("--frozen".to_string());
  }
 
  if resolve_no_prompt(&flags.permissions) {
  executable_args.push("--no-prompt".to_string());
  }

diff --git a/tests/specs/lockfile/frozen_lockfile/__test__.jsonc b/tests/specs/lockfile/frozen_lockfile/__test__.jsonc
@@ -0,0 1,101 @@
 {
  "tempDir": true,
  "tests": {
  "error_with_new_npm_dep": {
  "steps": [
  {
  "args": "cache add.ts",
  "output": "[WILDCARD]"
  },
  {
  // sub.ts imports from an npm package
  // that's not in the lockfile
  "args": "run --frozen sub.ts",
  "output": "frozen_new_dep_run.out",
  "exitCode": 1
  },
  {
  "args": "cache --frozen sub.ts",
  "output": "frozen_new_dep_cache.out",
  "exitCode": 1
  },
  {
  // now fix it
  "args": "cache sub.ts",
  "output": "update_lockfile.out"
  },
  {
  "args": "run --frozen sub.ts",
  "output": "3 - 2 = 1\n"
  }
  ]
  },
  "error_with_new_jsr_dep": {
  "steps": [
  {
  "args": "cache jsr.ts",
  "output": "[WILDCARD]"
  },
  {
  "args": "run --frozen jsr2.ts",
  "output": "frozen_new_dep_jsr_run.out",
  "exitCode": 1
  },
  {
  "args": "cache --frozen jsr2.ts",
  "output": "frozen_new_dep_jsr_cache.out",
  "exitCode": 1
  },
  {
  // update the lockfile
  "args": "cache jsr2.ts",
  "output": ""
  },
  {
  "args": "run --frozen jsr2.ts",
  "output": "1 2 = 3\n"
  }
  ]
  },
  "error_when_package_json_changed": {
  "steps": [
  {
  "args": "cache add.ts",
  "output": "[WILDCARD]"
  },
  {
  "args": [
  "eval",
  "Deno.writeTextFileSync(\"package.json\", JSON.stringify({ dependencies: { \"@denotest/bin\": \"0.7.0\" } }))"
  ],
  "output": ""
  },
  {
  "args": "cache --frozen add.ts",
  "output": "frozen_package_json_changed.out",
  "exitCode": 1
  },
  {
  "envs": {
  "DENO_FUTURE": "1"
  },
  "args": "install --frozen",
  "output": "frozen_package_json_changed_install.out",
  "exitCode": 1
  }
  ]
  },
  "no_error_when_in_lockfile": {
  "steps": [
  {
  "args": "cache add.ts",
  "output": "[WILDCARD]"
  },
  {
  "args": "run --frozen add.ts",
  "output": "1 2 = 3\n"
  }
  ]
  }
  }
 }
diff --git a/tests/specs/lockfile/frozen_lockfile/add.ts b/tests/specs/lockfile/frozen_lockfile/add.ts
@@ -0,0 1,2 @@
 import { add } from "npm:@denotest/add@1";
 console.log(`1 2 = ${add(1, 2)}`);