diff --git a/packages/plugin_source/lib/src/sources.dart b/packages/plugin_source/lib/src/sources.dart index a88eb30..3e8b2a9 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 when the caller asks for detail. +/// +/// 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} /// 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 289bed3..0fccd25 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;