SQLCipher build hook#375
Conversation
build for macOS openssl build android support sqlcipher output support ios disable linux 32 bits format fix download skip tests lint fix analyze no analyze no sanitizer build cleanup targetos crypto switch link with common crypto on macos/ios support tar xz build for iOS version 3.3.0 Revert "version 3.3.0" This reverts commit 4eb21c0. sqlite3-sqlcipher tag errata only prepare release link android log good hashes ignore wasm test allow prebuilt sqlcipher hashes hashes trigger deploy hashes whitespace cleanup
|
@simolus3 , I left a few general questions about how to aproach the PR |
|
@simolus3 This is ok to review now. But it appears to be stuck in the integration tests, but I'm not sure why, as I haven't touched those. |
simolus3
left a comment
There was a problem hiding this comment.
Should we reuse the download_sqlite and build_sqlite jobs?
Yes, download_sqlite should also download OpenSSL and SQLCipher sources. build_sqlite should also build SQLCipher.
Regarding build flags, what should we use as default?
Using options from our current build hook + the extra init/shutdown flags sounds correct to me. There's no reason to enable fts4 or extension loading through SQL functions.
|
@simolus3 I'm not able to change the target branch without creating a new PR. Are you able to change it as admin? |
simolus3
left a comment
There was a problem hiding this comment.
Thanks for the help! This is definitely helpful, I can still look at Windows and non-X64 Linux support before merging and releasing the sqlcipher branch.
| flutter test integration_test -Dsqlite3.sqlcipher=true | ||
| flutter test integration_test -Dsqlite3.sqlcipher=true -d macos | ||
|
|
||
| - name: sqlcipher Android emulator tests |
There was a problem hiding this comment.
Booting an emulator is quite slow. I wonder if the Android emulator tests should be a separate job now, where we might spawn a single Android emulator only and use a script: that runs multiple flutter test calls with different build options in sequence?
There was a problem hiding this comment.
Good idea. Should we create a bash script for the ci tests? Or would you like placing the "&&"s in the workflow yaml directly?
There was a problem hiding this comment.
Directly in the yaml should work
| @@ -60,15 +60,70 @@ ${usedSqliteSymbols.map((symbol) => ' $symbol;').join('\n')} | |||
| } | |||
|
|
|||
| final isSqlcipher = input.userDefines['is_sqlcipher'] as bool? ?? false; | |||
There was a problem hiding this comment.
I'm to blame for this, but there should be a comment here explaining that these options are internal, undocumented and only meant to be set via tool/build_sqlite.dart
I would appreciate if you look into the Windows and Linux Cross Compiling. I'm not very experienced with the build tools for those |
|
I'll merge this as-is since it's a clear improvement. Thanks for your help, I'll look into the cross-compiling issues too. |
Questions:
Should we reuse the download_sqlite and build_sqlite jobs?
Regarding build flags, what should we use as default?