[libcxx-commits] [PATCH] D89813: [CMake][NFC] Limit the use of uppercase_CMAKE_BUILD_TYPE

Tue Oct 20 12:15:18 PDT 2020

ctetreau marked an inline comment as done.
ctetreau added a comment.

In D89813#2342463 <https://reviews.llvm.org/D89813#2342463>, @ldionne wrote:

> I quite like this. We should strive to use the same name as CMake uses for its build types, i.e. `Debug`, `Release`, etc.
>
> If I configure CMake with `-DCMAKE_BUILD_TYPE=DEBUG`, is `CMAKE_BUILD_TYPE` normalized back to `Debug` by CMake, or is it defined to `DEBUG`? If the latter, then I suggest we might want to add a temporary error message when the `CMAKE_BUILD_TYPE` is defined to something all-uppercase. This would help catch cases where this patch will be a change in behavior. That would be a nice place to tell users to use the right build type names.

My interpretation of https://cmake.org/cmake/help/v3.13/variable/CMAKE_BUILD_TYPE.html, is that it can be the empty string, or CamelCase. However, experimentation shows that passing `-DCMAKE_BUILD_TYPE=RELEASE` results in an all-capitals version. Given this, I don't think this patch is correct.

It looks like we had case-sensitive CMAKE_BUILD_TYPE, but it was removed in 2015 (https://github.com/llvm/llvm-project/commit/74009be05158c72893e02c2930b771a334543f1b), so making it case sensitive again is probably going to be controversial.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D89813/new/

https://reviews.llvm.org/D89813