Allow Swift to enable LLDB tests in Windows CI #9513
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Nov 5, 2024
…lvm#111729) Use SEND_ERROR (continue processing, but skip generation) instead of FATAL_ERROR (stop processing and generation). This means that developers get to see all errors at once, instead of seeing just the first error and having to reconfigure to discover the next one.
…m#111747) This module is used in various helper scripts since llvm#93712
Many LLDB's dotest.py based tests require the `make` tool. If it's not found in Path, they fail with an obscure error and show up as `UNRESOLVED`. On Windows, llvm-lit takes care of MSYS based testing tools like cat, printf, etc., but `make` is not part of that. Let's catch the situation early and check for it at configuration time. This error isn't fatal: It should fail the build, but not immediately stop the configuration process. There might be other issues further down the line that can be caught in the same buildbot run.
…m#112342) In recent PR llvm#111531 for Windows support, we enabled tests that require the `make` tool. On Windows, default install directories likely contain spaces, in this case e.g. `C:\Program Files (x86)\GnuWin32\bin\make.exe`. It's typically handled well by CMake, so that today invocations from `dotest.py` don't cause issues. However, we also have nested invocations from a number of Makefiles themselves. These still failed if the path to the `make` tool contains spaces. This patch attempts to fix the functionalities/completion test by adding quotes in the respective Makefile. If it keeps passing on the bots, we can roll out the fix to all affected tests.
…m#112360) Following up from llvm#112342, we roll out the fix and quote nested `make` invocations in all API tests.
We need the manual lookup in lldb/source/Host/windows/HostInfoWindowsSwift.cpp to succeed. Otherwise SwiftASTContext::CreateInstance() fails with: Cannot create Swift scratch context (couldn't load the Swift stdlib)
…lit config These binaries only exists if the Swift runtime was built correctly. We cannot check them at configuration time, because they might not exist yet.
This adds a minor change to command-expr-diagnostics.test to make it pass on windows. Clang produces PDB on windows by default which was ignoring main symbol due to optimization. The problem is fixed by adding -gdwarf to commandline, making sure dwarf debug info gets generated on both Windows and Linux.
…5716) Since the remote Shell test execution feature was added, these tests should now be disabled on Windows target instead of Windows host. It should fix failures on https://lab.llvm.org/staging/#/builders/197/builds/76.
weliveindetail
force-pushed
the
lldb-tests-20240723
branch
from
df1a0d2 to
6b9b6e8
Compare
This patch removes XFAIL decorators from tests that are passing on x86 Windows. Corresponding XFAILs for AArch64 were removed here 7daa9a9.
weliveindetail
force-pushed
the
lldb-tests-20240723
branch
from
6b9b6e8 to
10fef7f
Compare
|
@swift-ci please test |
llvm#111531) Bot maintainers should be aware and it became too much of a burden for developers. In particular on Windows, where make.exe won't be found in Path typically.
|
Build failed because make wasn't found. We don't have detection for it until the swift PR lands and I missed the cherry-pick that turns it into a warning. |
|
@swift-ci please test |
|
@swift-ci please test Windows Platform |
|
Windows platform failed due to an unrelated build error. LLDB was building fine. We didn't get to the test stage. |
|
@swift-ci please test Windows platform |
JDevlieghere
reviewed
Dec 5, 2024
Comment on lines
+201
to
+218
| # Since Python3.8 the Windows runtime loads dependent DLLs only from the directory of the binary | ||
| # itself (and not Path). Windows has no RPATHs, so we must copy all DLLs that we depend on into | ||
| # the Python package. | ||
| if (WIN32) | ||
| # TARGET_RUNTIME_DLLS is supported in CMake 3.21+ | ||
| if ("${CMAKE_VERSION}" VERSION_LESS "3.21.0") | ||
| if (LLDB_INCLUDE_TESTS) | ||
| message(SEND_ERROR | ||
| "Your CMake version is ${CMAKE_VERSION}. In order to run LLDB tests " | ||
| "on Windows please upgrade to 3.21.0 at least (or disable tests with " | ||
| "LLDB_INCLUDE_TESTS=Off)") | ||
| endif() | ||
| else() | ||
| add_custom_command(TARGET ${swig_target} POST_BUILD | ||
| COMMAND ${CMAKE_COMMAND} -E copy -t ${lldb_python_target_dir} $<TARGET_RUNTIME_DLLS:liblldb> | ||
| COMMAND_EXPAND_LISTS) | ||
| endif() | ||
| endif() |
There was a problem hiding this comment.
Any reason this can't go upstream? Or is this a cherrypick?
There was a problem hiding this comment.
In upstream LLVM liblldb has no dependent DLLs (except Python itself) so there was no case for it. And yes, it's a cherry-pick from #9370
JDevlieghere
approved these changes
Dec 11, 2024
There was a problem hiding this comment.
LGTM. Most of these are cherrypicks from upstream and everything else is Swift specific. Thanks @weliveindetail!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the companion PR with all the llvm-project changes for swiftlang/swift#76894