From bc92f872988fd8b9cdf2ae1479278789911237a5 Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Mon, 22 Jan 2024 12:37:25 +0000 Subject: [PATCH] fix(runtime): only discard extension sources if a snapshot is provided (#22023) Fixes #21928. We have a code path which empties the extension sources because they're expected to be pre-executed in the snapshot. Instead of using conditional compilation for that, we now just check if a snapshot was provided. Removes the `dont_use_runtime_snapshot` feature. We didn't allow not providing a snapshot unless this feature was provided, now we always do. Adds the `only_snapshotted_js_sources` feature for us to use in CLI. This asserts that a snapshot is provided and gates the runtime transpilation code so it isn't included in the executable. --- cli/Cargo.toml | 2 +- runtime/Cargo.toml | 8 +++++--- runtime/web_worker.rs | 21 +++++++-------------- runtime/worker.rs | 36 +++++++++++++----------------------- 4 files changed, 26 insertions(+), 41 deletions(-) diff --git a/cli/Cargo.toml b/cli/Cargo.toml index e42b229471..16b561c05d 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -39,7 +39,7 @@ __runtime_js_sources = ["deno_runtime/__runtime_js_sources"] __vendored_zlib_ng = ["flate2/zlib-ng-compat", "libz-sys/zlib-ng"] [build-dependencies] -deno_runtime = { workspace = true, features = ["include_js_files_for_snapshotting"] } +deno_runtime = { workspace = true, features = ["include_js_files_for_snapshotting", "only_snapshotted_js_sources"] } deno_core = { workspace = true, features = ["include_js_files_for_snapshotting"] } lazy-regex.workspace = true serde.workspace = true diff --git a/runtime/Cargo.toml b/runtime/Cargo.toml index 0b07839b29..c3bcfb9bf7 100644 --- a/runtime/Cargo.toml +++ b/runtime/Cargo.toml @@ -12,8 +12,6 @@ description = "Provides the deno runtime library" [features] # "fake" feature that allows to generate docs on docs.rs docsrs = [] -# A feature that disables the requirement for startup snapshot to be provided. -dont_use_runtime_snapshot = [] # A feature that allows excluding `./js/99_main.js` from the exported extension. exclude_runtime_main_js = [] # A feature that disables embedding of the JavaScript source files in the binary. @@ -24,7 +22,11 @@ include_js_files_for_snapshotting = [ ] # A dev feature to disable creations and loading of snapshots in favor of # loading JS sources at runtime. -__runtime_js_sources = [] +__runtime_js_sources = ["include_js_files_for_snapshotting"] +# Signal that only snapshotted JS sources should be used. This will +# conditionally exclude the runtime source transpilation logic, and add an +# assertion that a snapshot is provided. +only_snapshotted_js_sources = ["include_js_files_for_snapshotting"] [lib] name = "deno_runtime" diff --git a/runtime/web_worker.rs b/runtime/web_worker.rs index 2b6eb19c91..78674af021 100644 --- a/runtime/web_worker.rs +++ b/runtime/web_worker.rs @@ -490,14 +490,16 @@ impl WebWorker { ops::web_worker::deno_web_worker::init_ops_and_esm(), ]; + #[cfg(__runtime_js_sources)] + assert!(cfg!(not(feature = "only_snapshotted_js_sources")), "'__runtime_js_sources' is incompatible with 'only_snapshotted_js_sources'."); + for extension in &mut extensions { - #[cfg(not(feature = "__runtime_js_sources"))] - { + if options.startup_snapshot.is_some() { extension.js_files = std::borrow::Cow::Borrowed(&[]); extension.esm_files = std::borrow::Cow::Borrowed(&[]); extension.esm_entry_point = None; } - #[cfg(feature = "__runtime_js_sources")] + #[cfg(not(feature = "only_snapshotted_js_sources"))] { use crate::shared::maybe_transpile_source; for source in extension.esm_files.to_mut() { @@ -511,17 +513,8 @@ impl WebWorker { extensions.extend(std::mem::take(&mut options.extensions)); - #[cfg(all( - feature = "include_js_files_for_snapshotting", - not(feature = "__runtime_js_sources") - ))] - options - .startup_snapshot - .as_ref() - .expect("Sources are not embedded and a user snapshot was not provided."); - - #[cfg(not(feature = "dont_use_runtime_snapshot"))] - options.startup_snapshot.as_ref().expect("A user snapshot was not provided, if you want to create a runtime without a snapshot use 'dont_use_runtime_snapshot' Cargo feature."); + #[cfg(feature = "only_snapshotted_js_sources")] + options.startup_snapshot.as_ref().expect("A user snapshot was not provided, even though 'only_snapshotted_js_sources' is used."); // Hook up the summary metrics if the user or subcommand requested them let (op_summary_metrics, op_metrics_factory_fn) = diff --git a/runtime/worker.rs b/runtime/worker.rs index 5dc5db71d6..e6da93d786 100644 --- a/runtime/worker.rs +++ b/runtime/worker.rs @@ -418,16 +418,13 @@ impl MainWorker { ops::signal::deno_signal::init_ops_and_esm(), ops::tty::deno_tty::init_ops_and_esm(), ops::http::deno_http_runtime::init_ops_and_esm(), - ops::bootstrap::deno_bootstrap::init_ops_and_esm({ - #[cfg(feature = "__runtime_js_sources")] - { - Some(Default::default()) - } - #[cfg(not(feature = "__runtime_js_sources"))] - { + ops::bootstrap::deno_bootstrap::init_ops_and_esm( + if options.startup_snapshot.is_some() { None - } - }), + } else { + Some(Default::default()) + }, + ), deno_permissions_worker::init_ops_and_esm( permissions, enable_testing_features, @@ -435,14 +432,16 @@ impl MainWorker { runtime::init_ops_and_esm(), ]; + #[cfg(__runtime_js_sources)] + assert!(cfg!(not(feature = "only_snapshotted_js_sources")), "'__runtime_js_sources' is incompatible with 'only_snapshotted_js_sources'."); + for extension in &mut extensions { - #[cfg(not(feature = "__runtime_js_sources"))] - { + if options.startup_snapshot.is_some() { extension.js_files = std::borrow::Cow::Borrowed(&[]); extension.esm_files = std::borrow::Cow::Borrowed(&[]); extension.esm_entry_point = None; } - #[cfg(feature = "__runtime_js_sources")] + #[cfg(not(feature = "only_snapshotted_js_sources"))] { use crate::shared::maybe_transpile_source; for source in extension.esm_files.to_mut() { @@ -456,17 +455,8 @@ impl MainWorker { extensions.extend(std::mem::take(&mut options.extensions)); - #[cfg(all( - feature = "include_js_files_for_snapshotting", - not(feature = "__runtime_js_sources") - ))] - options - .startup_snapshot - .as_ref() - .expect("Sources are not embedded and a user snapshot was not provided."); - - #[cfg(not(feature = "dont_use_runtime_snapshot"))] - options.startup_snapshot.as_ref().expect("A user snapshot was not provided, if you want to create a runtime without a snapshot use 'dont_use_runtime_snapshot' Cargo feature."); + #[cfg(feature = "only_snapshotted_js_sources")] + options.startup_snapshot.as_ref().expect("A user snapshot was not provided, even though 'only_snapshotted_js_sources' is used."); let has_notified_of_inspector_disconnect = AtomicBool::new(false); let wait_for_inspector_disconnect_callback = Box::new(move || {