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

fixes #1768 -- integrate boringssl into the build process more naturally #1806

Closed
wants to merge 1 commit into from

Conversation

alex
Copy link
Collaborator

@alex alex commented Feb 2, 2023

This PR uses the in-development bindgen support for static inline functions (rust-lang/rust-bindgen#2335) + an in-development boringssl patch (https://boringssl-review.googlesource.com/c/boringssl/+/56505) to allow using boringssl with rust-openssl without needing a .cargo/config override

This is not mergable as-is (requires a bindgen release + boringssl to merge my patch), putting this up for visibility only.

@alex
Copy link
Collaborator Author

alex commented Feb 2, 2023

One substantive issue I see:

BoringSSL uses core::ffi for its types, which requires the 2018 edition for -sys. The existing -sys crate has use *; which is not compatible with 2018. I suppose the solution is to just replace all the use *;? Any blocker to that?

@sfackler
Copy link
Owner

sfackler commented Feb 3, 2023

Should be totally fine to update -sys to the 2018 edition and just change the import to use ::*; I think?

@alex
Copy link
Collaborator Author

alex commented Feb 3, 2023

Ok! I'll endeavour to do a stand-alone PR for that.

@sfackler
Copy link
Owner

sfackler commented Feb 3, 2023

Our MSRV is at 1.56.0 so we can probably go all the way to the 2021 edition really.

@alex
Copy link
Collaborator Author

alex commented Feb 3, 2023

pyca/cryptography still has a lower MSRV (1.48 ATM), so I want to avoid creating a hard dep on 2021.

@sfackler
Copy link
Owner

sfackler commented Feb 3, 2023

Very much in favor of cleaning up the bssl-sys setup, but obviously I'll defer to @davidben and @maurer here.

@sfackler
Copy link
Owner

sfackler commented Feb 3, 2023

If you're depending on a 1.48 MSRV then we should probably change the existing CI check (or add a new one?) to enforce that.

alex added a commit to alex/rust-openssl that referenced this pull request Feb 3, 2023
alex added a commit to alex/rust-openssl that referenced this pull request Feb 3, 2023
alex added a commit to alex/rust-openssl that referenced this pull request Feb 3, 2023
alex added a commit to alex/rust-openssl that referenced this pull request Feb 3, 2023
alex added a commit to alex/rust-openssl that referenced this pull request Feb 3, 2023
@alex alex force-pushed the boringssl-normal branch 2 times, most recently from e6b83a3 to 84dba6e Compare February 3, 2023 19:49
@pvdrz
Copy link

pvdrz commented Feb 7, 2023

This feature is now available on bindgen 0.64.0 🎉

@alex
Copy link
Collaborator Author

alex commented Feb 7, 2023

🎉 🎉 🎉 -- as soon as it's available in brew (Homebrew/homebrew-core#122575) I plan to rebase my boringssl CL and hopefully advance the conversation there.

…e naturally

This PR uses the in-development bindgen support for static inline functions (rust-lang/rust-bindgen#2335) + an in-development boringssl patch (https://boringssl-review.googlesource.com/c/boringssl/+/56505) to allow using boringssl with rust-openssl without needing a .cargo/config override
alex added a commit to alex/rust-openssl that referenced this pull request Mar 4, 2023
This allows building it without the bssl-sys crate. This is an alternative approach to fixing sfackler#1768 (in contrast to sfackler#1806).

This maintains support for using the bssl-sys crate.
alex added a commit to alex/rust-openssl that referenced this pull request Mar 4, 2023
This allows building it without the bssl-sys crate. This is an alternative approach to fixing sfackler#1768 (in contrast to sfackler#1806).

This maintains support for using the bssl-sys crate.
alex added a commit to alex/rust-openssl that referenced this pull request Mar 4, 2023
This allows building it without the bssl-sys crate. This is an alternative approach to fixing sfackler#1768 (in contrast to sfackler#1806).

This maintains support for using the bssl-sys crate.
alex added a commit to alex/rust-openssl that referenced this pull request Mar 4, 2023
This allows building it without the bssl-sys crate. This is an alternative approach to fixing sfackler#1768 (in contrast to sfackler#1806).

This maintains support for using the bssl-sys crate.
alex added a commit to alex/rust-openssl that referenced this pull request Mar 4, 2023
This allows building it without the bssl-sys crate. This is an alternative approach to fixing sfackler#1768 (in contrast to sfackler#1806).

This maintains support for using the bssl-sys crate.
alex added a commit to alex/rust-openssl that referenced this pull request Mar 4, 2023
This allows building it without the bssl-sys crate. This is an alternative approach to fixing sfackler#1768 (in contrast to sfackler#1806).

This maintains support for using the bssl-sys crate.
alex added a commit to alex/rust-openssl that referenced this pull request Mar 4, 2023
This allows building it without the bssl-sys crate. This is an alternative approach to fixing sfackler#1768 (in contrast to sfackler#1806).

This maintains support for using the bssl-sys crate.
alex added a commit to alex/rust-openssl that referenced this pull request Mar 10, 2023
This allows building it without the bssl-sys crate. This is an alternative approach to fixing sfackler#1768 (in contrast to sfackler#1806).

This maintains support for using the bssl-sys crate.
alex added a commit to alex/rust-openssl that referenced this pull request Mar 10, 2023
This allows building it without the bssl-sys crate. This is an alternative approach to fixing sfackler#1768 (in contrast to sfackler#1806).

This maintains support for using the bssl-sys crate.
@alex alex closed this Mar 15, 2023
@alex alex deleted the boringssl-normal branch March 15, 2023 00:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants