wolfssl icon indicating copy to clipboard operation
wolfssl copied to clipboard

Published 20 hours ago • verified publisher icon wolfSSL

cmake: fix generation of options.h

Open oltolm opened this issue 9 months ago • 2 comments

Description

I fixed the way options.h is generated. It is now being generated properly using configure_file. Fixes #4489.

Testing

I built wolfssl with CMake.

oltolm avatar May 16 '24 17:05 oltolm

Can one of the admins verify this patch?

wolfSSL-Bot avatar May 16 '24 17:05 wolfSSL-Bot

Thank you @oltolm , I will review. Contributor agreement on file. Okay to test.

dgarske avatar May 16 '24 17:05 dgarske

Hi @oltolm,

What is this PR actually fixing? It seems to just change the way options.h is generated to use configure_file and adds additional maintenance. I checked and the current method for generating options.h works fine and doesn't require maintaining a list of macros.

Are there benefits to using configure_file?

Thanks, David Garske, wolfSSL

dgarske avatar May 28 '24 19:05 dgarske

Yes. This is from the official documentation:

If the input file is modified the build system will re-run CMake to re-configure the file and generate the build system again. The generated file is modified and its timestamp updated on subsequent cmake runs only if its content is changed.

If the options.h does not change, you don't have to rebuild your project. Also using configure_file is the standard way of doing things in CMake.

oltolm avatar May 28 '24 20:05 oltolm

Hi @oltolm ,

Thank you for the feedback. I'll review this with the engineering team tomorrow and made a decision.

Thanks, David Garske, wolfSSL

dgarske avatar May 29 '24 15:05 dgarske

Hi @oltolm ,

We discussed this internally provided a few items to adjust. Then this PR should be acceptable. Thank you again for your work on this improvement.

Thank you, David Garske, wolfSSL

dgarske avatar May 30 '24 22:05 dgarske