From 68faa86d36a2194cc8da7a6883b4d0f9e5e1f74d Mon Sep 17 00:00:00 2001 From: Bryan Oltman Date: Fri, 22 Mar 2024 12:14:55 -0400 Subject: [PATCH 1/5] Add gen_snapshot and kernel flags to aot_tools link --- .../lib/src/commands/patch/patch_ios_command.dart | 6 ++++++ .../lib/src/commands/patch/patch_ios_framework_command.dart | 2 ++ packages/shorebird_cli/lib/src/executables/aot_tools.dart | 4 ++++ 3 files changed, 12 insertions(+) diff --git a/packages/shorebird_cli/lib/src/commands/patch/patch_ios_command.dart b/packages/shorebird_cli/lib/src/commands/patch/patch_ios_command.dart index 3266c55ee..287563d31 100644 --- a/packages/shorebird_cli/lib/src/commands/patch/patch_ios_command.dart +++ b/packages/shorebird_cli/lib/src/commands/patch/patch_ios_command.dart @@ -513,14 +513,20 @@ ${summary.join('\n')} return ExitCode.software.code; } + final genSnapshot = shorebirdArtifacts.getArtifactPath( + artifact: ShorebirdArtifact.genSnapshot, + ); + final linkProgress = logger.progress('Linking AOT files'); try { await aotTools.link( base: releaseArtifact.path, patch: patch.path, analyzeSnapshot: analyzeSnapshot.path, + genSnapshot: genSnapshot, outputPath: _vmcodeOutputPath, workingDirectory: _buildDirectory, + kernel: newestAppDill().path, ); } catch (error) { linkProgress.fail('Failed to link AOT files: $error'); diff --git a/packages/shorebird_cli/lib/src/commands/patch/patch_ios_framework_command.dart b/packages/shorebird_cli/lib/src/commands/patch/patch_ios_framework_command.dart index 557f099c9..048521554 100644 --- a/packages/shorebird_cli/lib/src/commands/patch/patch_ios_framework_command.dart +++ b/packages/shorebird_cli/lib/src/commands/patch/patch_ios_framework_command.dart @@ -390,6 +390,8 @@ ${summary.join('\n')} base: releaseArtifact.path, patch: aotSnapshot.path, analyzeSnapshot: analyzeSnapshot.path, + genSnapshot: '', + kernel: '', outputPath: _vmcodeOutputPath, workingDirectory: _buildDirectory, ); diff --git a/packages/shorebird_cli/lib/src/executables/aot_tools.dart b/packages/shorebird_cli/lib/src/executables/aot_tools.dart index 72cf4b85c..ad38fee5b 100644 --- a/packages/shorebird_cli/lib/src/executables/aot_tools.dart +++ b/packages/shorebird_cli/lib/src/executables/aot_tools.dart @@ -99,6 +99,8 @@ class AotTools { required String base, required String patch, required String analyzeSnapshot, + required String genSnapshot, + required String kernel, required String outputPath, String? workingDirectory, }) async { @@ -108,6 +110,8 @@ class AotTools { '--base=$base', '--patch=$patch', '--analyze-snapshot=$analyzeSnapshot', + '--gen-snapshot=$genSnapshot', + '--kernel=$kernel', '--output=$outputPath', ], workingDirectory: workingDirectory, From 51a93abd2c583c9b43660223ac5a02e3210eafb7 Mon Sep 17 00:00:00 2001 From: Eric Seidel Date: Thu, 28 Mar 2024 11:22:09 -0700 Subject: [PATCH 2/5] Add version guard and fix tests --- .../patch/patch_ios_framework_command.dart | 8 +++++-- .../lib/src/executables/aot_tools.dart | 21 +++++++++++++++++-- .../patch/patch_ios_command_test.dart | 12 +++++++++++ .../patch_ios_framework_command_test.dart | 10 +++++++++ .../test/src/executables/aot_tools_test.dart | 12 ++++++++++- 5 files changed, 58 insertions(+), 5 deletions(-) diff --git a/packages/shorebird_cli/lib/src/commands/patch/patch_ios_framework_command.dart b/packages/shorebird_cli/lib/src/commands/patch/patch_ios_framework_command.dart index 048521554..5fd964c5a 100644 --- a/packages/shorebird_cli/lib/src/commands/patch/patch_ios_framework_command.dart +++ b/packages/shorebird_cli/lib/src/commands/patch/patch_ios_framework_command.dart @@ -384,14 +384,18 @@ ${summary.join('\n')} return ExitCode.software.code; } + final genSnapshot = shorebirdArtifacts.getArtifactPath( + artifact: ShorebirdArtifact.genSnapshot, + ); + final linkProgress = logger.progress('Linking AOT files'); try { await aotTools.link( base: releaseArtifact.path, patch: aotSnapshot.path, analyzeSnapshot: analyzeSnapshot.path, - genSnapshot: '', - kernel: '', + genSnapshot: genSnapshot, + kernel: newestAppDill().path, outputPath: _vmcodeOutputPath, workingDirectory: _buildDirectory, ); diff --git a/packages/shorebird_cli/lib/src/executables/aot_tools.dart b/packages/shorebird_cli/lib/src/executables/aot_tools.dart index ad38fee5b..1e5309e02 100644 --- a/packages/shorebird_cli/lib/src/executables/aot_tools.dart +++ b/packages/shorebird_cli/lib/src/executables/aot_tools.dart @@ -2,8 +2,10 @@ import 'dart:io'; import 'package:mason_logger/mason_logger.dart'; import 'package:path/path.dart' as p; +import 'package:pub_semver/pub_semver.dart'; import 'package:scoped/scoped.dart'; import 'package:shorebird_cli/src/cache.dart'; +import 'package:shorebird_cli/src/extensions/version.dart'; import 'package:shorebird_cli/src/shorebird_artifacts.dart'; import 'package:shorebird_cli/src/shorebird_env.dart'; import 'package:shorebird_cli/src/shorebird_process.dart'; @@ -94,6 +96,20 @@ class AotTools { ); } + Future _linkerUsesGenSnapshot() async { + final version = await _getVersion(); + return version != null && version >= Version(0, 0, 1); + } + + Future _getVersion() async { + final result = await _exec(['--version']); + if (result.exitCode != 0) { + return null; + } + final version = result.stdout.toString().trim(); + return tryParseVersion(version); + } + /// Generate a link vmcode file from two AOT snapshots. Future link({ required String base, @@ -104,14 +120,15 @@ class AotTools { required String outputPath, String? workingDirectory, }) async { + final linkerUsesGenSnapshot = await _linkerUsesGenSnapshot(); final result = await _exec( [ 'link', '--base=$base', '--patch=$patch', '--analyze-snapshot=$analyzeSnapshot', - '--gen-snapshot=$genSnapshot', - '--kernel=$kernel', + if (linkerUsesGenSnapshot) '--gen-snapshot=$genSnapshot', + if (linkerUsesGenSnapshot) '--kernel=$kernel', '--output=$outputPath', ], workingDirectory: workingDirectory, diff --git a/packages/shorebird_cli/test/src/commands/patch/patch_ios_command_test.dart b/packages/shorebird_cli/test/src/commands/patch/patch_ios_command_test.dart index 3a746e67a..16529e1f3 100644 --- a/packages/shorebird_cli/test/src/commands/patch/patch_ios_command_test.dart +++ b/packages/shorebird_cli/test/src/commands/patch/patch_ios_command_test.dart @@ -336,6 +336,8 @@ flutter: base: any(named: 'base'), patch: any(named: 'patch'), analyzeSnapshot: any(named: 'analyzeSnapshot'), + genSnapshot: any(named: 'genSnapshot'), + kernel: any(named: 'kernel'), workingDirectory: any(named: 'workingDirectory'), outputPath: any(named: 'outputPath'), ), @@ -819,6 +821,8 @@ Please re-run the release command for this version or create a new release.'''), base: any(named: 'base'), patch: any(named: 'patch'), analyzeSnapshot: any(named: 'analyzeSnapshot'), + genSnapshot: any(named: 'genSnapshot'), + kernel: any(named: 'kernel'), workingDirectory: any(named: 'workingDirectory'), outputPath: any(named: 'outputPath'), ), @@ -1142,6 +1146,8 @@ Please re-run the release command for this version or create a new release.'''), base: any(named: 'base'), patch: any(named: 'patch'), analyzeSnapshot: any(named: 'analyzeSnapshot'), + genSnapshot: any(named: 'genSnapshot'), + kernel: any(named: 'kernel'), workingDirectory: any(named: 'workingDirectory'), outputPath: any(named: 'outputPath'), ), @@ -1175,6 +1181,8 @@ Please re-run the release command for this version or create a new release.'''), base: any(named: 'base'), patch: any(named: 'patch'), analyzeSnapshot: any(named: 'analyzeSnapshot'), + genSnapshot: any(named: 'genSnapshot'), + kernel: any(named: 'kernel'), workingDirectory: any(named: 'workingDirectory'), outputPath: any(named: 'outputPath'), ), @@ -1189,6 +1197,8 @@ Please re-run the release command for this version or create a new release.'''), base: any(named: 'base'), patch: any(named: 'patch'), analyzeSnapshot: any(named: 'analyzeSnapshot'), + genSnapshot: any(named: 'genSnapshot'), + kernel: any(named: 'kernel'), workingDirectory: any(named: 'workingDirectory'), outputPath: any(named: 'outputPath'), ), @@ -1235,6 +1245,8 @@ Please re-run the release command for this version or create a new release.'''), base: any(named: 'base'), patch: any(named: 'patch'), analyzeSnapshot: any(named: 'analyzeSnapshot'), + genSnapshot: any(named: 'genSnapshot'), + kernel: any(named: 'kernel'), workingDirectory: any(named: 'workingDirectory'), outputPath: any(named: 'outputPath'), ), diff --git a/packages/shorebird_cli/test/src/commands/patch/patch_ios_framework_command_test.dart b/packages/shorebird_cli/test/src/commands/patch/patch_ios_framework_command_test.dart index 47f16a175..88b1bb1e1 100644 --- a/packages/shorebird_cli/test/src/commands/patch/patch_ios_framework_command_test.dart +++ b/packages/shorebird_cli/test/src/commands/patch/patch_ios_framework_command_test.dart @@ -285,6 +285,8 @@ flutter: base: any(named: 'base'), patch: any(named: 'patch'), analyzeSnapshot: any(named: 'analyzeSnapshot'), + genSnapshot: any(named: 'genSnapshot'), + kernel: any(named: 'kernel'), workingDirectory: any(named: 'workingDirectory'), outputPath: any(named: 'outputPath'), ), @@ -589,6 +591,8 @@ Please re-run the release command for this version or create a new release.'''), base: any(named: 'base'), patch: any(named: 'patch'), analyzeSnapshot: any(named: 'analyzeSnapshot'), + genSnapshot: any(named: 'genSnapshot'), + kernel: any(named: 'kernel'), workingDirectory: any(named: 'workingDirectory'), outputPath: any(named: 'outputPath'), ), @@ -1011,6 +1015,8 @@ Please re-run the release command for this version or create a new release.'''), base: any(named: 'base'), patch: any(named: 'patch'), analyzeSnapshot: any(named: 'analyzeSnapshot'), + genSnapshot: any(named: 'genSnapshot'), + kernel: any(named: 'kernel'), workingDirectory: any(named: 'workingDirectory'), outputPath: any(named: 'outputPath'), ), @@ -1025,6 +1031,8 @@ Please re-run the release command for this version or create a new release.'''), base: any(named: 'base'), patch: any(named: 'patch'), analyzeSnapshot: any(named: 'analyzeSnapshot'), + genSnapshot: any(named: 'genSnapshot'), + kernel: any(named: 'kernel'), workingDirectory: any(named: 'workingDirectory'), outputPath: any(named: 'outputPath'), ), @@ -1071,6 +1079,8 @@ Please re-run the release command for this version or create a new release.'''), base: any(named: 'base'), patch: any(named: 'patch'), analyzeSnapshot: any(named: 'analyzeSnapshot'), + genSnapshot: any(named: 'genSnapshot'), + kernel: any(named: 'kernel'), workingDirectory: any(named: 'workingDirectory'), outputPath: any(named: 'outputPath'), ), diff --git a/packages/shorebird_cli/test/src/executables/aot_tools_test.dart b/packages/shorebird_cli/test/src/executables/aot_tools_test.dart index bc9dbefed..9953e48cb 100644 --- a/packages/shorebird_cli/test/src/executables/aot_tools_test.dart +++ b/packages/shorebird_cli/test/src/executables/aot_tools_test.dart @@ -55,7 +55,9 @@ void main() { group('link', () { const base = './path/to/base.aot'; const patch = './path/to/patch.aot'; - const analyzeSnapshot = './path/to/analyze_snapshot.aot'; + const analyzeSnapshot = './path/to/analyze_snapshot'; + const genSnapshot = './path/to/gen_snapshot'; + const kernel = './path/to/kernel.dill'; const outputPath = './path/to/out.vmcode'; test('throws Exception when process exits with non-zero code', () async { @@ -83,6 +85,8 @@ void main() { base: base, patch: patch, analyzeSnapshot: analyzeSnapshot, + genSnapshot: genSnapshot, + kernel: kernel, outputPath: outputPath, ), ), @@ -127,6 +131,8 @@ void main() { base: base, patch: patch, analyzeSnapshot: analyzeSnapshot, + genSnapshot: genSnapshot, + kernel: kernel, workingDirectory: workingDirectory.path, outputPath: outputPath, ), @@ -180,6 +186,8 @@ void main() { base: base, patch: patch, analyzeSnapshot: analyzeSnapshot, + genSnapshot: genSnapshot, + kernel: kernel, workingDirectory: workingDirectory.path, outputPath: outputPath, ), @@ -235,6 +243,8 @@ void main() { base: base, patch: patch, analyzeSnapshot: analyzeSnapshot, + genSnapshot: genSnapshot, + kernel: kernel, workingDirectory: workingDirectory.path, outputPath: outputPath, ), From ebfa954532b4a8c9c750431a8f8348817066df75 Mon Sep 17 00:00:00 2001 From: Bryan Oltman Date: Thu, 28 Mar 2024 14:25:52 -0400 Subject: [PATCH 3/5] Update packages/shorebird_cli/lib/src/executables/aot_tools.dart Co-authored-by: Felix Angelov --- packages/shorebird_cli/lib/src/executables/aot_tools.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/shorebird_cli/lib/src/executables/aot_tools.dart b/packages/shorebird_cli/lib/src/executables/aot_tools.dart index 43e0df30b..4f833c567 100644 --- a/packages/shorebird_cli/lib/src/executables/aot_tools.dart +++ b/packages/shorebird_cli/lib/src/executables/aot_tools.dart @@ -118,7 +118,7 @@ class AotTools { Future _getVersion() async { final result = await _exec(['--version']); - if (result.exitCode != 0) { + if (result.exitCode != ExitCode.success.code) { return null; } final version = result.stdout.toString().trim(); From a5ca14bf3ef4bcca6f162281b879596a249463ba Mon Sep 17 00:00:00 2001 From: Eric Seidel Date: Thu, 28 Mar 2024 11:33:24 -0700 Subject: [PATCH 4/5] Don't use Version? --- .../lib/src/executables/aot_tools.dart | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/packages/shorebird_cli/lib/src/executables/aot_tools.dart b/packages/shorebird_cli/lib/src/executables/aot_tools.dart index 4f833c567..c15530a12 100644 --- a/packages/shorebird_cli/lib/src/executables/aot_tools.dart +++ b/packages/shorebird_cli/lib/src/executables/aot_tools.dart @@ -5,8 +5,8 @@ import 'package:path/path.dart' as p; import 'package:pub_semver/pub_semver.dart'; import 'package:scoped/scoped.dart'; import 'package:shorebird_cli/src/cache.dart'; -import 'package:shorebird_cli/src/extensions/version.dart'; import 'package:shorebird_cli/src/engine_config.dart'; +import 'package:shorebird_cli/src/extensions/version.dart'; import 'package:shorebird_cli/src/shorebird_artifacts.dart'; import 'package:shorebird_cli/src/shorebird_env.dart'; import 'package:shorebird_cli/src/shorebird_process.dart'; @@ -113,16 +113,20 @@ class AotTools { Future _linkerUsesGenSnapshot() async { final version = await _getVersion(); - return version != null && version >= Version(0, 0, 1); + return version >= Version(0, 0, 1); } - Future _getVersion() async { + Future _getVersion() async { + // Use 0.0.0 to allow callers to easily compare w/o checking for null. + // If callers need to care about null, we can change this function to + // return Version?. + final noVersion = Version(0, 0, 0); final result = await _exec(['--version']); if (result.exitCode != ExitCode.success.code) { - return null; + return noVersion; } final version = result.stdout.toString().trim(); - return tryParseVersion(version); + return tryParseVersion(version) ?? noVersion; } /// Generate a link vmcode file from two AOT snapshots. From 25685dee48d5a792dd00e9a6232d5d623be0e68f Mon Sep 17 00:00:00 2001 From: Eric Seidel Date: Thu, 28 Mar 2024 11:48:18 -0700 Subject: [PATCH 5/5] make offering to the coverage gods --- .../test/src/executables/aot_tools_test.dart | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/packages/shorebird_cli/test/src/executables/aot_tools_test.dart b/packages/shorebird_cli/test/src/executables/aot_tools_test.dart index 9953e48cb..4a5908cdc 100644 --- a/packages/shorebird_cli/test/src/executables/aot_tools_test.dart +++ b/packages/shorebird_cli/test/src/executables/aot_tools_test.dart @@ -268,6 +268,77 @@ void main() { ).called(1); }); }); + + group('when when link expects gen_snapshot', () { + const aotToolsPath = 'aot_tools'; + + setUp(() { + when( + () => shorebirdArtifacts.getArtifactPath( + artifact: ShorebirdArtifact.aotTools, + ), + ).thenReturn(aotToolsPath); + }); + + test('passes gen_snapshot to aot_tools', () async { + when( + () => process.run( + aotToolsPath, + ['--version'], + workingDirectory: any(named: 'workingDirectory'), + ), + ).thenAnswer( + (_) async => const ShorebirdProcessResult( + exitCode: 0, + stdout: '0.0.1', + stderr: '', + ), + ); + when( + () => process.run( + aotToolsPath, + any(that: contains('--gen-snapshot=$genSnapshot')), + workingDirectory: any(named: 'workingDirectory'), + ), + ).thenAnswer( + (_) async => const ShorebirdProcessResult( + exitCode: 0, + stdout: '', + stderr: '', + ), + ); + await expectLater( + runWithOverrides( + () => aotTools.link( + base: base, + patch: patch, + analyzeSnapshot: analyzeSnapshot, + genSnapshot: genSnapshot, + kernel: kernel, + workingDirectory: workingDirectory.path, + outputPath: outputPath, + ), + ), + completes, + ); + + verify( + () => process.run( + aotToolsPath, + [ + 'link', + '--base=$base', + '--patch=$patch', + '--analyze-snapshot=$analyzeSnapshot', + '--gen-snapshot=$genSnapshot', + '--kernel=$kernel', + '--output=$outputPath', + ], + workingDirectory: any(named: 'workingDirectory'), + ), + ).called(1); + }); + }); }); group('isGeneratePatchDiffBaseSupported', () {