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

Fix #8292: allow building curl when wolfssl configured with --enable-opensslextra #8315

Closed
wants to merge 4 commits into from

Conversation

harrysarson
Copy link
Contributor

@harrysarson harrysarson commented Jan 21, 2022

This MR puts all the #includes of openssl files behind wolfssl #ifdefs so that we can use the wolfssl/ prefixed include paths.

Without these curl only builds when wolfssl is built with enable-all. Running CI against the first commit should fail showing why the fix is useful.

fixes #8292

harrysarson added 3 commits Jan 21, 2022
put all #include of openssl files behind wolfssl ifdefs so that
we can use the wolfssl/ prefixed include paths. Without these
curl only builds when wolfssl is built with enable-all.
@bagder
Copy link
Member

@bagder bagder commented Jan 21, 2022

Without these curl only builds when wolfssl is built with enable-all

I don't think this is exactly true (you just needed more options than what you used and you didn't test other combos than --enable-all), but I also don't think it matters a lot.

@@ -35,7 +35,7 @@ jobs:
tar -xzf v5.0.0-stable.tar.gz
cd wolfssl-5.0.0-stable
./autogen.sh
./configure --enable-tls13 --enable-all --enable-harden --prefix=$HOME/wssl
./configure --enable-tls13 --enable-opensslextra --enable-harden --prefix=$HOME/wssl
Copy link
Member

@bagder bagder Jan 21, 2022

Choose a reason for hiding this comment

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

I don't think you should remove the old --enable-all build but I think adding an --enable-opensslextra is a good ideas. Note that the second build is a lesser wolfSSL build and for example NTLM will not be enabled in your curl build using that.

Copy link
Contributor Author

@harrysarson harrysarson Jan 21, 2022

Choose a reason for hiding this comment

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

I have had a go at splitting the wolfssl CI job in two to test the two configurations.

@harrysarson
Copy link
Contributor Author

@harrysarson harrysarson commented Jan 21, 2022

I don't think this is exactly true (you just needed more options than what you used and you didn't test other combos than --enable-all), but I also don't think it matters a lot.

Yep, good point. I guess the correct version of the statement is: this PR reduces the number of things you have to enable when configuring wolfssl in order to to build curl.

bagder
bagder approved these changes Jan 21, 2022
@bagder
Copy link
Member

@bagder bagder commented Jan 21, 2022

Thanks!

@harrysarson harrysarson deleted the harry/wolfssl-builds branch Jan 21, 2022
@harrysarson
Copy link
Contributor Author

@harrysarson harrysarson commented Jan 24, 2022

Thank you too!

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