From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 68CF61FF143 for ; Mon, 02 Feb 2026 11:55:57 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 587E917BE; Mon, 2 Feb 2026 11:56:27 +0100 (CET) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Mon, 02 Feb 2026 11:56:22 +0100 Message-Id: Subject: Re: [PATCH v5 proxmox-backup 03/16] datastore: add TempTestDir that is automatically deleted on drop From: "Lukas Wagner" To: "Robert Obkircher" , "Lukas Wagner" , X-Mailer: aerc 0.21.0-0-g5549850facc2-dirty References: <20260130164552.281581-1-r.obkircher@proxmox.com> <20260130164552.281581-4-r.obkircher@proxmox.com> In-Reply-To: X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1770029709771 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.037 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: KBR5QTZ7ENG54MKGVFKWTH73KLOG6XHA X-Message-ID-Hash: KBR5QTZ7ENG54MKGVFKWTH73KLOG6XHA X-MailFrom: l.wagner@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Backup Server development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Mon Feb 2, 2026 at 11:12 AM CET, Robert Obkircher wrote: > > On 2/2/26 09:30, Lukas Wagner wrote: >> On Fri Jan 30, 2026 at 5:45 PM CET, Robert Obkircher wrote: >>> This will be used to test the FixedIndexWriter and chunk store >>> creation. >>> >>> Signed-off-by: Robert Obkircher >>> --- >>> pbs-datastore/src/lib.rs | 4 ++++ >>> pbs-datastore/src/temp_test_dir.rs | 33 ++++++++++++++++++++++++++++++ >>> 2 files changed, 37 insertions(+) >>> create mode 100644 pbs-datastore/src/temp_test_dir.rs >>> >>> diff --git a/pbs-datastore/src/lib.rs b/pbs-datastore/src/lib.rs >>> index 1f7c54ae..b48ec93c 100644 >>> --- a/pbs-datastore/src/lib.rs >>> +++ b/pbs-datastore/src/lib.rs >>> @@ -233,3 +233,7 @@ pub use local_chunk_reader::LocalChunkReader; >>> =20 >>> mod local_datastore_lru_cache; >>> pub use local_datastore_lru_cache::LocalDatastoreLruCache; >>> + >>> +#[cfg(test)] >>> +mod temp_test_dir; >>> + >>> diff --git a/pbs-datastore/src/temp_test_dir.rs b/pbs-datastore/src/tem= p_test_dir.rs >>> new file mode 100644 >>> index 00000000..13e16c91 >>> --- /dev/null >>> +++ b/pbs-datastore/src/temp_test_dir.rs >>> @@ -0,0 +1,33 @@ >>> +use std::convert::From; >>> +use std::env; >>> +use std::path::{Path, PathBuf}; >>> + >>> +/// A temporary directory for tests that is removed on drop. >>> +/// Deletion is best-effort and errors are silently ignored. >>> +pub struct TempTestDir(PathBuf); >>> + >>> +impl TempTestDir { >>> + /// Create an empty directory. >>> + pub fn new() -> Self { >>> + Self(proxmox_sys::fs::make_tmp_dir(env::temp_dir(), None).unwr= ap()) >>> + } >>> +} >>> + >>> +impl std::ops::Deref for TempTestDir { >>> + type Target =3D Path; >>> + fn deref(&self) -> &Self::Target { >>> + &self.0 >>> + } >>> +} >>> + >>> +impl Drop for TempTestDir { >>> + fn drop(&mut self) { >>> + let _ =3D std::fs::remove_dir_all(&self.0); >>> + } >>> +} >>> + >>> +impl<'a> From<&'a TempTestDir> for PathBuf { >>> + fn from(temp: &'a TempTestDir) -> Self { >>> + temp.0.clone() >>> + } >>> +} >> >> Hey Robert, >> >> We do already have a very similar helper in the PDM repository [1]. >> Maybe it would sense move either this or the PDM version (or some hybrid= ) >> into a shared crate in proxmox.git? >> >> I would suggest creating a new crate for commonly used test-only >> helpers (proxmox-test-support, proxmox-test-helpers, proxmox-test-util, >> maybe you have a better suggestion?) >> >> What do you think? > I've noticed this as well when I skimmed your RFC. > > I'm not sure if combining unrelated things into a -test > crate is a good idea. Maybe a (proxmox-)tempfile crate > that is added to [dev-dependencies] would be slightly > nicer? Also okay to me. Then I would make sure that this helper is 'production-ready' - so maybe log the error in case `drop` could not clean up the directory, add support for `CreateOptions`, and also maybe add a method that allows one to clean up manually, so that any potential error can be handle by the application if needed (TmpDir from the tempfile crate has a `close` method, something like that). Maybe it could also make sense to move the tmpfile helpers from proxmox-sys there, if possible (IIRC proxmox-sys uses the tempfile helpers internally). > > I've also considered adding it to proxmox_sys::fs::dir, > but it does seem a little out of place there. Yeah, I wouldn't do that. proxmox-sys is pretty huge already and should rather be broken apart into meaningful smaller crates than made bigger. > >> >> [1] https://git.proxmox.com/?p=3Dproxmox-datacenter-manager.git;a=3Dblob= ;f=3Dserver/src/test_support/temp.rs;h=3Da93c914d9efcc60de3bb40bc79610abc98= 571b7f;hb=3DHEAD