From 5cad98224fcedf23047ba2a893836315508b6fbb Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Thu, 14 May 2026 21:21:09 +0200 Subject: [PATCH] keep the clide.dart barrel Flutter-free MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `lib/kernel/src/toolchain.dart` is a `ChangeNotifier`, so it pulls in `package:flutter/foundation.dart`. `GitClient` and `PqlClient` imported it for the resolved binary paths, which leaked Flutter through the `package:clide/clide.dart` barrel — breaking `dart test` on every core subsystem suite (`ci/test_core.sh`), since pure Dart can't compile Flutter packages. Split the Flutter-free pieces into `toolchain_paths.dart`: `ResolvedPaths`, `resolveToolchainPaths`, and a new read-only `ToolchainView` interface with a `ToolchainView.resolved()` const factory. `Toolchain` now implements `ToolchainView`; the clients depend on the interface. Core test setups that built a `Toolchain` just to call `applyResolved` switch to the factory. Co-Authored-By: Claude --- lib/kernel/src/toolchain.dart | 158 +++------------------- lib/kernel/src/toolchain_paths.dart | 135 ++++++++++++++++++ lib/src/git/client.dart | 6 +- lib/src/pql/client.dart | 4 +- test/daemon/git_commands_errors_test.dart | 5 +- test/daemon/git_commands_test.dart | 5 +- test/daemon/pql_commands_errors_test.dart | 5 +- test/daemon/pql_commands_test.dart | 5 +- test/git/client_test.dart | 11 +- test/pql/client_test.dart | 8 +- 10 files changed, 172 insertions(+), 170 deletions(-) create mode 100644 lib/kernel/src/toolchain_paths.dart diff --git a/lib/kernel/src/toolchain.dart b/lib/kernel/src/toolchain.dart index 67eececf..7cf7a9bd 100644 --- a/lib/kernel/src/toolchain.dart +++ b/lib/kernel/src/toolchain.dart @@ -3,31 +3,21 @@ /// Resolution runs in a background isolate via [Toolchain.resolvePaths] /// to avoid blocking the merged UI/platform thread on macOS. The result /// is applied on the main thread via [Toolchain.applyResolved]. +/// +/// The Flutter-free data types ([ResolvedPaths], [ToolchainView]) and +/// the isolate-side resolver ([resolveToolchainPaths]) live in +/// `toolchain_paths.dart` and are re-exported here for convenience. library; import 'dart:async'; -import 'dart:io'; import 'package:flutter/foundation.dart'; -/// Serializable result of tool resolution (crosses isolate boundary). -class ResolvedPaths { - const ResolvedPaths({ - this.git, - this.pql, - this.tmux, - this.shell, - this.gitEnv, - }); +import 'toolchain_paths.dart'; - final String? git; - final String? pql; - final String? tmux; - final String? shell; - final Map? gitEnv; -} +export 'toolchain_paths.dart'; -class Toolchain extends ChangeNotifier { +class Toolchain extends ChangeNotifier implements ToolchainView { String? _git; String? _pql; String? _tmux; @@ -35,17 +25,25 @@ class Toolchain extends ChangeNotifier { Map? _gitEnv; bool _resolved = false; + @override String get git => _git ?? 'git'; + @override String get pql => _pql ?? 'pql'; + @override String get tmux => _tmux ?? 'tmux'; + @override String get shell => _shell ?? '/bin/bash'; /// Extra environment variables for git (e.g. GIT_EXEC_PATH for dugite). + @override Map? get gitEnv => _gitEnv; + @override bool get resolved => _resolved; + @override bool get allOk => _resolved && missing.isEmpty; + @override List get missing => [ if (_git == null) 'git', if (_pql == null) 'pql', @@ -79,127 +77,7 @@ class Toolchain extends ChangeNotifier { } /// Pure function — runs in a background isolate. All file I/O happens - /// here, off the main thread. - static ResolvedPaths resolvePaths({required String workspaceRoot}) { - final dugite = '$workspaceRoot/native/dugite/bin'; - - String? git; - Map? gitEnv; - final dugiteGit = _firstExisting(['$dugite/git']); - if (dugiteGit != null) { - git = dugiteGit; - final dugiteRoot = File(dugiteGit).parent.parent.path; - gitEnv = { - 'GIT_EXEC_PATH': '$dugiteRoot/libexec/git-core', - 'GIT_TEMPLATE_DIR': '$dugiteRoot/share/git-core/templates', - }; - } else { - git = _findOnPath('git'); - } - - final pql = _findOnPath('pql'); - final tmux = _findOnPath('tmux'); - final shell = _findOnPath(Platform.environment['SHELL']?.split('/').last ?? 'bash'); - - return ResolvedPaths( - git: git, - pql: pql, - tmux: tmux, - shell: shell, - gitEnv: gitEnv, - ); - } - - static String? _findOnPath(String name) { - for (final dir in _expandedPath().split(':')) { - if (dir.isEmpty) continue; - final f = File('$dir/$name'); - if (f.existsSync()) return f.path; - } - return null; - } - - /// Build expanded PATH inline — must be self-contained for isolate use. - static String _expandedPath() { - final base = Platform.environment['PATH'] ?? ''; - if (!Platform.isMacOS) return base; - final home = Platform.environment['HOME'] ?? ''; - final extras = [ - if (home.isNotEmpty) '$home/.local/bin', - '/opt/homebrew/bin', - '/opt/homebrew/sbin', - '/usr/local/bin', - ]; - final existing = base.split(':').toSet(); - final missing = extras.where((p) => !existing.contains(p)); - if (missing.isEmpty) return base; - return [...missing, ...existing].join(':'); - } - - static String? _firstExisting(List candidates) { - for (final c in candidates) { - if (File(c).existsSync()) return c; - } - return null; - } -} - -/// Top-level function for compute/isolate use. Takes a single String -/// argument (the workspace root) and returns a plain-data result. -ResolvedPaths resolveToolchainPaths(String workspaceRoot) { - final dugite = '$workspaceRoot/native/dugite/bin'; - - String? git; - Map? gitEnv; - final dugiteGit = _firstExistingStandalone(['$dugite/git']); - if (dugiteGit != null) { - git = dugiteGit; - final dugiteRoot = File(dugiteGit).parent.parent.path; - gitEnv = { - 'GIT_EXEC_PATH': '$dugiteRoot/libexec/git-core', - 'GIT_TEMPLATE_DIR': '$dugiteRoot/share/git-core/templates', - }; - } else { - git = _findOnPathStandalone('git'); - } - - return ResolvedPaths( - git: git, - pql: _findOnPathStandalone('pql'), - tmux: _findOnPathStandalone('tmux'), - shell: _findOnPathStandalone(Platform.environment['SHELL']?.split('/').last ?? 'bash'), - gitEnv: gitEnv, - ); -} - -String? _findOnPathStandalone(String name) { - for (final dir in _expandedPathStandalone().split(':')) { - if (dir.isEmpty) continue; - final f = File('$dir/$name'); - if (f.existsSync()) return f.path; - } - return null; -} - -String? _firstExistingStandalone(List candidates) { - for (final c in candidates) { - if (File(c).existsSync()) return c; - } - return null; -} - -String _expandedPathStandalone() { - final base = Platform.environment['PATH'] ?? ''; - if (!Platform.isMacOS) return base; - final home = Platform.environment['HOME'] ?? ''; - final extras = [ - if (home.isNotEmpty) '$home/.local/bin', - '/opt/homebrew/bin', - '/opt/homebrew/sbin', - '/usr/local/bin', - ]; - final existing = base.split(':').toSet(); - final missing = extras.where((p) => !existing.contains(p)); - if (missing.isEmpty) return base; - return [...missing, ...existing].join(':'); + /// here, off the main thread. Delegates to the Flutter-free + /// [resolveToolchainPaths]. + static ResolvedPaths resolvePaths({required String workspaceRoot}) => resolveToolchainPaths(workspaceRoot); } diff --git a/lib/kernel/src/toolchain_paths.dart b/lib/kernel/src/toolchain_paths.dart new file mode 100644 index 00000000..51722b10 --- /dev/null +++ b/lib/kernel/src/toolchain_paths.dart @@ -0,0 +1,135 @@ +/// Flutter-free toolchain data + resolution. +/// +/// Split out of `toolchain.dart` so that pure-Dart consumers (the IPC +/// subsystems exported through `package:clide/clide.dart`, e.g. +/// [GitClient] and [PqlClient]) don't transitively pull in +/// `package:flutter/foundation.dart`. The live, listenable `Toolchain` +/// stays in `toolchain.dart`; everything here is plain Dart and runs +/// fine under `dart test`. +library; + +import 'dart:io'; + +/// Serializable result of tool resolution (crosses isolate boundary). +class ResolvedPaths { + const ResolvedPaths({ + this.git, + this.pql, + this.tmux, + this.shell, + this.gitEnv, + }); + + final String? git; + final String? pql; + final String? tmux; + final String? shell; + final Map? gitEnv; +} + +/// Read-only view of resolved tool paths. The concrete `Toolchain` +/// (in `toolchain.dart`) implements this on top of `ChangeNotifier`; +/// pure-Dart clients depend on the interface so they stay Flutter-free. +abstract class ToolchainView { + /// A fixed, already-resolved view over [paths]. Flutter-free — handy + /// for tests and isolate-side code that has a [ResolvedPaths] but no + /// need for the listenable `Toolchain`. + const factory ToolchainView.resolved(ResolvedPaths paths) = _StaticToolchain; + + String get git; + String get pql; + String get tmux; + String get shell; + Map? get gitEnv; + bool get resolved; + bool get allOk; + List get missing; +} + +class _StaticToolchain implements ToolchainView { + const _StaticToolchain(this._paths); + + final ResolvedPaths _paths; + + @override + String get git => _paths.git ?? 'git'; + @override + String get pql => _paths.pql ?? 'pql'; + @override + String get tmux => _paths.tmux ?? 'tmux'; + @override + String get shell => _paths.shell ?? '/bin/bash'; + @override + Map? get gitEnv => _paths.gitEnv; + @override + bool get resolved => true; + @override + bool get allOk => missing.isEmpty; + @override + List get missing => [ + if (_paths.git == null) 'git', + if (_paths.pql == null) 'pql', + if (_paths.tmux == null) 'tmux', + ]; +} + +/// Top-level function for compute/isolate use. Takes a single String +/// argument (the workspace root) and returns a plain-data result. +ResolvedPaths resolveToolchainPaths(String workspaceRoot) { + final dugite = '$workspaceRoot/native/dugite/bin'; + + String? git; + Map? gitEnv; + final dugiteGit = _firstExisting(['$dugite/git']); + if (dugiteGit != null) { + git = dugiteGit; + final dugiteRoot = File(dugiteGit).parent.parent.path; + gitEnv = { + 'GIT_EXEC_PATH': '$dugiteRoot/libexec/git-core', + 'GIT_TEMPLATE_DIR': '$dugiteRoot/share/git-core/templates', + }; + } else { + git = _findOnPath('git'); + } + + return ResolvedPaths( + git: git, + pql: _findOnPath('pql'), + tmux: _findOnPath('tmux'), + shell: _findOnPath(Platform.environment['SHELL']?.split('/').last ?? 'bash'), + gitEnv: gitEnv, + ); +} + +String? _findOnPath(String name) { + for (final dir in _expandedPath().split(':')) { + if (dir.isEmpty) continue; + final f = File('$dir/$name'); + if (f.existsSync()) return f.path; + } + return null; +} + +String? _firstExisting(List candidates) { + for (final c in candidates) { + if (File(c).existsSync()) return c; + } + return null; +} + +/// Build expanded PATH inline — must be self-contained for isolate use. +String _expandedPath() { + final base = Platform.environment['PATH'] ?? ''; + if (!Platform.isMacOS) return base; + final home = Platform.environment['HOME'] ?? ''; + final extras = [ + if (home.isNotEmpty) '$home/.local/bin', + '/opt/homebrew/bin', + '/opt/homebrew/sbin', + '/usr/local/bin', + ]; + final existing = base.split(':').toSet(); + final missing = extras.where((p) => !existing.contains(p)); + if (missing.isEmpty) return base; + return [...missing, ...existing].join(':'); +} diff --git a/lib/src/git/client.dart b/lib/src/git/client.dart index 6c89ff30..82a2e886 100644 --- a/lib/src/git/client.dart +++ b/lib/src/git/client.dart @@ -1,4 +1,4 @@ -/// Typed git client backed by [Toolchain]. +/// Typed git client backed by a [ToolchainView]. /// /// Every subprocess call goes through [_run] which uses the resolved /// absolute binary path from the toolchain. Parsing is delegated to @@ -7,7 +7,7 @@ library; import 'dart:io'; -import '../../kernel/src/toolchain.dart'; +import '../../kernel/src/toolchain_paths.dart'; import 'diff.dart' show GitDiff, parseDiffOutput; import 'operations.dart' show GitException, GitLogEntry; import 'status.dart'; @@ -15,7 +15,7 @@ import 'status.dart'; class GitClient { GitClient({required this.toolchain, required this.workDir}); - final Toolchain toolchain; + final ToolchainView toolchain; final Directory workDir; // -- queries -------------------------------------------------------------- diff --git a/lib/src/pql/client.dart b/lib/src/pql/client.dart index 97ae7e03..fe592b31 100644 --- a/lib/src/pql/client.dart +++ b/lib/src/pql/client.dart @@ -8,7 +8,7 @@ library; import 'dart:convert'; import 'dart:io'; -import '../../kernel/src/toolchain.dart'; +import '../../kernel/src/toolchain_paths.dart'; class PqlException implements Exception { const PqlException(this.message, {this.exitCode = 1, this.stderr = ''}); @@ -24,7 +24,7 @@ class PqlClient { PqlClient({required this.workDir, required this.toolchain}); final Directory workDir; - final Toolchain toolchain; + final ToolchainView toolchain; Future>> files({String? glob, int? limit}) async { final args = ['files']; diff --git a/test/daemon/git_commands_errors_test.dart b/test/daemon/git_commands_errors_test.dart index 3699730e..50170c8d 100644 --- a/test/daemon/git_commands_errors_test.dart +++ b/test/daemon/git_commands_errors_test.dart @@ -8,7 +8,7 @@ library; import 'dart:io'; import 'package:clide/clide.dart'; -import 'package:clide/kernel/src/toolchain.dart'; +import 'package:clide/kernel/src/toolchain_paths.dart'; import 'package:clide/src/daemon/git_commands.dart'; import 'package:test/test.dart'; @@ -18,8 +18,7 @@ void main() { setUp(() async { sandbox = await Directory.systemTemp.createTemp('clide-git-cmd-err-'); - final toolchain = Toolchain(); - toolchain.applyResolved(const ResolvedPaths(git: '/tmp/clide-no-such-git-binary')); + final toolchain = ToolchainView.resolved(const ResolvedPaths(git: '/tmp/clide-no-such-git-binary')); final git = GitClient(toolchain: toolchain, workDir: sandbox); dispatcher = DaemonDispatcher(); final sink = RecordingEventSink(); diff --git a/test/daemon/git_commands_test.dart b/test/daemon/git_commands_test.dart index 271cc69d..f66a9c21 100644 --- a/test/daemon/git_commands_test.dart +++ b/test/daemon/git_commands_test.dart @@ -1,7 +1,7 @@ import 'dart:io'; import 'package:clide/clide.dart'; -import 'package:clide/kernel/src/toolchain.dart'; +import 'package:clide/kernel/src/toolchain_paths.dart'; import 'package:clide/src/daemon/git_commands.dart'; import 'package:test/test.dart'; @@ -33,8 +33,7 @@ void main() { sink = RecordingEventSink(); dispatcher = DaemonDispatcher(); - final toolchain = Toolchain(); - toolchain.applyResolved(Toolchain.resolvePaths(workspaceRoot: sandbox.path)); + final toolchain = ToolchainView.resolved(resolveToolchainPaths(sandbox.path)); final gitClient = GitClient(toolchain: toolchain, workDir: sandbox); registerGitCommands(dispatcher, gitClient, sink); }); diff --git a/test/daemon/pql_commands_errors_test.dart b/test/daemon/pql_commands_errors_test.dart index a2072bd3..17fbe4df 100644 --- a/test/daemon/pql_commands_errors_test.dart +++ b/test/daemon/pql_commands_errors_test.dart @@ -8,7 +8,7 @@ library; import 'dart:io'; import 'package:clide/clide.dart'; -import 'package:clide/kernel/src/toolchain.dart'; +import 'package:clide/kernel/src/toolchain_paths.dart'; import 'package:clide/src/daemon/pql_commands.dart'; import 'package:clide/src/pql/client.dart'; import 'package:test/test.dart'; @@ -17,8 +17,7 @@ void main() { late DaemonDispatcher dispatcher; setUp(() { - final toolchain = Toolchain(); - toolchain.applyResolved(const ResolvedPaths(pql: '/tmp/clide-no-such-pql-binary')); + final toolchain = ToolchainView.resolved(const ResolvedPaths(pql: '/tmp/clide-no-such-pql-binary')); final pql = PqlClient(workDir: Directory.current, toolchain: toolchain); dispatcher = DaemonDispatcher(); registerPqlCommands(dispatcher, pql); diff --git a/test/daemon/pql_commands_test.dart b/test/daemon/pql_commands_test.dart index 7bf448ed..a0c209d1 100644 --- a/test/daemon/pql_commands_test.dart +++ b/test/daemon/pql_commands_test.dart @@ -1,7 +1,7 @@ import 'dart:io'; import 'package:clide/clide.dart'; -import 'package:clide/kernel/src/toolchain.dart'; +import 'package:clide/kernel/src/toolchain_paths.dart'; import 'package:clide/src/daemon/pql_commands.dart'; import 'package:test/test.dart'; @@ -10,8 +10,7 @@ void main() { late PqlClient pql; setUp(() { - final toolchain = Toolchain(); - toolchain.applyResolved(Toolchain.resolvePaths(workspaceRoot: Directory.current.path)); + final toolchain = ToolchainView.resolved(resolveToolchainPaths(Directory.current.path)); pql = PqlClient(workDir: Directory.current, toolchain: toolchain); dispatcher = DaemonDispatcher(); registerPqlCommands(dispatcher, pql); diff --git a/test/git/client_test.dart b/test/git/client_test.dart index b4191015..f34f6613 100644 --- a/test/git/client_test.dart +++ b/test/git/client_test.dart @@ -5,16 +5,12 @@ library; import 'dart:io'; -import 'package:clide/kernel/src/toolchain.dart'; +import 'package:clide/kernel/src/toolchain_paths.dart'; import 'package:clide/src/git/client.dart'; import 'package:clide/src/git/operations.dart' show GitException; import 'package:test/test.dart'; -Toolchain _toolchain() { - final t = Toolchain(); - t.applyResolved(Toolchain.resolvePaths(workspaceRoot: Directory.current.path)); - return t; -} +ToolchainView _toolchain() => ToolchainView.resolved(resolveToolchainPaths(Directory.current.path)); Future _newRepo({String filename = 'file.txt', String contents = 'hello\n'}) async { final dir = await Directory.systemTemp.createTemp('clide-git-client-'); @@ -195,8 +191,7 @@ void main() { group('GitClient — error surface', () { test('a bad git binary path makes _run throw GitException', () async { - final t = Toolchain(); - t.applyResolved(const ResolvedPaths(git: '/tmp/clide-no-such-git-binary')); + final t = ToolchainView.resolved(const ResolvedPaths(git: '/tmp/clide-no-such-git-binary')); final dir = await Directory.systemTemp.createTemp('clide-git-bad-'); addTearDown(() => dir.deleteSync(recursive: true)); final git = GitClient(toolchain: t, workDir: dir); diff --git a/test/pql/client_test.dart b/test/pql/client_test.dart index 10429777..51613239 100644 --- a/test/pql/client_test.dart +++ b/test/pql/client_test.dart @@ -5,15 +5,14 @@ library; import 'dart:io'; -import 'package:clide/kernel/src/toolchain.dart'; +import 'package:clide/kernel/src/toolchain_paths.dart'; import 'package:clide/src/pql/client.dart'; import 'package:test/test.dart'; void main() { late PqlClient pql; setUp(() { - final toolchain = Toolchain(); - toolchain.applyResolved(Toolchain.resolvePaths(workspaceRoot: Directory.current.path)); + final toolchain = ToolchainView.resolved(resolveToolchainPaths(Directory.current.path)); pql = PqlClient(workDir: Directory.current, toolchain: toolchain); }); @@ -123,9 +122,8 @@ void main() { group('PqlClient — error surface', () { test('non-existent pql binary raises a PqlException with ProcessException details', () async { - final t = Toolchain(); // Inject a bad path — Process.run will throw ProcessException. - t.applyResolved(const ResolvedPaths(pql: '/tmp/clide-no-such-pql-binary')); + final t = ToolchainView.resolved(const ResolvedPaths(pql: '/tmp/clide-no-such-pql-binary')); final bad = PqlClient(workDir: Directory.current, toolchain: t); try { await bad.files();