-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Build error on nightly-2017-12-21 #513
Comments
Unfortunately the new nightly has broken Until then, you'll need to stick with the latest working nightly |
The compiler change that caused ring's build to fail was intentional. Unfortunately,
These issues were fixed in As this point, I do not know of a path towards fixing Rocket on the latest nightlies. I will post updates on this issue as I have them. |
Quote by the author:
Pretty unhelpful and, in my opinion, a bit unprofessional, as the relevant work has already been done by others. |
Now i just self cloned jsonwebtoken (it uses ring="0.12.0") crate cause of ring's version. |
@SergioBenitez thanks for giving lengthy explanations and reasoning both here and in the various This is admittedly a combination of bad factors (a bug in rustc, being forced to nightly due to rocket, strong opinions of a maintainer...). I've already had to do regular dependency juggling with various projects due to ring though , and @briansmith s stances, while I'm sure they have a merit, make stable software development hard. Even if I didn't need nightly... I want my one year old projects to build on a new stable and I also don't want to constantly be constrained in dependency version updates due to one crate tying down everyone else.. Personally, I'll try to avoid |
Workaround for rwf2/Rocket#513, but the real blame goes to briansmith/ring#605 for not accepting the PR.
This works around #513 by patching 'ring' globally using the new '[patch]' Cargo section.
Given the previous discussion here and a seemingly hectic environment from the scale of this project, this comments might not be appreciated. But from a security standpoint, I do understand his firm stance on supporting only a single version. There has been at least one fix of a CVE issue as far as I can see¹, possibly more which I don't spot on first glance. With limited time, the author might be very uncomfortable with encouraging support for potentially insecure version of a crypto library. This, of course, isn't very helpful to the progress of this issue. But given that it is open source afterall, and forking can be done pretty much effortlessly, I think the outright spite coming here is disrespectful and very much out of place. @SirRade @SergioBenitez Thanks for your work and keeping it professional and constructive. ¹ Seems like this had not affected the library in any way, but it potentially could have. I made this point simply to show that non-linear versioning with backports can have security implications. Several linux distros are able to keep this up only as a result of dedicated people specifically maintaining high-quality backports. |
I guess you are referring to the code we imported from BoringSSL regarding CVE-2017-3736. CVE-2017-3736 didn't affect ring (nor did it affect BoringSSL) and those changes were just imported so they didn't get lost. In the future we might enable that optimization but only after more verification has been done; the verification process is actually what found that vulnerability in the first place. In any case, I wrote down the versioning story for ring at https://github.com/briansmith/ring#versioning--stability. I'm sure I'd already told the policy to one or more of the Rocket maintainers before so I don't think it's a surprise. It's actually the same policy that BoringSSL upstream has, more-or-less. It's OK if people don't like the policy but it's really not productive to keep debating it over and over. Basically dealing with the version number bumps is the price of ring. I hope that we've all made ring valuable enough where people think that price is fair; if we haven't, then I'd still rather increase ring's value than decrease its price. In any case, everybody using the same, newest, version is valuable to everybody on its own; it's sort of a herd immunity. |
classy |
As a result the origin error from cookie crate which use lower ring version, should bump to newest ring version |
The policy isn't a surprise. What is truly surprising is that you are unwilling to make an exception to your policy when it is likely warranted. In this particular instance, a mistake in
The issue isn't that crates want to lag behind using older versions of
Discussing a recurring issue with a multitude of implications feels like a productive use of conversation to me. Open discourse is at the core of open source software. Stifling it is antithetical. |
@HeroicKatora very good input, completely agree. @briansmith I definitely see your concern. I hope you see that we are not attacking you or your policy in general. I think we can all agree that keeping a consistent versioning policy is a very good thing for all participants in an Open Source environment and that we are all thankful for your work in both this regard and the |
According to https://crates.io/crates/rocket/reverse_dependencies there are less than 30 crates on crates.io that depend on Rocket. It turns out I did an experiment where I sent PRs to many ring-based projects to upgrade their dependencies. One tweet about it is at https://twitter.com/BRIAN_____/status/862017683054747648. In the end I think I updated close to 40 crates to the newest version of ring and/or Rustls and/or tokio-rustls. Most of the PRs were merged within 24 hours and almost all of them were merged within a week. This is just one person spending ~3 hours total effort. Based on that experiment I feel confident that it is reasonable to simply ask people to use the latest version of ring and update all the transitive dependencies. So that's what I recommend you do. Meanwhile I'll use the time I save from exiting this discussion to play with my small daughter and add new improvements to ring, webpki, and Rustls. Cheers! |
briansmith/ring#575 is a duplicate of briansmith/ring#535. See my response at briansmith/ring#535 (comment). |
Most projects which use For anyone else coming here because they're having the same issue, until all dependencies update to
Alternatively, with a couple tweaks from @SergioBenitez we'll be able to use the # Cargo.toml
[dependencies]
rocket = "0.3.5"
rocket_codegen = "0.3.5"
+ [replace]
+ "ring:0.11.0" = { git = "https://github.com/SergioBenitez/ring", branch = "v0.11" }
+ "ring:0.12.0" = { git = "https://github.com/SergioBenitez/ring", branch = "v0.12" } It's an ugly hack, but hopefully using the Instructions for @SergioBenitezFor the
|
In order to use @SergioBenitez's tweaks without changing the version number, we should be able to use the
I've tested this on the latest nightly, and it compiles no problem. |
This works around #513 by patching 'ring' globally using the new '[patch]' Cargo section.
I've pushed changes to the - rocket = "0.3.5"
+ rocket = { git = "https://github.com/SergioBenitez/Rocket", branch = "v0.3" }
- rocket_codegen = "0.3.5"
+ rocket_codegen = { git = "https://github.com/SergioBenitez/Rocket", branch = "v0.3" }
- rocket_contrib = "0.3.5"
+ rocket_contrib = { git = "https://github.com/SergioBenitez/Rocket", branch = "v0.3" }
+ [patch.crates-io]
+ ring = { git = "https://github.com/SergioBenitez/ring", branch = "v0.11" } Now that @briansmith has reconsidered his position on briansmith/ring#575 , I'm actively working on fixing Thank you, everyone, for your patience and support as we've worked through this issue. It's incredible to see such a supportive community forming around Rocket. |
Thank you for working so diligently to provide a short and long term solution. And also thanks for everything else. |
I am using the error: failed to run custom build command
I think this is related to briansmith/ring#523 |
@messense I pushed a commit to my |
Any idea when the official release will be for the workaround mentioned above in regarding briansmith/ring#575 now that stable 01-04-18 is out? The current git workaround is really messy on windows MSVC because it has to build ring from the git; the library uses YASM and perl to build through git but comes with precompiled libraries through cargo naively. No pressure of course, just curious because I would like to use rocket in a tutorial series of mine. |
@tensor-programming The PR is in review (briansmith/ring#619). As soon as it's accepted by @briansmith and a new version of |
Thank you @SergioBenitez for responding so quickly and thank you for your efforts. |
i just want to thank Sergio for his efforts - it saved my day. |
It would be nice to have these instructions in the README. Rocket has been broken for three weeks now (and it is forseeable that the PR on ring won't be merge anytime soon (i.e. it's going to take a few days / weeks to resolve errors)). This is not good. It's not nice to have to dive into issues in order to figure out how to build the library. My build error:
For anyone coming from Google: You can fix it by putting this in the Cargo.toml and deleting the Cargo.lock file: [dependencies]
rocket = { git = "https://github.com/SergioBenitez/Rocket", branch = "v0.3" }
rocket_codegen = { git = "https://github.com/SergioBenitez/Rocket", branch = "v0.3" }
rocket_contrib = { git = "https://github.com/SergioBenitez/Rocket", branch = "v0.3" }
[patch.crates-io]
ring = { git = "https://github.com/SergioBenitez/ring", branch = "v0.11" } This was already said before, I just wanted to comment again, so people can more easily copy-paste it in their Cargo.toml files. Should Rocket be broken longer, please put this in the README. Thank you for your effort. |
I agree, it should be the first thing in the Readme, it took me hours to figure out how to fix my build a few days ago when I updated my nightly, because I had to update rocket to the latest fix. (I even had to install perl and yasm on Windows to build FWIW, this is what I settled with, (with nightly 01-08) and it works: rocket = { git = "https://github.com/SergioBenitez/Rocket", version = "0.4.0-dev" }
rocket_codegen = { git = "https://github.com/SergioBenitez/Rocket", version = "0.4.0-dev" }
rocket_contrib = { git = "https://github.com/SergioBenitez/Rocket", version = "0.4.0-dev" }
cookie = { git = "https://github.com/alexcrichton/cookie-rs", rev = "a15b37a", features = ["percent-encode", "secure"] } Because I need that cookie SameSite fix (being able to set SameSite to None). And in my workspace root Cargo.toml: [patch.crates-io]
ring = { git = "https://github.com/SergioBenitez/ring", branch = "v0.12" } Is it a good idea to use But now, how can I |
@Boscop |
Yeah, that's what I did. |
It should be possible to override any indirect dependency with |
Adding a crates.io [patch] as @jplatte suggested should work. I had to do the same thing in my project to fix some issues with Maud (trait impls broke when I had two versions of Rocket) https://github.com/awalcutt/WeddingWebsite/blob/master/Cargo.toml#L14 |
Today's ( Thank you all, again, for your support! |
`juniper_rocket` now requires nightly >= 2018-01-12. See rwf2/Rocket#513 (comment). Fixes graphql-rust#125.
`juniper_rocket` now requires nightly >= 2018-01-12. See rwf2/Rocket#513 (comment). Fixes #125.
Remove workaround for rwf2/Rocket#513
The text was updated successfully, but these errors were encountered: