feat: Linux supported, automate with native assets fix sqlite support#613
feat: Linux supported, automate with native assets fix sqlite support#613bhola-dev58 wants to merge 13 commits intoCCExtractor:mainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds a native-assets build hook that compiles the Rust library for desktop targets, moves native-loading into a platform bootstrap at app startup, and introduces platform guards to avoid mobile-only widget/update operations on unsupported platforms. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as "native_assets_cli"
participant Hook as "hook/build.dart"
participant Cargo as "cargo (rust build)"
participant FS as "Filesystem"
participant Output as "native_assets_cli Output"
CLI->>Hook: invoke build(args, callback)
Hook->>Hook: inspect input.config.code.targetOS
alt target is linux or windows
Hook->>Cargo: run `cargo build --release` (workingDirectory: rust/)
Cargo-->>Hook: exitCode, stdout, stderr
alt exitCode != 0
Hook->>FS: write stdout/stderr to stdout/stderr
Hook-->>CLI: throw build failure
else exitCode == 0
Hook->>FS: check `rust/target/release/{libtc_helper.so|tc_helper.dll}` exists
alt exists
Hook->>Output: register CodeAsset(package: input.packageName, name: "main.dart", linkMode: DynamicLoadingBundled, file: libPath)
Hook-->>CLI: return success
else missing
Hook-->>CLI: throw missing artifact error
end
end
else
Hook-->>CLI: return without adding assets
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
lib/app/v3/db/task_database.dart (1)
300-311: SQL string literal fix is correct, butIS NOTusage is non-standard for string comparison.The change from double quotes to single quotes for the
'deleted'literal is correct—SQLite uses single quotes for string literals. However,IS NOTis typically used forNULLcomparisons in SQL. For string comparisons, prefer<>or!=:status <> 'deleted'Also, this method creates a new
TaskDatabaseinstance and opens it separately, rather than usingthis._database. This duplicates connection handling and may cause resource management issues.♻️ Proposed fix
Future<List<String>> fetchUniqueProjects() async { - var taskDatabase = TaskDatabase(); - await taskDatabase.open(); - await taskDatabase.ensureDatabaseIsOpen(); + await ensureDatabaseIsOpen(); final List< Map<String, - dynamic>> result = await taskDatabase._database!.rawQuery( - "SELECT DISTINCT project FROM Tasks WHERE project IS NOT NULL AND status IS NOT 'deleted'"); + dynamic>> result = await _database!.rawQuery( + "SELECT DISTINCT project FROM Tasks WHERE project IS NOT NULL AND status <> 'deleted'"); return result.map((row) => row['project'] as String).toList(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/app/v3/db/task_database.dart` around lines 300 - 311, The fetchUniqueProjects method creates a new TaskDatabase and opens it instead of using the current instance's connection and also uses non-standard "IS NOT" for string comparison; update fetchUniqueProjects to operate on this._database (call this.open() / this.ensureDatabaseIsOpen() or assume the instance is already open) rather than instantiating a new TaskDatabase, and change the rawQuery string to use standard string inequality (e.g., status <> 'deleted' or status != 'deleted') when querying with rawQuery to return distinct project values from _database.hook/build.dart (1)
16-26: Consider adding a check forcargoavailability.If
cargois not installed, theProcess.runwill fail with an unhelpful error. A pre-check would provide a clearer error message for developers setting up the project.♻️ Proposed improvement
+ // Verify cargo is available + final cargoCheck = await Process.run('which', ['cargo']); + if (cargoCheck.exitCode != 0) { + throw Exception( + 'Rust toolchain not found. Please install Rust: https://rustup.rs/'); + } + // 1. Run cargo build --release final result = await Process.run( 'cargo', ['build', '--release'], workingDirectory: rustDir.toFilePath(), );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hook/build.dart` around lines 16 - 26, The build step currently calls Process.run('cargo', ['build', '--release'], workingDirectory: rustDir.toFilePath()) and only checks result.exitCode; add a pre-check that verifies cargo is available (e.g. run Process.run('cargo', ['--version']) and/or catch OSError when invoking Process.run) and surface a clear error like "cargo not found or not in PATH" before attempting the build; also wrap the existing Process.run call in try/catch to handle and log invocation failures (use the existing result/stdout/stderr/error handling and update the thrown Exception message for clarity) so failures when cargo is missing produce an explicit, actionable error.pubspec.yaml (1)
72-74: Movenative_assets_clitodev_dependencies.
native_assets_cliis only used by the build hook (hook/build.dart) at compile time and is not imported in any runtime code. It should be indev_dependenciesto avoid unnecessarily increasing the app's dependency footprint.♻️ Proposed fix
dependencies: # ... other deps sqflite_common_ffi: ^2.3.6+1 path: ^1.9.1 - native_assets_cli: ^0.18.0 dev_dependencies: build_runner: null flutter_gen_runner: null flutter_lints: ^5.0.0 http_mock_adapter: ^0.6.1 ffigen: ^8.0.1 + native_assets_cli: ^0.18.0🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pubspec.yaml` around lines 72 - 74, The pubspec currently lists native_assets_cli under regular dependencies but it’s only used at build time (hook/build.dart); move the package entry named native_assets_cli from the dependencies section to the dev_dependencies section in pubspec.yaml so it is only a development-time dependency and does not bloat runtime installs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/app/modules/splash/controllers/splash_controller.dart`:
- Around line 141-142: The update check currently guards only against desktop
but should only run on Android to avoid calling the Android-only in_app_update
on iOS; in the checkForUpdate() method add/replace the platform guard to return
early when Platform.isAndroid is false (i.e., only proceed when
Platform.isAndroid) so the in_app_update calls are never invoked on iOS and
other unsupported platforms.
In `@lib/app/utils/app_settings/app_settings.dart`:
- Around line 18-20: The guard around HomeWidget.setAppGroupId currently uses
!Platform.isLinux which is too broad; change the condition to only run on iOS by
checking Platform.isIOS before calling
HomeWidget.setAppGroupId("group.taskwarrior"). Update the Platform check near
the HomeWidget.setAppGroupId call so it matches other HomeWidget usages (e.g.,
use Platform.isAndroid || Platform.isIOS elsewhere) and ensure only iOS executes
the App Group setup.
In `@lib/main.dart`:
- Around line 15-16: The unconditional native imports (dart:ffi, dart:io,
package:sqflite_common_ffi/sqflite) are causing web compile failures; change
them to conditional imports using Dart's if (dart.library.io) syntax by creating
a platform-specific implementation (e.g., a small shim file exported as a single
symbol like DatabaseProvider or initFfi) and an alternative stub for web (no-op
or web-compatible implementation), then update code that calls Platform.isLinux
and the FFI initialization (the block guarded by Platform.isLinux and any
initFfi/createFfiDatabase calls) to use the shared symbol from the conditional
import so web builds never reference dart:io or dart:ffi. Ensure all native-only
imports are moved into the io-specific implementation and the main file imports
only the conditional shim.
---
Nitpick comments:
In `@hook/build.dart`:
- Around line 16-26: The build step currently calls Process.run('cargo',
['build', '--release'], workingDirectory: rustDir.toFilePath()) and only checks
result.exitCode; add a pre-check that verifies cargo is available (e.g. run
Process.run('cargo', ['--version']) and/or catch OSError when invoking
Process.run) and surface a clear error like "cargo not found or not in PATH"
before attempting the build; also wrap the existing Process.run call in
try/catch to handle and log invocation failures (use the existing
result/stdout/stderr/error handling and update the thrown Exception message for
clarity) so failures when cargo is missing produce an explicit, actionable
error.
In `@lib/app/v3/db/task_database.dart`:
- Around line 300-311: The fetchUniqueProjects method creates a new TaskDatabase
and opens it instead of using the current instance's connection and also uses
non-standard "IS NOT" for string comparison; update fetchUniqueProjects to
operate on this._database (call this.open() / this.ensureDatabaseIsOpen() or
assume the instance is already open) rather than instantiating a new
TaskDatabase, and change the rawQuery string to use standard string inequality
(e.g., status <> 'deleted' or status != 'deleted') when querying with rawQuery
to return distinct project values from _database.
In `@pubspec.yaml`:
- Around line 72-74: The pubspec currently lists native_assets_cli under regular
dependencies but it’s only used at build time (hook/build.dart); move the
package entry named native_assets_cli from the dependencies section to the
dev_dependencies section in pubspec.yaml so it is only a development-time
dependency and does not bloat runtime installs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2e11baa1-9738-46c7-8cfe-f752346b4150
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
hook/build.dartlib/app/modules/home/controllers/home_controller.dartlib/app/modules/home/controllers/widget.controller.dartlib/app/modules/splash/controllers/splash_controller.dartlib/app/utils/app_settings/app_settings.dartlib/app/v3/db/task_database.dartlib/main.dartlinux/flutter/generated_plugin_registrant.cclinux/flutter/generated_plugins.cmakemacos/Flutter/GeneratedPluginRegistrant.swiftpubspec.yamlwindows/flutter/generated_plugin_registrant.ccwindows/flutter/generated_plugins.cmake
There was a problem hiding this comment.
♻️ Duplicate comments (1)
lib/main.dart (1)
1-3:⚠️ Potential issue | 🟠 MajorShared entrypoint still won't compile for web.
Lines 1-3 and Line 12 still pull native-only libraries into
main.dart. Those imports are resolved beforekIsWeborPlatform.isLinuxruns, so the Linux guards added here do not fix the original web build failure. Move the FFI /dart:io/sqflite_common_fficode behind conditional imports and keepmain.darton a platform-neutral API.Also applies to: 12-12, 18-37
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/main.dart` around lines 1 - 3, main.dart currently imports native-only libraries (dart:ffi, dart:io, sqflite_common_ffi) at top-level so the web build fails because those imports are resolved before kIsWeb/Platform checks; remove those native imports from lib/main.dart and keep main.dart platform-neutral, then create a separate platform-specific implementation file (e.g., sqlite_native.dart) and a web stub (e.g., sqlite_web.dart) and wire them via Dart conditional imports/exports to provide a single platform-neutral API (e.g., initializeNativeBindings or getDatabaseFactory) that main() calls; ensure the conditional import uses the proper library tests (dart.library.ffi / dart.library.io / dart.library.html) so native-only code (dart:ffi, sqflite_common_ffi, dart:io) is only compiled for supported platforms.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@lib/main.dart`:
- Around line 1-3: main.dart currently imports native-only libraries (dart:ffi,
dart:io, sqflite_common_ffi) at top-level so the web build fails because those
imports are resolved before kIsWeb/Platform checks; remove those native imports
from lib/main.dart and keep main.dart platform-neutral, then create a separate
platform-specific implementation file (e.g., sqlite_native.dart) and a web stub
(e.g., sqlite_web.dart) and wire them via Dart conditional imports/exports to
provide a single platform-neutral API (e.g., initializeNativeBindings or
getDatabaseFactory) that main() calls; ensure the conditional import uses the
proper library tests (dart.library.ffi / dart.library.io / dart.library.html) so
native-only code (dart:ffi, sqflite_common_ffi, dart:io) is only compiled for
supported platforms.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9a0821df-c2a9-4125-aa5d-356f3e63d4d8
📒 Files selected for processing (3)
lib/app/modules/splash/controllers/splash_controller.dartlib/app/utils/app_settings/app_settings.dartlib/main.dart
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/app/utils/app_settings/app_settings.dart
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/main.dart (1)
50-51:⚠️ Potential issue | 🟡 MinorRemove or integrate the
loadNativeLibrary()call properly.The function's return value is discarded at line 50, and
RustLib.init()(line 51) handles library loading automatically viaflutter_rust_bridge's default configuration (kDefaultExternalLibraryLoaderConfig). Either remove the call entirely if platform validation is handled elsewhere, or pass the returnedDynamicLibrarytoRustLib.init(externalLibrary: ...)if custom platform-specific paths are required. Currently, the function call is redundant and performs unnecessary work that is never used.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/main.dart` around lines 50 - 51, The call to loadNativeLibrary() is redundant because its return value is discarded and RustLib.init() already loads the library via flutter_rust_bridge; either remove the loadNativeLibrary() invocation, or capture its returned DynamicLibrary and pass it into RustLib.init(externalLibrary: ...) so the library you load is actually used (also keep any platform validation inside loadNativeLibrary() if that behavior is required). Update the call site accordingly: replace the standalone loadNativeLibrary() call with either nothing (remove) or with a variable assign and pass that variable to RustLib.init(externalLibrary: ...).
🧹 Nitpick comments (1)
lib/main.dart (1)
29-31: Linux native library loading is correct; minor style nit.The Linux branch correctly loads
libtc_helper.so, which aligns with thehook/build.dartoutput. Minor formatting: add a space beforeelse iffor consistency.🔧 Suggested formatting fix
- }else if (Platform.isLinux) { // Add this block + } else if (Platform.isLinux) { return DynamicLibrary.open('libtc_helper.so'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/main.dart` around lines 29 - 31, Minor style nit: change the " }else if (Platform.isLinux) { return DynamicLibrary.open('libtc_helper.so');" branch to include a space before else so it reads " } else if (Platform.isLinux) { ... }"; locate the Platform.isLinux check and the DynamicLibrary.open('libtc_helper.so') call in main.dart and update the spacing to match the surrounding formatting convention.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@lib/main.dart`:
- Around line 50-51: The call to loadNativeLibrary() is redundant because its
return value is discarded and RustLib.init() already loads the library via
flutter_rust_bridge; either remove the loadNativeLibrary() invocation, or
capture its returned DynamicLibrary and pass it into
RustLib.init(externalLibrary: ...) so the library you load is actually used
(also keep any platform validation inside loadNativeLibrary() if that behavior
is required). Update the call site accordingly: replace the standalone
loadNativeLibrary() call with either nothing (remove) or with a variable assign
and pass that variable to RustLib.init(externalLibrary: ...).
---
Nitpick comments:
In `@lib/main.dart`:
- Around line 29-31: Minor style nit: change the " }else if (Platform.isLinux) {
return DynamicLibrary.open('libtc_helper.so');" branch to include a space before
else so it reads " } else if (Platform.isLinux) { ... }"; locate the
Platform.isLinux check and the DynamicLibrary.open('libtc_helper.so') call in
main.dart and update the spacing to match the surrounding formatting convention.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
lib/main.dart (1)
1-3:⚠️ Potential issue | 🟠 MajorConditional imports are still required for web builds.
This is the same blocker raised on the earlier revision:
kIsWebonly guards execution, but these native-only imports are still part of the web compilation unit. As long aslib/main.dartimportsdart:ffi,dart:io, and the FFI helpers unconditionally, Chrome builds will fail beforeloadNativeLibrary()can throw. Please move the native loader behindif (dart.library.io)shims and keepmain.dartfree of native-only imports.In Flutter/Dart, do `kIsWeb` or `Platform.is...` runtime guards prevent web compile errors from unconditional `dart:ffi` / `dart:io` imports, or are conditional imports required instead?Also applies to: 12-13, 19-22
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/main.dart` around lines 1 - 3, Remove unconditional native-only imports (dart:ffi, dart:io) from lib/main.dart and replace them with a conditional import/shim that uses dart.library.io to load the native implementation; keep main.dart free of native imports and only import a platform-neutral interface. Move the native code currently referenced by loadNativeLibrary() into a separate file (e.g., native_loader_impl) and create a stub/web implementation (e.g., native_loader_stub) that is selected via conditional imports so the web build never sees dart:ffi/dart:io. Ensure main.dart imports only the interface (e.g., native_loader) and calls loadNativeLibrary() as before, with the actual native implementation behind the dart.library.io conditional import.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@lib/main.dart`:
- Around line 1-3: Remove unconditional native-only imports (dart:ffi, dart:io)
from lib/main.dart and replace them with a conditional import/shim that uses
dart.library.io to load the native implementation; keep main.dart free of native
imports and only import a platform-neutral interface. Move the native code
currently referenced by loadNativeLibrary() into a separate file (e.g.,
native_loader_impl) and create a stub/web implementation (e.g.,
native_loader_stub) that is selected via conditional imports so the web build
never sees dart:ffi/dart:io. Ensure main.dart imports only the interface (e.g.,
native_loader) and calls loadNativeLibrary() as before, with the actual native
implementation behind the dart.library.io conditional import.
Improvements: - In loadNativeLibrary(): Added Platform.isWindows check for tc_helper.dll. - In main(): Updated the platform guard to include Windows and macOS for SQLite initialization. - Goal: Ensures the database and native logic work correctly across all desktop platforms.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
lib/main.dart (1)
3-3:⚠️ Potential issue | 🟠 Major
kIsWebstill does not make this entrypoint web-compilable.Chrome builds fail before Line 20 can run, because this library still imports
dart:ffi,dart:io, andsqflite_common_ffiunconditionally. If web is intentionally unsupported, this branch is misleading; otherwise the native code needs to move behind conditional imports/exports.In Flutter/Dart, can a library that unconditionally imports `dart:ffi`, `dart:io`, or `package:sqflite_common_ffi/sqflite_ffi.dart` still compile for Flutter Web if every native call is wrapped in `kIsWeb` or `Platform.isX` guards, or are conditional imports required?Also applies to: 12-13, 19-21
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/main.dart` at line 3, The file unconditionally imports native-only libraries even though it checks kIsWeb; replace the direct imports of dart:ffi, dart:io and package:sqflite_common_ffi with conditional imports/exports and a web-friendly stub so the module can compile for web; locate the imports in lib/main.dart (the line importing kIsWeb/debugPrintSynchronously and surrounding native imports) and create two implementations (native and web) exported via conditional import, then update any calls that rely on the native API to use the conditionally-imported API so kIsWeb guards are not the only protection.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/main.dart`:
- Around line 29-33: The code is attempting to load macOS/Windows native libs
even though the build hook only builds Linux; remove the Platform.isWindows and
macOS branches that call ExternalLibrary.open (i.e., drop the tc_helper.dll and
tc_helper.framework/tc_helper cases) and ensure the Rust FFI initialization call
currently run for all desktops is gated by Platform.isLinux (so
ExternalLibrary.open and the FFI init only run when Platform.isLinux), or
alternatively update the build hook's early-return check (the targetOS !=
OS.linux guard) to include proper build steps for macOS and Windows if you
intend to support those platforms.
---
Duplicate comments:
In `@lib/main.dart`:
- Line 3: The file unconditionally imports native-only libraries even though it
checks kIsWeb; replace the direct imports of dart:ffi, dart:io and
package:sqflite_common_ffi with conditional imports/exports and a web-friendly
stub so the module can compile for web; locate the imports in lib/main.dart (the
line importing kIsWeb/debugPrintSynchronously and surrounding native imports)
and create two implementations (native and web) exported via conditional import,
then update any calls that rely on the native API to use the
conditionally-imported API so kIsWeb guards are not the only protection.
|
Many generated files. |
I've addressed the feedback regarding the generated files. I’ve cleaned up the PR by removing all irrelevant platform-generated files (like .cmake and lockfiles) and refactored
Ready for your review! |
|
hi @SGI-CAPP-AT2 @cfsmp3 so that i can move forward |
|
@coderabbitai resume |
✅ Actions performedReviews resumed. |
There was a problem hiding this comment.
♻️ Duplicate comments (2)
lib/main.dart (2)
30-33:⚠️ Potential issue | 🟠 MajorPlatform branches and native build outputs are not aligned yet.
Line 30-Line 33 and Line 41-Line 44 enable Linux/Windows/macOS runtime paths, but this PR’s build automation context indicates Linux-native asset generation. If Windows/macOS artifacts are not produced, these paths will fail at startup.
If Linux-only support is intended for now, scope runtime paths accordingly
- } else if (Platform.isMacOS) { - return DynamicLibrary.open('tc_helper.framework/tc_helper'); - } else if (Platform.isLinux) { + } else if (Platform.isLinux) { return DynamicLibrary.open('libtc_helper.so'); - } else if (Platform.isWindows) { - return DynamicLibrary.open('tc_helper.dll'); } @@ - if (!kIsWeb && (Platform.isLinux || Platform.isWindows || Platform.isMacOS)) { + if (!kIsWeb && Platform.isLinux) { sqfliteFfiInit(); databaseFactory = databaseFactoryFfi; }#!/bin/bash set -euo pipefail build_file="$(fd -i '^build\\.dart$' . | rg '/hook/' | head -n1 || true)" echo "Detected build hook: ${build_file:-<none>}" if [ -n "${build_file}" ]; then echo "== hook/build.dart (top) ==" sed -n '1,220p' "${build_file}" fi echo echo "== Platform/build signals in hook and main ==" rg -n -C2 "targetOS|OS\\.linux|OS\\.windows|OS\\.macos|tc_helper|DynamicLoadingBundled" \ "${build_file:-lib/main.dart}" lib/main.dartAlso applies to: 41-44
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/main.dart` around lines 30 - 33, The platform-specific DynamicLibrary.open branches (Platform.isLinux / Platform.isWindows / Platform.isMacOS) in the function that loads the native helper (the calls to DynamicLibrary.open('libtc_helper.so'), DynamicLibrary.open('tc_helper.dll'), and the macOS branch) must be aligned with build outputs; either restrict runtime paths to Linux-only (keep only Platform.isLinux and remove/replace the Windows/macOS branches) or add a runtime guard that checks for the presence of the built native artifact before attempting to open it and otherwise throw a clear error. Locate the loader that uses Platform.isLinux / Platform.isWindows / Platform.isMacOS and update it so it only tries Linux artifacts when only Linux-native assets are produced, or implements an existence check/explicit error for Windows/macOS paths to avoid failing at startup.
12-13:⚠️ Potential issue | 🟠 MajorUse conditional imports for native-only dependencies.
Line 12 and Line 13 are still unconditional native imports. Dart resolves imports at compile time, so Line 20 runtime guards do not prevent web compilation failures.
Suggested fix direction
-import 'dart:ffi'; -import 'dart:io'; ... -import 'package:sqflite_common_ffi/sqflite_ffi.dart'; -import 'package:flutter_rust_bridge/flutter_rust_bridge_for_generated.dart'; +import 'package:taskwarrior/platform/platform_bootstrap.dart';// lib/platform/platform_bootstrap.dart export 'platform_bootstrap_stub.dart' if (dart.library.io) 'platform_bootstrap_io.dart';#!/bin/bash set -euo pipefail echo "== Inspect native imports in lib/main.dart ==" sed -n '1,80p' lib/main.dart echo echo "== Search for conditional import/export usage in lib/ ==" rg -n --type=dart "dart\\.library\\.(io|html|js_interop)" lib🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/main.dart` around lines 12 - 13, Replace the unconditional native-only imports in lib/main.dart (the direct imports of sqflite_common_ffi and flutter_rust_bridge_for_generated) with a conditional platform bootstrap export: create platform_bootstrap.dart that exports platform_bootstrap_stub.dart if not on dart:io and platform_bootstrap_io.dart if dart.library.io, move the native imports into platform_bootstrap_io.dart (importing sqflite_common_ffi and flutter_rust_bridge_for_generated there) and keep platform_bootstrap_stub.dart as a no-op/stand-in for web, then update lib/main.dart to import the single platform_bootstrap.dart so web builds no longer see native-only packages at compile time.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@lib/main.dart`:
- Around line 30-33: The platform-specific DynamicLibrary.open branches
(Platform.isLinux / Platform.isWindows / Platform.isMacOS) in the function that
loads the native helper (the calls to DynamicLibrary.open('libtc_helper.so'),
DynamicLibrary.open('tc_helper.dll'), and the macOS branch) must be aligned with
build outputs; either restrict runtime paths to Linux-only (keep only
Platform.isLinux and remove/replace the Windows/macOS branches) or add a runtime
guard that checks for the presence of the built native artifact before
attempting to open it and otherwise throw a clear error. Locate the loader that
uses Platform.isLinux / Platform.isWindows / Platform.isMacOS and update it so
it only tries Linux artifacts when only Linux-native assets are produced, or
implements an existence check/explicit error for Windows/macOS paths to avoid
failing at startup.
- Around line 12-13: Replace the unconditional native-only imports in
lib/main.dart (the direct imports of sqflite_common_ffi and
flutter_rust_bridge_for_generated) with a conditional platform bootstrap export:
create platform_bootstrap.dart that exports platform_bootstrap_stub.dart if not
on dart:io and platform_bootstrap_io.dart if dart.library.io, move the native
imports into platform_bootstrap_io.dart (importing sqflite_common_ffi and
flutter_rust_bridge_for_generated there) and keep platform_bootstrap_stub.dart
as a no-op/stand-in for web, then update lib/main.dart to import the single
platform_bootstrap.dart so web builds no longer see native-only packages at
compile time.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d573f374-b856-47ec-9259-7e28f8a4120f
📒 Files selected for processing (3)
lib/app/modules/home/controllers/home_controller.dartlib/app/modules/splash/controllers/splash_controller.dartlib/main.dart
✅ Files skipped from review due to trivial changes (1)
- lib/app/modules/splash/controllers/splash_controller.dart
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/app/modules/home/controllers/home_controller.dart
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/main.dart (1)
30-31:⚠️ Potential issue | 🔴 CriticalGuard
loadNativeLibrary()call to desktop platforms only—currently crashes on Android and iOS.The unconditional
loadNativeLibrary()call at line 30 will crash on Android and iOS becauseplatform_bootstrap.dartselectsplatform_bootstrap_io.dartwhendart.library.iois available. On mobile, that implementation throwsUnsupportedErrorfor unsupported platforms. A stub exists that safely returnsnull, but it's never selected on mobile devices.Wrap the calls with a platform guard:
Proposed fix
+import 'dart:io' show Platform; + void main() async { WidgetsFlutterBinding.ensureInitialized(); initPlatform(); // Redirect debug prints to the local database logger debugPrint = (String? message, {int? wrapWidth}) { if (message != null) { debugPrintSynchronously(message, wrapWidth: wrapWidth); _logDatabaseHelper.insertLog(message); } }; debugPrint("🚀 BOOT: main() started"); - loadNativeLibrary(); - await RustLib.init(); + // Native Rust library only available on desktop platforms + if (Platform.isLinux || Platform.isWindows || Platform.isMacOS) { + loadNativeLibrary(); + await RustLib.init(); + } await AppSettings.init();Note: Adding
import 'dart:io'directly breaks web compilation. Use either abool get isDesktophelper in the platform bootstrap safe for all platforms, orfoundation'sdefaultTargetPlatformcheck, which is web-safe.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/main.dart` around lines 30 - 31, The unconditional call to loadNativeLibrary() (and subsequent await RustLib.init()) must be guarded to run only on desktop platforms; update the platform bootstrap (platform_bootstrap.dart / platform_bootstrap_io.dart) to expose a boolean like isDesktop (implemented in a web-safe way or via foundation's defaultTargetPlatform) or add a local desktop check, then wrap loadNativeLibrary() and RustLib.init() with that guard so they are skipped on Android/iOS/web where the native loader throws UnsupportedError; reference loadNativeLibrary(), RustLib.init(), platform_bootstrap.dart and platform_bootstrap_io.dart when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/platform_bootstrap_io.dart`:
- Around line 10-19: The loadNativeLibrary function currently throws on mobile
and uses incorrect macOS/Windows paths; change loadNativeLibrary to first guard
for mobile (Platform.isAndroid || Platform.isIOS) and return null or a safe
no-op stub so mobile apps don't crash, then restrict native loading to the only
supported artifact produced by the build hook (keep the Linux branch using
'libtc_helper.so' and remove the macOS/Windows DynamicLibrary.open branches for
now), and add a comment referencing hook/build.dart so when you later extend the
build hook to produce macOS (.dylib) or Windows artifacts you can restore
correct paths (macOS should use libtc_helper.dylib, not a framework).
---
Outside diff comments:
In `@lib/main.dart`:
- Around line 30-31: The unconditional call to loadNativeLibrary() (and
subsequent await RustLib.init()) must be guarded to run only on desktop
platforms; update the platform bootstrap (platform_bootstrap.dart /
platform_bootstrap_io.dart) to expose a boolean like isDesktop (implemented in a
web-safe way or via foundation's defaultTargetPlatform) or add a local desktop
check, then wrap loadNativeLibrary() and RustLib.init() with that guard so they
are skipped on Android/iOS/web where the native loader throws UnsupportedError;
reference loadNativeLibrary(), RustLib.init(), platform_bootstrap.dart and
platform_bootstrap_io.dart when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9c014505-aa65-474e-a336-a8e2ca62065d
📒 Files selected for processing (5)
hook/build.dartlib/main.dartlib/platform_bootstrap.dartlib/platform_bootstrap_io.dartlib/platform_bootstrap_stub.dart
✅ Files skipped from review due to trivial changes (2)
- lib/platform_bootstrap.dart
- lib/platform_bootstrap_stub.dart
🚧 Files skipped from review as they are similar to previous changes (1)
- hook/build.dart
lib/platform_bootstrap_io.dart
Outdated
| DynamicLibrary loadNativeLibrary() { | ||
| if (Platform.isMacOS) { | ||
| return DynamicLibrary.open('tc_helper.framework/tc_helper'); | ||
| } else if (Platform.isLinux) { | ||
| return DynamicLibrary.open('libtc_helper.so'); | ||
| } else if (Platform.isWindows) { | ||
| return DynamicLibrary.open('tc_helper.dll'); | ||
| } | ||
| throw UnsupportedError('Platform not supported'); | ||
| } No newline at end of file |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the build hook's platform gating and library filename expectations
echo "=== Build hook platform check ==="
rg -n "targetOS|OS\.linux|OS\.windows|OS\.macOS" hook/build.dart
echo ""
echo "=== Library filename in build hook ==="
rg -n "libName|\.so|\.dylib|\.dll" hook/build.dart
echo ""
echo "=== Cargo.toml crate-type ==="
cat rust/Cargo.toml | head -15Repository: CCExtractor/taskwarrior-flutter
Length of output: 671
🏁 Script executed:
rg -n "loadNativeLibrary" --type dartRepository: CCExtractor/taskwarrior-flutter
Length of output: 251
🏁 Script executed:
rg -n "main\(" lib/ --type dart -A 10 | head -50Repository: CCExtractor/taskwarrior-flutter
Length of output: 1116
🏁 Script executed:
rg -n "android|ios|mobile" --type dart -i | head -20Repository: CCExtractor/taskwarrior-flutter
Length of output: 2490
🏁 Script executed:
fd -e dart -e yaml | xargs rg -l "Android|iOS|mobile" -i | head -10Repository: CCExtractor/taskwarrior-flutter
Length of output: 612
🏁 Script executed:
rg -n "import.*platform_bootstrap" lib/ --type dartRepository: CCExtractor/taskwarrior-flutter
Length of output: 145
🏁 Script executed:
cat lib/platform_bootstrap_stub.dartRepository: CCExtractor/taskwarrior-flutter
Length of output: 294
🏁 Script executed:
cat lib/main.dart | head -40Repository: CCExtractor/taskwarrior-flutter
Length of output: 1451
🏁 Script executed:
find lib -name "platform_bootstrap.dart" -o -name "platform_bootstrap*" | head -20Repository: CCExtractor/taskwarrior-flutter
Length of output: 166
🏁 Script executed:
cat lib/platform_bootstrap.dart 2>/dev/null || echo "File not found, searching structure..."Repository: CCExtractor/taskwarrior-flutter
Length of output: 167
🏁 Script executed:
fd platform_bootstrap lib/ --type fRepository: CCExtractor/taskwarrior-flutter
Length of output: 166
Mobile platforms will crash; macOS/Windows library paths are incorrect.
Three critical issues with this function:
-
Mobile crash:
dart.library.iois true on Android/iOS, soplatform_bootstrap_io.dartis used on mobile. Since none of thePlatform.is*conditions match mobile platforms, line 17-18 throwsUnsupportedError, crashing the app at startup on all mobile devices. -
macOS library path mismatch: The Cargo
cdylibproduceslibtc_helper.dylibon macOS, nottc_helper.framework/tc_helper. Framework bundles require additional packaging not performed by the build hook. -
Windows/macOS not built: Per
hook/build.dartline 11, the build hook early-returns for non-Linux platforms, so these libraries won't exist at runtime.
The core fix requires either:
- Option A: Remove Windows/macOS branches entirely until the build hook supports them, and add mobile-safe handling (guard the call in
main.dartor return early inloadNativeLibrary()for mobile). - Option B: Extend
hook/build.dartto build for Windows/macOS and add a mobile guard inloadNativeLibrary()that returns null or uses a no-op stub.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/platform_bootstrap_io.dart` around lines 10 - 19, The loadNativeLibrary
function currently throws on mobile and uses incorrect macOS/Windows paths;
change loadNativeLibrary to first guard for mobile (Platform.isAndroid ||
Platform.isIOS) and return null or a safe no-op stub so mobile apps don't crash,
then restrict native loading to the only supported artifact produced by the
build hook (keep the Linux branch using 'libtc_helper.so' and remove the
macOS/Windows DynamicLibrary.open branches for now), and add a comment
referencing hook/build.dart so when you later extend the build hook to produce
macOS (.dylib) or Windows artifacts you can restore correct paths (macOS should
use libtc_helper.dylib, not a framework).
bhola-dev58
force-pushed
the
feature/linux-automation
branch
from
7c2ca95 to
38d8193
Compare
|
Tested on android with Linux OS . It running correctly |
2 participants
Description
This PR implements full Linux support and automates the native Rust library build process. It fixes the "Missing Library" crashes and database initialization errors previously encountered on Ubuntu.
Key Changes:
cargo buildduring the app launch.task_database.dartthat caused crashes on modern Linux environments.Note on Dependencies:
I noticed
native_assets_cliis being transitioned to thehookspackage by the Flutter team. I’ve kept the implementation compatible with version0.18.0for current SDK stability, but I am ready to migrate to thehookspackage once the API stabilizes.Screenshots in Phase 1
Final Verification (Linux System Proof)
Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores