diff --git a/.github/workflows/mason_cli.yaml b/.github/workflows/mason_cli.yaml index 940c1410c..6e29386bf 100644 --- a/.github/workflows/mason_cli.yaml +++ b/.github/workflows/mason_cli.yaml @@ -93,4 +93,4 @@ jobs: dart pub global activate pana - name: Verify Pub Score - run: ../../tool/verify_pub_score.sh + run: ../../tool/verify_pub_score.sh 100 diff --git a/packages/mason/lib/src/generator.dart b/packages/mason/lib/src/generator.dart index 3ddbb5f81..0f35bf146 100644 --- a/packages/mason/lib/src/generator.dart +++ b/packages/mason/lib/src/generator.dart @@ -640,3 +640,26 @@ extension on String { } } } + +extension on HookFile { + String get fileHash => sha1.convert(content).toString(); + + Directory get cacheDirectory { + return Directory( + p.join( + Directory(p.join(Directory.systemTemp.path, '.mason')).path, + sha1.convert(utf8.encode(File(path).parent.absolute.path)).toString(), + ), + ); + } + + Directory get buildDirectory { + return Directory( + p.join(cacheDirectory.path, 'build', p.basenameWithoutExtension(path)), + ); + } + + File get module { + return File(p.join(buildDirectory.path, '.$fileHash.dill')); + } +} diff --git a/packages/mason/lib/src/hooks.dart b/packages/mason/lib/src/hooks.dart index f55122bec..753909847 100644 --- a/packages/mason/lib/src/hooks.dart +++ b/packages/mason/lib/src/hooks.dart @@ -45,15 +45,15 @@ Ensure the hook contains a 'run' method: ); } -/// {@template hook_run_exception} -/// Thrown when an error occurs when trying to run hook. +/// {@template hook_compile_exception} +/// Thrown when an error occurs when trying to compile a hook. /// {@endtemplate} -class HookRunException extends MasonException { - /// {@macro hook_run_exception} - HookRunException(String path, String error) +class HookCompileException extends MasonException { + /// {@macro hook_compile_exception} + HookCompileException(String path, String error) : super( ''' -Unable to execute hook: $path. +Unable to compile hook: $path. Error: $error''', ); } @@ -202,6 +202,7 @@ class GeneratorHooks { Map vars = const {}, String? workingDirectory, void Function(Map vars)? onVarsChanged, + Logger? logger, }) async { final preGenHook = this.preGenHook; if (preGenHook != null && pubspec != null) { @@ -210,6 +211,7 @@ class GeneratorHooks { vars: vars, workingDirectory: workingDirectory, onVarsChanged: onVarsChanged, + logger: logger, ); } } @@ -220,6 +222,7 @@ class GeneratorHooks { Map vars = const {}, String? workingDirectory, void Function(Map vars)? onVarsChanged, + Logger? logger, }) async { final postGenHook = this.postGenHook; if (postGenHook != null && pubspec != null) { @@ -228,59 +231,49 @@ class GeneratorHooks { vars: vars, workingDirectory: workingDirectory, onVarsChanged: onVarsChanged, + logger: logger, ); } } - /// Runs the provided [hook] with the specified [vars]. - /// An optional [workingDirectory] can also be specified. - Future _runHook({ - required HookFile hook, - Map vars = const {}, - void Function(Map vars)? onVarsChanged, - String? workingDirectory, - }) async { - final pubspec = this.pubspec; - final subscriptions = []; - final messagePort = ReceivePort(); - final errorPort = ReceivePort(); - final exitPort = ReceivePort(); + /// Compile all hooks into modules for faster execution. + /// Hooks are compiled lazily by default but calling [compile] + /// can be used to compile hooks ahead of time. + Future compile({Logger? logger}) async { + await _installDependencies(); - dynamic hookError; - subscriptions.add(errorPort.listen((dynamic error) => hookError = error)); + if (preGenHook != null && !preGenHook!.module.existsSync()) { + await _compile(hook: preGenHook!, logger: logger); + } - if (onVarsChanged != null) { - subscriptions.add( - messagePort.listen((dynamic message) { - if (message is String) { - onVarsChanged( - json.decode(message) as Map, - ); - } - }), - ); + if (postGenHook != null && !postGenHook!.module.existsSync()) { + await _compile(hook: postGenHook!, logger: logger); } + } - Future _dartPubGet({required String workingDirectory}) async { - final result = await Process.run( - 'dart', - ['pub', 'get'], - workingDirectory: workingDirectory, - runInShell: true, + 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( + workingDirectory, + '${result.stderr}', ); - if (result.exitCode != ExitCode.success.code) { - throw HookDependencyInstallFailure(hook.path, '${result.stderr}'); - } } + } + + Future _installDependencies() async { + final hook = preGenHook ?? postGenHook; + if (hook == null) return; + + final hookCacheDir = hook.cacheDirectory; + final pubspec = this.pubspec; - var dependenciesInstalled = false; - Directory? hookCacheDir; - Uri? packageConfigUri; if (pubspec != null) { - final directoryHash = sha1.convert(pubspec).toString(); - hookCacheDir = Directory( - p.join(Directory.systemTemp.path, '.mason', directoryHash), - ); final packageConfigFile = File( p.join(hookCacheDir.path, '.dart_tool', 'package_config.json'), ); @@ -291,11 +284,18 @@ class GeneratorHooks { p.join(hookCacheDir.path, 'pubspec.yaml'), ).writeAsBytes(pubspec); await _dartPubGet(workingDirectory: hookCacheDir.path); - dependenciesInstalled = true; } - - packageConfigUri = packageConfigFile.uri; } + } + + Future _compile({required HookFile hook, Logger? logger}) async { + final hookCacheDir = hook.cacheDirectory; + final hookBuildDir = hook.buildDirectory; + final hookHash = hook.fileHash; + + try { + await hookBuildDir.delete(recursive: true); + } catch (_) {} Uri? uri; try { @@ -307,6 +307,67 @@ class GeneratorHooks { if (uri == null) throw HookMissingRunException(hook.path); + final hookFile = File(p.join(hookBuildDir.path, '.$hookHash.dart')) + ..createSync(recursive: true) + ..writeAsBytesSync(uri.data!.contentAsBytes()); + + final progress = logger?.progress('Compiling ${p.basename(hook.path)}'); + final result = await Process.run( + 'dart', + ['compile', 'kernel', hookFile.path], + workingDirectory: hookCacheDir.path, + runInShell: true, + ); + + if (result.exitCode != ExitCode.success.code) { + final error = result.stderr.toString(); + progress?.fail(error); + throw HookCompileException(hook.path, error); + } + + progress?.complete('Compiled ${p.basename(hook.path)}'); + } + + /// Runs the provided [hook] with the specified [vars]. + /// An optional [workingDirectory] can also be specified. + Future _runHook({ + required HookFile hook, + Map vars = const {}, + void Function(Map vars)? onVarsChanged, + String? workingDirectory, + Logger? logger, + }) async { + final subscriptions = []; + final messagePort = ReceivePort(); + final errorPort = ReceivePort(); + final exitPort = ReceivePort(); + + dynamic hookError; + subscriptions.add(errorPort.listen((dynamic error) => hookError = error)); + + if (onVarsChanged != null) { + subscriptions.add( + messagePort.listen((dynamic message) { + if (message is String) { + onVarsChanged( + json.decode(message) as Map, + ); + } + }), + ); + } + + await _installDependencies(); + final hookCacheDir = hook.cacheDirectory; + final packageConfigUri = File( + p.join(hookCacheDir.path, '.dart_tool', 'package_config.json'), + ).uri; + final module = hook.module; + + if (!module.existsSync()) { + await _compile(hook: hook, logger: logger); + } + Future spawnIsolate(Uri uri) { return Isolate.spawnUri( uri, @@ -318,35 +379,9 @@ class GeneratorHooks { } final cwd = Directory.current; - Isolate? isolate; - try { - if (workingDirectory != null) Directory.current = workingDirectory; - isolate = await spawnIsolate(uri); - } on IsolateSpawnException catch (error) { - 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 (workingDirectory != null) Directory.current = workingDirectory; - 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); - } - } + final isolate = await spawnIsolate(module.uri); isolate ..addErrorListener(errorPort.sendPort) diff --git a/packages/mason/test/fixtures/basic/hooks/post_gen.dart b/packages/mason/test/fixtures/basic/hooks/post_gen.dart new file mode 100644 index 000000000..b506987b3 --- /dev/null +++ b/packages/mason/test/fixtures/basic/hooks/post_gen.dart @@ -0,0 +1,3 @@ +import 'package:mason/mason.dart'; + +void run(HookContext context) {} diff --git a/packages/mason/test/src/hooks_test.dart b/packages/mason/test/src/hooks_test.dart index ba3e05f78..031d35ab3 100644 --- a/packages/mason/test/src/hooks_test.dart +++ b/packages/mason/test/src/hooks_test.dart @@ -2,11 +2,24 @@ import 'dart:io'; import 'package:mason/mason.dart'; import 'package:mason/src/generator.dart'; +import 'package:mocktail/mocktail.dart'; import 'package:path/path.dart' as path; import 'package:test/test.dart'; +class _MockLogger extends Mock implements Logger {} + +class _MockProgress extends Mock implements Progress {} + void main() { group('Hooks', () { + setUp(() async { + try { + await Directory( + path.join(Directory.systemTemp.path, '.mason'), + ).delete(recursive: true); + } catch (_) {} + }); + test( 'throws HookInvalidCharactersException ' 'when containining non-ascii characters', () async { @@ -40,7 +53,7 @@ void main() { }); test( - 'throws HookRunException ' + 'throws HookCompileException ' 'when unable to resolve a type', () async { final brick = Brick.path( path.join('test', 'fixtures', 'spawn_exception'), @@ -51,12 +64,12 @@ void main() { await generator.hooks.preGen(); fail('should throw'); } catch (error) { - expect(error, isA()); + expect(error, isA()); } }); test( - 'throws HookRunException ' + 'throws HookCompileException ' 'when unable to resolve a type (back-to-back)', () async { final brick = Brick.path( path.join('test', 'fixtures', 'spawn_exception'), @@ -67,14 +80,14 @@ void main() { await generator.hooks.preGen(); fail('should throw'); } catch (error) { - expect(error, isA()); + expect(error, isA()); } try { await generator.hooks.preGen(); fail('should throw'); } catch (error) { - expect(error, isA()); + expect(error, isA()); } }); @@ -94,7 +107,7 @@ void main() { } }); - test('throws HookRunException when hook cannot be run', () async { + test('throws HookCompileException when hook cannot be run', () async { final brick = Brick.path( path.join('test', 'fixtures', 'run_exception'), ); @@ -104,7 +117,7 @@ void main() { await generator.hooks.preGen(); fail('should throw'); } catch (error) { - expect(error, isA()); + expect(error, isA()); } }); @@ -138,6 +151,51 @@ void main() { } }); + test('installs dependencies and compiles hooks only once', () async { + final brick = Brick.path(path.join('test', 'fixtures', 'basic')); + final generator = await MasonGenerator.fromBrick(brick); + final logger = _MockLogger(); + final progress = _MockProgress(); + when(() => logger.progress(any())).thenReturn(progress); + + await expectLater(generator.hooks.compile(logger: logger), completes); + + verify(() => logger.progress('Compiling pre_gen.dart')).called(1); + verify(() => progress.complete('Compiled pre_gen.dart')).called(1); + verify(() => logger.progress('Compiling post_gen.dart')).called(1); + verify(() => progress.complete('Compiled post_gen.dart')).called(1); + + await expectLater(generator.hooks.compile(logger: logger), completes); + + verifyNever(() => logger.progress('Compiling pre_gen.dart')); + verifyNever(() => progress.complete('Compiled pre_gen.dart')); + verifyNever(() => logger.progress('Compiling post_gen.dart')); + verifyNever(() => progress.complete('Compiled post_gen.dart')); + }); + + test('compile reports compilation errors', () async { + final brick = Brick.path( + path.join('test', 'fixtures', 'run_exception'), + ); + final generator = await MasonGenerator.fromBrick(brick); + final logger = _MockLogger(); + final progress = _MockProgress(); + when(() => logger.progress(any())).thenReturn(progress); + + try { + await generator.hooks.compile(logger: logger); + fail('should throw'); + } catch (error) { + expect(error, isA()); + } + verify(() => logger.progress('Compiling pre_gen.dart')).called(1); + verify( + () => progress.fail( + any(that: contains("Error: Expected '{' before this.")), + ), + ).called(1); + }); + test('recovers from cleared pub cache', () async { final brick = Brick.path(path.join('test', 'fixtures', 'basic')); final generator = await MasonGenerator.fromBrick(brick); diff --git a/packages/mason_cli/lib/src/commands/add.dart b/packages/mason_cli/lib/src/commands/add.dart index d789c8eb4..138ada158 100644 --- a/packages/mason_cli/lib/src/commands/add.dart +++ b/packages/mason_cli/lib/src/commands/add.dart @@ -65,6 +65,18 @@ class AddCommand extends MasonCommand with InstallBrickMixin { final cachedBrick = await addBrick(brick, global: isGlobal); final file = File(p.join(cachedBrick.path, BrickYaml.file)); + final generator = await MasonGenerator.fromBrick( + Brick.path(cachedBrick.path), + ); + + final compileProgress = logger.progress('Compiling ${brick.name}'); + try { + await generator.hooks.compile(); + compileProgress.complete(); + } catch (_) { + compileProgress.fail(); + rethrow; + } final brickYaml = checkedYamlDecode( file.readAsStringSync(), diff --git a/packages/mason_cli/lib/src/commands/make.dart b/packages/mason_cli/lib/src/commands/make.dart index 9f8f957aa..f21482664 100644 --- a/packages/mason_cli/lib/src/commands/make.dart +++ b/packages/mason_cli/lib/src/commands/make.dart @@ -171,6 +171,7 @@ class _MakeCommand extends MasonCommand { vars: vars, workingDirectory: outputDir, onVarsChanged: (vars) => updatedVars = vars, + logger: logger, ); } @@ -189,6 +190,7 @@ class _MakeCommand extends MasonCommand { await generator.hooks.postGen( vars: updatedVars ?? vars, workingDirectory: outputDir, + logger: logger, ); } diff --git a/packages/mason_cli/lib/src/install_brick.dart b/packages/mason_cli/lib/src/install_brick.dart index 7d5b12dcc..ede304831 100644 --- a/packages/mason_cli/lib/src/install_brick.dart +++ b/packages/mason_cli/lib/src/install_brick.dart @@ -48,45 +48,56 @@ mixin InstallBrickMixin on MasonCommand { if (bricksJson == null) throw const MasonYamlNotFoundException(); final lockJson = global ? globalMasonLockJson : localMasonLockJson; final resolvedBricks = {}; - final getBricksProgress = logger.progress( - upgrade ? 'Upgrading bricks' : 'Getting bricks', - ); + final message = upgrade ? 'Upgrading bricks' : 'Getting bricks'; + final getBricksProgress = logger.progress(message); try { bricksJson.clear(); final masonYaml = global ? globalMasonYaml : localMasonYaml; if (masonYaml.bricks.entries.isNotEmpty) { - await Future.forEach>( - masonYaml.bricks.entries, - (entry) async { - final location = resolveBrickLocation( - location: entry.value, - lockedLocation: upgrade ? null : lockJson.bricks[entry.key], - ); - final masonYamlFile = - global ? globalMasonYamlFile : localMasonYamlFile; - final normalizedLocation = location.path != null - ? BrickLocation( - path: canonicalize( - p.join(masonYamlFile.parent.path, location.path), - ), - ) - : location; - final cachedBrick = await bricksJson.add( - Brick(name: entry.key, location: normalizedLocation), - ); - resolvedBricks.addAll( - {entry.key: cachedBrick.brick.location}, - ); - }, + await Future.wait( + masonYaml.bricks.entries.map( + (entry) { + return () async { + getBricksProgress.update('Resolving ${entry.key}'); + final location = resolveBrickLocation( + location: entry.value, + lockedLocation: upgrade ? null : lockJson.bricks[entry.key], + ); + final masonYamlFile = + global ? globalMasonYamlFile : localMasonYamlFile; + final normalizedLocation = location.path != null + ? BrickLocation( + path: canonicalize( + p.join(masonYamlFile.parent.path, location.path), + ), + ) + : location; + getBricksProgress.update('Installing ${entry.key}'); + final cachedBrick = await bricksJson.add( + Brick(name: entry.key, location: normalizedLocation), + ); + resolvedBricks.addAll( + { + entry.key: cachedBrick.brick.location + }, + ); + getBricksProgress.update('Compiling ${entry.key}'); + final generator = await MasonGenerator.fromBrick( + Brick.path(cachedBrick.path), + ); + await generator.hooks.compile(); + }(); + }, + ), ); } } finally { - getBricksProgress.complete(); + getBricksProgress.complete(message); await bricksJson.flush(); final masonLockJsonFile = global ? globalMasonLockJsonFile : localMasonLockJsonFile; await masonLockJsonFile.writeAsString( - json.encode(MasonLockJson(bricks: resolvedBricks)), + json.encode(MasonLockJson(bricks: resolvedBricks.sorted())), ); } } @@ -121,3 +132,11 @@ extension on GitPath { return url == other.url && path == other.path; } } + +extension on Map { + Map sorted() { + return Map.fromEntries( + entries.toList()..sort((a, b) => a.key.compareTo(b.key)), + ); + } +} diff --git a/packages/mason_cli/test/bricks/compilation_error/__brick__/.gitkeep b/packages/mason_cli/test/bricks/compilation_error/__brick__/.gitkeep new file mode 100644 index 000000000..e69de29bb diff --git a/packages/mason_cli/test/bricks/compilation_error/brick.yaml b/packages/mason_cli/test/bricks/compilation_error/brick.yaml new file mode 100644 index 000000000..5377ea885 --- /dev/null +++ b/packages/mason_cli/test/bricks/compilation_error/brick.yaml @@ -0,0 +1,3 @@ +name: compilation_error +description: A test brick +version: 0.1.0+1 diff --git a/packages/mason_cli/test/bricks/compilation_error/hooks/pre_gen.dart b/packages/mason_cli/test/bricks/compilation_error/hooks/pre_gen.dart new file mode 100644 index 000000000..5becf0f1d --- /dev/null +++ b/packages/mason_cli/test/bricks/compilation_error/hooks/pre_gen.dart @@ -0,0 +1,2 @@ +// ignore_for_file: missing_function_body +import 'package:mason/mason.dart';void run(HookContext context) diff --git a/packages/mason_cli/test/bricks/compilation_error/hooks/pubspec.yaml b/packages/mason_cli/test/bricks/compilation_error/hooks/pubspec.yaml new file mode 100644 index 000000000..9f5d90d8a --- /dev/null +++ b/packages/mason_cli/test/bricks/compilation_error/hooks/pubspec.yaml @@ -0,0 +1,10 @@ +name: compilation_error_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_cli/test/commands/add_test.dart b/packages/mason_cli/test/commands/add_test.dart index d01a2a6f1..d9d1220bc 100644 --- a/packages/mason_cli/test/commands/add_test.dart +++ b/packages/mason_cli/test/commands/add_test.dart @@ -78,6 +78,17 @@ void main() { verify(() => logger.err('oops')).called(1); }); + test('exits with code 70 on hook compilation exception', () async { + final progress = MockProgress(); + when(() => logger.progress(any())).thenReturn(progress); + final brickPath = path.join('..', '..', 'bricks', 'compilation_error'); + final result = await commandRunner.run( + ['add', 'compilation_error', '--path', brickPath], + ); + expect(result, equals(ExitCode.usage.code)); + verify(progress.fail).called(1); + }); + group('local', () { test('exits with code 64 when brick is not provided', () async { final result = await commandRunner.run(['add']); diff --git a/packages/mason_cli/test/commands/upgrade_test.dart b/packages/mason_cli/test/commands/upgrade_test.dart index 4b4751dd2..ddb9c90d4 100644 --- a/packages/mason_cli/test/commands/upgrade_test.dart +++ b/packages/mason_cli/test/commands/upgrade_test.dart @@ -77,7 +77,7 @@ bricks: ); }); - test('updates lockfile from nested directtory', () async { + test('updates lockfile from nested directory', () async { final bricksPath = path.join('..', '..', '..', '..', '..', 'bricks'); final simplePath = canonicalize( path.join(Directory.current.path, bricksPath, 'simple'),