Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update workspace feature resolver version to 2 #375

Merged
merged 3 commits into from
Apr 11, 2022

Conversation

zeegomo
Copy link
Contributor

@zeegomo zeegomo commented Apr 8, 2022

Version 1 of feature resolver always causes features unification of
build and dev dependencies, which is unwanted in some circumstances.
For example, the dev feature of the digest crate always depends on
alloc which requires to define a global allocator in static libs
even though that is never used by library code.

This might be useful in other repos as well

Version 1 of feature resolver always causes features unification of
build and dev dependencies, which is unwanted in some circumstances.
For example, the `dev` feature of the `digest` always depends on
`alloc` which requires to define a global allocator in static libs
even though that is never used by library code.
@newpavlov
Copy link
Member

newpavlov commented Apr 8, 2022

Isn't it default for 2021 edition crates?

UPD: Ah, I forgot we haven't migrated crates in this repository yet.

@zeegomo
Copy link
Contributor Author

zeegomo commented Apr 9, 2022

Seems like the clippy job is failing because the toolchain used is an old one which does not yet support the new resolver on master. Is this (or migrating to the 2021 edition) something you would be interested in given a MSRV bump is likely needed?

@tarcieri
Copy link
Member

tarcieri commented Apr 9, 2022

You can bump clippy to whatever version is needed (and really we should probably keep it close to stable to maximize the lints it performs)

@zeegomo
Copy link
Contributor Author

zeegomo commented Apr 10, 2022

I've bumped clippy to 1.60 to get the latest lints. This uncovered a bunch of warnings which I have currently fixed at the best of my judgement without deep knowledge of the codebase, maybe those should be better handled in a different PR.
However, the resolver entry in Cargo.toml is ignored by all MSRV builds, even though that does not cause any failures.

@@ -15,7 +15,7 @@ pub use self::simdty::{u32x4, u64x4};
pub trait Vector4<T>: Copy {
fn gather(src: &[T], i0: usize, i1: usize, i2: usize, i3: usize) -> Self;

fn from_le(self) -> Self;
fn from_le(vec: Self) -> Self;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why this was changed, but it's breaking the build:

https://github.com/RustCrypto/hashes/runs/5969024622?check_suite_focus=true#step:3:352

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clippy complained about a from_* method taking self as a parameter, so I changed it to be an associated function but forgot to update it everywhere

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can just disable the warning for now. FWIW we plan on switching to a different implementation eventually: #228

@tarcieri tarcieri merged commit dfcfb21 into RustCrypto:master Apr 11, 2022
@tarcieri
Copy link
Member

Thanks!

This might be useful in other repos as well

We have it enabled in certain repos, but if you see others that don't have it on, feel free to add it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants