Skip to content

fix: update build instructions for provider#243

Open
jenstopp wants to merge 2 commits intomainfrom
update-provider-readme
Open

fix: update build instructions for provider#243
jenstopp wants to merge 2 commits intomainfrom
update-provider-readme

Conversation

@jenstopp
Copy link
Collaborator

@jenstopp jenstopp commented Mar 6, 2026

No description provided.

Copilot AI review requested due to automatic review settings March 6, 2026 10:17
@jenstopp
Copy link
Collaborator Author

jenstopp commented Mar 6, 2026

@rajesh-gali had issues building and found this was the reason in his scenario.
@walterchris could you maybe provide context for why we initially put it there?

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the OpenSSL provider build documentation in plugins/ossl_prov/README.md.

Changes:

  • Simplifies the azihsm_api_native build command by removing OPENSSL_DIR / OPENSSL_STATIC environment variables.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comment on lines +87 to +89
# 3. Build the provider with static OpenSSL
OPENSSL_DIR=/opt/openssl-static OPENSSL_STATIC=1 \
cargo build -p azihsm_ossl_provider --features mock
Copy link

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

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

The new step text says the provider is built “with static OpenSSL”, but the build system for the provider uses CMake find_package(OpenSSL 3 REQUIRED) and links ${OPENSSL_CRYPTO_LIBRARY}. The OPENSSL_DIR/OPENSSL_STATIC environment variables set here won’t affect CMake’s OpenSSL selection (they’re for openssl-sys), so as written this is misleading and may confuse readers about what is actually static vs dynamic. Consider either (a) rewording step 3 to clarify the env vars are to ensure azihsm_api_native (built via Corrosion) links statically, while the provider uses the system OpenSSL, or (b) if the provider truly must link to the static OpenSSL, pass the appropriate CMake variables (e.g., OPENSSL_ROOT_DIR / OPENSSL_USE_STATIC_LIBS) and update the instructions accordingly.

Copilot uses AI. Check for mistakes.
@walterchris
Copy link
Collaborator

I'm a bit confused that this resolves it.

The reason we built the library against the static OpenSSL is that we want to resolve the circular dependency (OpenSSL -> Provider -> Library -> OpenSSL -> Provider -> ... ).

Building the provider against static OpenSSL will also work - not sure what issue it solves for @rajesh-gali .

@rajesh-gali : Can you give me more details on the error that you saw?

I never had to do this and I use Dockerfile's that also do not build the provider against static OpenSSL and everything works fine.

@jenstopp
Copy link
Collaborator Author

We should also update the OpenSSL version number

1. Build a static OpenSSL

OPENSSL_VERSION=3.0.16

to 3.0.3

Signed-off-by: Jens Topp <jens.topp@9elements.com>
@jenstopp jenstopp mentioned this pull request Mar 12, 2026
@jenstopp
Copy link
Collaborator Author

Also add a section on how to run the integration tests locally

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants