awslabs / mls-rs

An implementation of Messaging Layer Security (RFC 9420)
Apache License 2.0
105 stars 19 forks source link

[crypto-rustcrypto]: remove unused custom feature #161

Closed mgeisler closed 2 months ago

mgeisler commented 5 months ago

Description of changes:

We don’t actually use the “custom” feature of the getrandom crate. We don't have the feature enabled in all the places where I'm trying to use mls-rs, so it would be nice if this crate doesn't require it.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 and MIT license.

codecov-commenter commented 5 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 89.63%. Comparing base (9b55525) to head (68eaa11).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #161 +/- ## ========================================== - Coverage 89.65% 89.63% -0.02% ========================================== Files 175 175 Lines 31578 31578 ========================================== - Hits 28310 28305 -5 - Misses 3268 3273 +5 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

mulmarta commented 5 months ago

Seems to be used by embedded build @mgeisler. Maybe you could add this?

[target.'cfg(not(feature = "std"))'.dependencies]
getrandom = { version = "0.2", default-features = false, features = ["custom"] }
mgeisler commented 3 months ago

Thanks @mulmarta for the debugging! I finally came back to this and I think I understand what the problem is. Based on reading register_custom_getrandom!, I now believe it was wrong for the mls-rs-crypto-rustcrypto library to enable this feature.

It should instead be enabled by the final binary. I've updated the CI script to do this and I hope it'll work now.