Skip to content

Improve support for custom OpenSSL selection#5848

Open
masa-koz wants to merge 4 commits intomicrosoft:mainfrom
masa-koz:masa-koz/custom-openssl
Open

Improve support for custom OpenSSL selection#5848
masa-koz wants to merge 4 commits intomicrosoft:mainfrom
masa-koz:masa-koz/custom-openssl

Conversation

@masa-koz
Copy link
Copy Markdown
Contributor

@masa-koz masa-koz commented Mar 7, 2026

Description

This pull request introduces improved support for building with an external OpenSSL installation, primarily by adding a new openssl_external feature and enhancing the build system to handle more flexible OpenSSL configuration. The changes affect CMake configuration, Rust feature flags, and build scripts to better support both static and shared linking, and to allow specifying OpenSSL paths via environment variables.

Build system and CMake improvements:

  • Added a new CMake cache variable QUIC_OPENSSL_ROOT_DIR to allow specifying the OpenSSL root directory, and updated OpenSSL selection logic to support this variable for both static and shared builds. [1] [2]
  • Enhanced the CMake logic to properly select between static and shared OpenSSL libraries based on the BUILD_SHARED_LIBS flag, and improved handling of imported OpenSSL targets to avoid duplicate dependency linking.

Rust feature and build script updates:

  • Introduced a new Cargo feature openssl_external for building with an external OpenSSL, and updated the build script (scripts/build.rs) to configure CMake accordingly by passing the appropriate variables from environment variables if set. [1] [2]
  • Adjusted the output directory logic in the build script to use lib on Windows and artifacts elsewhere, improving compatibility with different platforms.

Test and platform compatibility:

  • Updated test logic in src/rs/config.rs to recognize the new openssl_external feature when determining which TLS provider is active on Windows.

Testing

No

Documentation

TBD

@masa-koz masa-koz requested a review from a team as a code owner March 7, 2026 10:37
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 9, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.86%. Comparing base (45a46f3) to head (a9bf108).
⚠️ Report is 21 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5848      +/-   ##
==========================================
- Coverage   86.30%   84.86%   -1.45%     
==========================================
  Files          60       60              
  Lines       18732    18732              
==========================================
- Hits        16167    15897     -270     
- Misses       2565     2835     +270     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Collaborator

@guhetier guhetier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a general comment, this start to be a lot of options to handle OpenSSL.
I would be interesting to find a way to keep the complexity from getting out of hands, although I don't have a solution in mind, and it doesn't have to be in this PR.

@masa-koz
Copy link
Copy Markdown
Contributor Author

@guhetier I'm sorry for my late response. I addressed your comments.

@masa-koz
Copy link
Copy Markdown
Contributor Author

@guhetier I've fixed the format. Please make rerun CI.

@masa-koz
Copy link
Copy Markdown
Contributor Author

masa-koz commented Apr 4, 2026

@guhetier What should I do before merge?

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.

2 participants