From 9114a2df69da9318c4e10887553b7daf77b0fa16 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Tue, 23 Jul 2024 19:00:48 -0400 Subject: [PATCH] fix(upgrade): do not error if config in cwd invalid (#24689) ``` > deno upgrade error: Unsupported lockfile version 'invalid'. Try upgrading Deno or recreating the lockfile. V:\scratch > V:\deno\target\debug\deno upgrade Looking up latest version Local deno version 1.45.3 is the most recent release ``` Closes #24517 Closes #20729 --- cli/args/lockfile.rs | 82 ++++++ cli/args/mod.rs | 73 ++---- cli/factory.rs | 395 +++++++++++++---------------- cli/lsp/language_server.rs | 4 +- cli/lsp/testing/execution.rs | 33 +-- cli/main.rs | 10 +- cli/module_loader.rs | 5 +- cli/tools/bench/mod.rs | 19 +- cli/tools/bundle.rs | 16 +- cli/tools/compile.rs | 6 +- cli/tools/coverage/mod.rs | 7 +- cli/tools/doc.rs | 12 +- cli/tools/fmt.rs | 13 +- cli/tools/info.rs | 12 +- cli/tools/installer.rs | 39 +-- cli/tools/jupyter/mod.rs | 8 +- cli/tools/lint/mod.rs | 13 +- cli/tools/registry/mod.rs | 10 +- cli/tools/registry/pm.rs | 19 +- cli/tools/repl/mod.rs | 9 +- cli/tools/run/mod.rs | 32 +-- cli/tools/task.rs | 6 +- cli/tools/test/mod.rs | 17 +- cli/tools/upgrade.rs | 15 +- cli/tools/vendor/mod.rs | 4 +- cli/util/file_watcher.rs | 15 +- tests/integration/upgrade_tests.rs | 69 +++-- tests/util/server/src/fs.rs | 7 + 28 files changed, 510 insertions(+), 440 deletions(-) diff --git a/cli/args/lockfile.rs b/cli/args/lockfile.rs index fa505e7b1e..c9afecd983 100644 --- a/cli/args/lockfile.rs +++ b/cli/args/lockfile.rs @@ -1,12 +1,17 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. +use std::collections::BTreeSet; use std::path::PathBuf; +use deno_config::deno_json::ConfigFile; use deno_config::workspace::Workspace; use deno_core::anyhow::Context; use deno_core::error::AnyError; use deno_core::parking_lot::Mutex; use deno_core::parking_lot::MutexGuard; +use deno_lockfile::WorkspaceMemberConfig; +use deno_package_json::PackageJsonDepValue; +use deno_runtime::deno_node::PackageJson; use crate::cache; use crate::util::fs::atomic_write_file_with_retries; @@ -93,6 +98,35 @@ impl CliLockfile { flags: &Flags, workspace: &Workspace, ) -> Result, AnyError> { + fn pkg_json_deps(maybe_pkg_json: Option<&PackageJson>) -> BTreeSet { + let Some(pkg_json) = maybe_pkg_json else { + return Default::default(); + }; + pkg_json + .resolve_local_package_json_deps() + .values() + .filter_map(|dep| dep.as_ref().ok()) + .filter_map(|dep| match dep { + PackageJsonDepValue::Req(req) => Some(req), + PackageJsonDepValue::Workspace(_) => None, + }) + .map(|r| format!("npm:{}", r)) + .collect() + } + + fn deno_json_deps( + maybe_deno_json: Option<&ConfigFile>, + ) -> BTreeSet { + maybe_deno_json + .map(|c| { + crate::args::deno_json::deno_json_deps(c) + .into_iter() + .map(|req| req.to_string()) + .collect() + }) + .unwrap_or_default() + } + if flags.no_lock || matches!( flags.subcommand, @@ -125,6 +159,54 @@ impl CliLockfile { } else { Self::read_from_path(filename, flags.frozen_lockfile)? }; + + // initialize the lockfile with the workspace's configuration + let root_url = workspace.root_dir(); + let root_folder = workspace.root_folder_configs(); + let config = deno_lockfile::WorkspaceConfig { + root: WorkspaceMemberConfig { + package_json_deps: pkg_json_deps(root_folder.pkg_json.as_deref()), + dependencies: deno_json_deps(root_folder.deno_json.as_deref()), + }, + members: workspace + .config_folders() + .iter() + .filter(|(folder_url, _)| *folder_url != root_url) + .filter_map(|(folder_url, folder)| { + Some(( + { + // should never be None here, but just ignore members that + // do fail for this + let mut relative_path = root_url.make_relative(folder_url)?; + if relative_path.ends_with('/') { + // make it slightly cleaner by removing the trailing slash + relative_path.pop(); + } + relative_path + }, + { + let config = WorkspaceMemberConfig { + package_json_deps: pkg_json_deps(folder.pkg_json.as_deref()), + dependencies: deno_json_deps(folder.deno_json.as_deref()), + }; + if config.package_json_deps.is_empty() + && config.dependencies.is_empty() + { + // exclude empty workspace members + return None; + } + config + }, + )) + }) + .collect(), + }; + lockfile.set_workspace_config(deno_lockfile::SetWorkspaceConfigOptions { + no_npm: flags.no_npm, + no_config: flags.config_flag == super::ConfigFlag::Disabled, + config, + }); + Ok(Some(lockfile)) } pub fn read_from_path( diff --git a/cli/args/mod.rs b/cli/args/mod.rs index ba2e06e067..aea6ed8a8d 100644 --- a/cli/args/mod.rs +++ b/cli/args/mod.rs @@ -26,7 +26,6 @@ use deno_npm::npm_rc::ResolvedNpmRc; use deno_npm::resolution::ValidSerializedNpmResolutionSnapshot; use deno_npm::NpmSystemInfo; use deno_runtime::deno_permissions::PermissionsContainer; -use deno_runtime::deno_tls::RootCertStoreProvider; use deno_semver::npm::NpmPackageReqReference; use import_map::resolve_import_map_value_from_specifier; @@ -62,7 +61,6 @@ use deno_runtime::inspector_server::InspectorServer; use deno_terminal::colors; use dotenvy::from_filename; use once_cell::sync::Lazy; -use once_cell::sync::OnceCell; use serde::Deserialize; use serde::Serialize; use std::collections::HashMap; @@ -599,43 +597,6 @@ pub fn create_default_npmrc() -> Arc { }) } -struct CliRootCertStoreProvider { - cell: OnceCell, - maybe_root_path: Option, - maybe_ca_stores: Option>, - maybe_ca_data: Option, -} - -impl CliRootCertStoreProvider { - pub fn new( - maybe_root_path: Option, - maybe_ca_stores: Option>, - maybe_ca_data: Option, - ) -> Self { - Self { - cell: Default::default(), - maybe_root_path, - maybe_ca_stores, - maybe_ca_data, - } - } -} - -impl RootCertStoreProvider for CliRootCertStoreProvider { - fn get_or_try_init(&self) -> Result<&RootCertStore, AnyError> { - self - .cell - .get_or_try_init(|| { - get_root_cert_store( - self.maybe_root_path.clone(), - self.maybe_ca_stores.clone(), - self.maybe_ca_data.clone(), - ) - }) - .map_err(|e| e.into()) - } -} - #[derive(Error, Debug, Clone)] pub enum RootCertStoreLoadError { #[error( @@ -761,7 +722,7 @@ struct CliOptionOverrides { pub struct CliOptions { // the source of the options is a detail the rest of the // application need not concern itself with, so keep these private - flags: Flags, + flags: Arc, initial_cwd: PathBuf, maybe_node_modules_folder: Option, npmrc: Arc, @@ -774,7 +735,7 @@ pub struct CliOptions { impl CliOptions { pub fn new( - flags: Flags, + flags: Arc, initial_cwd: PathBuf, maybe_lockfile: Option>, npmrc: Arc, @@ -830,7 +791,7 @@ impl CliOptions { }) } - pub fn from_flags(flags: Flags) -> Result { + pub fn from_flags(flags: Arc) -> Result { let initial_cwd = std::env::current_dir().with_context(|| "Failed getting cwd.")?; let maybe_vendor_override = flags.vendor.map(|v| match v { @@ -920,6 +881,16 @@ impl CliOptions { ) } + /// This method is purposefully verbose to disourage its use. Do not use it + /// except in the factory structs. Instead, prefer specific methods on `CliOptions` + /// that can take all sources of information into account (ex. config files or env vars). + pub fn into_self_and_flags( + self: Arc, + ) -> (Arc, Arc) { + let flags = self.flags.clone(); + (self, flags) + } + #[inline(always)] pub fn initial_cwd(&self) -> &Path { &self.initial_cwd @@ -1241,16 +1212,6 @@ impl CliOptions { self.workspace().vendor_dir_path() } - pub fn resolve_root_cert_store_provider( - &self, - ) -> Arc { - Arc::new(CliRootCertStoreProvider::new( - None, - self.flags.ca_stores.clone(), - self.flags.ca_data.clone(), - )) - } - pub fn resolve_ts_config_for_emit( &self, config_type: TsConfigType, @@ -1290,8 +1251,8 @@ impl CliOptions { Ok(Some(InspectorServer::new(host, version::get_user_agent())?)) } - pub fn maybe_lockfile(&self) -> Option> { - self.maybe_lockfile.clone() + pub fn maybe_lockfile(&self) -> Option<&Arc> { + self.maybe_lockfile.as_ref() } pub fn to_compiler_option_types( @@ -1537,10 +1498,6 @@ impl CliOptions { self.flags.no_npm } - pub fn no_config(&self) -> bool { - self.flags.config_flag == ConfigFlag::Disabled - } - pub fn permission_flags(&self) -> &PermissionFlags { &self.flags.permissions } diff --git a/cli/factory.rs b/cli/factory.rs index 18757cda64..d701c2719c 100644 --- a/cli/factory.rs +++ b/cli/factory.rs @@ -1,6 +1,7 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. -use crate::args::CliLockfile; +use crate::args::get_root_cert_store; +use crate::args::CaData; use crate::args::CliOptions; use crate::args::DenoSubcommand; use crate::args::Flags; @@ -51,59 +52,60 @@ use crate::util::progress_bar::ProgressBar; use crate::util::progress_bar::ProgressBarStyle; use crate::worker::CliMainWorkerFactory; use crate::worker::CliMainWorkerOptions; -use std::collections::BTreeSet; use std::path::PathBuf; -use deno_config::deno_json::ConfigFile; use deno_config::workspace::PackageJsonDepResolution; use deno_config::workspace::WorkspaceResolver; use deno_core::error::AnyError; use deno_core::futures::FutureExt; use deno_core::FeatureChecker; -use deno_package_json::PackageJsonDepValue; -use deno_lockfile::WorkspaceMemberConfig; use deno_runtime::deno_fs; use deno_runtime::deno_node::analyze::NodeCodeTranslator; use deno_runtime::deno_node::NodeResolver; -use deno_runtime::deno_node::PackageJson; +use deno_runtime::deno_tls::rustls::RootCertStore; use deno_runtime::deno_tls::RootCertStoreProvider; use deno_runtime::deno_web::BlobStore; use deno_runtime::inspector_server::InspectorServer; use log::warn; +use once_cell::sync::OnceCell; use std::future::Future; use std::sync::Arc; -pub struct CliFactoryBuilder { - watcher_communicator: Option>, +struct CliRootCertStoreProvider { + cell: OnceCell, + maybe_root_path: Option, + maybe_ca_stores: Option>, + maybe_ca_data: Option, } -impl CliFactoryBuilder { - pub fn new() -> Self { +impl CliRootCertStoreProvider { + pub fn new( + maybe_root_path: Option, + maybe_ca_stores: Option>, + maybe_ca_data: Option, + ) -> Self { Self { - watcher_communicator: None, + cell: Default::default(), + maybe_root_path, + maybe_ca_stores, + maybe_ca_data, } } +} - pub fn build_from_flags(self, flags: Flags) -> Result { - Ok(self.build_from_cli_options(Arc::new(CliOptions::from_flags(flags)?))) - } - - pub fn build_from_flags_for_watcher( - mut self, - flags: Flags, - watcher_communicator: Arc, - ) -> Result { - self.watcher_communicator = Some(watcher_communicator); - self.build_from_flags(flags) - } - - pub fn build_from_cli_options(self, options: Arc) -> CliFactory { - CliFactory { - watcher_communicator: self.watcher_communicator, - options, - services: Default::default(), - } +impl RootCertStoreProvider for CliRootCertStoreProvider { + fn get_or_try_init(&self) -> Result<&RootCertStore, AnyError> { + self + .cell + .get_or_try_init(|| { + get_root_cert_store( + self.maybe_root_path.clone(), + self.maybe_ca_stores.clone(), + self.maybe_ca_data.clone(), + ) + }) + .map_err(|e| e.into()) } } @@ -116,6 +118,10 @@ impl Default for Deferred { } impl Deferred { + pub fn from_value(value: T) -> Self { + Self(once_cell::unsync::OnceCell::from(value)) + } + #[inline(always)] pub fn get_or_try_init( &self, @@ -149,6 +155,7 @@ impl Deferred { #[derive(Default)] struct CliFactoryServices { + cli_options: Deferred>, deno_dir_provider: Deferred>, caches: Deferred>, file_fetcher: Deferred>, @@ -159,7 +166,6 @@ struct CliFactoryServices { emitter: Deferred>, fs: Deferred>, main_graph_container: Deferred>, - lockfile: Deferred>>, maybe_inspector_server: Deferred>>, root_cert_store_provider: Deferred>, blob_store: Deferred>, @@ -184,40 +190,66 @@ struct CliFactoryServices { pub struct CliFactory { watcher_communicator: Option>, - options: Arc, + flags: Arc, services: CliFactoryServices, } impl CliFactory { - pub fn from_flags(flags: Flags) -> Result { - CliFactoryBuilder::new().build_from_flags(flags) + pub fn from_flags(flags: Arc) -> Self { + Self { + flags, + watcher_communicator: None, + services: Default::default(), + } } - pub fn from_cli_options(options: Arc) -> Self { - CliFactoryBuilder::new().build_from_cli_options(options) + pub fn from_cli_options(cli_options: Arc) -> Self { + let (cli_options, flags) = cli_options.into_self_and_flags(); + CliFactory { + watcher_communicator: None, + flags, + services: CliFactoryServices { + cli_options: Deferred::from_value(cli_options), + ..Default::default() + }, + } } - pub fn cli_options(&self) -> &Arc { - &self.options + pub fn from_flags_for_watcher( + flags: Arc, + watcher_communicator: Arc, + ) -> Self { + CliFactory { + watcher_communicator: Some(watcher_communicator), + flags, + services: Default::default(), + } } - pub fn deno_dir_provider(&self) -> &Arc { - self.services.deno_dir_provider.get_or_init(|| { - Arc::new(DenoDirProvider::new( - self.options.maybe_custom_root().clone(), - )) + pub fn cli_options(&self) -> Result<&Arc, AnyError> { + self.services.cli_options.get_or_try_init(|| { + CliOptions::from_flags(self.flags.clone()).map(Arc::new) + }) + } + + pub fn deno_dir_provider(&self) -> Result<&Arc, AnyError> { + self.services.deno_dir_provider.get_or_try_init(|| { + Ok(Arc::new(DenoDirProvider::new( + self.cli_options()?.maybe_custom_root().clone(), + ))) }) } pub fn deno_dir(&self) -> Result<&DenoDir, AnyError> { - Ok(self.deno_dir_provider().get_or_create()?) + Ok(self.deno_dir_provider()?.get_or_create()?) } pub fn caches(&self) -> Result<&Arc, AnyError> { self.services.caches.get_or_try_init(|| { - let caches = Arc::new(Caches::new(self.deno_dir_provider().clone())); + let cli_options = self.cli_options()?; + let caches = Arc::new(Caches::new(self.deno_dir_provider()?.clone())); // Warm up the caches we know we'll likely need based on the CLI mode - match self.options.sub_command() { + match cli_options.sub_command() { DenoSubcommand::Run(_) | DenoSubcommand::Serve(_) | DenoSubcommand::Bench(_) @@ -225,11 +257,11 @@ impl CliFactory { | DenoSubcommand::Check(_) => { _ = caches.dep_analysis_db(); _ = caches.node_analysis_db(); - if self.options.type_check_mode().is_true() { + if cli_options.type_check_mode().is_true() { _ = caches.fast_check_db(); _ = caches.type_checking_cache_db(); } - if self.options.code_cache_enabled() { + if cli_options.code_cache_enabled() { _ = caches.code_cache_db(); } } @@ -244,10 +276,13 @@ impl CliFactory { } pub fn root_cert_store_provider(&self) -> &Arc { - self - .services - .root_cert_store_provider - .get_or_init(|| self.options.resolve_root_cert_store_provider()) + self.services.root_cert_store_provider.get_or_init(|| { + Arc::new(CliRootCertStoreProvider::new( + None, + self.flags.ca_stores.clone(), + self.flags.ca_data.clone(), + )) + }) } pub fn text_only_progress_bar(&self) -> &ProgressBar { @@ -269,7 +304,7 @@ impl CliFactory { pub fn http_cache(&self) -> Result<&Arc, AnyError> { self.services.http_cache.get_or_try_init(|| { let global_cache = self.global_http_cache()?.clone(); - match self.options.vendor_dir_path() { + match self.cli_options()?.vendor_dir_path() { Some(local_path) => { let local_cache = LocalHttpCache::new(local_path.clone(), global_cache); @@ -284,17 +319,18 @@ impl CliFactory { self.services.http_client_provider.get_or_init(|| { Arc::new(HttpClientProvider::new( Some(self.root_cert_store_provider().clone()), - self.options.unsafely_ignore_certificate_errors().clone(), + self.flags.unsafely_ignore_certificate_errors.clone(), )) }) } pub fn file_fetcher(&self) -> Result<&Arc, AnyError> { self.services.file_fetcher.get_or_try_init(|| { + let cli_options = self.cli_options()?; Ok(Arc::new(FileFetcher::new( self.http_cache()?.clone(), - self.options.cache_setting(), - !self.options.no_remote(), + cli_options.cache_setting(), + !cli_options.no_remote(), self.http_client_provider().clone(), self.blob_store().clone(), Some(self.text_only_progress_bar().clone()), @@ -306,98 +342,6 @@ impl CliFactory { self.services.fs.get_or_init(|| Arc::new(deno_fs::RealFs)) } - pub fn maybe_lockfile(&self) -> &Option> { - fn pkg_json_deps(maybe_pkg_json: Option<&PackageJson>) -> BTreeSet { - let Some(pkg_json) = maybe_pkg_json else { - return Default::default(); - }; - pkg_json - .resolve_local_package_json_deps() - .values() - .filter_map(|dep| dep.as_ref().ok()) - .filter_map(|dep| match dep { - PackageJsonDepValue::Req(req) => Some(req), - PackageJsonDepValue::Workspace(_) => None, - }) - .map(|r| format!("npm:{}", r)) - .collect() - } - - fn deno_json_deps( - maybe_deno_json: Option<&ConfigFile>, - ) -> BTreeSet { - maybe_deno_json - .map(|c| { - crate::args::deno_json::deno_json_deps(c) - .into_iter() - .map(|req| req.to_string()) - .collect() - }) - .unwrap_or_default() - } - - self.services.lockfile.get_or_init(|| { - let maybe_lockfile = self.options.maybe_lockfile(); - - // initialize the lockfile with the workspace's configuration - if let Some(lockfile) = &maybe_lockfile { - let root_url = self.options.workspace().root_dir(); - let root_folder = self.options.workspace().root_folder_configs(); - let config = deno_lockfile::WorkspaceConfig { - root: WorkspaceMemberConfig { - package_json_deps: pkg_json_deps(root_folder.pkg_json.as_deref()), - dependencies: deno_json_deps(root_folder.deno_json.as_deref()), - }, - members: self - .options - .workspace() - .config_folders() - .iter() - .filter(|(folder_url, _)| *folder_url != root_url) - .filter_map(|(folder_url, folder)| { - Some(( - { - // should never be None here, but just ignore members that - // do fail for this - let mut relative_path = root_url.make_relative(folder_url)?; - if relative_path.ends_with('/') { - // make it slightly cleaner by removing the trailing slash - relative_path.pop(); - } - relative_path - }, - { - let config = WorkspaceMemberConfig { - package_json_deps: pkg_json_deps( - folder.pkg_json.as_deref(), - ), - dependencies: deno_json_deps(folder.deno_json.as_deref()), - }; - if config.package_json_deps.is_empty() - && config.dependencies.is_empty() - { - // exclude empty workspace members - return None; - } - config - }, - )) - }) - .collect(), - }; - lockfile.set_workspace_config( - deno_lockfile::SetWorkspaceConfigOptions { - no_npm: self.options.no_npm(), - no_config: self.options.no_config(), - config, - }, - ); - } - - maybe_lockfile - }) - } - pub async fn npm_resolver( &self, ) -> Result<&Arc, AnyError> { @@ -406,25 +350,26 @@ impl CliFactory { .npm_resolver .get_or_try_init_async(async { let fs = self.fs(); + let cli_options = self.cli_options()?; // For `deno install` we want to force the managed resolver so it can set up `node_modules/` directory. - create_cli_npm_resolver(if self.options.use_byonm() && !matches!(self.options.sub_command(), DenoSubcommand::Install(_)) { + create_cli_npm_resolver(if cli_options.use_byonm() && !matches!(cli_options.sub_command(), DenoSubcommand::Install(_)) { CliNpmResolverCreateOptions::Byonm(CliNpmResolverByonmCreateOptions { fs: fs.clone(), - root_node_modules_dir: Some(match self.options.node_modules_dir_path() { + root_node_modules_dir: Some(match cli_options.node_modules_dir_path() { Some(node_modules_path) => node_modules_path.to_path_buf(), // path needs to be canonicalized for node resolution // (node_modules_dir_path above is already canonicalized) - None => canonicalize_path_maybe_not_exists(self.options.initial_cwd())? + None => canonicalize_path_maybe_not_exists(cli_options.initial_cwd())? .join("node_modules"), }), }) } else { CliNpmResolverCreateOptions::Managed(CliNpmResolverManagedCreateOptions { - snapshot: match self.options.resolve_npm_resolution_snapshot()? { + snapshot: match cli_options.resolve_npm_resolution_snapshot()? { Some(snapshot) => { CliNpmResolverManagedSnapshotOption::Specified(Some(snapshot)) } - None => match self.maybe_lockfile().as_ref() { + None => match cli_options.maybe_lockfile() { Some(lockfile) => { CliNpmResolverManagedSnapshotOption::ResolveFromLockfile( lockfile.clone(), @@ -433,19 +378,19 @@ impl CliFactory { None => CliNpmResolverManagedSnapshotOption::Specified(None), }, }, - maybe_lockfile: self.maybe_lockfile().as_ref().cloned(), + maybe_lockfile: cli_options.maybe_lockfile().cloned(), fs: fs.clone(), http_client_provider: self.http_client_provider().clone(), npm_global_cache_dir: self.deno_dir()?.npm_folder_path(), - cache_setting: self.options.cache_setting(), + cache_setting: cli_options.cache_setting(), text_only_progress_bar: self.text_only_progress_bar().clone(), - maybe_node_modules_path: self.options.node_modules_dir_path().cloned(), + maybe_node_modules_path: cli_options.node_modules_dir_path().cloned(), package_json_deps_provider: Arc::new(PackageJsonInstallDepsProvider::from_workspace( - self.options.workspace(), + cli_options.workspace(), )), - npm_system_info: self.options.npm_system_info(), - npmrc: self.options.npmrc().clone(), - lifecycle_scripts: self.options.lifecycle_scripts_config(), + npm_system_info: cli_options.npm_system_info(), + npmrc: cli_options.npmrc().clone(), + lifecycle_scripts: cli_options.lifecycle_scripts_config(), }) }).await }.boxed_local()) @@ -459,11 +404,11 @@ impl CliFactory { .services .workspace_resolver .get_or_try_init_async(async { - let resolver = self - .options + let cli_options = self.cli_options()?; + let resolver = cli_options .create_workspace_resolver( self.file_fetcher()?, - if self.options.use_byonm() { + if cli_options.use_byonm() { PackageJsonDepResolution::Disabled } else { // todo(dsherret): this should be false for nodeModulesDir: true @@ -493,27 +438,26 @@ impl CliFactory { .resolver .get_or_try_init_async( async { + let cli_options = self.cli_options()?; Ok(Arc::new(CliGraphResolver::new(CliGraphResolverOptions { - sloppy_imports_resolver: if self.options.unstable_sloppy_imports() { + sloppy_imports_resolver: if cli_options.unstable_sloppy_imports() { Some(SloppyImportsResolver::new(self.fs().clone())) } else { None }, node_resolver: Some(self.cli_node_resolver().await?.clone()), - npm_resolver: if self.options.no_npm() { + npm_resolver: if cli_options.no_npm() { None } else { Some(self.npm_resolver().await?.clone()) }, workspace_resolver: self.workspace_resolver().await?.clone(), - bare_node_builtins_enabled: self - .options + bare_node_builtins_enabled: cli_options .unstable_bare_node_builtins(), - maybe_jsx_import_source_config: self - .options + maybe_jsx_import_source_config: cli_options .workspace() .to_maybe_jsx_import_source_config()?, - maybe_vendor_dir: self.options.vendor_dir_path(), + maybe_vendor_dir: cli_options.vendor_dir_path(), }))) } .boxed_local(), @@ -561,9 +505,9 @@ impl CliFactory { pub fn emitter(&self) -> Result<&Arc, AnyError> { self.services.emitter.get_or_try_init(|| { - let ts_config_result = self - .options - .resolve_ts_config_for_emit(TsConfigType::Emit)?; + let cli_options = self.cli_options()?; + let ts_config_result = + cli_options.resolve_ts_config_for_emit(TsConfigType::Emit)?; if let Some(ignored_options) = ts_config_result.maybe_ignored_options { warn!("{}", ignored_options); } @@ -624,9 +568,10 @@ impl CliFactory { .services .type_checker .get_or_try_init_async(async { + let cli_options = self.cli_options()?; Ok(Arc::new(TypeChecker::new( self.caches()?.clone(), - self.options.clone(), + cli_options.clone(), self.module_graph_builder().await?.clone(), self.node_resolver().await?.clone(), self.npm_resolver().await?.clone(), @@ -642,15 +587,16 @@ impl CliFactory { .services .module_graph_builder .get_or_try_init_async(async { + let cli_options = self.cli_options()?; Ok(Arc::new(ModuleGraphBuilder::new( - self.options.clone(), + cli_options.clone(), self.caches()?.clone(), self.fs().clone(), self.resolver().await?.clone(), self.npm_resolver().await?.clone(), self.module_info_cache()?.clone(), self.parsed_source_cache().clone(), - self.maybe_lockfile().clone(), + cli_options.maybe_lockfile().cloned(), self.maybe_file_watcher_reporter().clone(), self.emit_cache()?.clone(), self.file_fetcher()?.clone(), @@ -667,8 +613,9 @@ impl CliFactory { .services .module_graph_creator .get_or_try_init_async(async { + let cli_options = self.cli_options()?; Ok(Arc::new(ModuleGraphCreator::new( - self.options.clone(), + cli_options.clone(), self.npm_resolver().await?.clone(), self.module_graph_builder().await?.clone(), self.type_checker().await?.clone(), @@ -685,7 +632,7 @@ impl CliFactory { .main_graph_container .get_or_try_init_async(async { Ok(Arc::new(MainModuleGraphContainer::new( - self.cli_options().clone(), + self.cli_options()?.clone(), self.module_load_preparer().await?.clone(), ))) }) @@ -696,7 +643,8 @@ impl CliFactory { &self, ) -> Result<&Option>, AnyError> { self.services.maybe_inspector_server.get_or_try_init(|| { - match self.options.resolve_inspector_server() { + let cli_options = self.cli_options()?; + match cli_options.resolve_inspector_server() { Ok(server) => Ok(server.map(Arc::new)), Err(err) => Err(err), } @@ -710,9 +658,10 @@ impl CliFactory { .services .module_load_preparer .get_or_try_init_async(async { + let cli_options = self.cli_options()?; Ok(Arc::new(ModuleLoadPreparer::new( - self.options.clone(), - self.maybe_lockfile().clone(), + cli_options.clone(), + cli_options.maybe_lockfile().cloned(), self.module_graph_builder().await?.clone(), self.text_only_progress_bar().clone(), self.type_checker().await?.clone(), @@ -742,61 +691,64 @@ impl CliFactory { .await } - pub fn feature_checker(&self) -> &Arc { - self.services.feature_checker.get_or_init(|| { + pub fn feature_checker(&self) -> Result<&Arc, AnyError> { + self.services.feature_checker.get_or_try_init(|| { + let cli_options = self.cli_options()?; let mut checker = FeatureChecker::default(); checker.set_exit_cb(Box::new(crate::unstable_exit_cb)); checker.set_warn_cb(Box::new(crate::unstable_warn_cb)); - if self.options.legacy_unstable_flag() { + if cli_options.legacy_unstable_flag() { checker.enable_legacy_unstable(); checker.warn_on_legacy_unstable(); } - let unstable_features = self.options.unstable_features(); + let unstable_features = cli_options.unstable_features(); for (flag_name, _, _) in crate::UNSTABLE_GRANULAR_FLAGS { if unstable_features.contains(&flag_name.to_string()) { checker.enable_feature(flag_name); } } - Arc::new(checker) + Ok(Arc::new(checker)) }) } pub async fn create_compile_binary_writer( &self, ) -> Result { + let cli_options = self.cli_options()?; Ok(DenoCompileBinaryWriter::new( self.deno_dir()?, self.file_fetcher()?, self.http_client_provider(), self.npm_resolver().await?.as_ref(), self.workspace_resolver().await?.as_ref(), - self.options.npm_system_info(), + cli_options.npm_system_info(), )) } pub async fn create_cli_main_worker_factory( &self, ) -> Result { + let cli_options = self.cli_options()?; let node_resolver = self.node_resolver().await?; let npm_resolver = self.npm_resolver().await?; let fs = self.fs(); let cli_node_resolver = self.cli_node_resolver().await?; - let maybe_file_watcher_communicator = if self.options.has_hmr() { + let maybe_file_watcher_communicator = if cli_options.has_hmr() { Some(self.watcher_communicator.clone().unwrap()) } else { None }; Ok(CliMainWorkerFactory::new( - StorageKeyResolver::from_options(&self.options), - self.options.sub_command().clone(), + StorageKeyResolver::from_options(cli_options), + cli_options.sub_command().clone(), npm_resolver.clone(), node_resolver.clone(), self.blob_store().clone(), Box::new(CliModuleLoaderFactory::new( - &self.options, - if self.options.code_cache_enabled() { + cli_options, + if cli_options.code_cache_enabled() { Some(self.code_cache()?.clone()) } else { None @@ -818,18 +770,18 @@ impl CliFactory { self.fs().clone(), maybe_file_watcher_communicator, self.maybe_inspector_server()?.clone(), - self.maybe_lockfile().clone(), - self.feature_checker().clone(), + cli_options.maybe_lockfile().cloned(), + self.feature_checker()?.clone(), self.create_cli_main_worker_options()?, - self.options.node_ipc_fd(), - self.options.serve_port(), - self.options.serve_host(), - self.options.enable_future_features(), + cli_options.node_ipc_fd(), + cli_options.serve_port(), + cli_options.serve_host(), + cli_options.enable_future_features(), // TODO(bartlomieju): temporarily disabled - // self.options.disable_deprecated_api_warning, + // cli_options.disable_deprecated_api_warning, true, - self.options.verbose_deprecated_api_warning, - if self.options.code_cache_enabled() { + cli_options.verbose_deprecated_api_warning, + if cli_options.code_cache_enabled() { Some(self.code_cache()?.clone()) } else { None @@ -840,7 +792,8 @@ impl CliFactory { fn create_cli_main_worker_options( &self, ) -> Result { - let create_hmr_runner = if self.options.has_hmr() { + let cli_options = self.cli_options()?; + let create_hmr_runner = if cli_options.has_hmr() { let watcher_communicator = self.watcher_communicator.clone().unwrap(); let emitter = self.emitter()?.clone(); let fn_: crate::worker::CreateHmrRunnerCb = Box::new(move |session| { @@ -855,7 +808,7 @@ impl CliFactory { None }; let create_coverage_collector = - if let Some(coverage_dir) = self.options.coverage_dir() { + if let Some(coverage_dir) = cli_options.coverage_dir() { let coverage_dir = PathBuf::from(coverage_dir); let fn_: crate::worker::CreateCoverageCollectorCb = Box::new(move |session| { @@ -867,36 +820,34 @@ impl CliFactory { }; Ok(CliMainWorkerOptions { - argv: self.options.argv().clone(), + argv: cli_options.argv().clone(), // This optimization is only available for "run" subcommand // because we need to register new ops for testing and jupyter // integration. - skip_op_registration: self.options.sub_command().is_run(), - log_level: self.options.log_level().unwrap_or(log::Level::Info).into(), - enable_op_summary_metrics: self.options.enable_op_summary_metrics(), - enable_testing_features: self.options.enable_testing_features(), - has_node_modules_dir: self.options.has_node_modules_dir(), - hmr: self.options.has_hmr(), - inspect_brk: self.options.inspect_brk().is_some(), - inspect_wait: self.options.inspect_wait().is_some(), - strace_ops: self.options.strace_ops().clone(), - is_inspecting: self.options.is_inspecting(), - is_npm_main: self.options.is_npm_main(), - location: self.options.location_flag().clone(), + skip_op_registration: cli_options.sub_command().is_run(), + log_level: cli_options.log_level().unwrap_or(log::Level::Info).into(), + enable_op_summary_metrics: cli_options.enable_op_summary_metrics(), + enable_testing_features: cli_options.enable_testing_features(), + has_node_modules_dir: cli_options.has_node_modules_dir(), + hmr: cli_options.has_hmr(), + inspect_brk: cli_options.inspect_brk().is_some(), + inspect_wait: cli_options.inspect_wait().is_some(), + strace_ops: cli_options.strace_ops().clone(), + is_inspecting: cli_options.is_inspecting(), + is_npm_main: cli_options.is_npm_main(), + location: cli_options.location_flag().clone(), // if the user ran a binary command, we'll need to set process.argv[0] // to be the name of the binary command instead of deno - argv0: self - .options + argv0: cli_options .take_binary_npm_command_name() .or(std::env::args().next()), node_debug: std::env::var("NODE_DEBUG").ok(), origin_data_folder_path: Some(self.deno_dir()?.origin_data_folder_path()), - seed: self.options.seed(), - unsafely_ignore_certificate_errors: self - .options + seed: cli_options.seed(), + unsafely_ignore_certificate_errors: cli_options .unsafely_ignore_certificate_errors() .clone(), - unstable: self.options.legacy_unstable_flag(), + unstable: cli_options.legacy_unstable_flag(), create_hmr_runner, create_coverage_collector, }) diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 0e65d8b255..62cbd58dd1 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -3556,7 +3556,7 @@ impl Inner { )?), }; let cli_options = CliOptions::new( - Flags { + Arc::new(Flags { cache_path: Some(self.cache.deno_dir().root.clone()), ca_stores: workspace_settings.certificate_stores.clone(), ca_data: workspace_settings.tls_certificate.clone().map(CaData::File), @@ -3576,7 +3576,7 @@ impl Inner { // bit of a hack to force the lsp to cache the @types/node package type_check_mode: crate::args::TypeCheckMode::Local, ..Default::default() - }, + }), initial_cwd, config_data.and_then(|d| d.lockfile.clone()), config_data diff --git a/cli/lsp/testing/execution.rs b/cli/lsp/testing/execution.rs index b73bcd1309..96f22a9b06 100644 --- a/cli/lsp/testing/execution.rs +++ b/cli/lsp/testing/execution.rs @@ -212,13 +212,15 @@ impl TestRun { ) -> Result<(), AnyError> { let args = self.get_args(); lsp_log!("Executing test run with arguments: {}", args.join(" ")); - let flags = flags_from_vec(args.into_iter().map(From::from).collect())?; - let factory = CliFactory::from_flags(flags)?; + let flags = + Arc::new(flags_from_vec(args.into_iter().map(From::from).collect())?); + let factory = CliFactory::from_flags(flags); + let cli_options = factory.cli_options()?; // Various test files should not share the same permissions in terms of // `PermissionsContainer` - otherwise granting/revoking permissions in one // file would have impact on other files, which is undesirable. let permissions = - Permissions::from_options(&factory.cli_options().permissions_options()?)?; + Permissions::from_options(&cli_options.permissions_options()?)?; let main_graph_container = factory.main_module_graph_container().await?; test::check_specifiers( factory.file_fetcher()?, @@ -231,19 +233,18 @@ impl TestRun { ) .await?; - let (concurrent_jobs, fail_fast) = if let DenoSubcommand::Test(test_flags) = - factory.cli_options().sub_command() - { - ( - test_flags - .concurrent_jobs - .unwrap_or_else(|| NonZeroUsize::new(1).unwrap()) - .into(), - test_flags.fail_fast, - ) - } else { - unreachable!("Should always be Test subcommand."); - }; + let (concurrent_jobs, fail_fast) = + if let DenoSubcommand::Test(test_flags) = cli_options.sub_command() { + ( + test_flags + .concurrent_jobs + .unwrap_or_else(|| NonZeroUsize::new(1).unwrap()) + .into(), + test_flags.fail_fast, + ) + } else { + unreachable!("Should always be Test subcommand."); + }; // TODO(mmastrac): Temporarily limit concurrency in windows testing to avoid named pipe issue: // *** Unexpected server pipe failure '"\\\\.\\pipe\\deno_pipe_e30f45c9df61b1e4.1198.222\\0"': 3 diff --git a/cli/main.rs b/cli/main.rs index 4264e1610c..760151d097 100644 --- a/cli/main.rs +++ b/cli/main.rs @@ -53,6 +53,7 @@ use factory::CliFactory; use std::env; use std::future::Future; use std::path::PathBuf; +use std::sync::Arc; /// Ensures that all subcommands return an i32 exit code and an [`AnyError`] error type. trait SubcommandOutput { @@ -90,7 +91,7 @@ fn spawn_subcommand + 'static, T: SubcommandOutput>( ) } -async fn run_subcommand(flags: Flags) -> Result { +async fn run_subcommand(flags: Arc) -> Result { let handle = match flags.subcommand.clone() { DenoSubcommand::Add(add_flags) => spawn_subcommand(async { tools::registry::add(flags, add_flags).await @@ -112,7 +113,7 @@ async fn run_subcommand(flags: Flags) -> Result { tools::run::eval_command(flags, eval_flags).await }), DenoSubcommand::Cache(cache_flags) => spawn_subcommand(async move { - let factory = CliFactory::from_flags(flags)?; + let factory = CliFactory::from_flags(flags); let emitter = factory.emitter()?; let main_graph_container = factory.main_module_graph_container().await?; @@ -122,7 +123,7 @@ async fn run_subcommand(flags: Flags) -> Result { emitter.cache_module_emits(&main_graph_container.graph()).await }), DenoSubcommand::Check(check_flags) => spawn_subcommand(async move { - let factory = CliFactory::from_flags(flags)?; + let factory = CliFactory::from_flags(flags); let main_graph_container = factory.main_module_graph_container().await?; main_graph_container @@ -231,7 +232,6 @@ async fn run_subcommand(flags: Flags) -> Result { DenoSubcommand::Vendor(vendor_flags) => spawn_subcommand(async { tools::vendor::vendor(flags, vendor_flags).await }), - // TODO: DenoSubcommand::Publish(publish_flags) => spawn_subcommand(async { tools::registry::publish(flags, publish_flags).await }), @@ -330,7 +330,7 @@ pub fn main() { // initialize the V8 platform on a parent thread of all threads that will spawn // V8 isolates. let flags = resolve_flags_and_init(args)?; - run_subcommand(flags).await + run_subcommand(Arc::new(flags)).await }; match create_and_run_current_thread_with_maybe_metrics(future) { diff --git a/cli/module_loader.rs b/cli/module_loader.rs index 5156e98e3c..45a79e1894 100644 --- a/cli/module_loader.rs +++ b/cli/module_loader.rs @@ -71,9 +71,10 @@ use deno_semver::npm::NpmPackageReqReference; pub async fn load_top_level_deps(factory: &CliFactory) -> Result<(), AnyError> { let npm_resolver = factory.npm_resolver().await?; + let cli_options = factory.cli_options()?; if let Some(npm_resolver) = npm_resolver.as_managed() { if !npm_resolver.ensure_top_level_package_json_install().await? { - if let Some(lockfile) = factory.maybe_lockfile() { + if let Some(lockfile) = cli_options.maybe_lockfile() { lockfile.error_if_changed()?; } @@ -107,7 +108,7 @@ pub async fn load_top_level_deps(factory: &CliFactory) -> Result<(), AnyError> { graph, &roots, false, - factory.cli_options().ts_type_lib_window(), + factory.cli_options()?.ts_type_lib_window(), deno_runtime::deno_permissions::PermissionsContainer::allow_all(), ) .await?; diff --git a/cli/tools/bench/mod.rs b/cli/tools/bench/mod.rs index 5bbf5ce8d9..9cf222c45d 100644 --- a/cli/tools/bench/mod.rs +++ b/cli/tools/bench/mod.rs @@ -1,12 +1,10 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. use crate::args::BenchFlags; -use crate::args::CliOptions; use crate::args::Flags; use crate::colors; use crate::display::write_json_to_stdout; use crate::factory::CliFactory; -use crate::factory::CliFactoryBuilder; use crate::graph_util::has_graph_root_local_dependent_changed; use crate::ops; use crate::tools::test::format_test_error; @@ -403,14 +401,13 @@ fn has_supported_bench_path_name(path: &Path) -> bool { } pub async fn run_benchmarks( - flags: Flags, + flags: Arc, bench_flags: BenchFlags, ) -> Result<(), AnyError> { - let cli_options = CliOptions::from_flags(flags)?; + let factory = CliFactory::from_flags(flags); + let cli_options = factory.cli_options()?; let workspace_bench_options = cli_options.resolve_workspace_bench_options(&bench_flags); - let factory = CliFactory::from_cli_options(Arc::new(cli_options)); - let cli_options = factory.cli_options(); // Various bench files should not share the same permissions in terms of // `PermissionsContainer` - otherwise granting/revoking permissions in one // file would have impact on other files, which is undesirable. @@ -464,7 +461,7 @@ pub async fn run_benchmarks( // TODO(bartlomieju): heavy duplication of code with `cli/tools/test.rs` pub async fn run_benchmarks_with_watch( - flags: Flags, + flags: Arc, bench_flags: BenchFlags, ) -> Result<(), AnyError> { file_watcher::watch_func( @@ -480,9 +477,11 @@ pub async fn run_benchmarks_with_watch( move |flags, watcher_communicator, changed_paths| { let bench_flags = bench_flags.clone(); Ok(async move { - let factory = CliFactoryBuilder::new() - .build_from_flags_for_watcher(flags, watcher_communicator.clone())?; - let cli_options = factory.cli_options(); + let factory = CliFactory::from_flags_for_watcher( + flags, + watcher_communicator.clone(), + ); + let cli_options = factory.cli_options()?; let workspace_bench_options = cli_options.resolve_workspace_bench_options(&bench_flags); diff --git a/cli/tools/bundle.rs b/cli/tools/bundle.rs index ef058f0d07..f2157ecd8c 100644 --- a/cli/tools/bundle.rs +++ b/cli/tools/bundle.rs @@ -1,6 +1,7 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. use std::path::PathBuf; +use std::sync::Arc; use deno_core::error::AnyError; use deno_graph::Module; @@ -11,13 +12,12 @@ use crate::args::CliOptions; use crate::args::Flags; use crate::args::TsConfigType; use crate::factory::CliFactory; -use crate::factory::CliFactoryBuilder; use crate::graph_util::error_for_any_npm_specifier; use crate::util; use crate::util::display; pub async fn bundle( - flags: Flags, + flags: Arc, bundle_flags: BundleFlags, ) -> Result<(), AnyError> { log::info!( @@ -35,11 +35,11 @@ pub async fn bundle( move |flags, watcher_communicator, _changed_paths| { let bundle_flags = bundle_flags.clone(); Ok(async move { - let factory = CliFactoryBuilder::new().build_from_flags_for_watcher( + let factory = CliFactory::from_flags_for_watcher( flags, watcher_communicator.clone(), - )?; - let cli_options = factory.cli_options(); + ); + let cli_options = factory.cli_options()?; let _ = watcher_communicator.watch_paths(cli_options.watch_paths()); bundle_action(factory, &bundle_flags).await?; @@ -49,7 +49,7 @@ pub async fn bundle( ) .await?; } else { - let factory = CliFactory::from_flags(flags)?; + let factory = CliFactory::from_flags(flags); bundle_action(factory, &bundle_flags).await?; } @@ -60,11 +60,11 @@ async fn bundle_action( factory: CliFactory, bundle_flags: &BundleFlags, ) -> Result<(), AnyError> { - let cli_options = factory.cli_options(); + let cli_options = factory.cli_options()?; let module_specifier = cli_options.resolve_main_module()?; log::debug!(">>>>> bundle START"); let module_graph_creator = factory.module_graph_creator().await?; - let cli_options = factory.cli_options(); + let cli_options = factory.cli_options()?; let graph = module_graph_creator .create_graph_and_maybe_check(vec![module_specifier.clone()]) diff --git a/cli/tools/compile.rs b/cli/tools/compile.rs index 5500f9512d..4df8e772bb 100644 --- a/cli/tools/compile.rs +++ b/cli/tools/compile.rs @@ -22,11 +22,11 @@ use std::sync::Arc; use super::installer::infer_name_from_url; pub async fn compile( - flags: Flags, + flags: Arc, compile_flags: CompileFlags, ) -> Result<(), AnyError> { - let factory = CliFactory::from_flags(flags)?; - let cli_options = factory.cli_options(); + let factory = CliFactory::from_flags(flags); + let cli_options = factory.cli_options()?; let module_graph_creator = factory.module_graph_creator().await?; let parsed_source_cache = factory.parsed_source_cache(); let binary_writer = factory.create_compile_binary_writer().await?; diff --git a/cli/tools/coverage/mod.rs b/cli/tools/coverage/mod.rs index c9eda3c195..f88b3bc645 100644 --- a/cli/tools/coverage/mod.rs +++ b/cli/tools/coverage/mod.rs @@ -32,6 +32,7 @@ use std::io::BufWriter; use std::io::Write; use std::path::Path; use std::path::PathBuf; +use std::sync::Arc; use text_lines::TextLines; use uuid::Uuid; @@ -473,17 +474,17 @@ fn filter_coverages( } pub async fn cover_files( - flags: Flags, + flags: Arc, coverage_flags: CoverageFlags, ) -> Result<(), AnyError> { if coverage_flags.files.include.is_empty() { return Err(generic_error("No matching coverage profiles found")); } - let factory = CliFactory::from_flags(flags)?; + let factory = CliFactory::from_flags(flags); + let cli_options = factory.cli_options()?; let npm_resolver = factory.npm_resolver().await?; let file_fetcher = factory.file_fetcher()?; - let cli_options = factory.cli_options(); let emitter = factory.emitter()?; assert!(!coverage_flags.files.include.is_empty()); diff --git a/cli/tools/doc.rs b/cli/tools/doc.rs index 138e0a47d1..824a3fc389 100644 --- a/cli/tools/doc.rs +++ b/cli/tools/doc.rs @@ -29,6 +29,7 @@ use doc::DocDiagnostic; use indexmap::IndexMap; use std::collections::BTreeMap; use std::rc::Rc; +use std::sync::Arc; async fn generate_doc_nodes_for_builtin_types( doc_flags: DocFlags, @@ -83,9 +84,12 @@ async fn generate_doc_nodes_for_builtin_types( Ok(IndexMap::from([(source_file_specifier, nodes)])) } -pub async fn doc(flags: Flags, doc_flags: DocFlags) -> Result<(), AnyError> { - let factory = CliFactory::from_flags(flags)?; - let cli_options = factory.cli_options(); +pub async fn doc( + flags: Arc, + doc_flags: DocFlags, +) -> Result<(), AnyError> { + let factory = CliFactory::from_flags(flags); + let cli_options = factory.cli_options()?; let module_info_cache = factory.module_info_cache()?; let parsed_source_cache = factory.parsed_source_cache(); let capturing_parser = parsed_source_cache.as_capturing_parser(); @@ -102,7 +106,7 @@ pub async fn doc(flags: Flags, doc_flags: DocFlags) -> Result<(), AnyError> { } DocSourceFileFlag::Paths(ref source_files) => { let module_graph_creator = factory.module_graph_creator().await?; - let maybe_lockfile = factory.maybe_lockfile(); + let maybe_lockfile = cli_options.maybe_lockfile(); let module_specifiers = collect_specifiers( FilePatterns { diff --git a/cli/tools/fmt.rs b/cli/tools/fmt.rs index 277a79cda0..baecb482fe 100644 --- a/cli/tools/fmt.rs +++ b/cli/tools/fmt.rs @@ -49,7 +49,10 @@ use std::sync::Arc; use crate::cache::IncrementalCache; /// Format JavaScript/TypeScript files. -pub async fn format(flags: Flags, fmt_flags: FmtFlags) -> Result<(), AnyError> { +pub async fn format( + flags: Arc, + fmt_flags: FmtFlags, +) -> Result<(), AnyError> { if fmt_flags.is_stdin() { let cli_options = CliOptions::from_flags(flags)?; let start_dir = &cli_options.start_dir; @@ -74,8 +77,8 @@ pub async fn format(flags: Flags, fmt_flags: FmtFlags) -> Result<(), AnyError> { move |flags, watcher_communicator, changed_paths| { let fmt_flags = fmt_flags.clone(); Ok(async move { - let factory = CliFactory::from_flags(flags)?; - let cli_options = factory.cli_options(); + let factory = CliFactory::from_flags(flags); + let cli_options = factory.cli_options()?; let caches = factory.caches()?; let mut paths_with_options_batches = resolve_paths_with_options_batches(cli_options, &fmt_flags)?; @@ -119,9 +122,9 @@ pub async fn format(flags: Flags, fmt_flags: FmtFlags) -> Result<(), AnyError> { ) .await?; } else { - let factory = CliFactory::from_flags(flags)?; + let factory = CliFactory::from_flags(flags); + let cli_options = factory.cli_options()?; let caches = factory.caches()?; - let cli_options = factory.cli_options(); let paths_with_options_batches = resolve_paths_with_options_batches(cli_options, &fmt_flags)?; format_files(caches, &fmt_flags, paths_with_options_batches).await?; diff --git a/cli/tools/info.rs b/cli/tools/info.rs index 18a4bed57c..a15252c7c4 100644 --- a/cli/tools/info.rs +++ b/cli/tools/info.rs @@ -4,6 +4,7 @@ use std::collections::HashMap; use std::collections::HashSet; use std::fmt; use std::fmt::Write; +use std::sync::Arc; use deno_ast::ModuleSpecifier; use deno_core::anyhow::bail; @@ -34,14 +35,17 @@ use crate::npm::CliNpmResolver; use crate::npm::ManagedCliNpmResolver; use crate::util::checksum; -pub async fn info(flags: Flags, info_flags: InfoFlags) -> Result<(), AnyError> { - let factory = CliFactory::from_flags(flags)?; - let cli_options = factory.cli_options(); +pub async fn info( + flags: Arc, + info_flags: InfoFlags, +) -> Result<(), AnyError> { + let factory = CliFactory::from_flags(flags); + let cli_options = factory.cli_options()?; if let Some(specifier) = info_flags.file { let module_graph_builder = factory.module_graph_builder().await?; let module_graph_creator = factory.module_graph_creator().await?; let npm_resolver = factory.npm_resolver().await?; - let maybe_lockfile = factory.maybe_lockfile(); + let maybe_lockfile = cli_options.maybe_lockfile(); let resolver = factory.workspace_resolver().await?; let maybe_import_specifier = diff --git a/cli/tools/installer.rs b/cli/tools/installer.rs index b7b9b4d96c..01ed367118 100644 --- a/cli/tools/installer.rs +++ b/cli/tools/installer.rs @@ -35,6 +35,7 @@ use std::path::PathBuf; #[cfg(not(windows))] use std::os::unix::fs::PermissionsExt; +use std::sync::Arc; static EXEC_NAME_RE: Lazy = Lazy::new(|| { RegexBuilder::new(r"^[a-z0-9][\w-]*$") @@ -261,17 +262,17 @@ pub fn uninstall(uninstall_flags: UninstallFlags) -> Result<(), AnyError> { } async fn install_local( - flags: Flags, + flags: Arc, maybe_add_flags: Option, ) -> Result<(), AnyError> { if let Some(add_flags) = maybe_add_flags { return super::registry::add(flags, add_flags).await; } - let factory = CliFactory::from_flags(flags)?; + let factory = CliFactory::from_flags(flags); crate::module_loader::load_top_level_deps(&factory).await?; - if let Some(lockfile) = factory.cli_options().maybe_lockfile() { + if let Some(lockfile) = factory.cli_options()?.maybe_lockfile() { lockfile.write_if_changed()?; } @@ -279,7 +280,7 @@ async fn install_local( } pub async fn install_command( - flags: Flags, + flags: Arc, install_flags: InstallFlags, ) -> Result<(), AnyError> { if !install_flags.global { @@ -297,11 +298,11 @@ pub async fn install_command( } async fn install_global( - flags: Flags, + flags: Arc, install_flags_global: InstallFlagsGlobal, ) -> Result<(), AnyError> { // ensure the module is cached - let factory = CliFactory::from_flags(flags.clone())?; + let factory = CliFactory::from_flags(flags.clone()); factory .main_module_graph_container() .await? @@ -310,16 +311,16 @@ async fn install_global( let http_client = factory.http_client_provider(); // create the install shim - create_install_shim(http_client, flags, install_flags_global).await + create_install_shim(http_client, &flags, install_flags_global).await } async fn create_install_shim( http_client_provider: &HttpClientProvider, - flags: Flags, + flags: &Flags, install_flags_global: InstallFlagsGlobal, ) -> Result<(), AnyError> { let shim_data = - resolve_shim_data(http_client_provider, &flags, &install_flags_global) + resolve_shim_data(http_client_provider, flags, &install_flags_global) .await?; // ensure directory exists @@ -778,7 +779,7 @@ mod tests { create_install_shim( &HttpClientProvider::new(None, None), - Flags { + &Flags { unstable_config: UnstableConfig { legacy_flag_enabled: true, ..Default::default() @@ -1173,7 +1174,7 @@ mod tests { create_install_shim( &HttpClientProvider::new(None, None), - Flags::default(), + &Flags::default(), InstallFlagsGlobal { module_url: local_module_str.to_string(), args: vec![], @@ -1203,7 +1204,7 @@ mod tests { create_install_shim( &HttpClientProvider::new(None, None), - Flags::default(), + &Flags::default(), InstallFlagsGlobal { module_url: "http://localhost:4545/echo_server.ts".to_string(), args: vec![], @@ -1224,7 +1225,7 @@ mod tests { // No force. Install failed. let no_force_result = create_install_shim( &HttpClientProvider::new(None, None), - Flags::default(), + &Flags::default(), InstallFlagsGlobal { module_url: "http://localhost:4545/cat.ts".to_string(), // using a different URL args: vec![], @@ -1246,7 +1247,7 @@ mod tests { // Force. Install success. let force_result = create_install_shim( &HttpClientProvider::new(None, None), - Flags::default(), + &Flags::default(), InstallFlagsGlobal { module_url: "http://localhost:4545/cat.ts".to_string(), // using a different URL args: vec![], @@ -1274,7 +1275,7 @@ mod tests { let result = create_install_shim( &HttpClientProvider::new(None, None), - Flags { + &Flags { config_flag: ConfigFlag::Path(config_file_path.to_string()), ..Flags::default() }, @@ -1307,7 +1308,7 @@ mod tests { create_install_shim( &HttpClientProvider::new(None, None), - Flags::default(), + &Flags::default(), InstallFlagsGlobal { module_url: "http://localhost:4545/echo_server.ts".to_string(), args: vec!["\"".to_string()], @@ -1348,7 +1349,7 @@ mod tests { create_install_shim( &HttpClientProvider::new(None, None), - Flags::default(), + &Flags::default(), InstallFlagsGlobal { module_url: local_module_str.to_string(), args: vec![], @@ -1390,7 +1391,7 @@ mod tests { let result = create_install_shim( &HttpClientProvider::new(None, None), - Flags { + &Flags { import_map_path: Some(import_map_path.to_string()), ..Flags::default() }, @@ -1436,7 +1437,7 @@ mod tests { let result = create_install_shim( &HttpClientProvider::new(None, None), - Flags::default(), + &Flags::default(), InstallFlagsGlobal { module_url: file_module_string.to_string(), args: vec![], diff --git a/cli/tools/jupyter/mod.rs b/cli/tools/jupyter/mod.rs index 1197d3df77..eff7f4f9d6 100644 --- a/cli/tools/jupyter/mod.rs +++ b/cli/tools/jupyter/mod.rs @@ -1,5 +1,7 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. +use std::sync::Arc; + use crate::args::Flags; use crate::args::JupyterFlags; use crate::cdp; @@ -36,7 +38,7 @@ mod install; pub mod server; pub async fn kernel( - flags: Flags, + flags: Arc, jupyter_flags: JupyterFlags, ) -> Result<(), AnyError> { log::info!( @@ -56,8 +58,8 @@ pub async fn kernel( let connection_filepath = jupyter_flags.conn_file.unwrap(); - let factory = CliFactory::from_flags(flags)?; - let cli_options = factory.cli_options(); + let factory = CliFactory::from_flags(flags); + let cli_options = factory.cli_options()?; let main_module = resolve_url_or_path("./$deno$jupyter.ts", cli_options.initial_cwd()) .unwrap(); diff --git a/cli/tools/lint/mod.rs b/cli/tools/lint/mod.rs index 3b2af12fba..9421291209 100644 --- a/cli/tools/lint/mod.rs +++ b/cli/tools/lint/mod.rs @@ -76,7 +76,10 @@ fn create_reporter(kind: LintReporterKind) -> Box { } } -pub async fn lint(flags: Flags, lint_flags: LintFlags) -> Result<(), AnyError> { +pub async fn lint( + flags: Arc, + lint_flags: LintFlags, +) -> Result<(), AnyError> { if let Some(watch_flags) = &lint_flags.watch { if lint_flags.is_stdin() { return Err(generic_error( @@ -89,8 +92,8 @@ pub async fn lint(flags: Flags, lint_flags: LintFlags) -> Result<(), AnyError> { move |flags, watcher_communicator, changed_paths| { let lint_flags = lint_flags.clone(); Ok(async move { - let factory = CliFactory::from_flags(flags)?; - let cli_options = factory.cli_options(); + let factory = CliFactory::from_flags(flags); + let cli_options = factory.cli_options()?; let lint_config = cli_options.resolve_deno_lint_config()?; let mut paths_with_options_batches = resolve_paths_with_options_batches(cli_options, &lint_flags)?; @@ -140,8 +143,8 @@ pub async fn lint(flags: Flags, lint_flags: LintFlags) -> Result<(), AnyError> { ) .await?; } else { - let factory = CliFactory::from_flags(flags)?; - let cli_options = factory.cli_options(); + let factory = CliFactory::from_flags(flags); + let cli_options = factory.cli_options()?; let is_stdin = lint_flags.is_stdin(); let deno_lint_config = cli_options.resolve_deno_lint_config()?; let workspace_lint_options = diff --git a/cli/tools/registry/mod.rs b/cli/tools/registry/mod.rs index 1c4e1bea76..bf9ce85861 100644 --- a/cli/tools/registry/mod.rs +++ b/cli/tools/registry/mod.rs @@ -72,16 +72,16 @@ use self::paths::CollectedPublishPath; use self::tar::PublishableTarball; pub async fn publish( - flags: Flags, + flags: Arc, publish_flags: PublishFlags, ) -> Result<(), AnyError> { - let cli_factory = CliFactory::from_flags(flags)?; + let cli_factory = CliFactory::from_flags(flags); let auth_method = get_auth_method(publish_flags.token, publish_flags.dry_run)?; - let directory_path = cli_factory.cli_options().initial_cwd(); - let cli_options = cli_factory.cli_options(); + let cli_options = cli_factory.cli_options()?; + let directory_path = cli_options.initial_cwd(); let publish_configs = cli_options.start_dir.jsr_packages_for_publish(); if publish_configs.is_empty() { match cli_options.start_dir.maybe_deno_json() { @@ -121,7 +121,7 @@ pub async fn publish( cli_factory.module_graph_creator().await?.clone(), cli_factory.parsed_source_cache().clone(), cli_factory.type_checker().await?.clone(), - cli_factory.cli_options().clone(), + cli_options.clone(), specifier_unfurler, ); diff --git a/cli/tools/registry/pm.rs b/cli/tools/registry/pm.rs index 4573f196db..233e682408 100644 --- a/cli/tools/registry/pm.rs +++ b/cli/tools/registry/pm.rs @@ -119,9 +119,9 @@ impl DenoOrPackageJson { /// creates a `deno.json` file - in this case /// we also return a new `CliFactory` that knows about /// the new config - fn from_flags(flags: Flags) -> Result<(Self, CliFactory), AnyError> { - let factory = CliFactory::from_flags(flags.clone())?; - let options = factory.cli_options(); + fn from_flags(flags: Arc) -> Result<(Self, CliFactory), AnyError> { + let factory = CliFactory::from_flags(flags.clone()); + let options = factory.cli_options()?; let start_dir = &options.start_dir; match (start_dir.maybe_deno_json(), start_dir.maybe_pkg_json()) { @@ -142,8 +142,8 @@ impl DenoOrPackageJson { .context("Failed to create deno.json file")?; drop(factory); // drop to prevent use log::info!("Created deno.json configuration file."); - let factory = CliFactory::from_flags(flags.clone())?; - let options = factory.cli_options().clone(); + let factory = CliFactory::from_flags(flags.clone()); + let options = factory.cli_options()?.clone(); let start_dir = &options.start_dir; Ok(( DenoOrPackageJson::Deno( @@ -175,7 +175,10 @@ fn package_json_dependency_entry( } } -pub async fn add(flags: Flags, add_flags: AddFlags) -> Result<(), AnyError> { +pub async fn add( + flags: Arc, + add_flags: AddFlags, +) -> Result<(), AnyError> { let (config_file, cli_factory) = DenoOrPackageJson::from_flags(flags.clone())?; @@ -307,9 +310,9 @@ pub async fn add(flags: Flags, add_flags: AddFlags) -> Result<(), AnyError> { // clear the previously cached package.json from memory before reloading it deno_node::PackageJsonThreadLocalCache::clear(); // make a new CliFactory to pick up the updated config file - let cli_factory = CliFactory::from_flags(flags)?; + let cli_factory = CliFactory::from_flags(flags); // cache deps - if cli_factory.cli_options().enable_future_features() { + if cli_factory.cli_options()?.enable_future_features() { crate::module_loader::load_top_level_deps(&cli_factory).await?; } diff --git a/cli/tools/repl/mod.rs b/cli/tools/repl/mod.rs index 5bd59888d1..9798a69674 100644 --- a/cli/tools/repl/mod.rs +++ b/cli/tools/repl/mod.rs @@ -156,9 +156,12 @@ async fn read_eval_file( } #[allow(clippy::print_stdout)] -pub async fn run(flags: Flags, repl_flags: ReplFlags) -> Result { - let factory = CliFactory::from_flags(flags)?; - let cli_options = factory.cli_options(); +pub async fn run( + flags: Arc, + repl_flags: ReplFlags, +) -> Result { + let factory = CliFactory::from_flags(flags); + let cli_options = factory.cli_options()?; let main_module = cli_options.resolve_main_module()?; let permissions = PermissionsContainer::new(Permissions::from_options( &cli_options.permissions_options()?, diff --git a/cli/tools/run/mod.rs b/cli/tools/run/mod.rs index f70cfd066f..baa8e72db0 100644 --- a/cli/tools/run/mod.rs +++ b/cli/tools/run/mod.rs @@ -1,6 +1,7 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. use std::io::Read; +use std::sync::Arc; use deno_core::error::AnyError; use deno_runtime::deno_permissions::Permissions; @@ -11,7 +12,6 @@ use crate::args::EvalFlags; use crate::args::Flags; use crate::args::WatchFlagsWithPaths; use crate::factory::CliFactory; -use crate::factory::CliFactoryBuilder; use crate::file_fetcher::File; use crate::util; use crate::util::file_watcher::WatcherRestartMode; @@ -20,7 +20,7 @@ pub mod hmr; pub async fn run_script( mode: WorkerExecutionMode, - flags: Flags, + flags: Arc, watch: Option, ) -> Result { if !flags.has_permission() && flags.has_permission_in_argv() { @@ -40,10 +40,10 @@ To grant permissions, set them before the script argument. For example: // TODO(bartlomieju): actually I think it will also fail if there's an import // map specified and bare specifier is used on the command line - let factory = CliFactory::from_flags(flags)?; + let factory = CliFactory::from_flags(flags); + let cli_options = factory.cli_options()?; let deno_dir = factory.deno_dir()?; let http_client = factory.http_client_provider(); - let cli_options = factory.cli_options(); if cli_options.unstable_sloppy_imports() { log::warn!( @@ -76,9 +76,9 @@ To grant permissions, set them before the script argument. For example: Ok(exit_code) } -pub async fn run_from_stdin(flags: Flags) -> Result { - let factory = CliFactory::from_flags(flags)?; - let cli_options = factory.cli_options(); +pub async fn run_from_stdin(flags: Arc) -> Result { + let factory = CliFactory::from_flags(flags); + let cli_options = factory.cli_options()?; let main_module = cli_options.resolve_main_module()?; maybe_npm_install(&factory).await?; @@ -109,7 +109,7 @@ pub async fn run_from_stdin(flags: Flags) -> Result { // code properly. async fn run_with_watch( mode: WorkerExecutionMode, - flags: Flags, + flags: Arc, watch_flags: WatchFlagsWithPaths, ) -> Result { util::file_watcher::watch_recv( @@ -122,9 +122,11 @@ async fn run_with_watch( WatcherRestartMode::Automatic, move |flags, watcher_communicator, _changed_paths| { Ok(async move { - let factory = CliFactoryBuilder::new() - .build_from_flags_for_watcher(flags, watcher_communicator.clone())?; - let cli_options = factory.cli_options(); + let factory = CliFactory::from_flags_for_watcher( + flags, + watcher_communicator.clone(), + ); + let cli_options = factory.cli_options()?; let main_module = cli_options.resolve_main_module()?; maybe_npm_install(&factory).await?; @@ -156,11 +158,11 @@ async fn run_with_watch( } pub async fn eval_command( - flags: Flags, + flags: Arc, eval_flags: EvalFlags, ) -> Result { - let factory = CliFactory::from_flags(flags)?; - let cli_options = factory.cli_options(); + let factory = CliFactory::from_flags(flags); + let cli_options = factory.cli_options()?; let file_fetcher = factory.file_fetcher()?; let main_module = cli_options.resolve_main_module()?; @@ -195,7 +197,7 @@ pub async fn eval_command( async fn maybe_npm_install(factory: &CliFactory) -> Result<(), AnyError> { // ensure an "npm install" is done if the user has explicitly // opted into using a managed node_modules directory - if factory.cli_options().node_modules_dir_enablement() == Some(true) { + if factory.cli_options()?.node_modules_dir_enablement() == Some(true) { if let Some(npm_resolver) = factory.npm_resolver().await?.as_managed() { npm_resolver.ensure_top_level_package_json_install().await?; } diff --git a/cli/tools/task.rs b/cli/tools/task.rs index d2e16fb121..f296a070fb 100644 --- a/cli/tools/task.rs +++ b/cli/tools/task.rs @@ -26,11 +26,11 @@ use std::rc::Rc; use std::sync::Arc; pub async fn execute_script( - flags: Flags, + flags: Arc, task_flags: TaskFlags, ) -> Result { - let factory = CliFactory::from_flags(flags)?; - let cli_options = factory.cli_options(); + let factory = CliFactory::from_flags(flags); + let cli_options = factory.cli_options()?; let start_dir = &cli_options.start_dir; if !start_dir.has_deno_or_pkg_json() { bail!("deno task couldn't find deno.json(c). See https://deno.land/manual@v{}/getting_started/configuration_file", env!("CARGO_PKG_VERSION")) diff --git a/cli/tools/test/mod.rs b/cli/tools/test/mod.rs index 587b737d63..904b9ecb7b 100644 --- a/cli/tools/test/mod.rs +++ b/cli/tools/test/mod.rs @@ -7,7 +7,6 @@ use crate::args::TestReporterConfig; use crate::colors; use crate::display; use crate::factory::CliFactory; -use crate::factory::CliFactoryBuilder; use crate::file_fetcher::File; use crate::file_fetcher::FileFetcher; use crate::graph_container::MainModuleGraphContainer; @@ -1738,11 +1737,11 @@ async fn fetch_specifiers_with_test_mode( } pub async fn run_tests( - flags: Flags, + flags: Arc, test_flags: TestFlags, ) -> Result<(), AnyError> { - let factory = CliFactory::from_flags(flags)?; - let cli_options = factory.cli_options(); + let factory = CliFactory::from_flags(flags); + let cli_options = factory.cli_options()?; let workspace_test_options = cli_options.resolve_workspace_test_options(&test_flags); let file_fetcher = factory.file_fetcher()?; @@ -1821,7 +1820,7 @@ pub async fn run_tests( } pub async fn run_tests_with_watch( - flags: Flags, + flags: Arc, test_flags: TestFlags, ) -> Result<(), AnyError> { // On top of the sigint handlers which are added and unbound for each test @@ -1850,9 +1849,11 @@ pub async fn run_tests_with_watch( move |flags, watcher_communicator, changed_paths| { let test_flags = test_flags.clone(); Ok(async move { - let factory = CliFactoryBuilder::new() - .build_from_flags_for_watcher(flags, watcher_communicator.clone())?; - let cli_options = factory.cli_options(); + let factory = CliFactory::from_flags_for_watcher( + flags, + watcher_communicator.clone(), + ); + let cli_options = factory.cli_options()?; let workspace_test_options = cli_options.resolve_workspace_test_options(&test_flags); diff --git a/cli/tools/upgrade.rs b/cli/tools/upgrade.rs index fd8394efa9..8349f060cf 100644 --- a/cli/tools/upgrade.rs +++ b/cli/tools/upgrade.rs @@ -371,15 +371,20 @@ async fn fetch_and_store_latest_version< } pub async fn upgrade( - flags: Flags, + flags: Arc, upgrade_flags: UpgradeFlags, ) -> Result<(), AnyError> { - let factory = CliFactory::from_flags(flags)?; + let factory = CliFactory::from_flags(flags); let client = factory.http_client_provider().get_or_create()?; let current_exe_path = std::env::current_exe()?; - let full_path_output_flag = upgrade_flags - .output - .map(|output| factory.cli_options().initial_cwd().join(output)); + let full_path_output_flag = match &upgrade_flags.output { + Some(output) => Some( + std::env::current_dir() + .context("failed getting cwd")? + .join(output), + ), + None => None, + }; let output_exe_path = full_path_output_flag.as_ref().unwrap_or(¤t_exe_path); diff --git a/cli/tools/vendor/mod.rs b/cli/tools/vendor/mod.rs index 73aada7cfe..e144523724 100644 --- a/cli/tools/vendor/mod.rs +++ b/cli/tools/vendor/mod.rs @@ -36,7 +36,7 @@ mod specifiers; mod test; pub async fn vendor( - flags: Flags, + flags: Arc, vendor_flags: VendorFlags, ) -> Result<(), AnyError> { log::info!( @@ -52,7 +52,7 @@ pub async fn vendor( validate_output_dir(&output_dir, &vendor_flags)?; validate_options(&mut cli_options, &output_dir)?; let factory = CliFactory::from_cli_options(Arc::new(cli_options)); - let cli_options = factory.cli_options(); + let cli_options = factory.cli_options()?; if cli_options.workspace().config_folders().len() > 1 { bail!("deno vendor is not supported in a workspace. Set `\"vendor\": true` in the workspace deno.json file instead"); } diff --git a/cli/util/file_watcher.rs b/cli/util/file_watcher.rs index 176ca43f04..51dde8404f 100644 --- a/cli/util/file_watcher.rs +++ b/cli/util/file_watcher.rs @@ -198,13 +198,13 @@ impl WatcherCommunicator { /// changes. For example, in the case where we would like to bundle, then `operation` would /// have the logic for it like bundling the code. pub async fn watch_func( - flags: Flags, + flags: Arc, print_config: PrintConfig, operation: O, ) -> Result<(), AnyError> where O: FnMut( - Flags, + Arc, Arc, Option>, ) -> Result, @@ -237,14 +237,14 @@ pub enum WatcherRestartMode { /// changes. For example, in the case where we would like to bundle, then `operation` would /// have the logic for it like bundling the code. pub async fn watch_recv( - mut flags: Flags, + mut flags: Arc, print_config: PrintConfig, restart_mode: WatcherRestartMode, mut operation: O, ) -> Result<(), AnyError> where O: FnMut( - Flags, + Arc, Arc, Option>, ) -> Result, @@ -321,7 +321,12 @@ where )?); // don't reload dependencies after the first run - flags.reload = false; + if flags.reload { + flags = Arc::new(Flags { + reload: false, + ..Arc::unwrap_or_clone(flags) + }); + } select! { _ = receiver_future => {}, diff --git a/tests/integration/upgrade_tests.rs b/tests/integration/upgrade_tests.rs index c016b61fcc..54408907e6 100644 --- a/tests/integration/upgrade_tests.rs +++ b/tests/integration/upgrade_tests.rs @@ -5,6 +5,7 @@ use std::process::Stdio; use std::time::Instant; use test_util as util; use test_util::TempDir; +use test_util::TestContext; use util::TestContextBuilder; // Warning: this test requires internet access. @@ -144,15 +145,14 @@ fn upgrade_with_out_in_tmpdir() { assert!(v.contains("1.11.5")); } -// Warning: this test requires internet access. -// TODO(#7412): reenable. test is flaky #[test] -#[ignore] fn upgrade_invalid_stable_version() { - let temp_dir = TempDir::new(); + let context = upgrade_context(); + let temp_dir = context.temp_dir(); let exe_path = temp_dir.path().join("deno"); util::deno_exe_path().copy(&exe_path); assert!(exe_path.exists()); + exe_path.mark_executable(); let output = Command::new(&exe_path) .arg("upgrade") .arg("--version") @@ -164,20 +164,19 @@ fn upgrade_invalid_stable_version() { .unwrap(); assert!(!output.status.success()); assert_eq!( - "error: Invalid semver passed\n", + "error: Invalid version passed\n", util::strip_ansi_codes(&String::from_utf8(output.stderr).unwrap()) ); } -// Warning: this test requires internet access. -// TODO(#7412): reenable. test is flaky #[test] -#[ignore] fn upgrade_invalid_canary_version() { - let temp_dir = TempDir::new(); + let context = upgrade_context(); + let temp_dir = context.temp_dir(); let exe_path = temp_dir.path().join("deno"); util::deno_exe_path().copy(&exe_path); assert!(exe_path.exists()); + exe_path.mark_executable(); let output = Command::new(&exe_path) .arg("upgrade") .arg("--canary") @@ -195,16 +194,41 @@ fn upgrade_invalid_canary_version() { ); } +#[test] +fn upgrade_invalid_lockfile() { + let context = upgrade_context(); + let temp_dir = context.temp_dir(); + temp_dir.write("deno.deno", r#"{ \"lock\": true }"#); + temp_dir.write( + "deno.lock", + r#"{ + "version": "invalid", +}"#, + ); + let exe_path = temp_dir.path().join("deno"); + util::deno_exe_path().copy(&exe_path); + assert!(exe_path.exists()); + exe_path.mark_executable(); + let output = Command::new(&exe_path) + .arg("upgrade") + .arg("--version") + .arg("foobar") + .stderr(Stdio::piped()) + .spawn() + .unwrap() + .wait_with_output() + .unwrap(); + assert!(!output.status.success()); + // should make it here instead of erroring on an invalid lockfile + assert_eq!( + "error: Invalid version passed\n", + util::strip_ansi_codes(&String::from_utf8(output.stderr).unwrap()) + ); +} + #[test] fn upgrade_prompt() { - let context = TestContextBuilder::new() - .use_http_server() - .use_temp_cwd() - .env( - "DENO_DONT_USE_INTERNAL_BASE_UPGRADE_URL", - "http://localhost:4545", - ) - .build(); + let context = upgrade_context(); let temp_dir = context.temp_dir(); // start a task that goes indefinitely in order to allow // the upgrade check to occur @@ -257,3 +281,14 @@ fn upgrade_lsp_repl_sleeps() { let elapsed_secs = start_instant.elapsed().as_secs(); assert!(elapsed_secs < 94, "elapsed_secs: {}", elapsed_secs); } + +fn upgrade_context() -> TestContext { + TestContextBuilder::new() + .use_http_server() + .use_temp_cwd() + .env( + "DENO_DONT_USE_INTERNAL_BASE_UPGRADE_URL", + "http://localhost:4545", + ) + .build() +} diff --git a/tests/util/server/src/fs.rs b/tests/util/server/src/fs.rs index 558d25ffbb..fc27e44859 100644 --- a/tests/util/server/src/fs.rs +++ b/tests/util/server/src/fs.rs @@ -255,6 +255,13 @@ impl PathRef { } } + #[track_caller] + pub fn mark_executable(&self) { + if cfg!(unix) { + Command::new("chmod").arg("+x").arg(self).output().unwrap(); + } + } + #[track_caller] pub fn make_dir_readonly(&self) { self.create_dir_all();