[libcxx-commits] [libcxx] Reverts around time_zone (PR #95058)
Louis Dionne via libcxx-commits
libcxx-commits at lists.llvm.org
Tue Jun 11 13:29:24 PDT 2024
ldionne wrote:
I think that's a false dilemma.
My understanding is that there are three failures reported:
1. Fuchsia: https://ci.chromium.org/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8745592240286102897/overview (assertion failure)
2. Windows/Linux cross builders: https://lab.llvm.org/buildbot/#/builders/60/builds/17468 (assertion failure)
3. HWasan: https://lab.llvm.org/buildbot/#/builders/236/builds/11695 (timeout)
The first two issues don't have a path to being resolved because the reporters have not provided a ways to reproduce the failures or investigated them. Therefore, they are not grounds for asking a revert of the patch. This is especially true given that TZDB is a library that is inherently coupled with the system in use, which means that system differences and quirks are intended. This means that the only viable option might very well be to `XFAIL` the test on the platforms that are failing because they e.g. have an incomplete or malformed time zone database. Of course we can't know until someone has investigated on these machines.
The last issue is a timeout. The fact that you run a build bot with instrumentation that greatly slows down runtime tests and can cause them to timeout is also not grounds to revert the patch. The right solution here is to mark the test as unsupported under that configuration, or to use a faster machine, or to make the tests faster, or to improve hwasan to have less runtime impact. It doesn't make the code that was committed wrong as in "incorrect". To further explain my point: if I enable a CI machine that is vastly underpowered and causes a bunch of tests to start timing out, whose fault is it? Can I then go to patch authors and ask them to revert their patches under the Dev Policy because of my bot? I think we'll all agree that it doesn't make sense. Instead, *I* should figure out a way to make my bot robust to longer-running tests and take it upon myself to deal with the implications of running such a bot.
So, to summarize: When we ship a bug and we can't quickly fix it forward, we revert. We all agree on that, it's the LLVM way. However, many things don't qualify as "a bug" yet still break CI. This is a prime example of such a situation, and I am really exhausted of folks trying to strong-arm others into reverting patches blindly just to get their bots green again with minimal effort while citing the LLVM policy.
@vitalybuka Is there even still a problem? You marked the test as unsupported under hwasan, so it's not running under your configuration. Did that work? A better fix would be to use `--param long_tests=False` to disable all the slow-running tests in the suite instead, and to mark the `local_time` test as `REQUIRES: long_tests`.
https://github.com/llvm/llvm-project/pull/95058
More information about the libcxx-commits
mailing list