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

Conditional include for openssl engines #828

Merged
merged 1 commit into from
Jul 29, 2024

Conversation

thalman
Copy link

@thalman thalman commented Jul 29, 2024

xmlsec can't be compiled on systems where openss/engine.h is missing. In some distributions this file is in extra package (Fedora) or not present at all.

This update makes the include conditional based on configure option --enable_openssl3_engines.

@thalman
Copy link
Author

thalman commented Jul 29, 2024

Hi Guys,

If you agree with this, I will make the same for master branch.

Tom

@beldmit
Copy link

beldmit commented Jul 29, 2024

Probably it's better to check for OPENSSL_NO_ENGINE. Or both.

@thalman thalman force-pushed the opensslengine_12 branch 2 times, most recently from f44532f to 745de36 Compare July 29, 2024 12:48
@thalman
Copy link
Author

thalman commented Jul 29, 2024

I updated the condition according other places in source code. I now checks both defines as well as openssl API version.

@thalman
Copy link
Author

thalman commented Jul 29, 2024

Checking API version was bad idea. It does not work with older openssl libraries correctly. Removed.

src/openssl/app.c Outdated Show resolved Hide resolved
@thalman
Copy link
Author

thalman commented Jul 29, 2024

I believe that the condition is correct now.

Tom

@lsh123 lsh123 merged commit 5545aff into lsh123:xmlsec-1_2_x Jul 29, 2024
17 checks passed
@lsh123
Copy link
Owner

lsh123 commented Jul 29, 2024

yep, looks great! Thanks for catching it and working on multiple iterations of the fix!

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