From 1923062841bfc63afb4bd34486b45d45a0fc6f89 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Sun, 18 Aug 2024 19:54:10 -0400 Subject: [PATCH 1/2] fix(compile): make output more deterministic --- Cargo.lock | 6 ++-- Cargo.toml | 3 ++ cli/mainrt.rs | 3 +- cli/standalone/binary.rs | 24 ++++++++++------ cli/standalone/virtual_fs.rs | 8 ++++-- .../compile/indeterminism/__test__.jsonc | 28 +++++++++++++++++++ .../compile/indeterminism/assert_equal.ts | 15 ++++++++++ tests/specs/compile/indeterminism/main.ts | 4 +++ 8 files changed, 75 insertions(+), 16 deletions(-) create mode 100644 tests/specs/compile/indeterminism/__test__.jsonc create mode 100644 tests/specs/compile/indeterminism/assert_equal.ts create mode 100644 tests/specs/compile/indeterminism/main.ts diff --git a/Cargo.lock b/Cargo.lock index c08fb45a92f0ab..745fdce9112111 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2779,8 +2779,6 @@ dependencies = [ [[package]] name = "eszip" version = "0.73.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e1de63c5d16c099e1164154ba0c6fd1f54e1147ef635aaccc47702f8a442392a" dependencies = [ "anyhow", "async-trait", @@ -8137,7 +8135,7 @@ dependencies = [ "log", "naga", "once_cell", - "parking_lot 0.12.3", + "parking_lot 0.11.2", "profiling", "raw-window-handle", "ron", @@ -8179,7 +8177,7 @@ dependencies = [ "ndk-sys", "objc", "once_cell", - "parking_lot 0.12.3", + "parking_lot 0.11.2", "profiling", "range-alloc", "raw-window-handle", diff --git a/Cargo.toml b/Cargo.toml index 3032b40b5f58ec..72a7812b0c232b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -394,3 +394,6 @@ opt-level = 3 opt-level = 3 [profile.release.package.zstd-sys] opt-level = 3 + +[patch.crates-io] +eszip = { path = "../eszip" } diff --git a/cli/mainrt.rs b/cli/mainrt.rs index 9bf31895325f58..fd03224bed256f 100644 --- a/cli/mainrt.rs +++ b/cli/mainrt.rs @@ -31,6 +31,7 @@ use deno_runtime::fmt_errors::format_js_error; use deno_runtime::tokio_util::create_and_run_current_thread_with_maybe_metrics; pub use deno_runtime::UNSTABLE_GRANULAR_FLAGS; use deno_terminal::colors; +use indexmap::IndexMap; use std::borrow::Cow; use std::collections::HashMap; @@ -73,7 +74,7 @@ fn unwrap_or_exit(result: Result) -> T { } } -fn load_env_vars(env_vars: &HashMap) { +fn load_env_vars(env_vars: &IndexMap) { env_vars.iter().for_each(|env_var| { if env::var(env_var.0).is_err() { std::env::set_var(env_var.0, env_var.1); diff --git a/cli/standalone/binary.rs b/cli/standalone/binary.rs index 168310efd31d13..d80f8a969d8bd8 100644 --- a/cli/standalone/binary.rs +++ b/cli/standalone/binary.rs @@ -100,6 +100,8 @@ pub struct SerializedWorkspaceResolver { pub pkg_json_resolution: PackageJsonDepResolution, } +// Note: Don't use hashmaps/hashsets. Ensure the serialization +// is deterministic. #[derive(Deserialize, Serialize)] pub struct Metadata { pub argv: Vec, @@ -111,7 +113,7 @@ pub struct Metadata { pub ca_stores: Option>, pub ca_data: Option>, pub unsafely_ignore_certificate_errors: Option>, - pub env_vars_from_env_file: HashMap, + pub env_vars_from_env_file: IndexMap, pub workspace_resolver: SerializedWorkspaceResolver, pub entrypoint_key: String, pub node_modules: Option, @@ -667,8 +669,10 @@ impl<'a> DenoCompileBinaryWriter<'a> { // but also don't make this dependent on the registry url let root_path = npm_resolver.global_cache_root_folder(); let mut builder = VfsBuilder::new(root_path)?; - for package in npm_resolver.all_system_packages(&self.npm_system_info) - { + let mut packages = + npm_resolver.all_system_packages(&self.npm_system_info); + packages.sort_by(|a, b| a.id.cmp(&b.id)); // determinism + for package in packages { let folder = npm_resolver.resolve_pkg_folder_from_pkg_id(&package.id)?; builder.add_dir_recursive(&folder)?; @@ -729,11 +733,13 @@ impl<'a> DenoCompileBinaryWriter<'a> { cli_options.workspace().root_dir().to_file_path().unwrap(), ); while let Some(pending_dir) = pending_dirs.pop_front() { - let entries = fs::read_dir(&pending_dir).with_context(|| { - format!("Failed reading: {}", pending_dir.display()) - })?; + let mut entries = fs::read_dir(&pending_dir) + .with_context(|| { + format!("Failed reading: {}", pending_dir.display()) + })? + .collect::, _>>()?; + entries.sort_by_cached_key(|entry| entry.file_name()); // determinism for entry in entries { - let entry = entry?; let path = entry.path(); if !path.is_dir() { continue; @@ -755,8 +761,8 @@ impl<'a> DenoCompileBinaryWriter<'a> { /// in the passed environment file. fn get_file_env_vars( filename: String, -) -> Result, dotenvy::Error> { - let mut file_env_vars = HashMap::new(); +) -> Result, dotenvy::Error> { + let mut file_env_vars = IndexMap::new(); for item in dotenvy::from_filename_iter(filename)? { let Ok((key, val)) = item else { continue; // this failure will be warned about on load diff --git a/cli/standalone/virtual_fs.rs b/cli/standalone/virtual_fs.rs index c44e2227babbfb..7e37bf95e8f4d2 100644 --- a/cli/standalone/virtual_fs.rs +++ b/cli/standalone/virtual_fs.rs @@ -90,8 +90,12 @@ impl VfsBuilder { let read_dir = std::fs::read_dir(path) .with_context(|| format!("Reading {}", path.display()))?; - for entry in read_dir { - let entry = entry?; + // create some determinism + let mut dir_entries = + read_dir.into_iter().collect::, _>>()?; + dir_entries.sort_by_key(|entry| entry.file_name()); + + for entry in dir_entries { let file_type = entry.file_type()?; let path = entry.path(); diff --git a/tests/specs/compile/indeterminism/__test__.jsonc b/tests/specs/compile/indeterminism/__test__.jsonc new file mode 100644 index 00000000000000..97045744f1e619 --- /dev/null +++ b/tests/specs/compile/indeterminism/__test__.jsonc @@ -0,0 +1,28 @@ +{ + "tempDir": true, + "steps": [{ + "if": "unix", + "args": "compile --output main1 main.ts", + "output": "[WILDCARD]" + }, { + "if": "unix", + "args": "compile --output main2 main.ts", + "output": "[WILDCARD]" + }, { + "if": "unix", + "args": "run --allow-read=. assert_equal.ts main1 main2", + "output": "Same\n" + }, { + "if": "windows", + "args": "compile --output main1.exe main.ts", + "output": "[WILDCARD]" + }, { + "if": "windows", + "args": "compile --output main2.exe main.ts", + "output": "[WILDCARD]" + }, { + "if": "windows", + "args": "run --allow-read=. assert_equal.ts main1.exe main2.exe", + "output": "Same\n" + }] +} diff --git a/tests/specs/compile/indeterminism/assert_equal.ts b/tests/specs/compile/indeterminism/assert_equal.ts new file mode 100644 index 00000000000000..c99bdf16c9d1d3 --- /dev/null +++ b/tests/specs/compile/indeterminism/assert_equal.ts @@ -0,0 +1,15 @@ +const file1 = Deno.readFileSync(Deno.args[0]); +const file2 = Deno.readFileSync(Deno.args[1]); + +if (file1.length !== file2.length) { + console.error("File lengths are different"); + Deno.exit(1); +} +for (let i = 0; i < file1.length; i++) { + if (file1[i] !== file2[i]) { + console.error("Files are different"); + Deno.exit(1); + } +} + +console.error("Same"); diff --git a/tests/specs/compile/indeterminism/main.ts b/tests/specs/compile/indeterminism/main.ts new file mode 100644 index 00000000000000..602388bb79fc54 --- /dev/null +++ b/tests/specs/compile/indeterminism/main.ts @@ -0,0 +1,4 @@ +import "npm:@denotest/esm-basic"; +import "npm:@denotest/add"; +import "npm:@denotest/subtract"; +import "npm:preact"; From 975dd7d9c50013192d2d8f2937f2ed665e75a23e Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 19 Aug 2024 11:28:56 -0400 Subject: [PATCH 2/2] update --- Cargo.lock | 27 ++++--------------- Cargo.toml | 3 --- cli/Cargo.toml | 4 +-- .../__test__.jsonc | 0 .../assert_equal.ts | 0 .../{indeterminism => determinism}/main.ts | 0 6 files changed, 7 insertions(+), 27 deletions(-) rename tests/specs/compile/{indeterminism => determinism}/__test__.jsonc (100%) rename tests/specs/compile/{indeterminism => determinism}/assert_equal.ts (100%) rename tests/specs/compile/{indeterminism => determinism}/main.ts (100%) diff --git a/Cargo.lock b/Cargo.lock index bdc8c4ddbaafd8..2582de2e27118b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1170,7 +1170,7 @@ dependencies = [ "deno_graph", "deno_lint", "deno_lockfile", - "deno_npm 0.22.0", + "deno_npm", "deno_package_json", "deno_runtime", "deno_semver", @@ -1867,25 +1867,6 @@ dependencies = [ "yoke", ] -[[package]] -name = "deno_npm" -version = "0.22.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "01fd279be9f522ebfaabce7ff0f4b069c2c1d737946ba57456b579f9246669e8" -dependencies = [ - "anyhow", - "async-trait", - "deno_lockfile", - "deno_semver", - "futures", - "log", - "monch", - "serde", - "serde_json", - "thiserror", - "url", -] - [[package]] name = "deno_npm" version = "0.23.0" @@ -2797,14 +2778,16 @@ dependencies = [ [[package]] name = "eszip" -version = "0.74.0" +version = "0.75.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "67ab3253e0e6a99b79ffc6d7a6243ad541f54543768cecd2198b64476a5971f8" dependencies = [ "anyhow", "async-trait", "base64 0.21.7", "deno_ast", "deno_graph", - "deno_npm 0.23.0", + "deno_npm", "deno_semver", "futures", "hashlink", diff --git a/Cargo.toml b/Cargo.toml index 8e2fab84c9d6fc..1176656e205e60 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -394,6 +394,3 @@ opt-level = 3 opt-level = 3 [profile.release.package.zstd-sys] opt-level = 3 - -[patch.crates-io] -eszip = { path = "../eszip" } diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 97f21deb2e2400..da4a701623a81c 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -72,13 +72,13 @@ deno_emit = "=0.44.0" deno_graph = { version = "=0.81.2" } deno_lint = { version = "=0.63.1", features = ["docs"] } deno_lockfile.workspace = true -deno_npm = "=0.22.0" +deno_npm = "=0.23.0" deno_package_json.workspace = true deno_runtime = { workspace = true, features = ["include_js_files_for_snapshotting"] } deno_semver = "=0.5.10" deno_task_shell = "=0.17.0" deno_terminal.workspace = true -eszip = "=0.74.0" +eszip = "=0.75.0" libsui = "0.3.0" napi_sym.workspace = true node_resolver.workspace = true diff --git a/tests/specs/compile/indeterminism/__test__.jsonc b/tests/specs/compile/determinism/__test__.jsonc similarity index 100% rename from tests/specs/compile/indeterminism/__test__.jsonc rename to tests/specs/compile/determinism/__test__.jsonc diff --git a/tests/specs/compile/indeterminism/assert_equal.ts b/tests/specs/compile/determinism/assert_equal.ts similarity index 100% rename from tests/specs/compile/indeterminism/assert_equal.ts rename to tests/specs/compile/determinism/assert_equal.ts diff --git a/tests/specs/compile/indeterminism/main.ts b/tests/specs/compile/determinism/main.ts similarity index 100% rename from tests/specs/compile/indeterminism/main.ts rename to tests/specs/compile/determinism/main.ts