From 66ea301fc25a07485286560c434a9fdaf460c431 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sun, 1 Nov 2020 16:10:00 +0000 Subject: Untangle caching code --- dhall/tests/spec.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'dhall/tests') diff --git a/dhall/tests/spec.rs b/dhall/tests/spec.rs index 8d67892..ae53f4a 100644 --- a/dhall/tests/spec.rs +++ b/dhall/tests/spec.rs @@ -670,7 +670,7 @@ fn run_test(test: &SpecTest) -> Result<()> { } SemanticHash => { let expr = expr.normalize()?.to_expr_alpha(); - let hash = hex::encode(expr.hash()?); + let hash = hex::encode(expr.sha256_hash()?); expected.compare_ui(format!("sha256:{}", hash))?; } TypeInferenceSuccess => { -- cgit v1.3.1 From 9b67852327aff5263a3c119c068170b2edef69f8 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sun, 1 Nov 2020 18:00:07 +0000 Subject: Don't pollute the dhall cache directory --- Cargo.lock | 7 +++++ dhall/Cargo.toml | 4 +-- dhall/tests/spec.rs | 80 +++++++++++++++++++++++++++++++---------------------- 3 files changed, 55 insertions(+), 36 deletions(-) (limited to 'dhall/tests') diff --git a/Cargo.lock b/Cargo.lock index 63522c2..503f711 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -300,6 +300,7 @@ dependencies = [ "annotate-snippets", "anyhow", "colored-diff", + "fs_extra", "hex", "itertools 0.9.0", "lazy_static", @@ -408,6 +409,12 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "00b0228411908ca8685dba7fc2cdd70ec9990a6e753e89b6ac91a84c40fbaf4b" +[[package]] +name = "fs_extra" +version = "1.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2022715d62ab30faffd124d40b76f4134a550a87792276512b18d63272333394" + [[package]] name = "fuchsia-zircon" version = "0.3.3" diff --git a/dhall/Cargo.toml b/dhall/Cargo.toml index 7923009..207539a 100644 --- a/dhall/Cargo.toml +++ b/dhall/Cargo.toml @@ -41,15 +41,13 @@ reqwest = { version = "0.10", features = ["blocking"], optional = true } [dev-dependencies] anyhow = "1.0.28" colored-diff = "0.2.2" +fs_extra = "1.2.0" libtest-mimic = "0.3.0" rand = "0.7" version-sync = "0.9" walkdir = "2" [build-dependencies] -walkdir = "2" abnf_to_pest = { version = "^0.5.0", path = "../abnf_to_pest" } pest_generator = "2.1.3" quote = "1.0" - - diff --git a/dhall/tests/spec.rs b/dhall/tests/spec.rs index ae53f4a..36cbd81 100644 --- a/dhall/tests/spec.rs +++ b/dhall/tests/spec.rs @@ -1,4 +1,6 @@ use anyhow::Result; +use rand::distributions::Alphanumeric; +use rand::Rng; use std::env; use std::ffi::OsString; use std::fmt::{Debug, Display}; @@ -593,26 +595,6 @@ fn unwrap_err(x: Result) -> Result { fn run_test(test: &SpecTest) -> Result<()> { use self::SpecTestKind::*; - // Setup current directory to the root of the repository. Important for `as Location` tests. - let root_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")) - .parent() - .unwrap() - .to_path_buf(); - env::set_current_dir(root_dir.as_path())?; - // Set environment variable for import tests. - env::set_var("DHALL_TEST_VAR", "6 * 7"); - - // Configure cache for import tests - env::set_var( - "XDG_CACHE_HOME", - root_dir - .join("dhall-lang") - .join("tests") - .join("import") - .join("cache") - .as_path(), - ); - let SpecTest { input: expr, output: expected, @@ -690,7 +672,6 @@ fn run_test(test: &SpecTest) -> Result<()> { expected.compare(expr)?; } } - Ok(()) } @@ -700,17 +681,50 @@ fn main() { .flat_map(discover_tests_for_feature) .collect(); - libtest_mimic::run_tests(&Arguments::from_args(), tests, |test| { - let result = std::panic::catch_unwind(move || { - run_test_stringy_error(&test.data) + // Setup current directory to the root of the repository. Important for `as Location` tests. + let root_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .parent() + .unwrap() + .to_path_buf(); + env::set_current_dir(root_dir.as_path()).unwrap(); + + // Set environment variable for import tests. + env::set_var("DHALL_TEST_VAR", "6 * 7"); + + // Configure cache for import tests + let dhall_cache_dir = root_dir + .join("dhall-lang") + .join("tests") + .join("import") + .join("cache") + .join("dhall"); + let random_id = rand::thread_rng() + .sample_iter(Alphanumeric) + .take(36) + .collect::(); + let cache_dir = format!("dhall-tests-{}", random_id); + let cache_dir = env::temp_dir().join(cache_dir); + + std::fs::create_dir_all(&cache_dir).unwrap(); + fs_extra::dir::copy(&dhall_cache_dir, &cache_dir, &Default::default()) + .unwrap(); + env::set_var("XDG_CACHE_HOME", &cache_dir); + + let res = + libtest_mimic::run_tests(&Arguments::from_args(), tests, |test| { + let result = std::panic::catch_unwind(move || { + run_test_stringy_error(&test.data) + }); + match result { + Ok(Ok(_)) => Outcome::Passed, + Ok(Err(e)) => Outcome::Failed { msg: Some(e) }, + Err(_) => Outcome::Failed { + msg: Some("thread panicked".to_string()), + }, + } }); - match result { - Ok(Ok(_)) => Outcome::Passed, - Ok(Err(e)) => Outcome::Failed { msg: Some(e) }, - Err(_) => Outcome::Failed { - msg: Some("thread panicked".to_string()), - }, - } - }) - .exit(); + + std::fs::remove_dir_all(&cache_dir).unwrap(); + + res.exit(); } -- cgit v1.3.1 From e5381c9b76f1d88dedb4a453cd026c8e98be5533 Mon Sep 17 00:00:00 2001 From: Nadrieril Date: Sun, 1 Nov 2020 18:32:33 +0000 Subject: Ensure that the hash always gets checked --- dhall/src/semantics/resolve/env.rs | 31 +++++++++++++--------- dhall/src/semantics/resolve/resolve.rs | 28 ++++++++++++++----- dhall/tests/import/data/simple.dhall | 1 + .../tests/import/failure/unit/HashMismatch2.dhall | 2 ++ dhall/tests/import/failure/unit/HashMismatch2.txt | 9 +++++++ 5 files changed, 52 insertions(+), 19 deletions(-) create mode 100644 dhall/tests/import/data/simple.dhall create mode 100644 dhall/tests/import/failure/unit/HashMismatch2.dhall create mode 100644 dhall/tests/import/failure/unit/HashMismatch2.txt (limited to 'dhall/tests') diff --git a/dhall/src/semantics/resolve/env.rs b/dhall/src/semantics/resolve/env.rs index a24c274..29dd16b 100644 --- a/dhall/src/semantics/resolve/env.rs +++ b/dhall/src/semantics/resolve/env.rs @@ -17,7 +17,7 @@ pub type CyclesStack = Vec; /// Environment for resolving imports #[derive(Debug)] pub struct ImportEnv { - file_cache: Option, + disk_cache: Option, // Missing if it failed to initialize mem_cache: MemCache, stack: CyclesStack, } @@ -69,37 +69,42 @@ impl NameEnv { impl ImportEnv { pub fn new() -> Self { ImportEnv { - file_cache: Cache::new().ok(), + disk_cache: Cache::new().ok(), mem_cache: Default::default(), stack: Default::default(), } } - pub fn get_from_cache( + pub fn get_from_mem_cache( &mut self, location: &ImportLocation, - hash: Option<&Hash>, ) -> Option { - if let Some(expr) = self.mem_cache.get(location) { - return Some(expr.clone()); - } + Some(self.mem_cache.get(location)?.clone()) + } + + pub fn get_from_disk_cache( + &mut self, + hash: &Option, + ) -> Option { let hash = hash.as_ref()?; - let expr = self.file_cache.as_ref()?.get(hash).ok()?; + let expr = self.disk_cache.as_ref()?.get(hash).ok()?; Some(expr) } - pub fn set_cache( + pub fn write_to_mem_cache( &mut self, location: ImportLocation, - hash: Option<&Hash>, expr: Typed, ) { - if let Some(file_cache) = self.file_cache.as_ref() { + self.mem_cache.insert(location, expr); + } + + pub fn write_to_disk_cache(&mut self, hash: &Option, expr: &Typed) { + if let Some(disk_cache) = self.disk_cache.as_ref() { if let Some(hash) = hash { - let _ = file_cache.insert(hash, &expr); + let _ = disk_cache.insert(hash, &expr); } } - self.mem_cache.insert(location, expr); } pub fn with_cycle_detection( diff --git a/dhall/src/semantics/resolve/resolve.rs b/dhall/src/semantics/resolve/resolve.rs index 002490b..614ea22 100644 --- a/dhall/src/semantics/resolve/resolve.rs +++ b/dhall/src/semantics/resolve/resolve.rs @@ -390,19 +390,35 @@ fn resolve_with_env( let do_sanity_check = import.mode != ImportMode::Location; let location = base_location.chain(&import.location, do_sanity_check)?; + // If the import is in the in-memory cache, or the hash is in the on-disk cache, return // the cached contents. - if let Some(resolved) = - env.get_from_cache(&location, import.hash.as_ref()) - { - return Ok(resolved); + if let Some(typed) = env.get_from_mem_cache(&location) { + // The same location may be used with different or no hashes. Thus we need to check + // the hashes every time. + check_hash(&import, &typed, span)?; + env.write_to_disk_cache(&import.hash, &typed); + return Ok(typed); + } + if let Some(typed) = env.get_from_disk_cache(&import.hash) { + // No need to check the hash, it was checked before reading the file. We also don't + // write to the in-memory cache, because the location might be completely unrelated + // to the cached file (e.g. `missing sha256:...` is valid). + // This actually means that importing many times a same hashed import will take + // longer than importing many times a same non-hashed import. + return Ok(typed); } + + // Resolve this import, making sure that recursive imports don't cycle back to the + // current one. let typed = env.with_cycle_detection(location.clone(), |env| { resolve_one_import(env, &import, location.clone(), span.clone()) })?; - check_hash(&import, &typed, span)?; + // Add the resolved import to the caches - env.set_cache(location, import.hash.as_ref(), typed.clone()); + check_hash(&import, &typed, span)?; + env.write_to_disk_cache(&import.hash, &typed); + env.write_to_mem_cache(location, typed.clone()); Ok(typed) }, )?; diff --git a/dhall/tests/import/data/simple.dhall b/dhall/tests/import/data/simple.dhall new file mode 100644 index 0000000..00750ed --- /dev/null +++ b/dhall/tests/import/data/simple.dhall @@ -0,0 +1 @@ +3 diff --git a/dhall/tests/import/failure/unit/HashMismatch2.dhall b/dhall/tests/import/failure/unit/HashMismatch2.dhall new file mode 100644 index 0000000..5fae772 --- /dev/null +++ b/dhall/tests/import/failure/unit/HashMismatch2.dhall @@ -0,0 +1,2 @@ +-- This ensures that even if the file gets imported without hash first, the hash check is not skipped later +../../data/simple.dhall + ../../data/simple.dhall sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + ../../data/simple.dhall diff --git a/dhall/tests/import/failure/unit/HashMismatch2.txt b/dhall/tests/import/failure/unit/HashMismatch2.txt new file mode 100644 index 0000000..f03ab24 --- /dev/null +++ b/dhall/tests/import/failure/unit/HashMismatch2.txt @@ -0,0 +1,9 @@ +Type error: error: hash mismatch + --> :2:27 + | +1 | -- This ensures that even if the file gets imported without hash first, the hash check is not skipped later +2 | ../../data/simple.dhall + ../../data/simple.dhall sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + ../../data/simple.dhall + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ hash mismatch + | + = note: Expected sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa + = note: Found sha256:15f52ecf91c94c1baac02d5a4964b2ed8fa401641a2c8a95e8306ec7c1e3b8d2 -- cgit v1.3.1