-
Notifications
You must be signed in to change notification settings - Fork 895
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
Bump some deps #3394
Bump some deps #3394
Conversation
Rustin170506
commented
Jun 30, 2023
- Bump the openssl v0.10.52 -> v0.10.55 to address the security alert. https://github.com/rust-lang/rustup/security/dependabot
- Bump proc-macro2 v1.0.51 -> v1.0.63 to make rustup complile. See Delete use of proc_macro::Span::before/after dtolnay/proc-macro2#391.
Signed-off-by: hi-rustin <[email protected]>
Signed-off-by: hi-rustin <[email protected]>
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.
Should we update here as well?
Lines 99 to 104 in 137a99b
[dependencies.openssl] | |
# Used by `curl` or `reqwest` backend although it isn't imported by our rustup : | |
# this allows controlling the vendoring status without exposing the presence of | |
# the download crate. | |
optional = true | |
version = "0.10" |
I updated it with ➜ rustup git:(master) cargo tree -i -p openssl --target all
openssl v0.10.55
└── native-tls v0.2.11
├── hyper-tls v0.5.0
│ └── reqwest v0.11.17
│ └── download v1.26.0 (/Volumes/t7/code/rustup/download)
│ └── rustup v1.26.0 (/Volumes/t7/code/rustup)
├── reqwest v0.11.17 (*)
└── tokio-native-tls v0.3.1
├── hyper-tls v0.5.0 (*)
└── reqwest v0.11.17 (*) We only have one version Thanks for your review! 💚 💙 💜 💛 ❤️ |
Yep but it depends on whether or not we allow openssl older 0.10.55 to be built with rustup. Some pros:
The counterargument is obviously we then force everyone use 0.10.55 or newer. |
I'd say it must be pretty rare that people build rustup on their own, but still there AFAIK. |
For this case, the CI may help us to avoid this kind of situation.
We already checked the lock file in our repo. Do you mean users will use it as a package? |
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.
Yes. As a dependency. Cargo usually tries to make them in sync for the manifest and the lockfile. It's not really a big deal. So go ahead!
The use of rustup as a dependency should be rare. Thank you for your review. |