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

[Martin Storsjö via Phabricator via libcxx-commits]

mstorsjo added inline comments.


================
Comment at: libcxx/utils/ci/buildkite-pipeline.yml:282
+    # The CI runner doesn't have bash in the path currently
+    command: "\"\\Program Files\\Git\\usr\\bin\\bash\" libcxx/utils/ci/run-buildbot generic-win"
+    artifact_paths:
----------------
ldionne wrote:
> How far are we from fixing this?
This runner uses the same base image as the main llvm premerge checks, and adding bash to the path there breaks the llvm tests. (The breakage is due to odd/untested combinations of windows containers and the posix emulation layer used by bash.) It's fixed on the main branch, but the same runner also tests release branches, so it needs backporting to all old releases it might test. So hopefully no biggie, but might take a little while.


================
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
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99093