From cbf958cd4034bf8bf13dbc98f7c632a2950fbfae Mon Sep 17 00:00:00 2001 From: Felix Angelov Date: Tue, 8 Nov 2022 10:02:39 -0600 Subject: [PATCH] fix(mason): hook execution after pub cache clean (#606) --- packages/mason/lib/src/hooks.dart | 76 +++++++++++++------ .../test/fixtures/basic/__brick__/.gitkeep | 0 packages/mason/test/fixtures/basic/brick.yaml | 3 + .../test/fixtures/basic/hooks/pre_gen.dart | 3 + .../test/fixtures/basic/hooks/pubspec.yaml | 10 +++ .../__brick__/.gitkeep | 0 .../dependency_install_failure/brick.yaml | 3 + .../hooks/pre_gen.dart | 3 + .../hooks/pubspec.yaml | 12 +++ .../spawn_exception/__brick__/.gitkeep | 0 .../test/fixtures/spawn_exception/brick.yaml | 3 + .../spawn_exception/hooks/pre_gen.dart | 1 + .../spawn_exception/hooks/pubspec.yaml | 4 + packages/mason/test/src/hooks_test.dart | 69 +++++++++++++++++ 14 files changed, 163 insertions(+), 24 deletions(-) create mode 100644 packages/mason/test/fixtures/basic/__brick__/.gitkeep create mode 100644 packages/mason/test/fixtures/basic/brick.yaml create mode 100644 packages/mason/test/fixtures/basic/hooks/pre_gen.dart create mode 100644 packages/mason/test/fixtures/basic/hooks/pubspec.yaml create mode 100644 packages/mason/test/fixtures/dependency_install_failure/__brick__/.gitkeep create mode 100644 packages/mason/test/fixtures/dependency_install_failure/brick.yaml create mode 100644 packages/mason/test/fixtures/dependency_install_failure/hooks/pre_gen.dart create mode 100644 packages/mason/test/fixtures/dependency_install_failure/hooks/pubspec.yaml create mode 100644 packages/mason/test/fixtures/spawn_exception/__brick__/.gitkeep create mode 100644 packages/mason/test/fixtures/spawn_exception/brick.yaml create mode 100644 packages/mason/test/fixtures/spawn_exception/hooks/pre_gen.dart create mode 100644 packages/mason/test/fixtures/spawn_exception/hooks/pubspec.yaml diff --git a/packages/mason/lib/src/hooks.dart b/packages/mason/lib/src/hooks.dart index 09fb44af7..f55122bec 100644 --- a/packages/mason/lib/src/hooks.dart +++ b/packages/mason/lib/src/hooks.dart @@ -261,32 +261,37 @@ class GeneratorHooks { ); } + Future _dartPubGet({required String workingDirectory}) async { + final result = await Process.run( + 'dart', + ['pub', 'get'], + workingDirectory: workingDirectory, + runInShell: true, + ); + if (result.exitCode != ExitCode.success.code) { + throw HookDependencyInstallFailure(hook.path, '${result.stderr}'); + } + } + + var dependenciesInstalled = false; + Directory? hookCacheDir; Uri? packageConfigUri; if (pubspec != null) { final directoryHash = sha1.convert(pubspec).toString(); - final directory = Directory( + hookCacheDir = Directory( p.join(Directory.systemTemp.path, '.mason', directoryHash), ); final packageConfigFile = File( - p.join(directory.path, '.dart_tool', 'package_config.json'), + p.join(hookCacheDir.path, '.dart_tool', 'package_config.json'), ); if (!packageConfigFile.existsSync()) { - await directory.create(recursive: true); + await hookCacheDir.create(recursive: true); await File( - p.join(directory.path, 'pubspec.yaml'), + p.join(hookCacheDir.path, 'pubspec.yaml'), ).writeAsBytes(pubspec); - - final result = await Process.run( - 'dart', - ['pub', 'get'], - workingDirectory: directory.path, - runInShell: true, - ); - - if (result.exitCode != 0) { - throw HookDependencyInstallFailure(hook.path, '${result.stderr}'); - } + await _dartPubGet(workingDirectory: hookCacheDir.path); + dependenciesInstalled = true; } packageConfigUri = packageConfigFile.uri; @@ -302,22 +307,45 @@ class GeneratorHooks { if (uri == null) throw HookMissingRunException(hook.path); - final cwd = Directory.current; - Isolate? isolate; - try { - if (workingDirectory != null) Directory.current = workingDirectory; - isolate = await Isolate.spawnUri( + Future spawnIsolate(Uri uri) { + return Isolate.spawnUri( uri, [json.encode(vars)], messagePort.sendPort, paused: true, packageConfig: packageConfigUri, ); + } + + final cwd = Directory.current; + Isolate? isolate; + try { + if (workingDirectory != null) Directory.current = workingDirectory; + isolate = await spawnIsolate(uri); } on IsolateSpawnException catch (error) { - Directory.current = cwd; - final msg = error.message; - final content = msg.contains('Error: ') ? msg.split('Error: ').last : msg; - throw HookRunException(hook.path, content.trim()); + Never throwHookRunException(IsolateSpawnException error) { + Directory.current = cwd; + final msg = error.message; + final content = + msg.contains('Error: ') ? msg.split('Error: ').last : msg; + throw HookRunException(hook.path, content.trim()); + } + + final shouldRetry = !dependenciesInstalled && hookCacheDir != null; + + if (!shouldRetry) throwHookRunException(error); + + // Failure to spawn the isolate could be due to changes in the pub cache. + // We attempt to reinstall hook dependencies. + await _dartPubGet(workingDirectory: hookCacheDir.path); + + // Retry spawning the isolate if the hook dependencies + // have been successfully reinstalled. + try { + isolate = await spawnIsolate(uri); + } on IsolateSpawnException catch (error) { + throwHookRunException(error); + } } isolate diff --git a/packages/mason/test/fixtures/basic/__brick__/.gitkeep b/packages/mason/test/fixtures/basic/__brick__/.gitkeep new file mode 100644 index 000000000..e69de29bb diff --git a/packages/mason/test/fixtures/basic/brick.yaml b/packages/mason/test/fixtures/basic/brick.yaml new file mode 100644 index 000000000..e852feefa --- /dev/null +++ b/packages/mason/test/fixtures/basic/brick.yaml @@ -0,0 +1,3 @@ +name: basic_hook +description: A basic Hook +version: 0.1.0+1 diff --git a/packages/mason/test/fixtures/basic/hooks/pre_gen.dart b/packages/mason/test/fixtures/basic/hooks/pre_gen.dart new file mode 100644 index 000000000..b506987b3 --- /dev/null +++ b/packages/mason/test/fixtures/basic/hooks/pre_gen.dart @@ -0,0 +1,3 @@ +import 'package:mason/mason.dart'; + +void run(HookContext context) {} diff --git a/packages/mason/test/fixtures/basic/hooks/pubspec.yaml b/packages/mason/test/fixtures/basic/hooks/pubspec.yaml new file mode 100644 index 000000000..7b25549b5 --- /dev/null +++ b/packages/mason/test/fixtures/basic/hooks/pubspec.yaml @@ -0,0 +1,10 @@ +name: basic_hooks + +environment: + sdk: ">=2.12.0 <3.0.0" + +dependencies: + mason: + git: + url: https://github.com/felangel/mason + path: packages/mason diff --git a/packages/mason/test/fixtures/dependency_install_failure/__brick__/.gitkeep b/packages/mason/test/fixtures/dependency_install_failure/__brick__/.gitkeep new file mode 100644 index 000000000..e69de29bb diff --git a/packages/mason/test/fixtures/dependency_install_failure/brick.yaml b/packages/mason/test/fixtures/dependency_install_failure/brick.yaml new file mode 100644 index 000000000..12f188a6f --- /dev/null +++ b/packages/mason/test/fixtures/dependency_install_failure/brick.yaml @@ -0,0 +1,3 @@ +name: dependency_install_failure_hook +description: A Test Hook +version: 0.1.0+1 diff --git a/packages/mason/test/fixtures/dependency_install_failure/hooks/pre_gen.dart b/packages/mason/test/fixtures/dependency_install_failure/hooks/pre_gen.dart new file mode 100644 index 000000000..b506987b3 --- /dev/null +++ b/packages/mason/test/fixtures/dependency_install_failure/hooks/pre_gen.dart @@ -0,0 +1,3 @@ +import 'package:mason/mason.dart'; + +void run(HookContext context) {} diff --git a/packages/mason/test/fixtures/dependency_install_failure/hooks/pubspec.yaml b/packages/mason/test/fixtures/dependency_install_failure/hooks/pubspec.yaml new file mode 100644 index 000000000..8e83ac7f0 --- /dev/null +++ b/packages/mason/test/fixtures/dependency_install_failure/hooks/pubspec.yaml @@ -0,0 +1,12 @@ +name: dependency_install_failure_hooks + +environment: + sdk: ">=2.12.0 <3.0.0" + +dependencies: + foo: + path: ./foo + mason: + git: + url: https://github.com/felangel/mason + path: packages/mason diff --git a/packages/mason/test/fixtures/spawn_exception/__brick__/.gitkeep b/packages/mason/test/fixtures/spawn_exception/__brick__/.gitkeep new file mode 100644 index 000000000..e69de29bb diff --git a/packages/mason/test/fixtures/spawn_exception/brick.yaml b/packages/mason/test/fixtures/spawn_exception/brick.yaml new file mode 100644 index 000000000..1d9f7c9ac --- /dev/null +++ b/packages/mason/test/fixtures/spawn_exception/brick.yaml @@ -0,0 +1,3 @@ +name: spawn_exception_hook +description: A Test Hook +version: 0.1.0+1 diff --git a/packages/mason/test/fixtures/spawn_exception/hooks/pre_gen.dart b/packages/mason/test/fixtures/spawn_exception/hooks/pre_gen.dart new file mode 100644 index 000000000..0c70f7bae --- /dev/null +++ b/packages/mason/test/fixtures/spawn_exception/hooks/pre_gen.dart @@ -0,0 +1 @@ +void run(HookContext context) {} diff --git a/packages/mason/test/fixtures/spawn_exception/hooks/pubspec.yaml b/packages/mason/test/fixtures/spawn_exception/hooks/pubspec.yaml new file mode 100644 index 000000000..4fce4a301 --- /dev/null +++ b/packages/mason/test/fixtures/spawn_exception/hooks/pubspec.yaml @@ -0,0 +1,4 @@ +name: spawn_exception_hooks + +environment: + sdk: ">=2.12.0 <3.0.0" diff --git a/packages/mason/test/src/hooks_test.dart b/packages/mason/test/src/hooks_test.dart index 24929a476..ba3e05f78 100644 --- a/packages/mason/test/src/hooks_test.dart +++ b/packages/mason/test/src/hooks_test.dart @@ -1,3 +1,5 @@ +import 'dart:io'; + import 'package:mason/mason.dart'; import 'package:mason/src/generator.dart'; import 'package:path/path.dart' as path; @@ -37,6 +39,45 @@ void main() { } }); + test( + 'throws HookRunException ' + 'when unable to resolve a type', () async { + final brick = Brick.path( + path.join('test', 'fixtures', 'spawn_exception'), + ); + final generator = await MasonGenerator.fromBrick(brick); + + try { + await generator.hooks.preGen(); + fail('should throw'); + } catch (error) { + expect(error, isA()); + } + }); + + test( + 'throws HookRunException ' + 'when unable to resolve a type (back-to-back)', () async { + final brick = Brick.path( + path.join('test', 'fixtures', 'spawn_exception'), + ); + final generator = await MasonGenerator.fromBrick(brick); + + try { + await generator.hooks.preGen(); + fail('should throw'); + } catch (error) { + expect(error, isA()); + } + + try { + await generator.hooks.preGen(); + fail('should throw'); + } catch (error) { + expect(error, isA()); + } + }); + test( 'throws HookMissingRunException ' 'when hook does not contain a valid run method', () async { @@ -80,5 +121,33 @@ void main() { expect(error, isA()); } }); + + test( + 'throws HookDependencyInstallFailure ' + 'when dependencies cannot be resolved', () async { + final brick = Brick.path( + path.join('test', 'fixtures', 'dependency_install_failure'), + ); + final generator = await MasonGenerator.fromBrick(brick); + + try { + await generator.hooks.preGen(); + fail('should throw'); + } catch (error) { + expect(error, isA()); + } + }); + + test('recovers from cleared pub cache', () async { + final brick = Brick.path(path.join('test', 'fixtures', 'basic')); + final generator = await MasonGenerator.fromBrick(brick); + + await expectLater(generator.hooks.preGen(), completes); + + final result = await Process.run('dart', ['pub', 'cache', 'clean']); + expect(result.exitCode, equals(ExitCode.success.code)); + + await expectLater(generator.hooks.preGen(), completes); + }); }); }