-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[windows][toolchain] Run non-executable Swift Runtime tests for aarch64 Android #79185
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
Merged
weliveindetail
merged 1 commit into
swiftlang:main
from
weliveindetail:windrd-test-nonexec-swift-runtime
Mar 16, 2025
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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 file contains hidden or 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,2 @@ | ||
config.substitutions.append((r'%refactor-check-compiles', f'{config.python} {config.refactor_check_compiles} -swift-refactor {config.swift_refactor} -swift-frontend {config.swift_frontend} -temp-dir %t {config.resource_dir_opt}')) | ||
config.substitutions.append((r'%refactor', f'{config.swift_refactor} {config.resource_dir_opt}')) | ||
config.substitutions.append((r'%refactor-check-compiles', f'{config.python} {config.refactor_check_compiles} -swift-refactor {config.swift_refactor} -swift-frontend {config.swift_frontend} -temp-dir %t {config.resource_dir_opt} -target {config.variant_triple}')) | ||
config.substitutions.append((r'%refactor', f'{config.swift_refactor} {config.resource_dir_opt} -target {config.variant_triple}')) |
This file contains hidden or 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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@weliveindetail, it appears this change broke several tests on the community Android CI, which cross-compiles them from a linux host for Android:
Are these
Interop/SwiftToCxx
tests passing for you on a Windows host when cross-compiled to Android? The good news is that this change has no effect when natively running the compiler validation suite on an Android AArch64 device, the same tests pass there. 😃It looks like you made this change because otherwise the file was not checked against the NDK, so I think you are right to add this flag for Android, but it will require more flags to work properly on the linux-hosted CI.
I plan to spend time in the coming month fixing the remaining tests on the Android CI run with a linux host, so it would be good to then communicate with whoever is checking these on your Android CI for a Windows host to make sure we get the same results.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our SDK has
cstdint
atswift-6.1-RELEASE-android-24-0.1.artifactbundle/swift-6.1-release-android-24-sdk/android-27c-sysroot/usr/include/c++/v1/cstdint
. I wonder if we manually add<sysroot>/usr/include/c++/v1/
to the include paths if it might find it…There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I tried that and then saw some other C++ macro errors. I've seen some weird logic in the clang Driver related to the "gcc-toolchain" which allows it to find the C++ headers automatically in the system linux sysroot, but I think not in the Android sysroot. This is probably caused by that, but then it should fail on Windows also?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIK clang on Windows always needs
--sysroot
in order to cross-compile for Android, because the location of the NDK cannot be inferred. This is one of the places that lacked the argument. It worked when it merged, but looking at this run of the nightly bot, it has regressed already:I didn't finish the integration of the downstream filters. In particular, the feature to skip tests in LIT doesn't seem to work in Swift. #80148 unblocked the nightly bots, but also removed visibility of these failures.
IMHO we won't get away without such build-platform specific filters. At least to mark parts of the test suite as "not yet supported" in the mid-term. The test suite is too clunky and toolchain requirements are too diverse. Also, we cannot specify the build-OS in the
REQUIRES
/UNSUPPORTED
/etc. clauses of individual tests (and it would add another layer of mess to the test suite if we did that).It would be great to fix the
skip
feature, so we can exclude flaky and xpass tests per build-platform.xfail
works. On Windows we load the filter for Android here:swift/utils/build.ps1
Lines 2230 to 2231 in 22a3ec0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yeah, as I said above, this change appears right, but incomplete when cross-compiling.
I'm surprised it ever worked on your Windows host, as it's listed as XFAIL for AArch64 in that output, and fails as expected for the other Android arches on Windows too:
Good to see you're still running the compiler validation tests for armv7 though, as that was broken on the community Android CI for a year or so before we finally shut it down, and I think you're the first to run the tests for Android i686. 😃
Understood, we can chip those lit fixes in over time.
Right now, I'm interested in fixing the compiler validation suite for Android when running on linux, which may cause regressions on your Windows host running the Android tests. Thanks for linking that Windows CI so I can check that: 😺 is there someone at TBC in charge of those Android tests who I can discuss such failures with?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am in another busy project this summer. For the moment I guess @compnerd might be able to help you here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
He seems super busy too though. :| My concern is that when I start fixing the Android tests on linux, some may then break on Windows. I can then check that in the toolchain CI you linked above and try to fix them, but may not be able to, as I don't use Windows myself. But I suppose if you don't have anybody in charge of these Android tests run on Windows, most of my changes will fix tests on Windows too, so let's just see how it goes...