Repository navigation
Conversation
|
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 | ||
| with: | ||
| path: ${{ github.workspace }}/llvm |
Check notice
Code scanning / zizmor
credential persistence through GitHub Actions artifacts Note
| -DLLVM_BUILD_LLVM_DYLIB_VIS=On ` | ||
| -DLLVM_LINK_LLVM_DYLIB=On ` | ||
| -DCMAKE_DISABLE_PRECOMPILE_HEADERS=ON | ||
| Error-On-Failure { cmake --build build --config RelWithDebInfo --parallel ${{ env.ncpus }} } |
Check notice
Code scanning / zizmor
code injection via template expansion Note
| -DLLVM_BUILD_LLVM_DYLIB_VIS=On ` | ||
| -DLLVM_LINK_LLVM_DYLIB=On ` | ||
| -DCMAKE_DISABLE_PRECOMPILE_HEADERS=ON | ||
| Error-On-Failure { cmake --build build --config RelWithDebInfo --parallel ${{ env.ncpus }} } |
Check notice
Code scanning / zizmor
code injection via template expansion Note
| -DLLVM_LINK_LLVM_DYLIB=On ` | ||
| -DCMAKE_DISABLE_PRECOMPILE_HEADERS=ON | ||
| Error-On-Failure { cmake --build build --config RelWithDebInfo --parallel ${{ env.ncpus }} } | ||
| Error-On-Failure { cmake --build build --target check-llvm-unit --config RelWithDebInfo --parallel ${{ env.ncpus }} } |
Check notice
Code scanning / zizmor
code injection via template expansion Note
| -DLLVM_BUILD_LLVM_DYLIB_VIS=On ` | ||
| -DLLVM_LINK_LLVM_DYLIB=On ` | ||
| -DCMAKE_DISABLE_PRECOMPILE_HEADERS=ON | ||
| Error-On-Failure { cmake --build build --config RelWithDebInfo --parallel ${{ env.ncpus }} } |
There was a problem hiding this comment.
Just realised I can remove the --parallel ${{ env.ncpus }} . It is there due to an earlier version of this ci where I defined the number of cpu cores. I will remove once I have review comments for elsewhere
| function Error-On-Failure { | ||
| param ( | ||
| [Parameter(Mandatory)] | ||
| [ScriptBlock]$Command | ||
| ) | ||
|
|
||
| & $Command | ||
|
|
||
| if ($LASTEXITCODE -ne 0) { | ||
| exit $LASTEXITCODE | ||
| } | ||
| } |
There was a problem hiding this comment.
The purpose of this is to act in the same way as set -e on Linux. I didn't know the equivalent on Windows, since I am not primarily a Windows developer. This was created by ChatGPT for me a long time ago.
nikic
left a comment
nikic
left a comment
There was a problem hiding this comment.
This is not at all what I am looking for. And I'm growing tired of reiterating this, so please see the discussion on #109483 for details. The tl;dr is that all ABI annotations need to be present, not just whatever is necessary for your pet use case. And the ids workflow is necessary to enforce that going forward, not just a Windows dylib build.
There should be no use of LLVM_ABI_FOR_TEST in exported headers.
Towards #109483 , and does the first 2 of plan laid out #188587 (comment)
The purpose of this is to add all the remaining LLVM_ABI annotations such that all llvm tools, tests and benchmarks can be built using the LLVM shared library on Windows. As of putting in this PR all unit tests can build, but 3 tests fail. I'm unsure what to with these tests. Putting in this PR as it stands allows people to review the ci, and suggest how to proceed with dealing with the failed tests. I could skip 2 of them by having
LLVM_ENABLE_PLUGINS=OFFfor now. It is only set to ON in the workflow for now as one of the purposes of #109483 is to enable plugin support on Windows. That would still leave one failing test.I'm aware this PR hits a large number of files, and can broken up into smaller PRs, once everyone is happy with the changes.
cc @nikic