Skip to content

Commit

Permalink
refactor: do not use deno_fs::FileSystem everywhere (#27508)
Browse files Browse the repository at this point in the history
This changes the cli to mostly use `std::fs` via `sys_traits` instead of
the implemention of `deno_fs::FileSystem`.
  • Loading branch information
dsherret authored Dec 31, 2024
1 parent 1cd3600 commit 4638caa
Show file tree
Hide file tree
Showing 74 changed files with 1,303 additions and 1,436 deletions.
10 changes: 6 additions & 4 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ slab = "0.4"
smallvec = "1.8"
socket2 = { version = "0.5.3", features = ["all"] }
spki = "0.7.2"
sys_traits = "=0.1.1"
sys_traits = "=0.1.4"
tar = "=0.4.40"
tempfile = "3.4.0"
termcolor = "1.1.3"
Expand Down
3 changes: 2 additions & 1 deletion cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ shell-escape = "=0.1.5"
spki = { version = "0.7", features = ["pem"] }
sqlformat = "=0.3.2"
strsim = "0.11.1"
sys_traits = { workspace = true, features = ["libc", "real", "winapi"] }
sys_traits = { workspace = true, features = ["getrandom", "filetime", "libc", "real", "strip_unc", "winapi"] }
tar.workspace = true
tempfile.workspace = true
text-size = "=1.1.0"
Expand Down Expand Up @@ -187,6 +187,7 @@ nix.workspace = true
[dev-dependencies]
deno_bench_util.workspace = true
pretty_assertions.workspace = true
sys_traits = { workspace = true, features = ["memory"] }
test_util.workspace = true

[package.metadata.winres]
Expand Down
21 changes: 14 additions & 7 deletions cli/args/lockfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@ use deno_core::serde_json;
use deno_lockfile::WorkspaceMemberConfig;
use deno_package_json::PackageJsonDepValue;
use deno_path_util::fs::atomic_write_file_with_retries;
use deno_runtime::deno_fs::FsSysTraitsAdapter;
use deno_runtime::deno_node::PackageJson;
use deno_semver::jsr::JsrDepPackageReq;

use crate::args::deno_json::import_map_deps;
use crate::cache;
use crate::sys::CliSys;
use crate::Flags;

use crate::args::DenoSubcommand;
Expand All @@ -36,6 +36,7 @@ pub struct CliLockfileReadFromPathOptions {

#[derive(Debug)]
pub struct CliLockfile {
sys: CliSys,
lockfile: Mutex<Lockfile>,
pub filename: PathBuf,
frozen: bool,
Expand Down Expand Up @@ -92,7 +93,7 @@ impl CliLockfile {
// do an atomic write to reduce the chance of multiple deno
// processes corrupting the file
atomic_write_file_with_retries(
&FsSysTraitsAdapter::new_real(),
&self.sys,
&lockfile.filename,
&bytes,
cache::CACHE_PERM,
Expand All @@ -103,6 +104,7 @@ impl CliLockfile {
}

pub fn discover(
sys: &CliSys,
flags: &Flags,
workspace: &Workspace,
maybe_external_import_map: Option<&serde_json::Value>,
Expand Down Expand Up @@ -165,11 +167,14 @@ impl CliLockfile {
.unwrap_or(false)
});

let lockfile = Self::read_from_path(CliLockfileReadFromPathOptions {
file_path,
frozen,
skip_write: flags.internal.lockfile_skip_write,
})?;
let lockfile = Self::read_from_path(
sys,
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();
Expand Down Expand Up @@ -225,6 +230,7 @@ impl CliLockfile {
}

pub fn read_from_path(
sys: &CliSys,
opts: CliLockfileReadFromPathOptions,
) -> Result<CliLockfile, AnyError> {
let lockfile = match std::fs::read_to_string(&opts.file_path) {
Expand All @@ -243,6 +249,7 @@ impl CliLockfile {
}
};
Ok(CliLockfile {
sys: sys.clone(),
filename: lockfile.filename.clone(),
lockfile: Mutex::new(lockfile),
frozen: opts.frozen,
Expand Down
20 changes: 13 additions & 7 deletions cli/args/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ use deno_npm::npm_rc::ResolvedNpmRc;
use deno_npm::resolution::ValidSerializedNpmResolutionSnapshot;
use deno_npm::NpmSystemInfo;
use deno_path_util::normalize_path;
use deno_runtime::deno_fs::FsSysTraitsAdapter;
use deno_semver::npm::NpmPackageReqReference;
use deno_semver::StackString;
use deno_telemetry::OtelConfig;
Expand Down Expand Up @@ -89,6 +88,7 @@ use thiserror::Error;

use crate::cache::DenoDirProvider;
use crate::file_fetcher::CliFileFetcher;
use crate::sys::CliSys;
use crate::util::fs::canonicalize_path_maybe_not_exists;
use crate::version;

Expand Down Expand Up @@ -573,7 +573,7 @@ fn discover_npmrc(
// TODO(bartlomieju): update to read both files - one in the project root and one and
// home dir and then merge them.
// 3. Try `.npmrc` in the user's home directory
if let Some(home_dir) = sys_traits::impls::RealSys.env_home_dir() {
if let Some(home_dir) = crate::sys::CliSys::default().env_home_dir() {
match try_to_read_npmrc(&home_dir) {
Ok(Some((source, path))) => {
return try_to_parse_npmrc(source, &path).map(|r| (r, Some(path)));
Expand Down Expand Up @@ -772,7 +772,9 @@ pub struct CliOptions {
}

impl CliOptions {
#[allow(clippy::too_many_arguments)]
pub fn new(
sys: &CliSys,
flags: Arc<Flags>,
initial_cwd: PathBuf,
maybe_lockfile: Option<Arc<CliLockfile>>,
Expand All @@ -797,8 +799,10 @@ impl CliOptions {
}

let maybe_lockfile = maybe_lockfile.filter(|_| !force_global_cache);
let deno_dir_provider =
Arc::new(DenoDirProvider::new(flags.internal.cache_path.clone()));
let deno_dir_provider = Arc::new(DenoDirProvider::new(
sys.clone(),
flags.internal.cache_path.clone(),
));
let maybe_node_modules_folder = resolve_node_modules_folder(
&initial_cwd,
&flags,
Expand All @@ -823,7 +827,7 @@ impl CliOptions {
})
}

pub fn from_flags(flags: Arc<Flags>) -> Result<Self, AnyError> {
pub fn from_flags(sys: &CliSys, flags: Arc<Flags>) -> Result<Self, AnyError> {
let initial_cwd =
std::env::current_dir().with_context(|| "Failed getting cwd.")?;
let maybe_vendor_override = flags.vendor.map(|v| match v {
Expand Down Expand Up @@ -867,7 +871,7 @@ impl CliOptions {
ConfigFlag::Discover => {
if let Some(start_paths) = flags.config_path_args(&initial_cwd) {
WorkspaceDirectory::discover(
&FsSysTraitsAdapter::new_real(),
sys,
WorkspaceDiscoverStart::Paths(&start_paths),
&resolve_workspace_discover_options(),
)?
Expand All @@ -878,7 +882,7 @@ impl CliOptions {
ConfigFlag::Path(path) => {
let config_path = normalize_path(initial_cwd.join(path));
WorkspaceDirectory::discover(
&FsSysTraitsAdapter::new_real(),
sys,
WorkspaceDiscoverStart::ConfigFile(&config_path),
&resolve_workspace_discover_options(),
)?
Expand Down Expand Up @@ -917,6 +921,7 @@ impl CliOptions {
};

let maybe_lock_file = CliLockfile::discover(
sys,
&flags,
&start_dir.workspace,
external_import_map.as_ref().map(|(_, v)| v),
Expand All @@ -925,6 +930,7 @@ impl CliOptions {
log::debug!("Finished config loading.");

Self::new(
sys,
flags,
initial_cwd,
maybe_lock_file.map(Arc::new),
Expand Down
13 changes: 10 additions & 3 deletions cli/cache/deno_dir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
use deno_cache_dir::DenoDirResolutionError;
use once_cell::sync::OnceCell;

use crate::sys::CliSys;

use super::DiskCache;

use std::env;
Expand All @@ -11,13 +13,15 @@ use std::path::PathBuf;
/// Lazily creates the deno dir which might be useful in scenarios
/// where functionality wants to continue if the DENO_DIR can't be created.
pub struct DenoDirProvider {
sys: CliSys,
maybe_custom_root: Option<PathBuf>,
deno_dir: OnceCell<Result<DenoDir, DenoDirResolutionError>>,
}

impl DenoDirProvider {
pub fn new(maybe_custom_root: Option<PathBuf>) -> Self {
pub fn new(sys: CliSys, maybe_custom_root: Option<PathBuf>) -> Self {
Self {
sys,
maybe_custom_root,
deno_dir: Default::default(),
}
Expand All @@ -26,7 +30,9 @@ impl DenoDirProvider {
pub fn get_or_create(&self) -> Result<&DenoDir, DenoDirResolutionError> {
self
.deno_dir
.get_or_init(|| DenoDir::new(self.maybe_custom_root.clone()))
.get_or_init(|| {
DenoDir::new(self.sys.clone(), self.maybe_custom_root.clone())
})
.as_ref()
.map_err(|err| match err {
DenoDirResolutionError::NoCacheOrHomeDir => {
Expand All @@ -53,6 +59,7 @@ pub struct DenoDir {

impl DenoDir {
pub fn new(
sys: CliSys,
maybe_custom_root: Option<PathBuf>,
) -> Result<Self, deno_cache_dir::DenoDirResolutionError> {
let root = deno_cache_dir::resolve_deno_dir(
Expand All @@ -64,7 +71,7 @@ impl DenoDir {

let deno_dir = Self {
root,
gen_cache: DiskCache::new(&gen_path),
gen_cache: DiskCache::new(sys, &gen_path),
};

Ok(deno_dir)
Expand Down
22 changes: 10 additions & 12 deletions cli/cache/disk_cache.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.

use crate::sys::CliSys;

use super::CACHE_PERM;

use deno_cache_dir::url_to_filename;
use deno_core::url::Host;
use deno_core::url::Url;
use deno_path_util::fs::atomic_write_file_with_retries;
use deno_runtime::deno_fs::FsSysTraitsAdapter;
use std::ffi::OsStr;
use std::fs;
use std::path::Component;
Expand All @@ -17,14 +18,16 @@ use std::str;

#[derive(Debug, Clone)]
pub struct DiskCache {
sys: CliSys,
pub location: PathBuf,
}

impl DiskCache {
/// `location` must be an absolute path.
pub fn new(location: &Path) -> Self {
pub fn new(sys: CliSys, location: &Path) -> Self {
assert!(location.is_absolute());
Self {
sys,
location: location.to_owned(),
}
}
Expand Down Expand Up @@ -121,12 +124,7 @@ impl DiskCache {

pub fn set(&self, filename: &Path, data: &[u8]) -> std::io::Result<()> {
let path = self.location.join(filename);
atomic_write_file_with_retries(
&FsSysTraitsAdapter::new_real(),
&path,
data,
CACHE_PERM,
)
atomic_write_file_with_retries(&self.sys, &path, data, CACHE_PERM)
}
}

Expand All @@ -139,7 +137,7 @@ mod tests {
fn test_set_get_cache_file() {
let temp_dir = TempDir::new();
let sub_dir = temp_dir.path().join("sub_dir");
let cache = DiskCache::new(&sub_dir.to_path_buf());
let cache = DiskCache::new(CliSys::default(), &sub_dir.to_path_buf());
let path = PathBuf::from("foo/bar.txt");
cache.set(&path, b"hello").unwrap();
assert_eq!(cache.get(&path).unwrap(), b"hello");
Expand All @@ -153,7 +151,7 @@ mod tests {
PathBuf::from("/deno_dir/")
};

let cache = DiskCache::new(&cache_location);
let cache = DiskCache::new(CliSys::default(), &cache_location);

let mut test_cases = vec![
(
Expand Down Expand Up @@ -209,7 +207,7 @@ mod tests {
} else {
"/foo"
};
let cache = DiskCache::new(&PathBuf::from(p));
let cache = DiskCache::new(CliSys::default(), &PathBuf::from(p));

let mut test_cases = vec![
(
Expand Down Expand Up @@ -257,7 +255,7 @@ mod tests {
PathBuf::from("/deno_dir/")
};

let cache = DiskCache::new(&cache_location);
let cache = DiskCache::new(CliSys::default(), &cache_location);

let mut test_cases = vec!["unknown://localhost/test.ts"];

Expand Down
5 changes: 4 additions & 1 deletion cli/cache/emit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,12 +159,15 @@ impl EmitFileSerializer {
mod test {
use test_util::TempDir;

use crate::sys::CliSys;

use super::*;

#[test]
pub fn emit_cache_general_use() {
let temp_dir = TempDir::new();
let disk_cache = DiskCache::new(temp_dir.path().as_path());
let disk_cache =
DiskCache::new(CliSys::default(), temp_dir.path().as_path());
let cache = EmitCache {
disk_cache: disk_cache.clone(),
file_serializer: EmitFileSerializer {
Expand Down
Loading

0 comments on commit 4638caa

Please sign in to comment.