[libcxx-commits] [PATCH] D99093: [libcxx] [ci] Add a Windows CI buildkite configuration

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxx/utils/ci/run-buildbot:430-437
+    # -D_LIBCPP_HAS_NO_INT128 allows building filesystem with a MSVC
+    # setup that lacks the necessary builtins for int128.
+
+    # -loldnames is needed for some tests unless using Clang 12 (the
+    # CI runner has Clang 11 currently).
+
+    # -DLIBCXX_ENABLE_EXPERIMENTAL_LIBRARY=NO is needed, as the testsuite
----------------
mstorsjo wrote:
> ldionne wrote:
> > Shouldn't we fix those issues instead of working around them? Since we're soft-failing the runner anyway, we could add the job without those work arounds and let it soft-fail until we've fixed all issues?
> You mean the experimental lib issue, or all the rest?
> 
> The int128 in msvc envs issue is a bit bigger (I'm not planning on working on that one as I mainly focus on mingw environments, and it's quite unclear how to fix best) - the lack of int128 precision doesn't matter for ABI here, and it only affects like 1 test in one file at most, so I'd like to have it tested this way for maximal CI coverage, so we can test fs even in msvc environments.
> 
> The experimental lib issue makes every single testcase fail to link, as things stand right now. I can look into it (it might be a pretty small thing) though, but the override here can be removed once that's fixed, too.
I meant all of it.

re: `int128`, can't we detect it properly automatically? We already try to detect it in `__config` but it looks like we're failing to detect it properly in this instance.

re: `-DLIBCXX_ENABLE_EXPERIMENTAL_LIBRARY=NO`, I think we should fix the underlying issue instead.

re: `-loldnames`, I think it's OK to wait until the CI upgrades to Clang 12

If you think those are unreasonable to wait for, I could be convinced to ship as-is. The only thing is that I dislike shipping something in a state where we know for sure we'll have to change in the future *without leaving a marker for us to find it again in the future*.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99093