Skip to content

CMake: improve build scripts#6331

Merged
JacobBarthelmeh merged 9 commits intowolfSSL:masterfrom
oltolm:cmake
May 17, 2023
Merged

CMake: improve build scripts#6331
JacobBarthelmeh merged 9 commits intowolfSSL:masterfrom
oltolm:cmake

Conversation

@oltolm
Copy link
Copy Markdown
Contributor

@oltolm oltolm commented Apr 22, 2023

Description

Improve CMake build scripts. WolfSSL is used by rpcs3, so some problems with the CMake build scripts became apparent.

  • WolfSSL needs the following target_compile_definitions(wolfssl INTERFACE SIZEOF_LONG=${SIZEOF_LONG} SIZEOF_LONG_LONG=${SIZEOF_LONG_LONG}) to compile with Mingw-w64 if WOLFSSL_FAST_MATH=ON. I fixed it in types.h.
  • Linking with WolfSSL fails because it needs to link against crypt32.lib.
  • The default for WOLFSSL_BUILD_OUT_OF_TREE is OFF, which means that config.h is generated in the source tree and is invisible because it is ignored by git. I changed the default to out-of-tree-build and removed config.h from .gitignore.
  • The build scripts do not export WolfSSL which makes it hard for other projects to use. Now you can use find_package(WolfSSL). To properly export WolfSSL i changed add_definitions to target_compile_definitions.

Testing

I compiled WolfSSL as part of rpcs3 and stand-alone.

@wolfSSL-Bot
Copy link
Copy Markdown

Can one of the admins verify this patch?

@dgarske
Copy link
Copy Markdown
Member

dgarske commented Apr 24, 2023

Hi @oltolm ,

Thank you for these CMake fixes. In order to accept this we'll need a signed contributor agreement. Can you email support@ wolfssl dot com and mention this PR and provide some additional details about your project and location?

Thanks,
David Garske, wolfSSL

@dgarske dgarske self-assigned this Apr 24, 2023
@oltolm
Copy link
Copy Markdown
Contributor Author

oltolm commented Apr 29, 2023 via email

@dgarske
Copy link
Copy Markdown
Member

dgarske commented May 8, 2023

Signed contributor agreement received and approved.

Copy link
Copy Markdown
Member

@dgarske dgarske left a comment

Choose a reason for hiding this comment

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

Great fixes here. Just one comment about the location of the new cmake file.

Comment thread Config.cmake.in
@dgarske
Copy link
Copy Markdown
Member

dgarske commented May 11, 2023

OK to test

Copy link
Copy Markdown
Member

@dgarske dgarske left a comment

Choose a reason for hiding this comment

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

Just one small change then it is ready for merge. Thank you @oltolm

Comment thread .gitignore Outdated
Comment thread cmake/Config.cmake.in
Copy link
Copy Markdown
Member

@dgarske dgarske left a comment

Choose a reason for hiding this comment

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

Over to @JacobBarthelmeh to finalize. Please squash on merge.

@dgarske dgarske assigned JacobBarthelmeh and unassigned dgarske May 12, 2023
@dgarske
Copy link
Copy Markdown
Member

dgarske commented May 15, 2023

Retest this please

@JacobBarthelmeh JacobBarthelmeh merged commit 90b8584 into wolfSSL:master May 17, 2023
@oltolm oltolm deleted the cmake branch May 17, 2023 21:41
@JacobBarthelmeh
Copy link
Copy Markdown
Contributor

@oltolm thank you for your work on this! It is a good step in the right direction for cmake + mingw32 support. When building wolfSSL were you able to successfully run the crypto unit tests? Using ./wolfcrypt/test/testwolfcrypt.exe. When testing locally with the mingw32 + cmake build I was running into some SHA384 failures.

@oltolm
Copy link
Copy Markdown
Contributor Author

oltolm commented May 17, 2023

I use the mingw32 and mingw64 subsystems of MSYS2. If I just build with default CMake options then the test runs successfully in mingw32 and mingw64.

joeftiger pushed a commit to joeftiger/wolfssl that referenced this pull request Sep 2, 2023
* make wolfssl compile with Mingw-w64

* cmake: CMAKE_SYSTEM_PROCESSOR is AMD64 on Windows

* cmake: use target_compile_definitions instead of add_definitions

* cmake: change default value of WOLFSSL_BUILD_OUT_OF_TREE_DEFAULT to ON

* cmake: link crypt32.lib on Windows

* cmake: export wolfssl

* move Config.cmake.in to cmake directory

* revert changes to .gitignore

* add Config.cmake.in to include.am
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.

4 participants