-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update expected skip condition for llvm framework test on msys2 #9591
Conversation
Test is now expected to run, not skip, on msys2 with clang. So change the skip_on_jobname from 'msys2' to 'msys2-gcc', as it's only expected to skip there. |
Now that the run is expected, we can see that it is failing during the build stage, for reasons I don't understand:
https://github.com/mesonbuild/meson/runs/4253951135?check_suite_focus=true#step:5:2768 |
Those like windows only libs, are they supposed to be used with msys? It almost looks like we've found an llvm we shouldn't have |
Codecov Report
@@ Coverage Diff @@
## master #9591 +/- ##
==========================================
+ Coverage 67.40% 67.45% +0.04%
==========================================
Files 400 400
Lines 85340 85340
Branches 17576 17576
==========================================
+ Hits 57524 57563 +39
+ Misses 23212 23171 -41
- Partials 4604 4606 +2
Continue to review full report at Codecov.
|
It looks like those are coming from msys2's LLVMExports.cmake, so guessing wildly, maybe that doesn't understand correctly about msys2? |
I'm not seeing anything in msys2's clang package to make me think it's doing something like that. Maybe meson/mesonbuild/dependencies/cmake.py Lines 619 to 625 in 4f4259d
I'm not familiar enough with meson to know if it already knows mingw vs 'normal' windows there |
Yeah, that does look suspicious. I was kind of assuming that dependency(method: 'cmake') works on msys2, but maybe that's not the case... |
On msys2, we will now find static llvm using cmake
Yeah, that seems to be the spot. I'm not sure that comment is correct. This list of libraries ends up in dep.link_args, which I thought is compiler-neutral and gets massaged into a compiler appropriate format when output by the backend? That line was added in 0f4e88b (PR #6980), which was accepted without a regression test. |
The good news is that comment isn't right, we do have a compiler environment. you should have libs += self.clib_compiler.find_library(j, self.env, []) |
Convert bare library names to a dependency linker argument using find_library(), rather than hardcoding the MSVC transformation.
Indeed, that seems to work. Thanks. We don't have any CI coverage of LLVM with MSVC, which I guess is the scenario in #6980, but I guess this is an improvement. |
Awesome, thanks for figuring this out! |
Looked at that a bit more since both LLVM and CMake are present in the Azure VM image, so perhaps only a bit of configuration was needed to find it. However, it seems that the Windows installer from llvm.org doesn't provide the required .cmake files (or llvm-config), so we are out of luck. |
Supersedes: #9534, #9547