enable exception support for shared libraries - #660
Conversation
|
lgtm, but CI is probably going to be a bit of a beast to get passing here |
03735ec to
0e27277
Compare
I've updated `tests/CMakeLists.txt` to run all p2 and p3 tests as shared libraries as well as normal executables. As usual my CMake skills are underwhelming; very open to feedback. This includes patches for the required LLVM PR backports: - llvm/llvm-project#209282 - llvm/llvm-project#222747
0e27277 to
719fe63
Compare
|
CI is green now, although I'm having trouble understanding "Build only sysroot - exceptions: Expected — Waiting for status to be reported" given I've completely commented out that part of the job matrix, so I don't understand why it's appearing at all. |
alexcrichton
left a comment
alexcrichton
left a comment
There was a problem hiding this comment.
For CI that's a repo configuration thing (a "required check") -- I'll go disable that
There was a problem hiding this comment.
Would it be possible to avoid manually splitting/merging *.patch files? I've historically attempted to keep the patch files as literal output of curl https://github.com/llvm/llvm-project/pull/208597.diff -L -o ./src/llvm-pr-208597.patch for example which makes it a bit easier to maintain over time. I couldn't do that for some versions in the past when things conflicted, however.
Another way to put this: if literal *.patch files-from-prs don't work, could you expand on why? (also the rationale for the sysroot/toolchain split)
There was a problem hiding this comment.
A few reasons:
- The actual upstream
git difffor each PR doesn't apply to LLVM 23 due to other changes which have happened on LLVMmain, so I had to edit them it as part of the backport - Both PRs involve both toolchain and sysroot changes which depend on each other. And if I apply either patch only in
wasi-sdk-toolchain.cmakeor only inwasi-sdk-sysroot.cmake, but only the other one is built, then it won't have the required changes, hence the split. - Since 222747 depends on 209282 having been applied first, the
git diff ... -R --checkwas erroring when they were two separate files. I later saw your reverse-the-list trick in thewasi-sdk-toolchain.cmakehistory which might address that, though. - I figured the test
.llfiles weren't relevant, so I stripped them out
Happy to revisit any of that if there's a better way.
There was a problem hiding this comment.
Ok I kind of figure it was something like that but wanted to double check, yeah, and it's ok to land as-is just wanted to have some written-down rationale as well
|
|
||
| # Apply test-specific compile options and link flags. | ||
| if(${arg_EMULATED_CLOCKS}) | ||
| set(so_files ${so_files} "${wasi_sysroot}/lib/${target}/libwasi-emulated-process-clocks.so") |
There was a problem hiding this comment.
For these set calls would a list(APPEND so_files ...) work?
There was a problem hiding this comment.
FWIW, I was just starting to test this when I saw you merged the PR, but I can make a small follow up PR if it works out.
There was a problem hiding this comment.
Ah yeah it's fine either way, I figure no need to block this for another ~6 hours for a small nit
2 participants
I've updated
tests/CMakeLists.txtto run all p2 and p3 tests as shared libraries as well as normal executables. As usual my CMake skills are underwhelming; very open to feedback.This includes patches for the required LLVM PR backports:
Unwind_CallPersonalityllvm/llvm-project#209282