Skip to content

Commit

Permalink
refactor(cli): Create wrapper around deno_lockfile::Lockfile (#24366)
Browse files Browse the repository at this point in the history 
As suggested in
#24355 (comment).

I wasn't able to hide the mutex stuff as much as I'd like (ended up just
adding an escape hatch `inner()` method that locks the inner mutex),
because you can't return references to the inner fields through a mutex.

This is mostly motivated by the frozen lockfile changes
  • Loading branch information
@nathanwhit
nathanwhit committed Jun 29, 2024
1 parent 2ddae87 commit bc8a0e6
Show file tree
Hide file tree
Showing 12 changed files with 178 additions and 138 deletions.
194 changes: 126 additions & 68 deletions cli/args/lockfile.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,88 4,146 @@ use std::path::PathBuf;

use deno_core::anyhow::Context;
use deno_core::error::AnyError;
use deno_core::parking_lot::Mutex;
use deno_core::parking_lot::MutexGuard;
use deno_runtime::deno_node::PackageJson;

use crate::args::ConfigFile;
use crate::cache;
use crate::util::fs::atomic_write_file_with_retries;
use crate::Flags;

use super::DenoSubcommand;
use super::InstallFlags;
use super::InstallKind;

pub use deno_lockfile::Lockfile;

pub fn discover(
flags: &Flags,
maybe_config_file: Option<&ConfigFile>,
maybe_package_json: Option<&PackageJson>,
) -> Result<Option<Lockfile>, AnyError> {
if flags.no_lock
|| matches!(
flags.subcommand,
DenoSubcommand::Install(InstallFlags {
kind: InstallKind::Global(..),
..
}) | DenoSubcommand::Uninstall(_)
use crate::args::DenoSubcommand;
use crate::args::InstallFlags;
use crate::args::InstallKind;

use deno_lockfile::Lockfile;

#[derive(Debug)]
pub struct CliLockfile {
lockfile: Mutex<Lockfile>,
pub filename: PathBuf,
}

pub struct Guard<'a, T> {
guard: MutexGuard<'a, T>,
}

impl<'a, T> std::ops::Deref for Guard<'a, T> {
type Target = T;

fn deref(&self) -> &Self::Target {
&self.guard
}
}

impl<'a, T> std::ops::DerefMut for Guard<'a, T> {
fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.guard
}
}

impl CliLockfile {
pub fn new(lockfile: Lockfile) -> Self {
let filename = lockfile.filename.clone();
Self {
lockfile: Mutex::new(lockfile),
filename,
}
}

/// Get the inner deno_lockfile::Lockfile.
pub fn lock(&self) -> Guard<Lockfile> {
Guard {
guard: self.lockfile.lock(),
}
}

pub fn set_workspace_config(
&self,
options: deno_lockfile::SetWorkspaceConfigOptions,
) {
self.lockfile.lock().set_workspace_config(options);
}

pub fn overwrite(&self) -> bool {
self.lockfile.lock().overwrite
}

pub fn write_if_changed(&self) -> Result<(), AnyError> {
let mut lockfile = self.lockfile.lock();
let Some(bytes) = lockfile.resolve_write_bytes() else {
return Ok(()); // nothing to do
};
// do an atomic write to reduce the chance of multiple deno
// processes corrupting the file
atomic_write_file_with_retries(
&lockfile.filename,
bytes,
cache::CACHE_PERM,
)
{
return Ok(None);
.context("Failed writing lockfile.")?;
lockfile.has_content_changed = false;
Ok(())
}

let filename = match flags.lock {
Some(ref lock) => PathBuf::from(lock),
None => match maybe_config_file {
Some(config_file) => {
if config_file.specifier.scheme() == "file" {
match config_file.resolve_lockfile_path()? {
Some(path) => path,
None => return Ok(None),
pub fn discover(
flags: &Flags,
maybe_config_file: Option<&ConfigFile>,
maybe_package_json: Option<&PackageJson>,
) -> Result<Option<CliLockfile>, AnyError> {
if flags.no_lock
|| matches!(
flags.subcommand,
DenoSubcommand::Install(InstallFlags {
kind: InstallKind::Global(..),
..
}) | DenoSubcommand::Uninstall(_)
)
{
return Ok(None);
}

let filename = match flags.lock {
Some(ref lock) => PathBuf::from(lock),
None => match maybe_config_file {
Some(config_file) => {
if config_file.specifier.scheme() == "file" {
match config_file.resolve_lockfile_path()? {
Some(path) => path,
None => return Ok(None),
}
} else {
return Ok(None);
}
} else {
return Ok(None);
}
}
None => match maybe_package_json {
Some(package_json) => {
package_json.path.parent().unwrap().join("deno.lock")
}
None => return Ok(None),
None => match maybe_package_json {
Some(package_json) => {
package_json.path.parent().unwrap().join("deno.lock")
}
None => return Ok(None),
},
},
},
};

let lockfile = if flags.lock_write {
Lockfile::new_empty(filename, true)
} else {
read_lockfile_at_path(filename)?
};
Ok(Some(lockfile))
}
};

pub fn read_lockfile_at_path(filename: PathBuf) -> Result<Lockfile, AnyError> {
match std::fs::read_to_string(&filename) {
Ok(text) => Ok(Lockfile::with_lockfile_content(filename, &text, false)?),
Err(err) if err.kind() == std::io::ErrorKind::NotFound => {
Ok(Lockfile::new_empty(filename, false))
let lockfile = if flags.lock_write {
CliLockfile::new(Lockfile::new_empty(filename, true))
} else {
Self::read_from_path(filename)?
};
Ok(Some(lockfile))
}
pub fn read_from_path(filename: PathBuf) -> 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)))
}
Err(err) => Err(err).with_context(|| {
format!("Failed reading lockfile '{}'", filename.display())
}),
}
Err(err) => Err(err).with_context(|| {
format!("Failed reading lockfile '{}'", filename.display())
}),
}
}

pub fn write_lockfile_if_has_changes(
lockfile: &mut Lockfile,
) -> Result<(), AnyError> {
let Some(bytes) = lockfile.resolve_write_bytes() else {
return Ok(()); // nothing to do
};
// do an atomic write to reduce the chance of multiple deno
// processes corrupting the file
atomic_write_file_with_retries(&lockfile.filename, bytes, cache::CACHE_PERM)
.context("Failed writing lockfile.")?;
lockfile.has_content_changed = false;
Ok(())
}
15 changes: 6 additions & 9 deletions cli/args/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 34,13 @@ pub use deno_config::TsConfigType;
pub use deno_config::TsTypeLib;
pub use deno_config::WorkspaceConfig;
pub use flags::*;
pub use lockfile::read_lockfile_at_path;
pub use lockfile::write_lockfile_if_has_changes;
pub use lockfile::Lockfile;
pub use lockfile::CliLockfile;
pub use package_json::PackageJsonDepsProvider;

use deno_ast::ModuleSpecifier;
use deno_core::anyhow::bail;
use deno_core::anyhow::Context;
use deno_core::error::AnyError;
use deno_core::parking_lot::Mutex;
use deno_core::serde_json;
use deno_core::url::Url;
use deno_runtime::deno_node::PackageJson;
Expand Down Expand Up @@ -812,7 809,7 @@ pub struct CliOptions {
maybe_config_file: Option<ConfigFile>,
maybe_package_json: Option<Arc<PackageJson>>,
npmrc: Arc<ResolvedNpmRc>,
maybe_lockfile: Option<Arc<Mutex<Lockfile>>>,
maybe_lockfile: Option<Arc<CliLockfile>>,
overrides: CliOptionOverrides,
maybe_workspace_config: Option<WorkspaceConfig>,
pub disable_deprecated_api_warning: bool,
Expand All @@ -824,7 821,7 @@ impl CliOptions {
flags: Flags,
initial_cwd: PathBuf,
maybe_config_file: Option<ConfigFile>,
maybe_lockfile: Option<Arc<Mutex<Lockfile>>>,
maybe_lockfile: Option<Arc<CliLockfile>>,
maybe_package_json: Option<Arc<PackageJson>>,
npmrc: Arc<ResolvedNpmRc>,
force_global_cache: bool,
Expand Down Expand Up @@ -958,7 955,7 @@ impl CliOptions {
}),
)?;

let maybe_lock_file = lockfile::discover(
let maybe_lock_file = CliLockfile::discover(
&flags,
maybe_config_file.as_ref(),
maybe_package_json.as_deref(),
Expand All @@ -967,7 964,7 @@ impl CliOptions {
flags,
initial_cwd,
maybe_config_file,
maybe_lock_file.map(|l| Arc::new(Mutex::new(l))),
maybe_lock_file.map(Arc::new),
maybe_package_json,
npmrc,
false,
Expand Down Expand Up @@ -1353,7 1350,7 @@ impl CliOptions {
Ok(Some(InspectorServer::new(host, version::get_user_agent())?))
}

pub fn maybe_lockfile(&self) -> Option<Arc<Mutex<Lockfile>>> {
pub fn maybe_lockfile(&self) -> Option<Arc<CliLockfile>> {
self.maybe_lockfile.clone()
}

Expand Down
12 changes: 5 additions & 7 deletions cli/factory.rs
View file
Original file line number Diff line number Diff line change
@@ -1,10 1,10 @@
// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.

use crate::args::deno_json::deno_json_deps;
use crate::args::CliLockfile;
use crate::args::CliOptions;
use crate::args::DenoSubcommand;
use crate::args::Flags;
use crate::args::Lockfile;
use crate::args::PackageJsonDepsProvider;
use crate::args::StorageKeyResolver;
use crate::args::TsConfigType;
Expand Down Expand Up @@ -56,7 56,6 @@ use std::path::PathBuf;

use deno_core::error::AnyError;
use deno_core::futures::FutureExt;
use deno_core::parking_lot::Mutex;
use deno_core::FeatureChecker;

use deno_lockfile::WorkspaceMemberConfig;
Expand Down Expand Up @@ -156,7 155,7 @@ struct CliFactoryServices {
emitter: Deferred<Arc<Emitter>>,
fs: Deferred<Arc<dyn deno_fs::FileSystem>>,
main_graph_container: Deferred<Arc<MainModuleGraphContainer>>,
lockfile: Deferred<Option<Arc<Mutex<Lockfile>>>>,
lockfile: Deferred<Option<Arc<CliLockfile>>>,
maybe_import_map: Deferred<Option<Arc<ImportMap>>>,
maybe_inspector_server: Deferred<Option<Arc<InspectorServer>>>,
root_cert_store_provider: Deferred<Arc<dyn RootCertStoreProvider>>,
Expand Down Expand Up @@ -304,8 303,8 @@ impl CliFactory {
self.services.fs.get_or_init(|| Arc::new(deno_fs::RealFs))
}

pub fn maybe_lockfile(&self) -> &Option<Arc<Mutex<Lockfile>>> {
fn check_no_npm(lockfile: &Mutex<Lockfile>, options: &CliOptions) -> bool {
pub fn maybe_lockfile(&self) -> &Option<Arc<CliLockfile>> {
fn check_no_npm(lockfile: &CliLockfile, options: &CliOptions) -> bool {
if options.no_npm() {
return true;
}
Expand All @@ -315,7 314,7 @@ impl CliFactory {
options
.maybe_package_json()
.map(|package_json| {
package_json.path.parent() != lockfile.lock().filename.parent()
package_json.path.parent() != lockfile.filename.parent()
})
.unwrap_or(false)
}
Expand All @@ -337,7 336,6 @@ impl CliFactory {
.unwrap_or_default()
})
.unwrap_or_default();
let mut lockfile = lockfile.lock();
let config = match self.options.maybe_workspace_config() {
Some(workspace_config) => deno_lockfile::WorkspaceConfig {
root: WorkspaceMemberConfig {
Expand Down
8 changes: 4 additions & 4 deletions cli/graph_util.rs
View file
Original file line number Diff line number Diff line change
@@ -1,8 1,8 @@
// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.

use crate::args::jsr_url;
use crate::args::CliLockfile;
use crate::args::CliOptions;
use crate::args::Lockfile;
use crate::args::DENO_DISABLE_PEDANTIC_NODE_WARNINGS;
use crate::cache;
use crate::cache::GlobalHttpCache;
Expand Down Expand Up @@ -354,7 354,7 @@ pub struct ModuleGraphBuilder {
npm_resolver: Arc<dyn CliNpmResolver>,
module_info_cache: Arc<ModuleInfoCache>,
parsed_source_cache: Arc<ParsedSourceCache>,
lockfile: Option<Arc<Mutex<Lockfile>>>,
lockfile: Option<Arc<CliLockfile>>,
maybe_file_watcher_reporter: Option<FileWatcherReporter>,
emit_cache: cache::EmitCache,
file_fetcher: Arc<FileFetcher>,
Expand All @@ -371,7 371,7 @@ impl ModuleGraphBuilder {
npm_resolver: Arc<dyn CliNpmResolver>,
module_info_cache: Arc<ModuleInfoCache>,
parsed_source_cache: Arc<ParsedSourceCache>,
lockfile: Option<Arc<Mutex<Lockfile>>>,
lockfile: Option<Arc<CliLockfile>>,
maybe_file_watcher_reporter: Option<FileWatcherReporter>,
emit_cache: cache::EmitCache,
file_fetcher: Arc<FileFetcher>,
Expand Down Expand Up @@ -412,7 412,7 @@ impl ModuleGraphBuilder {
}
}

struct LockfileLocker<'a>(&'a Mutex<Lockfile>);
struct LockfileLocker<'a>(&'a CliLockfile);

impl<'a> deno_graph::source::Locker for LockfileLocker<'a> {
fn get_remote_checksum(
Expand Down
Loading

0 comments on commit bc8a0e6

Please sign in to comment.