-
Notifications
You must be signed in to change notification settings - Fork 2.7k
"cmake -DMBEDTLS_CONFIG_FILE" does not *install* the specified mbedtls_config.h #9947
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
Comments
Good point! But with the added wrinkle that it isn't enough to install the alternative config file. Unless you pass I'm not sure how to fully resolve this. I wonder what is the right way to resolve this. Inject the setting into the installed Note that this applies to |
(I'm not sure whether to label this “bug” or “enhancement”. We mostly intended this for the convenience of developer builds and test scripts, and I guess we just didn't think about usage outside the development team. But once the possibility is there, it is natural to expect it to work together with installation.) |
To be more specific, here is the use case:
Notice that the user has no option to set This is a bug, not an enhancement request. When cmake builds mbedtls with a certain configuration C, it is imperative that the installed |
This is only the case when you don't pass compiler flags that change the meaning of the header files. And passing If you pass There is also a CMake variable with the name Using both I do agree that the current situation is not good though. But if we don't find a better solution, we may resolve it by throwing an error on |
By the way, we don't have a lot of CMake expertise (although we're learning). If you (or anyone) knows how to do this right, we would welcome a patch. |
What would be the use case for building mbedtls with cmake and not using the install phase of the build? It seems fairly hypothetical. We're not willing to include mbedtls defines in all downstream projects, so we are simply adding a post-install step to clobber the incompatible |
I think it is just the case that in |
fixes Mbed-TLS#9947 Signed-off-by: Salem Bouraoui <[email protected]>
Simply to say, the file given by |
Only if Same concern with Are these preprocessor macros fundamentally incompatible with installation? |
From the description, the suppilied CMake file build creates the library. Then the library is installed - to be used later without this library build script.
Unlimited number of ways to cause a failure could be invented. By the way, there was my feature request to provide a standard place for user-defined files. If alternative configuration was using other (non-standard) locations, the files could be copied to the standard place. At least, this would provide a copy of original files. |
Summary
In cmake, option
-DMBEDTLS_CONFIG_FILE=my_mbedtls_config.h
makes that file take the place ofinclude/mbedtls/mbedtls_config.h
. However, during cmakeinstall
, the override is ignored and the originalinclude/mbedtls/mbedtls_config.h
is copied into the build area.System information
Mbed TLS version (number or commit id):
2ca6c285a0dd3f33982dd57299012dacab1ff206
(though no change is apparent on the defaultdevelopment
branch)Operating system and version: Fedora 41
Configuration (if not default, please attach
mbedtls_config.h
): (not relevant)Compiler and options (if you used a pre-built binary, please indicate how you obtained it): (not relevant)
Additional environment information:
Expected behavior
When the user specifies an alternative
mbedtls_config.h
, it should not only be used in the build, but also be installed. Otherwise thembedtls_config.h
in the installed includes is not consistent with the build it corresponds to.Actual behavior
cmake copies the original
include/mbedtls/mbedtls_config.h
instead of the one that was actually used to produce the corresponding build.Steps to reproduce
Set flag
-DMBEDTLS_CONFIG_FILE
to any file other thaninclude/mbedtls/mbedtls_config.h
and observe that in fact it is the latter file which is installed, not the former.Additional information
The text was updated successfully, but these errors were encountered: