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

Reduce install friction for macOS users #149

Open
mleonhard opened this issue Nov 27, 2020 · 2 comments
Open

Reduce install friction for macOS users #149

mleonhard opened this issue Nov 27, 2020 · 2 comments
Labels
environment good first issue Good for newcomers help wanted Extra attention is needed
Milestone

Comments

@mleonhard
Copy link

The build fails on macOS 10.15.7:

$ cargo install cargo-geiger
...
  It looks like you're compiling on macOS, where the system contains a version of
  OpenSSL 0.9.8. This crate no longer supports OpenSSL 0.9.8.

  As a consumer of this crate, you can fix this error by using Homebrew to
  install the `openssl` package, or as a maintainer you can use the openssl-sys
  0.7 crate for support with OpenSSL 0.9.8.

Many macOS users do not install Homebrew. "As a maintainer" sounds like I would need to download the source code and modify Cargo.toml to specify the different dependency. This is a extra work.

It turns out that on macOS, one can simply add --feature vendored-openssl to build successfully. I saw that, but didn't know what "vendored" meant. I assumed it was a special third-party SSL library that one would buy from a vendor.

I think this experience could be improved a lot by preventing the build error. And if it can't be prevented, then prevent confusion. Ideas for doing this:

  1. Switch to rustls. OpenSSL is a security risk.
  2. Require users to explicitly choose which OpenSSL library to use. Add a local-openssl feature.
  3. Rename vendored-openssl to a term that is understood by more engineers. Suggestion: openssl-crate
  4. Update the readme to state that vendored-openssl feature uses "OpenSSL from the openssl-sys crate" and builds on macOS.
  5. Automatically use the crate-bundled OpenSSL library on macOS.
  6. Update the build error to say "Run cargo install cargo-geiger --features vendored-openssl to build with OpenSSL from the openssl-sys crate."

I will prepare a PR, if you wish. Please let me know if you want to address this issue and what you prefer.

@anderejd
Copy link
Contributor

anderejd commented Nov 29, 2020

Many macOS users do not install Homebrew. "As a maintainer" sounds like I would need to download the source code and modify Cargo.toml to specify the different dependency. This is a extra work.

I'm one of those users that don't use Homebrew as well :)

It turns out that on macOS, one can simply add --feature vendored-openssl to build successfully. I saw that, but didn't know what "vendored" meant. I assumed it was a special third-party SSL library that one would buy from a vendor.

This information is available at the top of the readme file, did you find this information somewhere else?

I think this experience could be improved a lot by preventing the build error. And if it can't be prevented, then prevent confusion. Ideas for doing this:

Yes, getting the OpenSSL error message is a garbage user experience. cargo-geiger is currently using cargo as a library and the relentless @jmcconnell26 is chipping away at #16 to move us towards a future where we are no longer depending on cargo in this way. Also related to #69.

Switch to rustls. OpenSSL is a security risk.

I like this direction, but that means fully decoupling from cargo (#16 + #69) or making cargo switch to rustls or native-tls which would also be very cool imho.

Require users to explicitly choose which OpenSSL library to use. Add a local-openssl feature.

If we can make this explicit only for macOS users that would be fine with me. Let's not make the experience worse for everyone else.

Rename vendored-openssl to a term that is understood by more engineers. Suggestion: openssl-crate

No, this cargo feature name should mirror the same feature name in the upstream library cargo:

https://github.com/rust-lang/cargo/blob/12c107ade5a3b173cdac5d06c8ba92429fe93c74/Cargo.toml#L120, this will make it easier to search for both in source code and on google.

Update the readme to state that vendored-openssl feature uses "OpenSSL from the openssl-sys crate" and builds on macOS.

A PR with README.md improvements would be most welcome.

Automatically use the crate-bundled OpenSSL library on macOS.

This suggestion was discussed in #99 and we opted for the explicit cargo feature instead.

Update the build error to say "Run cargo install cargo-geiger --features vendored-openssl to build with OpenSSL from the openssl-sys crate."

This would be a nice improvement for the user experience on macOS! If you would like to make a PR for this, that would be great! Make sure to preserve the current install behaviour for other platforms, cargo install cargo-geiger should work on linux and windows.

@tarcieri
Copy link
Collaborator

No, this cargo feature name should mirror the same feature name in the upstream library cargo

+1 to this. We use the same feature name for cargo-audit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
environment good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants