feat: replace reqwest with attohttpc #999
Conversation
|
reqwest is already an optional dependency. See PR 979 |
True and I made that pull request myself but lightening the load when https is enabled in is still beneficial. |
Cargo.toml
Outdated
| unicode-width = "0.1.7" | ||
| textwrap = "0.11.0" | ||
| term_size = "0.3.1" | ||
|
|
||
| # OpenSSL causes problems when building a MUSL release. |
There was a problem hiding this comment.
Oof that's hairy. I wish there was a more concise way to address this. 😰
There was a problem hiding this comment.
In #933 @q66 mentions everything builds fine for them with MUSL. Is the trouble related with MUSL and openssl related to the CI setup or is it something else?
Should I start a test run to see if MUSL builds fine with openssl on CI now? Beyond that do you know how well the rust openssl crate handles different openssl library versions and locations across different systems these days? I remember having trouble with that back in the day.
There was a problem hiding this comment.
yes, no issues building on musl (x86_64 or any other architecture) on my system when using openssl-sys, it just dynamically links against that and works fine
i think the issue for you might be that upstream rust musl builds prefer static linkage by default, while we override this in Void to use dynamic linkage by default; that may result in the openssl crate building its own openssl rather than linking system's...
so, maybe make the condition something like cfg(target_feature = "crt-static")? if using crt-static, use rustls, otherwise use native-tls... then see if it builds on CI, and I can test if it builds here.
Ideally, it'd be good to make this overridable too, to force either of these options even if the default is different.
There was a problem hiding this comment.
i think the issue for you might be that upstream rust musl builds prefer static linkage by default, while we override this in Void to use dynamic linkage by default; that may result in the openssl crate building its own openssl rather than linking system's...
Well I just tried cross-compiling to FreeBSD and I just the same error as when I tried to compile to MUSL (Could not find directory of OpenSSL installation which apparently means it needs a cross-compiled openssl). This means that the error is likely not related to crt-static (and isn't that anly about the c runtime and not other dependencies?). I suppose vendoring openssl could be a fix as opposed it to being the issue.
Ideally, it'd be good to make this overridable too, to force either of these options even if the default is different.
Not sure how this could be solved. Is it possible to do feature-aware dependency specification?
There was a problem hiding this comment.
Yes, if you cross compile you obviously need OpenSSL/libressl for target in the cross root, that works fine though (we build all ARM packages as crosscompiled in Void and there are no issues)
There was a problem hiding this comment.
I think the MUSL builds here are mainly intended for maximum platform compablity so I guess vendoring works fine here.
There was a problem hiding this comment.
It makes sense as an option, i don't think musl is otherwise special though
There was a problem hiding this comment.
Yes. I added a disabled-by-default tls-vendored feature in the recent commits that gets enabled only in the CI MUSL release builds.
|
Oopsie my bad. Sorrry I did not saw your name in that PR @davidkna |
* upstream/master: build(deps): bump regex from 1.3.4 to 1.3.5 refactor: Fix new Clippy suggestions (#1006) build(deps): bump sysinfo from 0.11.6 to 0.11.7
Description
reqwestis a heavy dependency. Replace it with the lighter alternativeattohttpc(-35% binary size on windows release build).Also (fixes #933):
Opt to use
native-tlsinstead ofrustlson cpu architectures not supported byrustls(architectures other than ARM and x86) and on MacOS/iOS and Windows because it's shrinks binary size (at least on Windows) and doesn't appear to cause any trouble there.Motivation and Context
Context #844 #895
Types of changes
Screenshots (if appropriate):
How Has This Been Tested?
Checklist: