Skip to content

Commit

Permalink
build: fix clippy lints and MSRV check (#2680)
Browse files Browse the repository at this point in the history
Rust 1.72 beta introduced new clippy lints that are failing on CI, most
of them are easy to fix (remove ` as` conversion), but [the one
changing](https://rust-lang.github.io/rust-clippy/master/index.html#/arc_with_non_send_sync)
```
pub struct InnerStorage(Arc<Mutex<dyn Storage>>);
```
to
```
pub struct InnerStorage(Rc<RwLock<dyn Storage>>);
```
is a bit more involved. Still need to better evaluate how this will go
with threads, but since it is an opaque type we can also change the
internal representation without breaking semver.

#2629 introduced an MSRV
bump to 1.64 on the test dependencies, added an extra argument `--tests`
in the CI checks to verify test deps too.
  • Loading branch information
luizirber authored Jul 18, 2023
1 parent c7c320f commit caccb58
Show file tree
Hide file tree
Showing 14 changed files with 44 additions and 42 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/python.yml
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ jobs:

- name: Start Redis
if: startsWith(runner.os, 'Linux') && (matrix.py == '3.9')
uses: supercharge/redis-github-action@f63fe516254d0af5df91755a4488274c2e71e38c
uses: supercharge/redis-github-action@1.5.0
with:
redis-version: 6

Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -268,17 +268,17 @@ jobs:

- uses: actions-rs/toolchain@v1
with:
toolchain: "1.60.0"
toolchain: "1.64.0"
override: true

- name: check if README matches MSRV defined here
run: grep '1.60.0' src/core/README.md
run: grep '1.64.0' src/core/README.md

- name: Check if it builds properly
uses: actions-rs/cargo@v1
with:
command: build
args: --all-features
args: --all-features --tests

check_cbindgen:
name: "Check if cbindgen runs cleanly for generating the C headers"
Expand Down
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ members = [
"src/core",
]
default-members = ["src/core"]
resolver = "2"

[profile.test]
opt-level = 1
24 changes: 12 additions & 12 deletions flake.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 5 additions & 5 deletions flake.nix
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@
rustc = rustVersion;
};
naersk-lib = naersk.lib."${system}".override {
cargo = rustPlatform.rust.cargo;
rustc = rustPlatform.rust.rustc;
cargo = rustVersion;
rustc = rustVersion;
};

python = pkgs.python310Packages;
python = pkgs.python311Packages;

in

Expand Down Expand Up @@ -104,8 +104,8 @@

git
stdenv.cc.cc.lib
(python310.withPackages (ps: with ps; [ virtualenv tox cffi ]))
(python311.withPackages (ps: with ps; [ virtualenv ]))
(python311.withPackages (ps: with ps; [ virtualenv tox cffi ]))
(python310.withPackages (ps: with ps; [ virtualenv ]))
(python39.withPackages (ps: with ps; [ virtualenv ]))
(python38.withPackages (ps: with ps; [ virtualenv ]))

Expand Down
4 changes: 2 additions & 2 deletions src/core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ edition = "2021"
readme = "README.md"
autoexamples = false
autobins = false
rust-version = "1.60.0"
rust-version = "1.64.0"

[lib]
name = "sourmash"
Expand All @@ -31,6 +31,7 @@ cfg-if = "1.0"
counter = "0.5.7"
finch = { version = "0.5.0", optional = true }
fixedbitset = "0.4.0"
getrandom = { version = "0.2", features = ["js"] }
getset = "0.1.1"
log = "0.4.19"
md5 = "0.7.0"
Expand All @@ -57,7 +58,6 @@ criterion = "0.5.1"
needletail = { version = "0.5.1", default-features = false }
proptest = { version = "1.2.0", default-features = false, features = ["std"]}
rand = "0.8.2"
getrandom = { version = "0.2", features = ["js"] }
tempfile = "3.6.0"

[[bench]]
Expand Down
2 changes: 1 addition & 1 deletion src/core/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,4 @@ Development happens on github at

## Minimum supported Rust version

Currently the minimum supported Rust version is 1.60.0.
Currently the minimum supported Rust version is 1.64.0.
4 changes: 2 additions & 2 deletions src/core/src/ffi/cmd/compute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ pub unsafe extern "C" fn computeparams_ksizes_free(ptr: *mut u32, insize: usize)
if ptr.is_null() {
return;
}
Vec::from_raw_parts(ptr as *mut u32, insize, insize);
Vec::from_raw_parts(ptr, insize, insize);
}

ffi_fn! {
Expand All @@ -65,7 +65,7 @@ unsafe fn computeparams_set_ksizes(

let ksizes = {
assert!(!ksizes_ptr.is_null());
slice::from_raw_parts(ksizes_ptr as *const u32, insize)
slice::from_raw_parts(ksizes_ptr, insize)
};

cp.set_ksizes(ksizes.into());
Expand Down
8 changes: 4 additions & 4 deletions src/core/src/ffi/minhash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ pub unsafe extern "C" fn kmerminhash_slice_free(ptr: *mut u64, insize: usize) {
if ptr.is_null() {
return;
}
Vec::from_raw_parts(ptr as *mut u64, insize, insize);
Vec::from_raw_parts(ptr, insize, insize);
}

ffi_fn! {
Expand Down Expand Up @@ -232,7 +232,7 @@ unsafe fn kmerminhash_add_many(
// FIXME: make a SourmashSlice_u64 type?
let hashes = {
assert!(!hashes_ptr.is_null());
slice::from_raw_parts(hashes_ptr as *const u64, insize)
slice::from_raw_parts(hashes_ptr, insize)
};

for hash in hashes {
Expand Down Expand Up @@ -277,13 +277,13 @@ unsafe fn kmerminhash_set_abundances(
// FIXME: make a SourmashSlice_u64 type?
let hashes = {
assert!(!hashes_ptr.is_null());
slice::from_raw_parts(hashes_ptr as *const u64, insize)
slice::from_raw_parts(hashes_ptr, insize)
};

// FIXME: make a SourmashSlice_u64 type?
let abunds = {
assert!(!abunds_ptr.is_null());
slice::from_raw_parts(abunds_ptr as *const u64, insize)
slice::from_raw_parts(abunds_ptr, insize)
};

let mut pairs: Vec<_> = hashes.iter().cloned().zip(abunds.iter().cloned()).collect();
Expand Down
2 changes: 1 addition & 1 deletion src/core/src/ffi/nodegraph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ pub unsafe extern "C" fn nodegraph_buffer_free(ptr: *mut u8, insize: usize) {
if ptr.is_null() {
return;
}
Vec::from_raw_parts(ptr as *mut u8, insize, insize);
Vec::from_raw_parts(ptr, insize, insize);
}

#[no_mangle]
Expand Down
4 changes: 2 additions & 2 deletions src/core/src/ffi/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ unsafe fn zipstorage_list_sbts(
// FIXME: use the ForeignObject trait, maybe define new method there...
let ptr_sigs: Vec<*mut SourmashStr> = sbts
.into_iter()
.map(|x| Box::into_raw(Box::new(SourmashStr::from_string(x))) as *mut SourmashStr)
.map(|x| Box::into_raw(Box::new(SourmashStr::from_string(x))))
.collect();

let b = ptr_sigs.into_boxed_slice();
Expand All @@ -86,7 +86,7 @@ unsafe fn zipstorage_filenames(
// FIXME: use the ForeignObject trait, maybe define new method there...
let ptr_sigs: Vec<*mut SourmashStr> = files
.into_iter()
.map(|x| Box::into_raw(Box::new(SourmashStr::from_string(x))) as *mut SourmashStr)
.map(|x| Box::into_raw(Box::new(SourmashStr::from_string(x))))
.collect();

let b = ptr_sigs.into_boxed_slice();
Expand Down
2 changes: 1 addition & 1 deletion src/core/src/sketch/minhash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1655,7 +1655,7 @@ impl From<KmerMinHash> for KmerMinHashBTree {
let mins: BTreeSet<u64> = other.mins.into_iter().collect();
let abunds = other
.abunds
.map(|abunds| mins.iter().cloned().zip(abunds.into_iter()).collect());
.map(|abunds| mins.iter().cloned().zip(abunds).collect());

new_mh.mins = mins;
new_mh.abunds = abunds;
Expand Down
15 changes: 8 additions & 7 deletions src/core/src/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ use std::ffi::OsStr;
use std::fs::{DirBuilder, File};
use std::io::{BufReader, BufWriter, Read, Write};
use std::path::{Path, PathBuf};
use std::sync::{Arc, Mutex};
use std::rc::Rc;
use std::sync::RwLock;

use serde::{Deserialize, Serialize};
use thiserror::Error;
Expand All @@ -24,11 +25,11 @@ pub trait Storage {
}

#[derive(Clone)]
pub struct InnerStorage(Arc<Mutex<dyn Storage>>);
pub struct InnerStorage(Rc<RwLock<dyn Storage>>);

impl InnerStorage {
pub fn new(inner: impl Storage + 'static) -> InnerStorage {
InnerStorage(Arc::new(Mutex::new(inner)))
InnerStorage(Rc::new(RwLock::new(inner)))
}
}

Expand Down Expand Up @@ -85,20 +86,20 @@ impl From<&StorageArgs> for FSStorage {
}
}

impl<L> Storage for Mutex<L>
impl<L> Storage for RwLock<L>
where
L: ?Sized + Storage,
{
fn save(&self, path: &str, content: &[u8]) -> Result<String, Error> {
self.lock().unwrap().save(path, content)
self.read().unwrap().save(path, content)
}

fn load(&self, path: &str) -> Result<Vec<u8>, Error> {
self.lock().unwrap().load(path)
self.read().unwrap().load(path)
}

fn args(&self) -> StorageArgs {
self.lock().unwrap().args()
self.read().unwrap().args()
}
}

Expand Down
2 changes: 1 addition & 1 deletion tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ commands = pytest \
[testenv:khmer_master]
basepython = python3.8
deps =
-e git+/~https://github.com/dib-lab/khmer.git#egg=khmer
-e git+/~https://github.com/dib-lab/khmer.git\#egg=khmer
commands = pytest \
--cov "{envsitepackagesdir}/sourmash" \
--cov-config "{toxinidir}/tox.ini" \
Expand Down

0 comments on commit caccb58

Please sign in to comment.