From 9b0b622161a4a3de416ba933aa24a95842a74fdf Mon Sep 17 00:00:00 2001 From: Dominik Simonik Date: Wed, 21 Oct 2026 14:01:12 +0200 Subject: [PATCH 1/2] fix(plugin_source): give up on a git clone that outlasts its timeout The clone had no timeout, so an unreachable github.com left install and doctor waiting forever. HarnessDetector already bounds its own probe at five seconds. The git process is not killed, but nothing waits on it and the half-filled temp directory is thrown away, so the next run starts clean. GitPluginSource also takes an optional log sink, which carries the command line and git's stderr to --verbose. --- packages/plugin_source/lib/src/sources.dart | 52 +++++++++++++++---- .../plugin_source/test/src/sources_test.dart | 35 +++++++++++++ 2 files changed, 77 insertions(+), 10 deletions(-) diff --git a/packages/plugin_source/lib/src/sources.dart b/packages/plugin_source/lib/src/sources.dart index a88eb302..29ff61ba 100644 --- a/packages/plugin_source/lib/src/sources.dart +++ b/packages/plugin_source/lib/src/sources.dart @@ -1,3 +1,4 @@ +import 'dart:async' show TimeoutException; import 'dart:io' show ProcessException, ProcessResult; import 'package:file/file.dart'; @@ -12,10 +13,19 @@ import 'package:plugin_source/src/resolution.dart'; /// process wiring belongs with the caller that owns the rest of it. typedef RunGit = Future Function(List args); +/// Reports a line worth showing only under `--verbose`. +/// +/// Kept as a function rather than a `Logger` so this package does not depend +/// on the CLI's logging package. +typedef LogDetail = void Function(String message); + /// {@template plugin_source} /// Resolves a [PluginRef] to a directory on disk. /// {@endtemplate} abstract class const PluginSource() { + /// {@macro plugin_source} + this; + /// Resolves [ref] from this source. Future resolve(PluginRef ref); } @@ -35,6 +45,9 @@ class const LocalCachePluginSource({ required final Directory home, final String marketplace = vgvMarketplaceName, }) extends PluginSource { + /// {@macro local_cache_plugin_source} + this; + @override Future resolve(PluginRef ref) async { final fileSystem = home.fileSystem; @@ -90,12 +103,19 @@ class const LocalCachePluginSource({ /// Resolves plugins by cloning them at their pinned tag. /// /// [cacheRoot] is where clones are kept between runs. [runGit] runs git, and is -/// required so tests never hit the network. +/// required so tests never hit the network. A clone that takes longer than +/// [timeout] is given up on, so an unreachable host cannot hang the command +/// that is waiting. [log] receives the command line and git's stderr. /// {@endtemplate} class const GitPluginSource({ required final Directory cacheRoot, required final RunGit runGit, + final Duration timeout = const Duration(minutes: 2), + final LogDetail? log, }) extends PluginSource { + /// {@macro git_plugin_source} + this; + @override Future resolve(PluginRef ref) async { final fileSystem = cacheRoot.fileSystem; @@ -121,17 +141,28 @@ class const GitPluginSource({ ); } + final args = [ + 'clone', + '--depth', + '1', + '--branch', + ref.gitRef, + 'https://github.com/${ref.repo}.git', + temp.path, + ]; + log?.call('Running: git ${args.join(' ')}'); + final ProcessResult result; try { - result = await runGit([ - 'clone', - '--depth', - '1', - '--branch', - ref.gitRef, - 'https://github.com/${ref.repo}.git', - temp.path, - ]); + result = await runGit(args).timeout(timeout); + } on TimeoutException { + // The git process is not killed, but nothing waits on it any more and + // the temp directory it was filling is thrown away. + _deleteTemp(temp); + return UnavailablePlugin.single( + 'Cloning ${ref.repo} took longer than ${timeout.inSeconds} seconds ' + 'and was given up on. Check your connection to github.com.', + ); } on ProcessException { // Process.run throws rather than returning a non-zero exit code when the // executable is missing, so this is specifically "no git", not "the @@ -143,6 +174,7 @@ class const GitPluginSource({ } if (result.exitCode != 0) { + log?.call('git stderr:\n${result.stderr}'); _deleteTemp(temp); return UnavailablePlugin.single( 'Could not clone ${ref.repo} at ${ref.gitRef}: ${result.stderr}', diff --git a/packages/plugin_source/test/src/sources_test.dart b/packages/plugin_source/test/src/sources_test.dart index 289bed39..0fccd25f 100644 --- a/packages/plugin_source/test/src/sources_test.dart +++ b/packages/plugin_source/test/src/sources_test.dart @@ -1,3 +1,4 @@ +import 'dart:async' show Completer; import 'dart:io' show ProcessException, ProcessResult; import 'package:file/file.dart'; @@ -220,6 +221,40 @@ void main() { ); }); + test('gives up on a clone that outlasts the timeout', () async { + final resolution = await GitPluginSource( + cacheRoot: cacheRoot, + // A clone that never finishes, as an unreachable host would be. + runGit: (a) => Completer().future, + timeout: const Duration(milliseconds: 10), + ).resolve(ref); + + final reason = (resolution as UnavailablePlugin).reasons.single; + expect(reason, contains('took longer than')); + expect(reason, contains('github.com')); + final leftovers = fileSystem + .directory(p.join(cacheRoot.path, ref.name)) + .listSync(); + expect(leftovers, isEmpty); + }); + + test('logs the command it runs and the stderr of a failed clone', () async { + final logged = []; + + await GitPluginSource( + cacheRoot: cacheRoot, + runGit: (a) async { + fileSystem.directory(a.last).createSync(recursive: true); + return ProcessResult(0, 128, '', 'Remote branch v0.0.5 not found'); + }, + log: logged.add, + ).resolve(ref); + + expect(logged.first, startsWith('Running: git clone --depth 1')); + expect(logged.first, contains('v0.0.5')); + expect(logged.last, contains('Remote branch v0.0.5 not found')); + }); + test('reports a cache root it cannot create', () async { fileSystem.file(p.join(cacheRoot.path, ref.name)).createSync(); var ran = false; From bf4f931de45cecf053ba420ac612b1f20003cb17 Mon Sep 17 00:00:00 2001 From: Dominik Simonik Date: Wed, 21 Oct 2026 15:53:25 +0200 Subject: [PATCH 2/2] docs(plugin_source): describe LogDetail without naming the caller's logger --- packages/plugin_source/lib/src/sources.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/plugin_source/lib/src/sources.dart b/packages/plugin_source/lib/src/sources.dart index 29ff61ba..3e8b2a99 100644 --- a/packages/plugin_source/lib/src/sources.dart +++ b/packages/plugin_source/lib/src/sources.dart @@ -13,10 +13,10 @@ import 'package:plugin_source/src/resolution.dart'; /// process wiring belongs with the caller that owns the rest of it. typedef RunGit = Future Function(List args); -/// Reports a line worth showing only under `--verbose`. +/// Reports a line worth showing only when the caller asks for detail. /// -/// Kept as a function rather than a `Logger` so this package does not depend -/// on the CLI's logging package. +/// A function rather than a logger, so this package stays independent of how +/// its caller logs. typedef LogDetail = void Function(String message); /// {@template plugin_source}