[libcxx-commits] [PATCH] D91574: [libc++] Simplify how we pick the typeinfo comparison

[Jonas Devlieghere via Phabricator via libcxx-commits]

JDevlieghere added a comment.

In D91574#2404340 <https://reviews.llvm.org/D91574#2404340>, @ldionne wrote:

> You need to remove your build directory before running CMake. The issue is that CMake cache variables don't get overwritten with new values unless the cache is changed manually. If you don't remove the cache, it gets stale, and in this case, it's wrong.
>
> Generally speaking, any CI job that doesn't either remove the whole build directory and start from scratch, or at least remove the CMake cache, is bound to be flaky and break in this way. This is a problem of the build job, not of this change.
>
> Note that if you're trying to setup an incremental build and save on full rebuilds, I think it's better to use something like ccache than trying to keep the build directory around.

LLVM has a bunch of incremental bots and in addition to trying to be be fast, they also mirror what developers do when they pull. I can only speak for myself, but I certainly don't remove the CMake cache every time I do that. I've been involved in a bunch of CMake-related code reviews and as this is a pretty common problem, which I've seen people address by introducing a temporary CMake variable to let the bots (and developers) catch up. Is there any reason that couldn't have worked here?

FWIW if you disagree with how the bots deal with incremental builds, you're of course free to change that, but in the meantime the burden is still on the author of the patch and anyone could've reverted this in accordance with LLVM's "revert-to-green" policy.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91574