zulip / zulip-flutter

Upcoming Zulip mobile apps for Android and iOS, using Flutter
Apache License 2.0
197 stars 192 forks source link

check: Add step to install `libsqlite3-dev` system dependency #999

Closed rajveermalviya closed 1 month ago

rajveermalviya commented 1 month ago

The package:sqlite3 requires the system-installed sqlite3 shared library (libsqlite3.so) when running directly via Dart, on Linux. Whereas, when running under Flutter, it uses bundled libraries provided by package:sqlite3_flutter_libs.

Recently, the ubuntu-latest runner image has switched to defaulting to ubuntu-24.04: https://github.com/actions/runner-images/issues/10636 Previously, it used ubuntu-22.04, which had the libsqlite3-dev package pre-installed: https://github.com/actions/runner-images/blob/main/images/ubuntu/Ubuntu2204-Readme.md#installed-apt-packages However, the ubuntu-24.04 image no longer includes this package by default: https://github.com/actions/runner-images/blob/main/images/ubuntu/Ubuntu2404-Readme.md#installed-apt-packages

As a result, five unit tests now fail with the following error:

Invalid argument(s): Failed to load dynamic library 'libsqlite3.so': libsqlite3.so: cannot open shared object file: No such file or directory
  dart:ffi                                                        new DynamicLibrary.open
  package:sqlite3/src/ffi/load_library.dart 52:27                 _defaultOpen
  package:sqlite3/src/ffi/load_library.dart 127:12                OpenDynamicLibrary.openSqlite
  package:sqlite3/src/ffi/api.dart 13:39                          sqlite3
  package:drift/native.dart 313:12                                _NativeDelegate.openDatabase
  package:drift/src/sqlite3/database.dart 79:19                   Sqlite3Delegate.open
  package:drift/src/runtime/executor/helpers/engines.dart 431:22  DelegatedDatabase.ensureOpen.<fn>

To resolve these test failures, add a step to manually install the libsqlite3-dev package.

rajveermalviya commented 1 month ago

Well, just as I was putting this PR up, it looks like the ubuntu-latest == ubuntu-24.04 rollout is being rolled back: https://github.com/actions/runner-images/issues/10636#issuecomment-2417303444

fun 😄

fombalang commented 1 month ago

Well, just as I was putting this PR up, it looks like the ubuntu-latest == ubuntu-24.04 rollout is being rolled back: actions/runner-images#10636 (comment)

fun 😄

Oh, I was wondering why some test suddenly failed on the pipeline when I sent in my PR.

gnprice commented 1 month ago

Thanks @rajveermalviya for sorting this out! I'd run into the same issue yesterday at https://github.com/zulip/zulip-flutter/pull/998#issuecomment-2415545081 but hadn't yet spent time to investigate.

This seems like a good change even if for the moment it's no longer needed. How about combining it with a change to use ubuntu-24.04 specifically? That way CI would validate for us that the change fully solves the problem.

rajveermalviya commented 1 month ago

Thanks for the review @gnprice! I've updated the commit to also change the checks' runner image to use ubuntu-24.04 explicitly. PTAL.

gnprice commented 1 month ago

Thanks! Looks good; merging.

I tweaked the commit message, mainly here:

-    As a result, five unit tests now fail with the following error:
+    Without that package, five unit tests would fail with the following
+    error:

because (since GitHub rolled back the change of ubuntu-latest to 24.04) there's no longer a "now" when the unit tests actually do fail; rather it's a hypothetical, that if we just bumped the version without adding this setup step, those tests would fail.