[libcxx-commits] [PATCH] D101565: [libcxx] [ci] Add a Windows CI configuration for a statically linked libc++

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added inline comments.
This revision is now accepted and ready to land.


================
Comment at: libcxx/utils/ci/run-buildbot:97-108
+    echo "--- Generating CMake"
+    cmake -S "${MONOREPO_ROOT}/libcxx" \
+          -B "${BUILD_DIR}" \
+          -GNinja -DCMAKE_MAKE_PROGRAM="${NINJA}" \
+          -DCMAKE_BUILD_TYPE=RelWithDebInfo \
+          -DCMAKE_C_COMPILER=clang-cl \
+          -DCMAKE_CXX_COMPILER=clang-cl \
----------------
curdeius wrote:
> mstorsjo wrote:
> > ldionne wrote:
> > > Could we instead do this to reuse as much between the Windows and non-Windows cases? I'm a bit worried that the two invocations are going to diverge with time.
> > > 
> > > ```
> > > generate-cmake -DCMAKE_CXX_FLAGS="-D_LIBCPP_HAS_NO_INT128" \
> > >                    -DLIBCXX_TEST_COMPILER_FLAGS="-D_LIBCPP_HAS_NO_INT128" \
> > >                    -DLIBCXX_ENABLE_FILESYSTEM=YES
> > > ```
> > > 
> > > Then, from below:
> > > 
> > > ```
> > > export CC=clang-cl
> > > export CXX=clang-cl
> > > generate-cmake-libcxx-windows -DOPTIONS
> > > ```
> > > 
> > Yeah we could do something like that.
> > 
> > I'd have to split up the existing `generate-cmake` into a base configuration usable for the windows target though - using the current `generate-cmake` as such doesn't work for the clang-cl targets for a number of reasons.
> > 
> > The current `generate-cmake` configures cmake pointing at the llvm subdir. It's possible to build libcxx for windows that way, but it requires adding the option `LLVM_FORCE_BUILD_RUNTIME` (see llvm/projects/CMakeLists.txt). Additionally the current `generate-cmake` adds libunwind and libcxxabi to the build; libunwind fails to cmake configure for clang-cl targets, and adding libcxxabi also isn't buildable as such for clang-cl targets. Finally `generate-cmake` also adds `LIBCXX_CXX_ABI=libcxxabi` which isn't right for our configuration.
> > 
> > (As an aside - would it be good to have a CI configuration where libcxx is built on top of another ABI library like libsupc++?)
> > 
> > I kinda also prefer using `LIBCXX_CXX_COMPILER=clang-cl` instead of `export CXX=clang-cl` even though both would work.
> > I kinda also prefer using `LIBCXX_CXX_COMPILER=clang-cl` instead of `export CXX=clang-cl` even though both would work.
> 
> +1.
> Also, it has a minor (because doesn't apply to CI) advantage that CMake won't let you change the compiler by mistake in an already created build dir when using `-DCMAKE_C_COMPILER`, but will happily discard a change in CC/CXX env. vars.
> Yeah we could do something like that.
> 
> I'd have to split up the existing `generate-cmake` into a base configuration usable for the windows target though - using the current `generate-cmake` as such doesn't work for the clang-cl targets for a number of reasons.

I see.

> 
> The current `generate-cmake` configures cmake pointing at the llvm subdir. It's possible to build libcxx for windows that way, but it requires adding the option `LLVM_FORCE_BUILD_RUNTIME` (see llvm/projects/CMakeLists.txt). Additionally the current `generate-cmake` adds libunwind and libcxxabi to the build; libunwind fails to cmake configure for clang-cl targets, and adding libcxxabi also isn't buildable as such for clang-cl targets. 

It's kind of lame that our Windows support is *that* bad.

> Finally `generate-cmake` also adds `LIBCXX_CXX_ABI=libcxxabi` which isn't right for our configuration.

Indeed.

> 
> (As an aside - would it be good to have a CI configuration where libcxx is built on top of another ABI library like libsupc++?)

Yes, I think it would be useful since we claim it's supported.

> I kinda also prefer using `LIBCXX_CXX_COMPILER=clang-cl` instead of `export CXX=clang-cl` even though both would work.

I think we should do this throughout the script then. I'll do it separately for other jobs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101565