-
Notifications
You must be signed in to change notification settings - Fork 58
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
chore(deps): update reqwest #769
Conversation
I'll look into the failed unit tests, sorry... I didn't spot them locally |
It's an unfortunate lint that we have using Cargo Deny. The lint causes an error when two major versions of a dependency exist in Cargo.lock. To fix the lint, the list of ignored dependencies has to be updated in Line 69 in eb5e25e
Line 16 in eb5e25e
|
@webern I've updated |
@webern sorry, this should be fixed now |
Can you also turn off auto-formatting of TOML files and restore them so that the PR has the minimum possible diff? We prefer to avoid auto-formatting churn in our git history. Thank you! |
@webern sorry about that, I fixed it. Moreover, |
Looks good, thank you for dealing with our |
Sorry to bother you, do you have any ETA about when you plan to merge this PR and even tag a new release of the crate? 🙏 |
Sorry for the radio silence here. I was concerned about the number of duplicated crates that were being pulled in. I'm going to poke at this early next week and see how things look once we bump smithy and aws-sdk-rust. Do you mind if I add your commit to that larger dependency update PR, or would you rather rebase this PR on top of it? |
Thanks for looking into that! I'm fine with you adding this commit to a larger dependency PR |
After bumping some things in #781, I still couldn't seem to get out of the situation where we pull two versions of |
I need tough to consume latest version of |
tough/Cargo.toml
Outdated
@@ -22,7 +22,7 @@ log = "0.4" | |||
olpc-cjson = { version = "0.1", path = "../olpc-cjson" } | |||
pem = "3" | |||
percent-encoding = "2" | |||
reqwest = { version = "0.11", optional = true, default-features = false, features = ["stream"] } | |||
reqwest = { version = "0.12", optional = true, default-features = false, features = ["stream"] } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reqwest = { version = "0.12", optional = true, default-features = false, features = ["stream"] } | |
reqwest = { version = "0.11", optional = true, default-features = false, features = ["stream"] } | |
reqwest-hyper-v1 = { package = "reqwest", version = "0.12", optional = true, default-features = false, features = ["stream"] } |
I wonder if we could do this and separate them by feature. Crates stuck on the old hyper would continue to use http
and those on the new could use a http-hyper-v1
. Curious if @bcressey or @cbgbt have thoughts around this as well.
From the crate description:
This will be required soon for FIPS support in |
Hello, is there any update on bumping the hyper/reqwest? This is blocking the release of |
Sigstore's TUF repository format changed in way that breaks the tough crate. This commit updates to a forked release of the tough crate that includes fixes for these issues: * awslabs/tough#778 - the fix for the TUF repo change * awslabs/tough#769 - another long standing issue Signed-off-by: Flavio Castelli <[email protected]>
@flavio, sorry about the delay on this. I'm still trying to get some answers on |
Update to latest version of reqwest, v0.12. This version depends on hyper v1, while previous versions (like v0.11) rely on hyper v0.14. This hyper upgrade is massive, having a unified version of hyper inside of projects consuming the tough crate is beneficial. For example, hyper testing frameworks can work only with one version of the library at a time. `cargo deny` had to be configured to ignore some duplicated crates. The reqwest update is massive, is the pinnacle of a major update of hyper. A lot of hyper dependencies received major bumps too. Some dependencies of tough (including development only ones), have not been updated to consume these new crates. Signed-off-by: Flavio Castelli <[email protected]>
Signed-off-by: Flavio Castelli <[email protected]>
4cd7203
to
c903eaa
Compare
@jpculp I think using the feature flag is the only way to go, and it works (I tested it). I've force pushed the following changes:
Once you're happy about the changes I'll squash the 2 commits into a single one. I just wanted to make easy to understand the amount of changes requested by the feature flag. |
Fantastic news folks, because of a happy coincidence I finally understood why sigstore About this PRThe Sigstore project doesn't need any bump of reqwest by tough. We can solve the issue all on our own (more details below). Having tough bump reqwest would still be nice, but this commit is definitely not needed. There might still be value inside of the 1st commit, because of the updates to Feel free to close this PR. The root cause and the fixIt can be found here |
@flavio, that's fantastic news! On our side we're actually working on the code to jump to |
It's still in progress, but we're working on the |
Good news. We merged #826 and should be ready for |
@jpculp, @ginglis13 thanks for having handled that. I'm sorry, but the last days have been pretty busy for me |
Issue #, if available:
The current version of tough depends on an older version of reqwest which in cause a old version of hyper to be consumed.
This hyper upgrade is massive, having a unified version of hyper inside of projects consuming the tough crate is beneficial. For example, hyper testing frameworks can work only with one version of the library at a time.
Description of changes:
Update to latest version of reqwest, v0.12. This version depends on hyper v1, while previous versions (like v0.11) rely on hyper v0.14.