From 314c4d10493e271af63a2ddace298ca3ffb0d9cf Mon Sep 17 00:00:00 2001 From: Miles Date: Tue, 7 May 2024 14:59:04 +0200 Subject: [PATCH] Fix early drops of params (#22) * Keep c/dparams alive when passed to storage * Update CI and propagate regenerate-bindings to blosc2-rs * Remove Windows skipped test_schunk_basic --- .github/workflows/CI.yml | 10 ++++---- Cargo.toml | 5 ++-- blosc2-sys/Cargo.toml | 2 +- src/lib.rs | 50 +++++++++++++++++++++++----------------- 4 files changed, 38 insertions(+), 29 deletions(-) diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 83599fd2..10c710b6 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -59,7 +59,7 @@ jobs: - name: Build shell: bash -el {0} - run: cargo build ${{ matrix.flags }} + run: cargo build ${{ matrix.flags }} --release - name: Test # Running tests using shared library is ugly since conda doesn't @@ -69,7 +69,7 @@ jobs: # shared library within the conda environment. if: ${{ matrix.flags == '--features static' }} shell: bash -el {0} - run: cargo test ${{ matrix.flags }} + run: cargo test ${{ matrix.flags }} --release test-musllinux: runs-on: ubuntu-latest @@ -85,7 +85,7 @@ jobs: run: cargo install cross --git https://github.com/cross-rs/cross --rev 6d097fb - name: Test - run: cross test --target x86_64-unknown-linux-musl --no-default-features --features static + run: cross test --target x86_64-unknown-linux-musl --no-default-features --features static --release test-native: runs-on: ${{ matrix.os }} @@ -113,9 +113,9 @@ jobs: # Known issue where testing w/ shared linked lib doesn't work with --doc testing - name: Test (shared) - run: cargo test --features shared --lib -vv + run: cargo test --features shared --lib -vv --release - name: Test (static) run: | cargo clean # ensure we're starting fresh, no funny business - cargo test --no-default-features --features static -vv + cargo test --no-default-features --features static -vv --release diff --git a/Cargo.toml b/Cargo.toml index e962ef33..3df02d2b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "blosc2-rs" -version = "0.2.6+2.14.3" +version = "0.3.0+2.14.3" description = "Blosc2" license = "MIT" edition = "2021" @@ -12,11 +12,12 @@ name = "blosc2" [features] default = ["shared"] use-system-blosc2 = ["blosc2-sys/use-system-blosc2"] +regenerate-bindings = ["blosc2-sys/regenerate-bindings"] static = ["blosc2-sys/static"] shared = ["blosc2-sys/shared"] [dependencies] -blosc2-sys = { path = "blosc2-sys", version = "0.2.6+2.14.3" } +blosc2-sys = { path = "blosc2-sys", version = "0.3.0+2.14.3" } parking_lot = "^0.12" [dev-dependencies] diff --git a/blosc2-sys/Cargo.toml b/blosc2-sys/Cargo.toml index 211f5152..1b1e8fb5 100644 --- a/blosc2-sys/Cargo.toml +++ b/blosc2-sys/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "blosc2-sys" -version = "0.2.6+2.14.3" +version = "0.3.0+2.14.3" edition = "2021" description = "Bindings to C Blosc2" license = "MIT" diff --git a/src/lib.rs b/src/lib.rs index dd5044e2..387ac72b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -356,12 +356,20 @@ pub mod schunk { /// /// let storage = Storage::default().set_urlpath("/some/path.blosc2"); /// ``` - pub struct Storage(ffi::blosc2_storage); + pub struct Storage { + inner: ffi::blosc2_storage, + cparams: Option, + dparams: Option, + } impl Default for Storage { fn default() -> Self { let storage: ffi::blosc2_storage = unsafe { mem::MaybeUninit::zeroed().assume_init() }; - Storage(storage) + Storage { + inner: storage, + cparams: None, + dparams: None, + } } } @@ -369,7 +377,7 @@ pub mod schunk { /// Set url/file path to specify a file-backed `schunk`. /// if not set, defaults to an in-memory `schunk` pub fn set_urlpath>(mut self, urlpath: S) -> Result { - self.0.urlpath = CString::new(urlpath.as_ref().to_string_lossy().to_string()) + self.inner.urlpath = CString::new(urlpath.as_ref().to_string_lossy().to_string()) .map_err(|e| Error::Other(e.to_string()))? .into_raw(); Ok(self) @@ -385,11 +393,11 @@ pub mod schunk { /// assert_eq!(storage.get_urlpath().unwrap().unwrap(), "/some/path.blosc2"); /// ``` pub fn get_urlpath(&self) -> Result> { - if self.0.urlpath.is_null() { + if self.inner.urlpath.is_null() { return Ok(None); } unsafe { - CStr::from_ptr(self.0.urlpath) + CStr::from_ptr(self.inner.urlpath) .to_str() .map(|v| Some(v)) .map_err(|e| Error::Other(e.to_string())) @@ -397,21 +405,23 @@ pub mod schunk { } /// Set the contiguous nature of the `schunk`. pub fn set_contiguous(mut self, contiguous: bool) -> Self { - self.0.contiguous = contiguous; + self.inner.contiguous = contiguous; self } /// Set compression parameters - pub fn set_cparams(mut self, cparams: &mut CParams) -> Self { - self.0.cparams = &mut cparams.0; + pub fn set_cparams(mut self, cparams: CParams) -> Self { + self.cparams = Some(cparams); + self.inner.cparams = &mut self.cparams.as_mut().unwrap().0; self } /// Get compression parameters - pub fn get_cparams(&self) -> CParams { - CParams(unsafe { *self.0.cparams }) + pub fn get_cparams(&self) -> Option<&CParams> { + self.cparams.as_ref() } /// Set decompression parameters - pub fn set_dparams(mut self, dparams: &mut DParams) -> Self { - self.0.dparams = &mut dparams.0; + pub fn set_dparams(mut self, dparams: DParams) -> Self { + self.dparams = Some(dparams); + self.inner.dparams = &mut self.dparams.as_mut().unwrap().0; self } } @@ -430,8 +440,8 @@ pub mod schunk { /// let input = b"some data"; /// let storage = Storage::default() /// .set_contiguous(true) - /// .set_cparams(&mut CParams::from(&input[0])) - /// .set_dparams(&mut DParams::default()); + /// .set_cparams(CParams::from(&input[0])) + /// .set_dparams(DParams::default()); /// let mut schunk = SChunk::new(storage); /// /// let n = schunk.write(input).unwrap(); // same as schunk.append_buffer(input)?; @@ -781,7 +791,7 @@ pub mod schunk { impl SChunk { pub fn new(storage: Storage) -> Self { let mut storage = storage; - let schunk = unsafe { ffi::blosc2_schunk_new(&mut storage.0) }; + let schunk = unsafe { ffi::blosc2_schunk_new(&mut storage.inner) }; Self(Arc::new(RwLock::new(schunk))) } @@ -1858,15 +1868,13 @@ mod tests { Ok(()) } - // Something wrong w/ Windows' into_vec or something - #[cfg(not(target_os = "windows"))] #[test] fn test_schunk_basic() -> Result<()> { let input = b"some data"; let storage = schunk::Storage::default() .set_contiguous(true) - .set_cparams(&mut CParams::from(&input[0])) - .set_dparams(&mut DParams::default()); + .set_cparams(CParams::from(&input[0])) + .set_dparams(DParams::default()); let mut schunk = schunk::SChunk::new(storage); assert!(schunk.is_contiguous()); @@ -1902,8 +1910,8 @@ mod tests { .collect::>(); let storage = schunk::Storage::default() .set_contiguous(true) - .set_cparams(&mut CParams::from(&input[0])) - .set_dparams(&mut DParams::default()); + .set_cparams(CParams::from(&input[0])) + .set_dparams(DParams::default()); let mut schunk = schunk::SChunk::new(storage); let nbytes = std::io::copy(&mut Cursor::new(input.clone()), &mut schunk)