refactor: remove PermissionsContainer in deno_runtime (#24119)

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.
denoland · Jun 7, 2024 · 386d5c8 · 386d5c8
1 parent a17794d
commit 386d5c8
Show file tree

Hide file tree

Showing 69 changed files with 428 additions and 564 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/cli/args/flags.rs b/cli/args/flags.rs
@@ -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;

diff --git a/cli/args/import_map.rs b/cli/args/import_map.rs
@@ -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;

diff --git a/cli/args/mod.rs b/cli/args/mod.rs
@@ -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;

diff --git a/cli/cache/mod.rs b/cli/cache/mod.rs
@@ -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;

diff --git a/cli/file_fetcher.rs b/cli/file_fetcher.rs
@@ -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;

diff --git a/cli/graph_container.rs b/cli/graph_container.rs
@@ -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;

diff --git a/cli/graph_util.rs b/cli/graph_util.rs
@@ -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;

diff --git a/cli/jsr.rs b/cli/jsr.rs
@@ -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;

diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs
@@ -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;

diff --git a/cli/lsp/jsr.rs b/cli/lsp/jsr.rs
@@ -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;

diff --git a/cli/lsp/npm.rs b/cli/lsp/npm.rs
@@ -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;

diff --git a/cli/lsp/registries.rs b/cli/lsp/registries.rs
@@ -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;

diff --git a/cli/lsp/resolver.rs b/cli/lsp/resolver.rs
@@ -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;
@@ -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(),
     ))
   }
@@ -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(

diff --git a/cli/lsp/testing/execution.rs b/cli/lsp/testing/execution.rs
@@ -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;

diff --git a/cli/main.rs b/cli/main.rs
@@ -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),
   );

diff --git a/cli/module_loader.rs b/cli/module_loader.rs
@@ -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> {
@@ -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?;
   }
@@ -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?
@@ -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()),
@@ -505,7 +491,6 @@ impl<TGraphContainer: ModuleGraphContainer>
           .node_resolver
           .resolve_req_reference(
             &reference,
-            permissions,
             referrer,
             NodeResolutionMode::Execution,
           )
@@ -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)
@@ -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,
@@ -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)
   }
@@ -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();
@@ -757,7 +741,6 @@ impl<TGraphContainer: ModuleGraphContainer> ModuleLoader
           .load_inner(
             &specifier,
             maybe_referrer.as_ref(),
-            is_dynamic,
             requested_module_type,
           )
           .await

diff --git a/cli/npm/byonm.rs b/cli/npm/byonm.rs
@@ -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

diff --git a/cli/npm/managed/mod.rs b/cli/npm/managed/mod.rs
@@ -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)

diff --git a/cli/npm/managed/resolvers/common.rs b/cli/npm/managed/resolvers/common.rs
@@ -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>;
 }
@@ -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

diff --git a/cli/npm/managed/resolvers/global.rs b/cli/npm/managed/resolvers/global.rs
@@ -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

diff --git a/cli/npm/managed/resolvers/local.rs b/cli/npm/managed/resolvers/local.rs
@@ -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