Skip to content

Commit

Permalink
refactor: remove PermissionsContainer in deno_runtime (#24119)
Browse files Browse the repository at this point in the history
Also removes permissions being passed in for node resolution. It was
completely useless because we only checked it for reading package.json
files, but Deno reading package.json files for resolution is perfectly
fine.

My guess is this is also a perf improvement because Deno is doing less
work.
  • Loading branch information
dsherret committed Jun 7, 2024
1 parent a17794d commit 386d5c8
Show file tree
Hide file tree
Showing 69 changed files with 428 additions and 564 deletions.
8 changes: 8 additions & 0 deletions Cargo.lock

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

4 changes: 2 additions & 2 deletions cli/args/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 17,8 @@ use deno_core::error::AnyError;
use deno_core::resolve_url_or_path;
use deno_core::url::Url;
use deno_graph::GraphKind;
use deno_runtime::permissions::parse_sys_kind;
use deno_runtime::permissions::PermissionsOptions;
use deno_runtime::deno_permissions::parse_sys_kind;
use deno_runtime::deno_permissions::PermissionsOptions;
use log::debug;
use log::Level;
use serde::Deserialize;
Expand Down
2 changes: 1 addition & 1 deletion cli/args/import_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 4,7 @@ use deno_core::anyhow::Context;
use deno_core::error::AnyError;
use deno_core::serde_json;
use deno_core::url::Url;
use deno_runtime::permissions::PermissionsContainer;
use deno_runtime::deno_permissions::PermissionsContainer;
use import_map::ImportMap;
use import_map::ImportMapDiagnostic;
use log::warn;
Expand Down
2 changes: 1 addition & 1 deletion cli/args/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 47,13 @@ use deno_core::parking_lot::Mutex;
use deno_core::serde_json;
use deno_core::url::Url;
use deno_runtime::deno_node::PackageJson;
use deno_runtime::deno_permissions::PermissionsOptions;
use deno_runtime::deno_tls::deno_native_certs::load_native_certs;
use deno_runtime::deno_tls::rustls;
use deno_runtime::deno_tls::rustls::RootCertStore;
use deno_runtime::deno_tls::rustls_pemfile;
use deno_runtime::deno_tls::webpki_roots;
use deno_runtime::inspector_server::InspectorServer;
use deno_runtime::permissions::PermissionsOptions;
use deno_terminal::colors;
use dotenvy::from_filename;
use once_cell::sync::Lazy;
Expand Down
2 changes: 1 addition & 1 deletion cli/cache/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 17,7 @@ use deno_graph::source::CacheInfo;
use deno_graph::source::LoadFuture;
use deno_graph::source::LoadResponse;
use deno_graph::source::Loader;
use deno_runtime::permissions::PermissionsContainer;
use deno_runtime::deno_permissions::PermissionsContainer;
use std::collections::HashMap;
use std::path::Path;
use std::path::PathBuf;
Expand Down
2 changes: 1 addition & 1 deletion cli/file_fetcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 22,8 @@ use deno_core::url::Url;
use deno_core::ModuleSpecifier;
use deno_graph::source::LoaderChecksum;

use deno_runtime::deno_permissions::PermissionsContainer;
use deno_runtime::deno_web::BlobStore;
use deno_runtime::permissions::PermissionsContainer;
use log::debug;
use std::borrow::Cow;
use std::collections::HashMap;
Expand Down
2 changes: 1 addition & 1 deletion cli/graph_container.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 8,7 @@ use deno_core::parking_lot::RwLock;
use deno_core::resolve_url_or_path;
use deno_graph::ModuleGraph;
use deno_runtime::colors;
use deno_runtime::permissions::PermissionsContainer;
use deno_runtime::deno_permissions::PermissionsContainer;

use crate::args::CliOptions;
use crate::module_loader::ModuleLoadPreparer;
Expand Down
2 changes: 1 addition & 1 deletion cli/graph_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 41,7 @@ use deno_graph::ResolutionError;
use deno_graph::SpecifierError;
use deno_runtime::deno_fs::FileSystem;
use deno_runtime::deno_node;
use deno_runtime::permissions::PermissionsContainer;
use deno_runtime::deno_permissions::PermissionsContainer;
use deno_semver::package::PackageNv;
use deno_semver::package::PackageReq;
use import_map::ImportMapError;
Expand Down
2 changes: 1 addition & 1 deletion cli/jsr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 10,7 @@ use deno_core::ModuleSpecifier;
use deno_graph::packages::JsrPackageInfo;
use deno_graph::packages::JsrPackageVersionInfo;
use deno_lockfile::Lockfile;
use deno_runtime::permissions::PermissionsContainer;
use deno_runtime::deno_permissions::PermissionsContainer;
use deno_semver::jsr::JsrPackageReqReference;
use deno_semver::package::PackageNv;
use deno_semver::package::PackageReq;
Expand Down
2 changes: 1 addition & 1 deletion cli/lsp/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 29,8 @@ use deno_lint::linter::LintConfig;
use deno_lockfile::Lockfile;
use deno_npm::npm_rc::ResolvedNpmRc;
use deno_runtime::deno_node::PackageJson;
use deno_runtime::deno_permissions::PermissionsContainer;
use deno_runtime::fs_util::specifier_to_file_path;
use deno_runtime::permissions::PermissionsContainer;
use import_map::ImportMap;
use lsp::Url;
use lsp_types::ClientCapabilities;
Expand Down
2 changes: 1 addition & 1 deletion cli/lsp/jsr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 7,7 @@ use dashmap::DashMap;
use deno_core::anyhow::anyhow;
use deno_core::error::AnyError;
use deno_core::serde_json;
use deno_runtime::permissions::PermissionsContainer;
use deno_runtime::deno_permissions::PermissionsContainer;
use deno_semver::package::PackageNv;
use deno_semver::Version;
use serde::Deserialize;
Expand Down
2 changes: 1 addition & 1 deletion cli/lsp/npm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 4,7 @@ use dashmap::DashMap;
use deno_core::anyhow::anyhow;
use deno_core::error::AnyError;
use deno_core::serde_json;
use deno_runtime::permissions::PermissionsContainer;
use deno_runtime::deno_permissions::PermissionsContainer;
use deno_semver::package::PackageNv;
use deno_semver::Version;
use serde::Deserialize;
Expand Down
2 changes: 1 addition & 1 deletion cli/lsp/registries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 30,7 @@ use deno_core::url::Position;
use deno_core::url::Url;
use deno_core::ModuleSpecifier;
use deno_graph::Dependency;
use deno_runtime::permissions::PermissionsContainer;
use deno_runtime::deno_permissions::PermissionsContainer;
use log::error;
use once_cell::sync::Lazy;
use std::collections::HashMap;
Expand Down
14 changes: 5 additions & 9 deletions cli/lsp/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 38,6 @@ use deno_runtime::deno_node::NodeResolutionMode;
use deno_runtime::deno_node::NodeResolver;
use deno_runtime::deno_node::PackageJson;
use deno_runtime::fs_util::specifier_to_file_path;
use deno_runtime::permissions::PermissionsContainer;
use deno_semver::jsr::JsrPackageReqReference;
use deno_semver::npm::NpmPackageReqReference;
use deno_semver::package::PackageNv;
Expand Down Expand Up @@ -247,12 246,7 @@ impl LspResolver {
let node_resolver = self.node_resolver.as_ref()?;
Some(NodeResolution::into_specifier_and_media_type(
node_resolver
.resolve_req_reference(
req_ref,
&PermissionsContainer::allow_all(),
referrer,
NodeResolutionMode::Types,
)
.resolve_req_reference(req_ref, referrer, NodeResolutionMode::Types)
.ok(),
))
}
Expand Down Expand Up @@ -282,8 276,10 @@ impl LspResolver {
let Some(node_resolver) = self.node_resolver.as_ref() else {
return Ok(None);
};
node_resolver
.get_closest_package_json(referrer, &PermissionsContainer::allow_all())
node_resolver.get_closest_package_json(
referrer,
&mut deno_runtime::deno_node::AllowAllNodePermissions,
)
}

pub fn resolve_redirects(
Expand Down
2 changes: 1 addition & 1 deletion cli/lsp/testing/execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 26,7 @@ use deno_core::parking_lot::RwLock;
use deno_core::unsync::spawn;
use deno_core::unsync::spawn_blocking;
use deno_core::ModuleSpecifier;
use deno_runtime::permissions::Permissions;
use deno_runtime::deno_permissions::Permissions;
use deno_runtime::tokio_util::create_and_run_current_thread;
use indexmap::IndexMap;
use std::collections::HashMap;
Expand Down
2 changes: 1 addition & 1 deletion cli/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 318,7 @@ pub fn main() {
util::windows::ensure_stdio_open();
#[cfg(windows)]
colors::enable_ansi(); // For Windows 10
deno_runtime::permissions::set_prompt_callbacks(
deno_runtime::deno_permissions::set_prompt_callbacks(
Box::new(util::draw_thread::DrawThread::hide),
Box::new(util::draw_thread::DrawThread::show),
);
Expand Down
29 changes: 6 additions & 23 deletions cli/module_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 68,7 @@ use deno_graph::Resolution;
use deno_lockfile::Lockfile;
use deno_runtime::code_cache;
use deno_runtime::deno_node::NodeResolutionMode;
use deno_runtime::permissions::PermissionsContainer;
use deno_runtime::deno_permissions::PermissionsContainer;
use deno_semver::npm::NpmPackageReqReference;

pub async fn load_top_level_deps(factory: &CliFactory) -> Result<(), AnyError> {
Expand Down Expand Up @@ -104,7 104,7 @@ pub async fn load_top_level_deps(factory: &CliFactory) -> Result<(), AnyError> {
&roots,
false,
factory.cli_options().ts_type_lib_window(),
deno_runtime::permissions::PermissionsContainer::allow_all(),
deno_runtime::deno_permissions::PermissionsContainer::allow_all(),
)
.await?;
}
Expand Down Expand Up @@ -348,18 348,12 @@ impl<TGraphContainer: ModuleGraphContainer>
&self,
specifier: &ModuleSpecifier,
maybe_referrer: Option<&ModuleSpecifier>,
is_dynamic: bool,
requested_module_type: RequestedModuleType,
) -> Result<ModuleSource, AnyError> {
let permissions = if is_dynamic {
&self.dynamic_permissions
} else {
&self.root_permissions
};
let code_source = if let Some(result) = self
.shared
.npm_module_loader
.load_if_in_npm_package(specifier, maybe_referrer, permissions)
.load_if_in_npm_package(specifier, maybe_referrer)
.await
{
result?
Expand Down Expand Up @@ -448,19 442,11 @@ impl<TGraphContainer: ModuleGraphContainer>
&self,
specifier: &str,
referrer: &ModuleSpecifier,
kind: ResolutionKind,
) -> Result<ModuleSpecifier, AnyError> {
let permissions = if matches!(kind, ResolutionKind::DynamicImport) {
&self.dynamic_permissions
} else {
&self.root_permissions
};

if let Some(result) = self.shared.node_resolver.resolve_if_in_npm_package(
specifier,
referrer,
NodeResolutionMode::Execution,
permissions,
) {
return match result? {
Some(res) => Ok(res.into_url()),
Expand Down Expand Up @@ -505,7 491,6 @@ impl<TGraphContainer: ModuleGraphContainer>
.node_resolver
.resolve_req_reference(
&reference,
permissions,
referrer,
NodeResolutionMode::Execution,
)
Expand All @@ -530,7 515,6 @@ impl<TGraphContainer: ModuleGraphContainer>
module.nv_reference.sub_path(),
referrer,
NodeResolutionMode::Execution,
permissions,
)
.with_context(|| {
format!("Could not resolve '{}'.", module.nv_reference)
Expand Down Expand Up @@ -720,7 704,7 @@ impl<TGraphContainer: ModuleGraphContainer> ModuleLoader
&self,
specifier: &str,
referrer: &str,
kind: ResolutionKind,
_kind: ResolutionKind,
) -> Result<ModuleSpecifier, AnyError> {
fn ensure_not_jsr_non_jsr_remote_import(
specifier: &ModuleSpecifier,
Expand All @@ -736,7 720,7 @@ impl<TGraphContainer: ModuleGraphContainer> ModuleLoader
}

let referrer = self.0.resolve_referrer(referrer)?;
let specifier = self.0.inner_resolve(specifier, &referrer, kind)?;
let specifier = self.0.inner_resolve(specifier, &referrer)?;
ensure_not_jsr_non_jsr_remote_import(&specifier, &referrer)?;
Ok(specifier)
}
Expand All @@ -745,7 729,7 @@ impl<TGraphContainer: ModuleGraphContainer> ModuleLoader
&self,
specifier: &ModuleSpecifier,
maybe_referrer: Option<&ModuleSpecifier>,
is_dynamic: bool,
_is_dynamic: bool,
requested_module_type: RequestedModuleType,
) -> deno_core::ModuleLoadResponse {
let inner = self.0.clone();
Expand All @@ -757,7 741,6 @@ impl<TGraphContainer: ModuleGraphContainer> ModuleLoader
.load_inner(
&specifier,
maybe_referrer.as_ref(),
is_dynamic,
requested_module_type,
)
.await
Expand Down
2 changes: 1 addition & 1 deletion cli/npm/byonm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 160,7 @@ impl NpmResolver for ByonmCliNpmResolver {

fn ensure_read_permission(
&self,
permissions: &dyn NodePermissions,
permissions: &mut dyn NodePermissions,
path: &Path,
) -> Result<(), AnyError> {
if !path
Expand Down
2 changes: 1 addition & 1 deletion cli/npm/managed/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -550,7 550,7 @@ impl NpmResolver for ManagedCliNpmResolver {

fn ensure_read_permission(
&self,
permissions: &dyn NodePermissions,
permissions: &mut dyn NodePermissions,
path: &Path,
) -> Result<(), AnyError> {
self.fs_resolver.ensure_read_permission(permissions, path)
Expand Down
4 changes: 2 additions & 2 deletions cli/npm/managed/resolvers/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 53,7 @@ pub trait NpmPackageFsResolver: Send Sync {

fn ensure_read_permission(
&self,
permissions: &dyn NodePermissions,
permissions: &mut dyn NodePermissions,
path: &Path,
) -> Result<(), AnyError>;
}
Expand All @@ -76,7 76,7 @@ impl RegistryReadPermissionChecker {

pub fn ensure_registry_read_permission(
&self,
permissions: &dyn NodePermissions,
permissions: &mut dyn NodePermissions,
path: &Path,
) -> Result<(), AnyError> {
// allow reading if it's in the node_modules
Expand Down
2 changes: 1 addition & 1 deletion cli/npm/managed/resolvers/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 146,7 @@ impl NpmPackageFsResolver for GlobalNpmPackageResolver {

fn ensure_read_permission(
&self,
permissions: &dyn NodePermissions,
permissions: &mut dyn NodePermissions,
path: &Path,
) -> Result<(), AnyError> {
self
Expand Down
2 changes: 1 addition & 1 deletion cli/npm/managed/resolvers/local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 243,7 @@ impl NpmPackageFsResolver for LocalNpmPackageResolver {

fn ensure_read_permission(
&self,
permissions: &dyn NodePermissions,
permissions: &mut dyn NodePermissions,
path: &Path,
) -> Result<(), AnyError> {
self
Expand Down
Loading

0 comments on commit 386d5c8

Please sign in to comment.