From e880cf7e611b877dc63aebf0b5a6a3caf27090a1 Mon Sep 17 00:00:00 2001 From: Axel Viala Date: Sat, 8 Jul 2023 21:04:35 +0200 Subject: [PATCH 01/11] Check env file already open with samefile. Samefile abstract away differences between Unixes and windows to check if two files lead to the same file on the underlying filesystem. Now it should follow through soft-links, hard-links and file moved. --- heed/Cargo.toml | 1 + heed/src/env.rs | 183 ++++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 172 insertions(+), 12 deletions(-) diff --git a/heed/Cargo.toml b/heed/Cargo.toml index f055e7cf..56dfbe8e 100644 --- a/heed/Cargo.toml +++ b/heed/Cargo.toml @@ -21,6 +21,7 @@ once_cell = "1.16.0" page_size = "0.5.0" serde = { version = "1.0.151", features = ["derive"], optional = true } synchronoise = "1.0.1" +same-file = "1.0.6" [dev-dependencies] serde = { version = "1.0.151", features = ["derive"] } diff --git a/heed/src/env.rs b/heed/src/env.rs index b589dc52..82c00a2f 100644 --- a/heed/src/env.rs +++ b/heed/src/env.rs @@ -19,6 +19,7 @@ use std::{ use std::{fmt, io, mem, ptr, sync}; use once_cell::sync::Lazy; +use same_file::Handle; use synchronoise::event::SignalEvent; use crate::mdb::error::mdb_result; @@ -32,7 +33,10 @@ use crate::{ /// noone tries to open the same environment between these two phases. /// /// Trying to open a None marked environment returns an error to the user trying to open it. -static OPENED_ENV: Lazy>> = Lazy::new(RwLock::default); +/// +/// [samefile::Handle] abstract the platform details of checking if the given paths points to +/// the same file on the filesystem. +static OPENED_ENV: Lazy>> = Lazy::new(RwLock::default); struct EnvEntry { env: Option, @@ -180,22 +184,29 @@ impl EnvOpenOptions { pub fn open>(&self, path: P) -> Result { let mut lock = OPENED_ENV.write().unwrap(); - let path = match canonicalize_path(path.as_ref()) { + let (path, handle) = match canonicalize_path(path.as_ref()) { Err(err) => { if err.kind() == NotFound && self.flags & (Flag::NoSubDir as u32) != 0 { let path = path.as_ref(); match path.parent().zip(path.file_name()) { - Some((dir, file_name)) => canonicalize_path(dir)?.join(file_name), + Some((dir, file_name)) => { + let handle = Handle::from_path(dir)?; + let path = canonicalize_path(dir)?.join(file_name); + (path, handle) + } None => return Err(err.into()), } } else { return Err(err.into()); } } - Ok(path) => path, + Ok(path) => { + let handle = Handle::from_path(&path)?; + (path, handle) + } }; - match lock.entry(path) { + match lock.entry(handle) { Entry::Occupied(entry) => { let env = entry.get().env.clone().ok_or(Error::DatabaseClosing)?; let options = entry.get().options.clone(); @@ -206,7 +217,6 @@ impl EnvOpenOptions { } } Entry::Vacant(entry) => { - let path = entry.key(); let path_str = CString::new(path.as_os_str().as_bytes()).unwrap(); unsafe { @@ -255,6 +265,7 @@ impl EnvOpenOptions { env, dbi_open_mutex: sync::Mutex::default(), path: path.clone(), + handle: Handle::from_path(path)?, }; let env = Env(Arc::new(inner)); let cache_entry = EnvEntry { @@ -277,9 +288,11 @@ impl EnvOpenOptions { } /// Returns a struct that allows to wait for the effective closing of an environment. -pub fn env_closing_event>(path: P) -> Option { +pub fn env_closing_event>(path: P) -> Result> { let lock = OPENED_ENV.read().unwrap(); - lock.get(path.as_ref()).map(|e| EnvClosingEvent(e.signal_event.clone())) + let handle = Handle::from_path(path)?; + + Ok(lock.get(&handle).map(|e| EnvClosingEvent(e.signal_event.clone()))) } /// An environment handle constructed by using [`EnvOpenOptions`]. @@ -288,7 +301,7 @@ pub struct Env(Arc); impl fmt::Debug for Env { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - let EnvInner { env: _, dbi_open_mutex: _, path } = self.0.as_ref(); + let EnvInner { env: _, dbi_open_mutex: _, path, .. } = self.0.as_ref(); f.debug_struct("Env").field("path", &path.display()).finish_non_exhaustive() } } @@ -297,6 +310,7 @@ struct EnvInner { env: *mut ffi::MDB_env, dbi_open_mutex: sync::Mutex>>, path: PathBuf, + handle: Handle, } unsafe impl Send for EnvInner {} @@ -307,7 +321,7 @@ impl Drop for EnvInner { fn drop(&mut self) { let mut lock = OPENED_ENV.write().unwrap(); - match lock.remove(&self.path) { + match lock.remove(&self.handle) { None => panic!("It seems another env closed this env before"), Some(EnvEntry { signal_event, .. }) => { unsafe { @@ -653,7 +667,7 @@ impl Env { /// when all references are dropped, the last one will eventually close the environment. pub fn prepare_for_closing(self) -> EnvClosingEvent { let mut lock = OPENED_ENV.write().unwrap(); - match lock.get_mut(self.path()) { + match lock.get_mut(&self.0.handle) { None => panic!("cannot find the env that we are trying to close"), Some(EnvEntry { env, signal_event, .. }) => { // We remove the env from the global list and replace it with a None. @@ -797,7 +811,7 @@ mod tests { eprintln!("env closed successfully"); // Make sure we don't have a reference to the env - assert!(env_closing_event(&dir.path()).is_none()); + assert!(env_closing_event(&dir.path()).unwrap().is_none()); } #[test] @@ -830,6 +844,75 @@ mod tests { .unwrap(); } + #[test] + fn open_env_with_named_path_hardlink_and_no_subdir() { + let dir = tempfile::tempdir().unwrap(); + let dir_symlink = tempfile::tempdir().unwrap(); + let env_name = dir.path().join("babar.mdb"); + let hardlink_name = dir_symlink.path().join("babar.mdb.link"); + + let mut envbuilder = EnvOpenOptions::new(); + unsafe { envbuilder.flag(crate::Flag::NoSubDir) }; + let _env = envbuilder + .map_size(10 * 1024 * 1024) // 10MB + .open(&env_name) + .unwrap(); + + std::os::unix::fs::symlink(&dir.path(), &hardlink_name).unwrap(); + let _env = envbuilder + .map_size(10 * 1024 * 1024) // 10MB + .open(&hardlink_name) + .unwrap(); + + let _env = envbuilder + .map_size(10 * 1024 * 1024) // 10MB + .open(&env_name) + .unwrap(); + } + + #[test] + #[cfg(unix)] + fn open_env_with_named_path_symlink() { + let dir = tempfile::tempdir().unwrap(); + let dir_symlink = tempfile::tempdir().unwrap(); + + let env_name = dir.path().join("babar.mdb"); + let symlink_name = dir_symlink.path().join("babar.mdb.link"); + fs::create_dir_all(&env_name).unwrap(); + + std::os::unix::fs::symlink(&env_name, &symlink_name).unwrap(); + let _env = EnvOpenOptions::new() + .map_size(10 * 1024 * 1024) // 10MB + .open(&symlink_name) + .unwrap(); + + let _env = EnvOpenOptions::new() + .map_size(10 * 1024 * 1024) // 10MB + .open(&env_name) + .unwrap(); + } + + #[test] + fn open_env_with_named_path_rename() { + let dir = tempfile::tempdir().unwrap(); + + let env_name = dir.path().join("babar.mdb"); + fs::create_dir_all(&env_name).unwrap(); + + let _env = EnvOpenOptions::new() + .map_size(10 * 1024 * 1024) // 10MB + .open(&env_name) + .unwrap(); + + let env_renamed = dir.path().join("serafina.mdb"); + std::fs::rename(&env_name, &env_renamed).unwrap(); + + let _env = EnvOpenOptions::new() + .map_size(10 * 1024 * 1024) // 10MB + .open(&env_renamed) + .unwrap(); + } + #[test] #[cfg(not(windows))] fn open_database_with_writemap_flag() { @@ -853,6 +936,82 @@ mod tests { let _env = envbuilder.open(&dir.path().join("data.mdb")).unwrap(); } + #[test] + #[cfg(unix)] + fn open_env_with_named_path_symlink_and_no_subdir() { + let dir = tempfile::tempdir().unwrap(); + let dir_symlink = tempfile::tempdir().unwrap(); + let env_name = dir.path().join("babar.mdb"); + let symlink_name = dir_symlink.path().join("babar.mdb.link"); + + let mut envbuilder = EnvOpenOptions::new(); + unsafe { envbuilder.flag(crate::Flag::NoSubDir) }; + let _env = envbuilder + .map_size(10 * 1024 * 1024) // 10MB + .open(&env_name) + .unwrap(); + + std::os::unix::fs::symlink(&dir.path(), &symlink_name).unwrap(); + let _env = envbuilder + .map_size(10 * 1024 * 1024) // 10MB + .open(&symlink_name) + .unwrap(); + + let _env = envbuilder + .map_size(10 * 1024 * 1024) // 10MB + .open(&env_name) + .unwrap(); + } + + #[test] + #[cfg(windows)] + fn open_env_with_named_path_symlinkfile_and_no_subdir() { + let dir = tempfile::tempdir().unwrap(); + let dir_symlink = tempfile::tempdir().unwrap(); + let env_name = dir.path().join("babar.mdb"); + let symlink_name = dir_symlink.path().join("babar.mdb.link"); + + let mut envbuilder = EnvOpenOptions::new(); + unsafe { envbuilder.flag(crate::Flag::NoSubDir) }; + let _env = envbuilder + .map_size(10 * 1024 * 1024) // 10MB + .open(&env_name) + .unwrap(); + + std::os::windows::fs::symlink_file(&dir.path(), &symlink_name).unwrap(); + let _env = envbuilder + .map_size(10 * 1024 * 1024) // 10MB + .open(&symlink_name) + .unwrap(); + + let _env = envbuilder + .map_size(10 * 1024 * 1024) // 10MB + .open(&env_name) + .unwrap(); + } + + #[test] + #[cfg(windows)] + fn open_env_with_named_path_symlink() { + let dir = tempfile::tempdir().unwrap(); + let dir_symlink = tempfile::tempdir().unwrap(); + + let env_name = dir.path().join("babar.mdb"); + let symlink_name = dir_symlink.path().join("babar.mdb.link"); + fs::create_dir_all(&env_name).unwrap(); + + std::os::windows::fs::symlink_dir(&env_name, &symlink_name).unwrap(); + let _env = EnvOpenOptions::new() + .map_size(10 * 1024 * 1024) // 10MB + .open(&symlink_name) + .unwrap(); + + let _env = EnvOpenOptions::new() + .map_size(10 * 1024 * 1024) // 10MB + .open(&env_name) + .unwrap(); + } + #[test] fn create_database_without_commit() { let dir = tempfile::tempdir().unwrap(); From 092ab5491086355309f97ff8f96b71fadf874b75 Mon Sep 17 00:00:00 2001 From: Axel Viala Date: Mon, 10 Jul 2023 15:07:54 +0200 Subject: [PATCH 02/11] Apply Review suggestion: deconstruct explicitly handle. --- heed/src/env.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/heed/src/env.rs b/heed/src/env.rs index 82c00a2f..aa389ea7 100644 --- a/heed/src/env.rs +++ b/heed/src/env.rs @@ -301,7 +301,8 @@ pub struct Env(Arc); impl fmt::Debug for Env { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - let EnvInner { env: _, dbi_open_mutex: _, path, .. } = self.0.as_ref(); + // We don't include the header since it's containing platform specifics details + let EnvInner { env: _, dbi_open_mutex: _, path, handle: _ } = self.0.as_ref(); f.debug_struct("Env").field("path", &path.display()).finish_non_exhaustive() } } From ded5dd8397459af179f5d0617f988a24e71ee30c Mon Sep 17 00:00:00 2001 From: Axel Viala Date: Mon, 10 Jul 2023 15:09:02 +0200 Subject: [PATCH 03/11] Fixup documentation of the OPENED_ENV. Precise that Handle open the file. --- heed/src/env.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/heed/src/env.rs b/heed/src/env.rs index aa389ea7..fa9b379a 100644 --- a/heed/src/env.rs +++ b/heed/src/env.rs @@ -30,12 +30,17 @@ use crate::{ /// The list of opened environments, the value is an optional environment, it is None /// when someone asks to close the environment, closing is a two-phase step, to make sure -/// noone tries to open the same environment between these two phases. +/// none tries to open the same environment between these two phases. /// -/// Trying to open a None marked environment returns an error to the user trying to open it. +/// Trying to open a `None` marked environment returns an error to the user trying to open it. /// -/// [samefile::Handle] abstract the platform details of checking if the given paths points to +/// [same_file::Handle] abstract the platform details of checking if the given paths points to /// the same file on the filesystem. +/// +/// ## Safety +/// +/// Mind that Handle currently open the file, so avoid writing through the fd held by [same_file::Handle], +/// since the file will also be opened by LMDB. static OPENED_ENV: Lazy>> = Lazy::new(RwLock::default); struct EnvEntry { From 86da304ddbeb65807441c33d5c35718845dcb529 Mon Sep 17 00:00:00 2001 From: Axel Viala Date: Mon, 10 Jul 2023 15:17:41 +0200 Subject: [PATCH 04/11] Review: we don't need canonnicalize anymore. --- heed/src/env.rs | 28 ++++------------------------ 1 file changed, 4 insertions(+), 24 deletions(-) diff --git a/heed/src/env.rs b/heed/src/env.rs index fa9b379a..e2819107 100644 --- a/heed/src/env.rs +++ b/heed/src/env.rs @@ -49,23 +49,6 @@ struct EnvEntry { options: EnvOpenOptions, } -// Thanks to the mozilla/rkv project -// Workaround the UNC path on Windows, see https://github.com/rust-lang/rust/issues/42869. -// Otherwise, `Env::from_env()` will panic with error_no(123). -#[cfg(not(windows))] -fn canonicalize_path(path: &Path) -> io::Result { - path.canonicalize() -} - -#[cfg(windows)] -fn canonicalize_path(path: &Path) -> io::Result { - let canonical = path.canonicalize()?; - let url = url::Url::from_file_path(&canonical) - .map_err(|_e| io::Error::new(io::ErrorKind::Other, "URL passing error"))?; - url.to_file_path() - .map_err(|_e| io::Error::new(io::ErrorKind::Other, "path canonicalization error")) -} - #[cfg(windows)] /// Adding a 'missing' trait from windows OsStrExt trait OsStrExtLmdb { @@ -189,14 +172,14 @@ impl EnvOpenOptions { pub fn open>(&self, path: P) -> Result { let mut lock = OPENED_ENV.write().unwrap(); - let (path, handle) = match canonicalize_path(path.as_ref()) { + let (path, handle) = match Handle::from_path(&path) { Err(err) => { if err.kind() == NotFound && self.flags & (Flag::NoSubDir as u32) != 0 { let path = path.as_ref(); match path.parent().zip(path.file_name()) { Some((dir, file_name)) => { - let handle = Handle::from_path(dir)?; - let path = canonicalize_path(dir)?.join(file_name); + let handle = Handle::from_path(&dir)?; + let path = dir.join(file_name); (path, handle) } None => return Err(err.into()), @@ -205,10 +188,7 @@ impl EnvOpenOptions { return Err(err.into()); } } - Ok(path) => { - let handle = Handle::from_path(&path)?; - (path, handle) - } + Ok(handle) => (path.as_ref().to_path_buf(), handle), }; match lock.entry(handle) { From 295d15996cdd838da64e7810042008ae59d0d365 Mon Sep 17 00:00:00 2001 From: Axel Viala Date: Mon, 10 Jul 2023 15:33:56 +0200 Subject: [PATCH 05/11] Fix hardlink test. --- heed/src/env.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/heed/src/env.rs b/heed/src/env.rs index e2819107..3ba7c9c8 100644 --- a/heed/src/env.rs +++ b/heed/src/env.rs @@ -833,9 +833,8 @@ mod tests { #[test] fn open_env_with_named_path_hardlink_and_no_subdir() { let dir = tempfile::tempdir().unwrap(); - let dir_symlink = tempfile::tempdir().unwrap(); let env_name = dir.path().join("babar.mdb"); - let hardlink_name = dir_symlink.path().join("babar.mdb.link"); + let hardlink_name = dir.path().join("babar.mdb.link"); let mut envbuilder = EnvOpenOptions::new(); unsafe { envbuilder.flag(crate::Flag::NoSubDir) }; @@ -844,7 +843,7 @@ mod tests { .open(&env_name) .unwrap(); - std::os::unix::fs::symlink(&dir.path(), &hardlink_name).unwrap(); + std::fs::hard_link(&env_name, &hardlink_name).unwrap(); let _env = envbuilder .map_size(10 * 1024 * 1024) // 10MB .open(&hardlink_name) From d720f09e2ce463d1e781c09662514f7ad7deaa83 Mon Sep 17 00:00:00 2001 From: Axel Viala Date: Mon, 10 Jul 2023 18:16:32 +0200 Subject: [PATCH 06/11] fixup! document path changes --- heed/src/env.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/heed/src/env.rs b/heed/src/env.rs index 3ba7c9c8..1d8416b0 100644 --- a/heed/src/env.rs +++ b/heed/src/env.rs @@ -641,7 +641,10 @@ impl Env { Ok(()) } - /// Returns the canonicalized path where this env lives. + /// Returns the `Path` where this [Env] lives. + /// + /// The `Path` returned will be the one from the first opening. Like if you `EnvOpenOptions::open` through + /// a symlink or a moved the directory containing it will be the case. pub fn path(&self) -> &Path { &self.0.path } From b065e9124ac3c02af51a31148f1dc937d49aa798 Mon Sep 17 00:00:00 2001 From: Axel Viala Date: Tue, 11 Jul 2023 11:28:52 +0200 Subject: [PATCH 07/11] Fixup! Hardlink will not work reviewed my unixbasics --- heed/src/env.rs | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/heed/src/env.rs b/heed/src/env.rs index 1d8416b0..33b93888 100644 --- a/heed/src/env.rs +++ b/heed/src/env.rs @@ -833,31 +833,6 @@ mod tests { .unwrap(); } - #[test] - fn open_env_with_named_path_hardlink_and_no_subdir() { - let dir = tempfile::tempdir().unwrap(); - let env_name = dir.path().join("babar.mdb"); - let hardlink_name = dir.path().join("babar.mdb.link"); - - let mut envbuilder = EnvOpenOptions::new(); - unsafe { envbuilder.flag(crate::Flag::NoSubDir) }; - let _env = envbuilder - .map_size(10 * 1024 * 1024) // 10MB - .open(&env_name) - .unwrap(); - - std::fs::hard_link(&env_name, &hardlink_name).unwrap(); - let _env = envbuilder - .map_size(10 * 1024 * 1024) // 10MB - .open(&hardlink_name) - .unwrap(); - - let _env = envbuilder - .map_size(10 * 1024 * 1024) // 10MB - .open(&env_name) - .unwrap(); - } - #[test] #[cfg(unix)] fn open_env_with_named_path_symlink() { From 641d2297f52e546b1a33d3ff119af7d1ce2a7a28 Mon Sep 17 00:00:00 2001 From: Axel Viala Date: Tue, 11 Jul 2023 11:29:45 +0200 Subject: [PATCH 08/11] Fixup! Integrate suggestion from review about the tests. --- heed/src/env.rs | 38 +++++++++++++++++++++++++++----------- 1 file changed, 27 insertions(+), 11 deletions(-) diff --git a/heed/src/env.rs b/heed/src/env.rs index 33b93888..8f9cdaf8 100644 --- a/heed/src/env.rs +++ b/heed/src/env.rs @@ -843,37 +843,47 @@ mod tests { let symlink_name = dir_symlink.path().join("babar.mdb.link"); fs::create_dir_all(&env_name).unwrap(); - std::os::unix::fs::symlink(&env_name, &symlink_name).unwrap(); - let _env = EnvOpenOptions::new() + let env = EnvOpenOptions::new() .map_size(10 * 1024 * 1024) // 10MB - .open(&symlink_name) + .open(&env_name) .unwrap(); + assert_eq!(env_name, env.path()); - let _env = EnvOpenOptions::new() + std::os::unix::fs::symlink(&env_name, &symlink_name).unwrap(); + + let env = EnvOpenOptions::new() .map_size(10 * 1024 * 1024) // 10MB - .open(&env_name) + .open(&symlink_name) .unwrap(); + + // The path is recycle from the first opening of the env. + assert_eq!(env_name, env.path()); } #[test] fn open_env_with_named_path_rename() { let dir = tempfile::tempdir().unwrap(); + // We make a tmp dir where to move the env to avoid directly writing to /tmp. + // Windows CI had issue if not done. + let moved_parent = tempfile::tempdir().unwrap(); let env_name = dir.path().join("babar.mdb"); fs::create_dir_all(&env_name).unwrap(); - let _env = EnvOpenOptions::new() + let env = EnvOpenOptions::new() .map_size(10 * 1024 * 1024) // 10MB .open(&env_name) .unwrap(); + assert_eq!(env_name, env.path()); - let env_renamed = dir.path().join("serafina.mdb"); + let env_renamed = moved_parent.path().join("serafina.mdb"); std::fs::rename(&env_name, &env_renamed).unwrap(); - let _env = EnvOpenOptions::new() + let env = EnvOpenOptions::new() .map_size(10 * 1024 * 1024) // 10MB .open(&env_renamed) .unwrap(); + assert_eq!(env_name, env.path()); } #[test] @@ -909,10 +919,12 @@ mod tests { let mut envbuilder = EnvOpenOptions::new(); unsafe { envbuilder.flag(crate::Flag::NoSubDir) }; - let _env = envbuilder + let env = envbuilder .map_size(10 * 1024 * 1024) // 10MB .open(&env_name) .unwrap(); + assert_eq!(env_name, env.path()); + assert_ne!(symlink_name, env.path()); std::os::unix::fs::symlink(&dir.path(), &symlink_name).unwrap(); let _env = envbuilder @@ -920,6 +932,10 @@ mod tests { .open(&symlink_name) .unwrap(); + // Checkout that we get the path of the first openning. + assert_eq!(env_name, env.path()); + assert_ne!(symlink_name, env.path()); + let _env = envbuilder .map_size(10 * 1024 * 1024) // 10MB .open(&env_name) @@ -964,12 +980,12 @@ mod tests { fs::create_dir_all(&env_name).unwrap(); std::os::windows::fs::symlink_dir(&env_name, &symlink_name).unwrap(); - let _env = EnvOpenOptions::new() + let env = EnvOpenOptions::new() .map_size(10 * 1024 * 1024) // 10MB .open(&symlink_name) .unwrap(); - let _env = EnvOpenOptions::new() + let env = EnvOpenOptions::new() .map_size(10 * 1024 * 1024) // 10MB .open(&env_name) .unwrap(); From 833e43368a59f588ff85f6c3621529ddb3f65d3c Mon Sep 17 00:00:00 2001 From: Axel Viala Date: Tue, 11 Jul 2023 12:11:33 +0200 Subject: [PATCH 09/11] Fix symlink no subdir --- heed/src/env.rs | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/heed/src/env.rs b/heed/src/env.rs index 8f9cdaf8..cb7bbac5 100644 --- a/heed/src/env.rs +++ b/heed/src/env.rs @@ -912,34 +912,37 @@ mod tests { #[test] #[cfg(unix)] fn open_env_with_named_path_symlink_and_no_subdir() { - let dir = tempfile::tempdir().unwrap(); - let dir_symlink = tempfile::tempdir().unwrap(); - let env_name = dir.path().join("babar.mdb"); - let symlink_name = dir_symlink.path().join("babar.mdb.link"); + let env_dir = tempfile::Builder::new().suffix("heed_test_no_subdir_env").tempdir().unwrap(); + let symlink_parent = tempfile::tempdir().unwrap(); + let env_path = env_dir.path(); + let db_file = env_dir.path().join("data.mdb"); + + let symlink_path = symlink_parent.path().join("env.link"); let mut envbuilder = EnvOpenOptions::new(); unsafe { envbuilder.flag(crate::Flag::NoSubDir) }; let env = envbuilder .map_size(10 * 1024 * 1024) // 10MB - .open(&env_name) + .open(&db_file) .unwrap(); - assert_eq!(env_name, env.path()); - assert_ne!(symlink_name, env.path()); + assert_eq!(db_file, env.path()); - std::os::unix::fs::symlink(&dir.path(), &symlink_name).unwrap(); + std::os::unix::fs::symlink(&env_path, &symlink_path).unwrap(); let _env = envbuilder .map_size(10 * 1024 * 1024) // 10MB - .open(&symlink_name) + .open(&symlink_path) .unwrap(); // Checkout that we get the path of the first openning. - assert_eq!(env_name, env.path()); - assert_ne!(symlink_name, env.path()); + assert_eq!(db_file, env.path()); + assert_ne!(symlink_path, env.path()); let _env = envbuilder .map_size(10 * 1024 * 1024) // 10MB - .open(&env_name) + .open(&env_path) .unwrap(); + assert_eq!(db_file, env.path()); + assert_ne!(symlink_path, env.path()); } #[test] From 911ba89cb50f2f5a6b0563cf6cb4f2ff9b202a17 Mon Sep 17 00:00:00 2001 From: Axel Viala Date: Tue, 11 Jul 2023 12:11:48 +0200 Subject: [PATCH 10/11] Fix windows details. --- heed/src/env.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/heed/src/env.rs b/heed/src/env.rs index cb7bbac5..6ed64fad 100644 --- a/heed/src/env.rs +++ b/heed/src/env.rs @@ -950,8 +950,12 @@ mod tests { fn open_env_with_named_path_symlinkfile_and_no_subdir() { let dir = tempfile::tempdir().unwrap(); let dir_symlink = tempfile::tempdir().unwrap(); + // We do a temp dir because CI returned + // Os { code: 5, kind: PermissionDenied, message: "Access is denied." } + let parent_symlink = tempfile::tempdir().unwrap(); + let env_name = dir.path().join("babar.mdb"); - let symlink_name = dir_symlink.path().join("babar.mdb.link"); + let symlink_name = parent_symlink.path().join("babar.mdb.link"); let mut envbuilder = EnvOpenOptions::new(); unsafe { envbuilder.flag(crate::Flag::NoSubDir) }; @@ -960,7 +964,7 @@ mod tests { .open(&env_name) .unwrap(); - std::os::windows::fs::symlink_file(&dir.path(), &symlink_name).unwrap(); + std::os::windows::fs::symlink_dir(&dir.path(), &symlink_name).unwrap(); let _env = envbuilder .map_size(10 * 1024 * 1024) // 10MB .open(&symlink_name) From 83b43861c6e7137fb591467bc7ec6ed03abb5d2c Mon Sep 17 00:00:00 2001 From: Axel Viala Date: Tue, 11 Jul 2023 14:21:17 +0200 Subject: [PATCH 11/11] Windows moving dir is just a burden. --- heed/src/env.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/heed/src/env.rs b/heed/src/env.rs index 6ed64fad..912dcff0 100644 --- a/heed/src/env.rs +++ b/heed/src/env.rs @@ -860,13 +860,13 @@ mod tests { assert_eq!(env_name, env.path()); } + // Unix only since managing to support *moving* directory on windows was too + // much pain. By moving we mean not copy-recreating, just changing name. + // https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-movefileexa #[test] + #[cfg(unix)] fn open_env_with_named_path_rename() { let dir = tempfile::tempdir().unwrap(); - // We make a tmp dir where to move the env to avoid directly writing to /tmp. - // Windows CI had issue if not done. - let moved_parent = tempfile::tempdir().unwrap(); - let env_name = dir.path().join("babar.mdb"); fs::create_dir_all(&env_name).unwrap(); @@ -876,7 +876,7 @@ mod tests { .unwrap(); assert_eq!(env_name, env.path()); - let env_renamed = moved_parent.path().join("serafina.mdb"); + let env_renamed = dir.path().join("serafina.mdb"); std::fs::rename(&env_name, &env_renamed).unwrap(); let env = EnvOpenOptions::new()