Skip to content
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

Status of unused dependencies #223

Closed
lebensterben opened this issue Apr 14, 2021 · 9 comments
Closed

Status of unused dependencies #223

lebensterben opened this issue Apr 14, 2021 · 9 comments
Labels
help wanted Extra attention is needed maintenance

Comments

@lebensterben
Copy link
Member

lebensterben commented Apr 14, 2021

There are a few unused dependencies

  • openssl_sys
  • ring

ring is supposed to solve build issue on Apple M1 architecture. But is it still needed?
Someone mentioned that exa had the issue and ring can solve it. But how? There's no
use ring in the entire crate. Also, I don't see anything mentioned about ring, either
in its manifest, or in the homebrew recipe: https://github.com/Homebrew/homebrew-core/blob/HEAD/Formula/exa.rb

Also, ring at least should be gated behind a cfg attribute for darwin only.

Similarly, openssl_sys is never used. Could that be safely removed?

@mre
Copy link
Member

mre commented Apr 14, 2021

hubcaps depends on jsonwebtoken 7, which depends on ring, which is not ready for Apple Silicon. The advice is to explicitly depend on a newer version which does. (See cargo tree --invert ring in lychee.)

openssl_sys can't build on macOS with the openssl bindings (issue), which is why we use ring to statically link SSL support.

At least that's what I think is going on. It's all a bit of a mess. 🤷‍♀️

@mre
Copy link
Member

mre commented Apr 14, 2021

But I'd be more than happy to remove that or at least put it behind a gate.
Also, maybe we should be good OSS citizens and get send PRs to the deps in question.

@lebensterben
Copy link
Member Author

lebensterben commented Apr 14, 2021

Also I think we should have a new feature flag serde that controls whether serde is enabled for dependencies.

Since lychee is used in CI, I want to cut as much dependency and compile time as possible.

@mre
Copy link
Member

mre commented Apr 14, 2021

Good idea. 👍

@lebensterben
Copy link
Member Author

lebensterben commented Apr 15, 2021

As for ring and openssl_sys, I confirm that they cannot be built for aarch-apple-darwin target.
hubcaps requires ring. So one simple solution is to replace it with github_rs with rust-native-tls feature.

@mre
Copy link
Member

mre commented Apr 26, 2021

Can we close this?

@lebensterben
Copy link
Member Author

not really..
They're not added to optional dependencies yet.

@mre
Copy link
Member

mre commented Feb 4, 2022

This has been in the queue for a while. @lebensterben do you want to tackle this? I personally don't have the resources right now. 😕

@mre mre added help wanted Extra attention is needed maintenance labels Feb 4, 2022
@mre
Copy link
Member

mre commented Mar 28, 2022

Closing this to keep the issue tracker clean. @lebensterben, feel free to reopen if you're planning to work on this. 😉

@mre mre closed this as completed Mar 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed maintenance
Projects
None yet
Development

No branches or pull requests

2 participants