Skip to content

Conversation

@honsunrise
Copy link

Description of changes:

Define CMAKE_BUILD_TYPE so that CMake can use the correct CMAKE_XXX_FLAGS_XXX.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

@honsunrise honsunrise requested a review from a team as a code owner November 12, 2025 03:28
@justsmth
Copy link
Contributor

Hello! Thanks for the PR. For CMAKE_BUILD_TYPE, we defer to the logic provided by cmake-rs here.

Is there a particular problem that you're trying to address with this change?

@honsunrise
Copy link
Author

Hello! Thanks for the PR. For CMAKE_BUILD_TYPE, we defer to the logic provided by cmake-rs here.

Is there a particular problem that you're trying to address with this change?

Maintain consistency with the logic in aws-lc-fips-sys, so that the CMAKE_BUILD_TYPE has a consistent definition under the same build profile (it seems unnecessary on Windows to require aws-lc-sys to follow "relwithdebinfo"). Of course, if this is not mandatory, following the logic of cmake-rs would seem more elegant.

However, another approach would be for aws-lc-fips-sys to migrate to the cmake-rs logic and handle the Windows case separately, which also seems better. If this is acceptable, I can modify this MR.

@justsmth
Copy link
Contributor

However, another approach would be for aws-lc-fips-sys to migrate to the cmake-rs logic and handle the Windows case separately, which also seems better. If this is acceptable, I can modify this MR.

Yeah, that logic in aws-lc-fips-sys is really only needed on Windows due to our FIPS build on Windows not supporting debug.

@honsunrise honsunrise force-pushed the feat/cmake-build-type branch from 71e08e2 to cd0ab51 Compare November 13, 2025 15:42
@honsunrise honsunrise changed the title Set CMAKE_BUILD_TYPE according to OPT_LEVEL Make aws-lc-fips-sys's config CMAKE_BUILD_TYPE logic same with aws-lc-sys Nov 13, 2025
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