Skip to content

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

Merged
merged 1 commit into from
Apr 14, 2021

Conversation

finagolfin
Copy link
Member

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:

Test Suite 'Selected tests' started at 2021-04-08 16:09:18.868
Test Suite 'CertificatePolicyTests' started at 2021-04-08 16:09:18.890
Test Case 'CertificatePolicyTests.test_EC_validate_happyCase' started at 2021-04-08 16:09:18.891
/data/data/com.termux/files/home/src/swiftpm/Tests/PackageCollectionsSigningTests/CertificatePolicyTests.swift:56: error: CertificatePolicyTests.test_EC_validate_happyCase : XCTAssertNoThrow failed: threw error "invalidCertChain" - 
Test Case 'CertificatePolicyTests.test_EC_validate_happyCase' failed (0.636 seconds)
Test Suite 'CertificatePolicyTests' failed at 2021-04-08 16:09:19.527
	 Executed 1 test, with 1 failure (0 unexpected) in 0.636 (0.636) seconds
Test Suite 'Selected tests' failed at 2021-04-08 16:09:19.527
	 Executed 1 test, with 1 failure (0 unexpected) in 0.636 (0.636) seconds

Test Suite 'Selected tests' started at 2021-04-08 16:09:21.009
Test Suite 'PackageCollectionSigningTests' started at 2021-04-08 16:09:21.012
Test Case 'PackageCollectionSigningTests.test_EC_signAndValidate_collectionMismatch' started at 2021-04-08 16:09:21.012
/data/data/com.termux/files/home/src/swiftpm/Tests/PackageCollectionsSigningTests/PackageCollectionSigningTest.swift:144: error: PackageCollectionSigningTests.test_EC_signAndValidate_collectionMismatch : failed - invalidCertChain
Test Case 'PackageCollectionSigningTests.test_EC_signAndValidate_collectionMismatch' failed (0.764 seconds)
Test Suite 'PackageCollectionSigningTests' failed at 2021-04-08 16:09:21.776
	 Executed 1 test, with 1 failure (0 unexpected) in 0.764 (0.764) seconds
Test Suite 'Selected tests' failed at 2021-04-08 16:09:21.776
	 Executed 1 test, with 1 failure (0 unexpected) in 0.764 (0.764) seconds

Test Suite 'Selected tests' started at 2021-04-08 16:09:21.156
Test Suite 'PackageCollectionSigningTests' started at 2021-04-08 16:09:21.167
Test Case 'PackageCollectionSigningTests.test_EC_signAndValidate_happyCase' started at 2021-04-08 16:09:21.167
/data/data/com.termux/files/home/src/swiftpm/Tests/PackageCollectionsSigningTests/PackageCollectionSigningTest.swift:112: error: PackageCollectionSigningTests.test_EC_signAndValidate_happyCase : failed - invalidCertChain
Test Case 'PackageCollectionSigningTests.test_EC_signAndValidate_happyCase' failed (0.741 seconds)
Test Suite 'PackageCollectionSigningTests' failed at 2021-04-08 16:09:21.908
	 Executed 1 test, with 1 failure (0 unexpected) in 0.741 (0.741) seconds
Test Suite 'Selected tests' failed at 2021-04-08 16:09:21.908
	 Executed 1 test, with 1 failure (0 unexpected) in 0.741 (0.741) seconds

Test Suite 'Selected tests' started at 2021-04-08 16:09:27.357
Test Suite 'PackageCollectionsTests' started at 2021-04-08 16:09:27.368
Test Case 'PackageCollectionsTests.testPackageSearch' started at 2021-04-08 16:09:27.368
/data/data/com.termux/files/home/src/swiftpm/Tests/PackageCollectionsTests/PackageCollectionsTests.swift:644: error: PackageCollectionsTests.testPackageSearch : XCTAssertEqual failed: ("0") is not equal to ("1") - list count should match
/data/data/com.termux/files/home/src/swiftpm/Tests/PackageCollectionsTests/PackageCollectionsTests.swift:645: error: PackageCollectionsTests.testPackageSearch : XCTAssertEqual failed: ("nil") is not equal to ("Optional([PackageCollections.PackageCollectionsModel.CollectionIdentifier.json(https://feed.mock/45754C6E-1B17-4755-82CF-03BD333F42F8), PackageCollections.PackageCollectionsModel.CollectionIdentifier.json(https://feed.mock/C0211A59-EF95-4DE1-B16A-2B92C4610122)])") - list count should match
/data/data/com.termux/files/home/src/swiftpm/Tests/PackageCollectionsTests/PackageCollectionsTests.swift:651: error: PackageCollectionsTests.testPackageSearch : XCTAssertEqual failed: ("0") is not equal to ("1") - list count should match
/data/data/com.termux/files/home/src/swiftpm/Tests/PackageCollectionsTests/PackageCollectionsTests.swift:652: error: PackageCollectionsTests.testPackageSearch : XCTAssertEqual failed: ("nil") is not equal to ("Optional([PackageCollections.PackageCollectionsModel.CollectionIdentifier.json(https://feed.mock/45754C6E-1B17-4755-82CF-03BD333F42F8), PackageCollections.PackageCollectionsModel.CollectionIdentifier.json(https://feed.mock/C0211A59-EF95-4DE1-B16A-2B92C4610122)])") - list count should match
/data/data/com.termux/files/home/src/swiftpm/Tests/PackageCollectionsTests/PackageCollectionsTests.swift:658: error: PackageCollectionsTests.testPackageSearch : XCTAssertEqual failed: ("0") is not equal to ("1") - list count should match
/data/data/com.termux/files/home/src/swiftpm/Tests/PackageCollectionsTests/PackageCollectionsTests.swift:659: error: PackageCollectionsTests.testPackageSearch : XCTAssertEqual failed: ("nil") is not equal to ("Optional([PackageCollections.PackageCollectionsModel.CollectionIdentifier.json(https://feed.mock/45754C6E-1B17-4755-82CF-03BD333F42F8), PackageCollections.PackageCollectionsModel.CollectionIdentifier.json(https://feed.mock/C0211A59-EF95-4DE1-B16A-2B92C4610122)])") - list count should match
/data/data/com.termux/files/home/src/swiftpm/Tests/PackageCollectionsTests/PackageCollectionsTests.swift:665: error: PackageCollectionsTests.testPackageSearch : XCTAssertEqual failed: ("0") is not equal to ("1") - list count should match
/data/data/com.termux/files/home/src/swiftpm/Tests/PackageCollectionsTests/PackageCollectionsTests.swift:666: error: PackageCollectionsTests.testPackageSearch : XCTAssertEqual failed: ("nil") is not equal to ("Optional([PackageCollections.PackageCollectionsModel.CollectionIdentifier.json(https://feed.mock/45754C6E-1B17-4755-82CF-03BD333F42F8), PackageCollections.PackageCollectionsModel.CollectionIdentifier.json(https://feed.mock/C0211A59-EF95-4DE1-B16A-2B92C4610122)])") - collections should match
/data/data/com.termux/files/home/src/swiftpm/Tests/PackageCollectionsTests/PackageCollectionsTests.swift:672: error: PackageCollectionsTests.testPackageSearch : XCTAssertEqual failed: ("0") is not equal to ("1") - list count should match
/data/data/com.termux/files/home/src/swiftpm/Tests/PackageCollectionsTests/PackageCollectionsTests.swift:673: error: PackageCollectionsTests.testPackageSearch : XCTAssertEqual failed: ("nil") is not equal to ("Optional([PackageCollections.PackageCollectionsModel.CollectionIdentifier.json(https://feed.mock/45754C6E-1B17-4755-82CF-03BD333F42F8), PackageCollections.PackageCollectionsModel.CollectionIdentifier.json(https://feed.mock/C0211A59-EF95-4DE1-B16A-2B92C4610122)])") - collections should match
/data/data/com.termux/files/home/src/swiftpm/Tests/PackageCollectionsTests/PackageCollectionsTests.swift:679: error: PackageCollectionsTests.testPackageSearch : XCTAssertEqual failed: ("0") is not equal to ("1") - list count should match
/data/data/com.termux/files/home/src/swiftpm/Tests/PackageCollectionsTests/PackageCollectionsTests.swift:680: error: PackageCollectionsTests.testPackageSearch : XCTAssertEqual failed: ("nil") is not equal to ("Optional([PackageCollections.PackageCollectionsModel.CollectionIdentifier.json(https://feed.mock/45754C6E-1B17-4755-82CF-03BD333F42F8), PackageCollections.PackageCollectionsModel.CollectionIdentifier.json(https://feed.mock/C0211A59-EF95-4DE1-B16A-2B92C4610122)])") - list count should match
/data/data/com.termux/files/home/src/swiftpm/Tests/PackageCollectionsTests/PackageCollectionsTests.swift:686: error: PackageCollectionsTests.testPackageSearch : XCTAssertEqual failed: ("0") is not equal to ("1") - list count should match
/data/data/com.termux/files/home/src/swiftpm/Tests/PackageCollectionsTests/PackageCollectionsTests.swift:687: error: PackageCollectionsTests.testPackageSearch : XCTAssertEqual failed: ("nil") is not equal to ("Optional([PackageCollections.PackageCollectionsModel.CollectionIdentifier.json(https://feed.mock/45754C6E-1B17-4755-82CF-03BD333F42F8), PackageCollections.PackageCollectionsModel.CollectionIdentifier.json(https://feed.mock/C0211A59-EF95-4DE1-B16A-2B92C4610122)])") - collections should match
Test Case 'PackageCollectionsTests.testPackageSearch' failed (4.432 seconds)
Test Suite 'PackageCollectionsTests' failed at 2021-04-08 16:09:31.801
	 Executed 1 test, with 14 failures (0 unexpected) in 4.432 (4.432) seconds
Test Suite 'Selected tests' failed at 2021-04-08 16:09:31.801
	 Executed 1 test, with 14 failures (0 unexpected) in 4.432 (4.432) seconds

I'm not sure why these four tests fail, but there's nothing important that can't be fixed later.

@yim-lee, mind reviewing?

@tomerd
Copy link
Contributor

tomerd commented Apr 13, 2021

@swift-ci please smoke test

Copy link
Contributor

@yim-lee yim-lee left a 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;?

@finagolfin
Copy link
Member Author

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.

@finagolfin
Copy link
Member Author

The BoringSSL error is certificate is not yet valid, maybe the certs aren't set up properly or there's a bug when compiling BoringSSL for Android AArch64.

The SQLite output is:

sqlite> PRAGMA compile_options;
COMPILER=clang-9.0.8
ENABLE_DBSTAT_VTAB
ENABLE_FTS4
ENABLE_FTS5
ENABLE_GEOPOLY
ENABLE_JSON1
ENABLE_RTREE
ENABLE_STMTVTAB
THREADSAFE=1

yim-lee added a commit to yim-lee/swift-package-manager that referenced this pull request Apr 13, 2021
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.
@yim-lee
Copy link
Contributor

yim-lee commented Apr 13, 2021

Thanks for the info @buttaface.

I think #3402 should fix the PackageCollectionSigningTests failures.

I will look into PackageCollectionsTests.testPackageSearch next.

@yim-lee
Copy link
Contributor

yim-lee commented Apr 13, 2021

There seems to be a difference in how SQLite FTS handles -.

Suppose we set up a simplified FTS table:

create virtual table if not exists fts1 using fts4(id,name,url,tokenize=unicode61);
insert into fts1 values('foo bar', 'foobar', 'http://test.com/foo-bar');

All of these queries should return the foo bar row:

select * from fts1 where fts1 match 'foo';
select * from fts1 where fts1 match 'foo bar';
select * from fts1 where fts1 match 'bar foo';
select * from fts1 where fts1 match 'foobar';
select * from fts1 where fts1 match 'foo-bar';
select * from fts1 where fts1 match 'http://test.com/foo-bar';

This is true on macOS and Linux.

I try on an Android device by installing Termux then pkg install sqlite. For foo-bar and 'http://test.com/foo-bar' there is no result; the other queries work as expected. This causes PackageCollectionsTests.testPackageSearch to fail because the tests use UUIDs which contain -.

We have the following options:

  1. Disable FTS on Android. Search would be less robust--slower and not all sample queries are supported, but the test should pass. (To make sure, @buttaface can you please try commenting out this block of code in SQLitePackageCollectionsStorage and make sure useSearchIndices.put(false) then rerun PackageCollectionsTests.testPackageSearch?) This might be OK if we don't anticipate the package collections feature to be used on Android.
  2. Disable PackageCollectionsTests.testPackageSearch on Android.
  3. There might be FTS options that we can tweak though we should be cautious since it might impact the feature as a whole.

I prefer Option 1 (if it works). @buttaface @tomerd wdyt?

yim-lee added a commit that referenced this pull request Apr 13, 2021
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.
@tomerd
Copy link
Contributor

tomerd commented Apr 13, 2021

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)
Copy link
Member Author

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.

@yim-lee
Copy link
Contributor

yim-lee commented Apr 14, 2021

@swift-ci please smoke test

@yim-lee yim-lee merged commit d4a488b into swiftlang:main Apr 14, 2021
@finagolfin
Copy link
Member Author

Thanks for fixing the tests and merging, @yim-lee.

@yim-lee
Copy link
Contributor

yim-lee commented Apr 14, 2021

Thanks for the help @buttaface

@finagolfin finagolfin deleted the droid branch April 14, 2021 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants