-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Android: enable BoringSSL and other Package Collections support #3393
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
Conversation
@swift-ci please smoke test |
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.
The code changes LGTM, but I don't know why the tests mentioned by @buttaface are failing.
The first three test failures share the same underlying cause I think. It could be problem with that specific set of test certificates (tests that use RSA certs pass), but we need the BoringSSL error to know more.
The PackageCollectionsTests.testPackageSearch
failure could be SQLite related. Which SQLite version is installed? What is the output of PRAGMA compile_options;
?
I will look into the BoringSSL error in more detail. I'm using the Termux package of SQLite 3.34.0. I will paste that SQLite info later. |
The BoringSSL error is The SQLite output is:
|
Motivation: Tests in `PackageCollectionsSigningTests` that involve `Test_ec.cer` are failing. See swiftlang#3393. I think this is a timezone-related issue. `Test_ec.cer` is valid from November 15, 2020 10:45:20am PST for a year. In the tests we manipulate the timestamp at which the cert validation is done so we don't have to regenerate new test certs and so we can test for different scenarios. That timestamp is set to November 16, 2020 midnight, and depending on where the tests are run, it might potentially mean a time before `Test_ec.cer` is valid and that's why we see "certificate is not yet valid" error. Note that this doesn't happen with tests that use `Test_rsa.cer` since the certificate is valid from November 8, 2020. Modification: Change verify date to November 18, 2020, which should give us more than enough buffer for various timezones.
Thanks for the info @buttaface. I think #3402 should fix the I will look into |
There seems to be a difference in how SQLite FTS handles Suppose we set up a simplified FTS table:
All of these queries should return the
This is true on macOS and Linux. I try on an Android device by installing Termux then We have the following options:
I prefer Option 1 (if it works). @buttaface @tomerd wdyt? |
Motivation: Tests in `PackageCollectionsSigningTests` that involve `Test_ec.cer` are failing. See #3393. I think this is a timezone-related issue. `Test_ec.cer` is valid from November 15, 2020 10:45:20am PST for a year. In the tests we manipulate the timestamp at which the cert validation is done so we don't have to regenerate new test certs and so we can test for different scenarios. That timestamp is set to November 16, 2020 midnight, and depending on where the tests are run, it might potentially mean a time before `Test_ec.cer` is valid and that's why we see "certificate is not yet valid" error. Note that this doesn't happen with tests that use `Test_rsa.cer` since the certificate is valid from November 8, 2020. Modification: Change verify date to November 18, 2020, which should give us more than enough buffer for various timezones.
option 1 seems reasonable to me |
Also, disable FTS search through collections.
#if os(Android) | ||
// FTS queries for strings containing hyphens isn't working in SQLite on | ||
// Android, so disable for now. | ||
useSearchIndices.put(false) |
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.
Rebased and disabled FTS on Android, all package collections tests now pass on Android with this change and #3402.
@swift-ci please smoke test |
Thanks for fixing the tests and merging, @yim-lee. |
Thanks for the help @buttaface |
Fix building package collections for Android
Motivation:
Building latest trunk or the last March 25 trunk source snapshot for Android fails because Android is not added to these supported OS lists.
Modifications:
Add Android to the list of supported OS's.
Result:
I'm now able to both cross-compile and natively compile SPM for Android. When running the tests, I get the following failures:
I'm not sure why these four tests fail, but there's nothing important that can't be fixed later.
@yim-lee, mind reviewing?