v1.7.0 breaking changes
#143
Labels
Comments
|
Check this out: https://docs.rs/rustls/latest/rustls/crypto/struct.CryptoProvider.html#using-the-per-process-default-cryptoprovider Let me know if that doesn't work and we can go from there. If it does work we can add that one liner to the examples. |
|
@RoloEdits can you share full build errors on windows? To see if due to dependencies |
|
The full link error(seems like its messing up the pathing to the Visual Studio path?): |
|
Adding rustls = "0.23"and using let crypto_provider = rustls::crypto::aws_lc_rs::default_provider();
crypto_provider.install_default().unwrap();Seems to have fixed the runtime panic. |
|
@RoloEdits , apparently it is due to the space in the path "/LIBPATH:C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\MSVC\14.41.34120\atlmfc\lib\x64", unfortunately, I do not have window env to try this. is it only with tls enabled, right? Can you build the dependent rustls in windows? |
|
Hmm, even without |
|
if you enable verbose build, do you see any difference in linking phase between release and debug build? |
|
Yes, I was running Caused by: |
|
Hi in release build you see the output of env LIBPATH? |
|
I think the workaround could be you install microsoft visual studio on a path without space |
|
It seems like this should be split up into two issues: the build failing and the runtime rustls failure. Updating my project from |
|
see above comments that below has to call |
This is still a breaking change, so |
|
OK, I would yank the 1.70 and make a v2 release |
Regarding the runtime error, I disagree, we are following rustls's recommendations per their documentation on how to handle this. |
|
@LockedThread , can we make this call inside our tls setup, so that user does not need to do that? Because cargo can auto upgrade to new version, api-wise, it is backward compatible, but some new deps need to document |
Does it really matter what rustls says? My application is not using rustls (directly), but amqprs is. The update from |
|
I treat backward-compatible seriously, so I think better to make it v2.0.0 to follow semantic versioning |
The point of the CryptoProvider system is so that people could swap out different providers. What if I need to use a FIPS 140 compliant crypto library but because amqprs is saying use system default I am now screwed. Per the rustls specification on CryptoProvider, it is the application's responsibility to setup the CryptoProvider; not the library. If we do this I will have to maintain a fork for my use cases as we need to be 140-2 compliant. |
Do that |
|
This might provide a good middle ground: seanmonstar/reqwest#2225 Please also look at this: https://github.com/rustls/hyper-rustls/blob/main/examples/server.rs |
|
@LockedThread , thanks, I did not follow all threads here, seems quite a bit subtles to handle, do you have suggestions? |
gftea
added
dependencies
|
Thanks ❤️ |
|
@gftea Sorry for responding late, the linker error doesn't happen with the default linker. Only when using: [target.x86_64-pc-windows-msvc]
linker = "lld-link.exe"It still only happens with the new version of the crate though, which is weird. And thanks for the SemVer adherence. Things can be tricky in this realm, as Semver doesn't cover everything, so thanks for going beyond it's guarantees. |
|
By the way, are you considering windows GNU toolchain support? Unfortunately, most libraries don't focus on it, including AWS Libcrypto (that is a huge pain to build) which is used by rustls, which leaves Windows users with the need for a proprietary license at best and often with hours of trying to conform to sometimes hidden dependency requirements. P.S. If anyone knows a way to build amqprs with "tls" on Windows GNU, please share, because I'm yet to see a successful build with mingw64 ucrt gcc and nasm. |
wez
added a commit
to KumoCorp/kumomta
that referenced
this issue
Sep 17, 2024
I was going to run cargo update, but was blocked because amqprs 1.7 got yanked (gftea/amqprs#143) and re-released as 2.0.
|
This seems to be enough to trigger it: [package]
name = "foo"
version = "0.1.0"
edition = "2021"
[dependencies]
amqprs = { version = "2.0.0", default-features = false, features = ["tls", "urispec"] }
reqwest = { version = "0.12.7", default-features = false, features = ["rustls-tls"] }
tokio = { version = "1.40.0", features = ["full"] }use std::error::Error;
use amqprs::connection::{Connection, OpenConnectionArguments};
#[tokio::main]
async fn main() -> Result<(), Box<dyn Error>> {
let response = reqwest::get("https://example.org").await?;
println!("{response:?}");
let url = std::env::var("AMQP_URL")?;
let args = OpenConnectionArguments::try_from(url.as_str())?;
let connection = Connection::open(&args).await?;
println!("{}", connection.connection_name());
Ok(())
}When So |
|
Another problem: This must also be a problem in |
|
can you do cargo tree to see which dep use |
Unfortunately, I have no windows env and have no time to think about it, so to be honest, no plan to support windows |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
After updating to
1.7.0from1.6.3Windows fails to build with a linking error:This is the
Cargo.toml:Usage:
On
Alpine Linux, it builds properly, but panics with:Given that it wasn't updated to
2.0.0, I assume that SemVer is still maintained this behavior is not intended?The text was updated successfully, but these errors were encountered: