bazel: remove CMAKE_BUILD_TYPE override#17679
Conversation
rules_foreign_cc sets this automatically to either Debug or Release based on bazel's compilation_mode. Overriding this can cause issues when deps' cmake configs expect to be in a specific subset of configs. Unlike LLVM most cmake configs seem to silently fail in that case by not setting an expected var correctly. I hit this issue when updating rules_foreign_cc (I'm not sure of why the update broke it) and wamr missing some settings for the custom Bazel config. Fixes: envoyproxy#17106 Signed-off-by: Keith Smiley <keithbsmiley@gmail.com>
repokitteh-read-only
bot
added
the
deps
|
Looks like the original reason for adding this is still valid, will try again after #17445 lands if it works without this |
| cmake_files_dir = "$BUILD_TMPDIR/CMakeFiles", | ||
| generate_crosstool_file = False, | ||
| **kwargs): | ||
| cache_entries.update({"CMAKE_BUILD_TYPE": "Bazel"}) |
There was a problem hiding this comment.
Does RELWITHDEBINFO work?
|
This fix is -not- correct. Bazel rules_foreign_cc provided all necessary flags to cmake in order to build the targets in exactly the same configuration as envoy itself, which is why a custom, no-op flavor target is used. |
|
This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions! |
github-actions
bot
added
the
stale
|
Makes sense, I could see this causing more issues in the future though. I was thinking that either:
|
3 participants
rules_foreign_cc sets this automatically to either Debug or Release
based on bazel's compilation_mode. Overriding this can cause issues when
deps' cmake configs expect to be in a specific subset of configs. Unlike
LLVM most cmake configs seem to silently fail in that case by not
setting an expected var correctly. I hit this issue when updating
rules_foreign_cc (I'm not sure of why the update broke it) and wamr
missing some settings for the custom Bazel config.
Fixes: #17106
Signed-off-by: Keith Smiley keithbsmiley@gmail.com