From 6ed14c3e3474effdb73e0a059944feef4228436c Mon Sep 17 00:00:00 2001 From: Oleksandr Anyshchenko Date: Tue, 11 Jan 2022 11:42:22 +0200 Subject: [PATCH] Fix CI builds (#582) --- Cargo.toml | 2 +- librocksdb-sys/rocksdb_lib_sources.txt | 7 --- src/backup.rs | 10 ++-- src/db.rs | 16 ++--- src/db_options.rs | 60 ++++++++++--------- src/lib.rs | 3 - src/perf.rs | 5 +- tests/fail/checkpoint_outlive_db.stderr | 2 +- tests/fail/iterator_outlive_db.stderr | 4 +- ...th_multiple_refs_as_single_threaded.stderr | 8 +-- tests/fail/snapshot_outlive_db.stderr | 4 +- tests/test_db.rs | 14 ++--- tests/test_merge_operator.rs | 2 +- tests/test_raw_iterator.rs | 22 +++---- 14 files changed, 73 insertions(+), 86 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 07c02a8..926e85c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -37,6 +37,6 @@ serde = { version = "1", features = [ "derive" ], optional = true } [dev-dependencies] trybuild = "1.0" tempfile = "3.1" -pretty_assertions = "0.7" +pretty_assertions = "1.0" bincode = "1.3" serde = { version = "1", features = [ "derive" ] } diff --git a/librocksdb-sys/rocksdb_lib_sources.txt b/librocksdb-sys/rocksdb_lib_sources.txt index 40217e0..d326c86 100644 --- a/librocksdb-sys/rocksdb_lib_sources.txt +++ b/librocksdb-sys/rocksdb_lib_sources.txt @@ -136,12 +136,6 @@ options/options.cc options/options_helper.cc options/options_parser.cc port/port_posix.cc -port/win/env_default.cc -port/win/env_win.cc -port/win/io_win.cc -port/win/port_win.cc -port/win/win_logger.cc -port/win/win_thread.cc port/stack_trace.cc table/adaptive/adaptive_table_factory.cc table/block_based/binary_search_index_reader.cc @@ -205,7 +199,6 @@ util/comparator.cc util/compression_context_cache.cc util/concurrent_task_limiter_impl.cc util/crc32c.cc -util/crc32c_arm64.cc util/dynamic_bloom.cc util/hash.cc util/murmurhash.cc diff --git a/src/backup.rs b/src/backup.rs index 75aed70..cddfae8 100644 --- a/src/backup.rs +++ b/src/backup.rs @@ -277,9 +277,8 @@ impl Default for BackupEngineOptions { fn default() -> Self { unsafe { let opts = ffi::rocksdb_options_create(); - if opts.is_null() { - panic!("Could not create RocksDB backup options"); - } + assert!(!opts.is_null(), "Could not create RocksDB backup options"); + Self { inner: opts } } } @@ -289,9 +288,8 @@ impl Default for RestoreOptions { fn default() -> Self { unsafe { let opts = ffi::rocksdb_restore_options_create(); - if opts.is_null() { - panic!("Could not create RocksDB restore options"); - } + assert!(!opts.is_null(), "Could not create RocksDB restore options"); + Self { inner: opts } } } diff --git a/src/db.rs b/src/db.rs index 6a31b42..131a8fc 100644 --- a/src/db.rs +++ b/src/db.rs @@ -1330,9 +1330,9 @@ impl DBWithThreadMode { ffi::rocksdb_compact_range( self.inner, opt_bytes_to_ptr(start), - start.map_or(0, |s| s.len()) as size_t, + start.map_or(0, <[u8]>::len) as size_t, opt_bytes_to_ptr(end), - end.map_or(0, |e| e.len()) as size_t, + end.map_or(0, <[u8]>::len) as size_t, ); } } @@ -1352,9 +1352,9 @@ impl DBWithThreadMode { self.inner, opts.inner, opt_bytes_to_ptr(start), - start.map_or(0, |s| s.len()) as size_t, + start.map_or(0, <[u8]>::len) as size_t, opt_bytes_to_ptr(end), - end.map_or(0, |e| e.len()) as size_t, + end.map_or(0, <[u8]>::len) as size_t, ); } } @@ -1375,9 +1375,9 @@ impl DBWithThreadMode { self.inner, cf.inner(), opt_bytes_to_ptr(start), - start.map_or(0, |s| s.len()) as size_t, + start.map_or(0, <[u8]>::len) as size_t, opt_bytes_to_ptr(end), - end.map_or(0, |e| e.len()) as size_t, + end.map_or(0, <[u8]>::len) as size_t, ); } } @@ -1399,9 +1399,9 @@ impl DBWithThreadMode { cf.inner(), opts.inner, opt_bytes_to_ptr(start), - start.map_or(0, |s| s.len()) as size_t, + start.map_or(0, <[u8]>::len) as size_t, opt_bytes_to_ptr(end), - end.map_or(0, |e| e.len()) as size_t, + end.map_or(0, <[u8]>::len) as size_t, ); } } diff --git a/src/db_options.rs b/src/db_options.rs index 09f0f91..c629ba4 100644 --- a/src/db_options.rs +++ b/src/db_options.rs @@ -407,9 +407,8 @@ impl Drop for Options { impl Clone for Options { fn clone(&self) -> Self { let inner = unsafe { ffi::rocksdb_options_create_copy(self.inner) }; - if inner.is_null() { - panic!("Could not copy RocksDB options"); - } + assert!(!inner.is_null(), "Could not copy RocksDB options"); + Self { inner, outlive: self.outlive.clone(), @@ -710,9 +709,11 @@ impl BlockBasedOptions { impl Default for BlockBasedOptions { fn default() -> Self { let block_opts = unsafe { ffi::rocksdb_block_based_options_create() }; - if block_opts.is_null() { - panic!("Could not create RocksDB block based options"); - } + assert!( + !block_opts.is_null(), + "Could not create RocksDB block based options" + ); + Self { inner: block_opts, outlive: BlockBasedOptionsMustOutliveDB::default(), @@ -782,9 +783,8 @@ impl CuckooTableOptions { impl Default for CuckooTableOptions { fn default() -> Self { let opts = unsafe { ffi::rocksdb_cuckoo_options_create() }; - if opts.is_null() { - panic!("Could not create RocksDB cuckoo options"); - } + assert!(!opts.is_null(), "Could not create RocksDB cuckoo options"); + Self { inner: opts } } } @@ -2951,9 +2951,8 @@ impl Default for Options { fn default() -> Self { unsafe { let opts = ffi::rocksdb_options_create(); - if opts.is_null() { - panic!("Could not create RocksDB options"); - } + assert!(!opts.is_null(), "Could not create RocksDB options"); + Self { inner: opts, outlive: OptionsMustOutliveDB::default(), @@ -2989,9 +2988,11 @@ impl FlushOptions { impl Default for FlushOptions { fn default() -> Self { let flush_opts = unsafe { ffi::rocksdb_flushoptions_create() }; - if flush_opts.is_null() { - panic!("Could not create RocksDB flush options"); - } + assert!( + !flush_opts.is_null(), + "Could not create RocksDB flush options" + ); + Self { inner: flush_opts } } } @@ -3077,9 +3078,11 @@ impl WriteOptions { impl Default for WriteOptions { fn default() -> Self { let write_opts = unsafe { ffi::rocksdb_writeoptions_create() }; - if write_opts.is_null() { - panic!("Could not create RocksDB write options"); - } + assert!( + !write_opts.is_null(), + "Could not create RocksDB write options" + ); + Self { inner: write_opts } } } @@ -3456,9 +3459,11 @@ pub struct FifoCompactOptions { impl Default for FifoCompactOptions { fn default() -> Self { let opts = unsafe { ffi::rocksdb_fifo_compaction_options_create() }; - if opts.is_null() { - panic!("Could not create RocksDB Fifo Compaction Options"); - } + assert!( + !opts.is_null(), + "Could not create RocksDB Fifo Compaction Options" + ); + Self { inner: opts } } } @@ -3499,9 +3504,11 @@ pub struct UniversalCompactOptions { impl Default for UniversalCompactOptions { fn default() -> Self { let opts = unsafe { ffi::rocksdb_universal_compaction_options_create() }; - if opts.is_null() { - panic!("Could not create RocksDB Universal Compaction Options"); - } + assert!( + !opts.is_null(), + "Could not create RocksDB Universal Compaction Options" + ); + Self { inner: opts } } } @@ -3622,9 +3629,8 @@ pub struct CompactOptions { impl Default for CompactOptions { fn default() -> Self { let opts = unsafe { ffi::rocksdb_compactoptions_create() }; - if opts.is_null() { - panic!("Could not create RocksDB Compact Options"); - } + assert!(!opts.is_null(), "Could not create RocksDB Compact Options"); + Self { inner: opts } } } diff --git a/src/lib.rs b/src/lib.rs index 298fa08..1b2fba6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -61,18 +61,15 @@ // Next lints produce too much noise/false positives. clippy::module_name_repetitions, clippy::similar_names, clippy::must_use_candidate, // '... may panic' lints. - clippy::indexing_slicing, // Too much work to fix. clippy::missing_errors_doc, // False positive: WebSocket clippy::doc_markdown, clippy::missing_safety_doc, clippy::needless_pass_by_value, - clippy::option_if_let_else, clippy::ptr_as_ptr, clippy::missing_panics_doc, clippy::from_over_into, - clippy::upper_case_acronyms, )] #[macro_use] diff --git a/src/perf.rs b/src/perf.rs index a44b671..624a6ec 100644 --- a/src/perf.rs +++ b/src/perf.rs @@ -127,9 +127,8 @@ pub struct PerfContext { impl Default for PerfContext { fn default() -> Self { let ctx = unsafe { ffi::rocksdb_perfcontext_create() }; - if ctx.is_null() { - panic!("Could not create Perf Context"); - } + assert!(!ctx.is_null(), "Could not create Perf Context"); + Self { inner: ctx } } } diff --git a/tests/fail/checkpoint_outlive_db.stderr b/tests/fail/checkpoint_outlive_db.stderr index 7bb598c..a8ff0cc 100644 --- a/tests/fail/checkpoint_outlive_db.stderr +++ b/tests/fail/checkpoint_outlive_db.stderr @@ -1,5 +1,5 @@ error[E0597]: `db` does not live long enough - --> $DIR/checkpoint_outlive_db.rs:6:25 + --> tests/fail/checkpoint_outlive_db.rs:6:25 | 4 | let _checkpoint = { | ----------- borrow later stored here diff --git a/tests/fail/iterator_outlive_db.stderr b/tests/fail/iterator_outlive_db.stderr index 08b7d0d..9fbaf15 100644 --- a/tests/fail/iterator_outlive_db.stderr +++ b/tests/fail/iterator_outlive_db.stderr @@ -1,10 +1,10 @@ error[E0597]: `db` does not live long enough - --> $DIR/iterator_outlive_db.rs:6:9 + --> tests/fail/iterator_outlive_db.rs:6:9 | 4 | let _iter = { | ----- borrow later stored here 5 | let db = DB::open_default("foo").unwrap(); 6 | db.iterator(IteratorMode::Start) - | ^^ borrowed value does not live long enough + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ borrowed value does not live long enough 7 | }; | - `db` dropped here while still borrowed diff --git a/tests/fail/open_with_multiple_refs_as_single_threaded.stderr b/tests/fail/open_with_multiple_refs_as_single_threaded.stderr index 61879c0..0f17e9d 100644 --- a/tests/fail/open_with_multiple_refs_as_single_threaded.stderr +++ b/tests/fail/open_with_multiple_refs_as_single_threaded.stderr @@ -1,17 +1,17 @@ error[E0596]: cannot borrow `*db_ref1` as mutable, as it is behind a `&` reference - --> $DIR/open_with_multiple_refs_as_single_threaded.rs:8:5 + --> tests/fail/open_with_multiple_refs_as_single_threaded.rs:8:5 | 5 | let db_ref1 = &db; | --- help: consider changing this to be a mutable reference: `&mut db` ... 8 | db_ref1.create_cf("cf1", &opts).unwrap(); - | ^^^^^^^ `db_ref1` is a `&` reference, so the data it refers to cannot be borrowed as mutable + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `db_ref1` is a `&` reference, so the data it refers to cannot be borrowed as mutable error[E0596]: cannot borrow `*db_ref2` as mutable, as it is behind a `&` reference - --> $DIR/open_with_multiple_refs_as_single_threaded.rs:9:5 + --> tests/fail/open_with_multiple_refs_as_single_threaded.rs:9:5 | 6 | let db_ref2 = &db; | --- help: consider changing this to be a mutable reference: `&mut db` ... 9 | db_ref2.create_cf("cf2", &opts).unwrap(); - | ^^^^^^^ `db_ref2` is a `&` reference, so the data it refers to cannot be borrowed as mutable + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `db_ref2` is a `&` reference, so the data it refers to cannot be borrowed as mutable diff --git a/tests/fail/snapshot_outlive_db.stderr b/tests/fail/snapshot_outlive_db.stderr index 9720e6b..e0ae012 100644 --- a/tests/fail/snapshot_outlive_db.stderr +++ b/tests/fail/snapshot_outlive_db.stderr @@ -1,10 +1,10 @@ error[E0597]: `db` does not live long enough - --> $DIR/snapshot_outlive_db.rs:6:9 + --> tests/fail/snapshot_outlive_db.rs:6:9 | 4 | let _snapshot = { | --------- borrow later stored here 5 | let db = DB::open_default("foo").unwrap(); 6 | db.snapshot() - | ^^ borrowed value does not live long enough + | ^^^^^^^^^^^^^ borrowed value does not live long enough 7 | }; | - `db` dropped here while still borrowed diff --git a/tests/test_db.rs b/tests/test_db.rs index 8f229ec..394d05e 100644 --- a/tests/test_db.rs +++ b/tests/test_db.rs @@ -979,11 +979,8 @@ fn key_may_exist() { { let db = DB::open_default(&path).unwrap(); - assert_eq!(false, db.key_may_exist("nonexistent")); - assert_eq!( - false, - db.key_may_exist_opt("nonexistent", &ReadOptions::default()) - ); + assert!(!db.key_may_exist("nonexistent")); + assert!(!db.key_may_exist_opt("nonexistent", &ReadOptions::default())); } } @@ -998,11 +995,8 @@ fn key_may_exist_cf() { let db = DB::open_cf(&opts, &path, &["cf"]).unwrap(); let cf = db.cf_handle("cf").unwrap(); - assert_eq!(false, db.key_may_exist_cf(&cf, "nonexistent")); - assert_eq!( - false, - db.key_may_exist_cf_opt(&cf, "nonexistent", &ReadOptions::default()) - ); + assert!(!db.key_may_exist_cf(&cf, "nonexistent")); + assert!(!db.key_may_exist_cf_opt(&cf, "nonexistent", &ReadOptions::default())); } } diff --git a/tests/test_merge_operator.rs b/tests/test_merge_operator.rs index 5068519..1e75f19 100644 --- a/tests/test_merge_operator.rs +++ b/tests/test_merge_operator.rs @@ -115,7 +115,7 @@ fn test_counting_full_merge( operands: &MergeOperands, ) -> Option> { let mut counts = existing_val - .map(|v| ValueCounts::from_slice(v)) + .map(ValueCounts::from_slice) .flatten() .unwrap_or_default(); diff --git a/tests/test_raw_iterator.rs b/tests/test_raw_iterator.rs index b9efbab..f48e5d3 100644 --- a/tests/test_raw_iterator.rs +++ b/tests/test_raw_iterator.rs @@ -32,13 +32,13 @@ pub fn test_forwards_iteration() { let mut iter = db.raw_iterator(); iter.seek_to_first(); - assert_eq!(iter.valid(), true); + assert!(iter.valid()); assert_eq!(iter.key(), Some(b"k1".as_ref())); assert_eq!(iter.value(), Some(b"v1".as_ref())); iter.next(); - assert_eq!(iter.valid(), true); + assert!(iter.valid()); assert_eq!(iter.key(), Some(b"k2".as_ref())); assert_eq!(iter.value(), Some(b"v2".as_ref())); @@ -46,7 +46,7 @@ pub fn test_forwards_iteration() { iter.next(); // k4 iter.next(); // invalid! - assert_eq!(iter.valid(), false); + assert!(!iter.valid()); assert_eq!(iter.key(), None); assert_eq!(iter.value(), None); } @@ -65,13 +65,13 @@ pub fn test_seek_last() { let mut iter = db.raw_iterator(); iter.seek_to_last(); - assert_eq!(iter.valid(), true); + assert!(iter.valid()); assert_eq!(iter.key(), Some(b"k4".as_ref())); assert_eq!(iter.value(), Some(b"v4".as_ref())); iter.prev(); - assert_eq!(iter.valid(), true); + assert!(iter.valid()); assert_eq!(iter.key(), Some(b"k3".as_ref())); assert_eq!(iter.value(), Some(b"v3".as_ref())); @@ -79,7 +79,7 @@ pub fn test_seek_last() { iter.prev(); // k1 iter.prev(); // invalid! - assert_eq!(iter.valid(), false); + assert!(!iter.valid()); assert_eq!(iter.key(), None); assert_eq!(iter.value(), None); } @@ -97,14 +97,14 @@ pub fn test_seek() { let mut iter = db.raw_iterator(); iter.seek(b"k2"); - assert_eq!(iter.valid(), true); + assert!(iter.valid()); assert_eq!(iter.key(), Some(b"k2".as_ref())); assert_eq!(iter.value(), Some(b"v2".as_ref())); // Check it gets the next key when the key doesn't exist iter.seek(b"k3"); - assert_eq!(iter.valid(), true); + assert!(iter.valid()); assert_eq!(iter.key(), Some(b"k4".as_ref())); assert_eq!(iter.value(), Some(b"v4".as_ref())); } @@ -122,7 +122,7 @@ pub fn test_seek_to_nonexistant() { let mut iter = db.raw_iterator(); iter.seek(b"k2"); - assert_eq!(iter.valid(), true); + assert!(iter.valid()); assert_eq!(iter.key(), Some(b"k3".as_ref())); assert_eq!(iter.value(), Some(b"v3".as_ref())); } @@ -140,14 +140,14 @@ pub fn test_seek_for_prev() { let mut iter = db.raw_iterator(); iter.seek(b"k2"); - assert_eq!(iter.valid(), true); + assert!(iter.valid()); assert_eq!(iter.key(), Some(b"k2".as_ref())); assert_eq!(iter.value(), Some(b"v2".as_ref())); // Check it gets the previous key when the key doesn't exist iter.seek_for_prev(b"k3"); - assert_eq!(iter.valid(), true); + assert!(iter.valid()); assert_eq!(iter.key(), Some(b"k2".as_ref())); assert_eq!(iter.value(), Some(b"v2".as_ref())); }