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

Thu Nov 19 10:15:42 PST 2020

ldionne added a comment.

In D91574#2405977 <https://reviews.llvm.org/D91574#2405977>, @shafik wrote:

> In D91574#2405909 <https://reviews.llvm.org/D91574#2405909>, @ldionne wrote:
>
>> 
>
>
>
>> I don't disagree with how they deal with incremental builds -- I just think we should be fine with a bit of human intervention from time to time if we're going to assume the cache never gets invalidated. And at the same time, I suggested a technical approach (full build + `ccache`) to make the bots more robust so that these issues don't happen again in the future.
>
> If you foresee the need for possible human intervention maybe the discussion about that should have happened before the change was landed? Then at least folks would have been prepared to fix the problem right after the it landed, assuming they agreed on that approach.

TBH, I didn't know which bots, if any, would break. We've moved some of the incremental builds to ccache + full build in the past, and I don't have a full mental map of all the bots in LLVM (I'm sure you don't, either). What I meant above is that I didn't purposefully break the build because I didn't care -- I thought it was inevitable (and it's still not clear to me it's not), and I wasn't sure whether it would actually break bots. When asked whether there was a technical way to unbreak the cache, I replied "no", which is the answer I thought was correct at the time (@JDevlieghere can LMK if I got his suggestion right).

Is it possible we're making this way bigger than it needs to be? If there's an agreed-upon way of making these CMake cache-affecting changes, then let's document it and be done with it. I'll be happy to do it in the future. If there's no such technical way, let's fix the bots to make sure they're robust to these changes, and somehow find a way to avoid breaking people's build as well when they `git pull`.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91574