diff --git a/packages/shorebird_cli/lib/src/commands/patch/ios_framework_patcher.dart b/packages/shorebird_cli/lib/src/commands/patch/ios_framework_patcher.dart index e70d045d..13ab92a8 100644 --- a/packages/shorebird_cli/lib/src/commands/patch/ios_framework_patcher.dart +++ b/packages/shorebird_cli/lib/src/commands/patch/ios_framework_patcher.dart @@ -37,15 +37,7 @@ class IosFrameworkPatcher extends Patcher { required super.target, }); - String get _buildDirectory => p.join( - shorebirdEnv.getShorebirdProjectRoot()!.path, - 'build', - ); - - String get _vmcodeOutputPath => p.join( - _buildDirectory, - 'out.vmcode', - ); + String get _vmcodeOutputPath => p.join(buildDirectory.path, 'out.vmcode'); @override ArchiveDiffer get archiveDiffer => IosArchiveDiffer(); @@ -56,6 +48,9 @@ class IosFrameworkPatcher extends Patcher { @override ReleaseType get releaseType => ReleaseType.iosFramework; + @override + double? get linkPercentage => lastBuildLinkPercentage; + @visibleForTesting double? lastBuildLinkPercentage; @@ -278,7 +273,7 @@ class IosFrameworkPatcher extends Patcher { genSnapshot: genSnapshot, kernel: artifactManager.newestAppDill().path, outputPath: _vmcodeOutputPath, - workingDirectory: _buildDirectory, + workingDirectory: buildDirectory.path, ); } catch (error) { linkProgress.fail('Failed to link AOT files: $error'); diff --git a/packages/shorebird_cli/lib/src/commands/patch/ios_patcher.dart b/packages/shorebird_cli/lib/src/commands/patch/ios_patcher.dart index c7cfd7b8..6bd95395 100644 --- a/packages/shorebird_cli/lib/src/commands/patch/ios_patcher.dart +++ b/packages/shorebird_cli/lib/src/commands/patch/ios_patcher.dart @@ -25,8 +25,6 @@ import 'package:shorebird_cli/src/third_party/flutter_tools/lib/flutter_tools.da import 'package:shorebird_cli/src/version.dart'; import 'package:shorebird_code_push_client/shorebird_code_push_client.dart'; -const linkDebugInfoFileName = 'linker_diagnostic.zip'; - typedef _LinkResult = ({int exitCode, double? linkPercentage}); /// {@template ios_patcher} @@ -40,46 +38,16 @@ class IosPatcher extends Patcher { required super.target, }); - // Link percentage that is considered the minimum before a user might notice. - // Our early testing has shown that about: - // - 1/3rd of patches link at 99% - // - 1/3rd of patches link between 20% and 99% - // - 1/3rd of patches link below 20% - // Most lowering is likely due to: - // https://github.com/shorebirdtech/shorebird/issues/1825 - static const double minLinkPercentage = 75; + String get _aotOutputPath => p.join(buildDirectory.path, 'out.aot'); - static String lowLinkPercentageWarning(double linkPercentage) { - return ''' -${lightCyan.wrap('shorebird patch')} was only able to share ${linkPercentage.toStringAsFixed(1)}% of Dart code with the released app. -This means the patched code may execute slower than expected. -https://docs.shorebird.dev/status#link-percentage-ios -'''; - } - - String get _buildDirectory => p.join( - shorebirdEnv.getShorebirdProjectRoot()!.path, - 'build', - ); - - String get _aotOutputPath => p.join( - _buildDirectory, - 'out.aot', - ); - - String get _vmcodeOutputPath => p.join( - _buildDirectory, - 'out.vmcode', - ); - - String get _debugInfoOutputPath => p.join( - _buildDirectory, - linkDebugInfoFileName, - ); + String get _vmcodeOutputPath => p.join(buildDirectory.path, 'out.vmcode'); @visibleForTesting double? lastBuildLinkPercentage; + @override + double? get linkPercentage => lastBuildLinkPercentage; + @override ReleaseType get releaseType => ReleaseType.ios; @@ -220,8 +188,9 @@ https://docs.shorebird.dev/status#link-percentage-ios releaseArtifact: releaseArtifactFile, ); if (exitCode != ExitCode.success.code) return exit(exitCode); - if (linkPercentage != null && linkPercentage < minLinkPercentage) { - logger.warn(lowLinkPercentageWarning(linkPercentage)); + if (linkPercentage != null && + linkPercentage < Patcher.minLinkPercentage) { + logger.warn(Patcher.lowLinkPercentageWarning(linkPercentage)); } lastBuildLinkPercentage = linkPercentage; } @@ -352,19 +321,14 @@ https://docs.shorebird.dev/status#link-percentage-ios analyzeSnapshot: analyzeSnapshot.path, genSnapshot: genSnapshot, outputPath: _vmcodeOutputPath, - workingDirectory: _buildDirectory, + workingDirectory: buildDirectory.path, kernel: artifactManager.newestAppDill().path, dumpDebugInfoPath: dumpDebugInfoDir?.path, ); if (dumpDebugInfo && dumpDebugInfoDir != null) { final debugInfoZip = await dumpDebugInfoDir.zipToTempFile(); - debugInfoZip.copySync( - p.join( - 'build', - _debugInfoOutputPath, - ), - ); + debugInfoZip.copySync(p.join('build', debugInfoFile.path)); } } catch (error) { linkProgress.fail('Failed to link AOT files: $error'); diff --git a/packages/shorebird_cli/lib/src/commands/patch/patch_command.dart b/packages/shorebird_cli/lib/src/commands/patch/patch_command.dart index 1218e9e0..a53c9a5c 100644 --- a/packages/shorebird_cli/lib/src/commands/patch/patch_command.dart +++ b/packages/shorebird_cli/lib/src/commands/patch/patch_command.dart @@ -89,8 +89,7 @@ of the iOS app that is using this module.''', 'debug-linker', defaultsTo: true, help: 'Collects linker diagnostic information to help troubleshoot low ' - 'link percentages. File is saved to build/$linkDebugInfoFileName. ' - 'iOS only.', + 'link percentages (iOS only.)', ) ..addFlag( 'dry-run', @@ -298,6 +297,12 @@ NOTE: this is ${styleBold.wrap('not')} recommended. Asset changes cannot be incl '🟠 Track: ${lightCyan.wrap('Staging')}' else 'đŸŸĸ Track: ${lightCyan.wrap('Production')}', + if (patcher.linkPercentage != null) + '''🔗 Running ${lightCyan.wrap('${patcher.linkPercentage!.toStringAsFixed(1)}%')} on CPU''', + if (results['debug-linker'] == true && + (patcher.linkPercentage != null && + patcher.linkPercentage! < Patcher.minLinkPercentage)) + '''🔍 Debug Info: ${lightCyan.wrap(patcher.debugInfoFile.path)}''', ]; logger.info( diff --git a/packages/shorebird_cli/lib/src/commands/patch/patcher.dart b/packages/shorebird_cli/lib/src/commands/patch/patcher.dart index fd3f51fc..4cb9a71a 100644 --- a/packages/shorebird_cli/lib/src/commands/patch/patcher.dart +++ b/packages/shorebird_cli/lib/src/commands/patch/patcher.dart @@ -1,6 +1,8 @@ import 'dart:io'; import 'package:args/args.dart'; +import 'package:mason_logger/mason_logger.dart'; +import 'package:path/path.dart' as p; import 'package:shorebird_cli/src/archive_analysis/archive_differ.dart'; import 'package:shorebird_cli/src/code_push_client_wrapper.dart'; import 'package:shorebird_cli/src/patch_diff_checker.dart'; @@ -20,6 +22,24 @@ abstract class Patcher { required this.target, }); + // Link percentage that is considered the minimum before a user might notice. + // Our early testing has shown that about: + // - 1/3rd of patches link at 99% + // - 1/3rd of patches link between 20% and 99% + // - 1/3rd of patches link below 20% + // Most lowering is likely due to: + // https://github.com/shorebirdtech/shorebird/issues/1825 + static const double minLinkPercentage = 75; + + /// The standard link percentage warning. + static String lowLinkPercentageWarning(double linkPercentage) { + return ''' +${lightCyan.wrap('shorebird patch')} was only able to share ${linkPercentage.toStringAsFixed(1)}% of Dart code with the released app. +This means the patched code may execute slower than expected. +https://docs.shorebird.dev/status#link-percentage-ios +'''; + } + /// The arguments passed to the command. final ArgResults argResults; @@ -71,4 +91,21 @@ abstract class Patcher { /// Whether to allow changes in native code (--allow-native-diffs). bool get allowNativeDiffs => argResults['allow-native-diffs'] == true; + + /// The link percentage for the generated patch artifact if applicable. + /// Returns `null` if the platform does not use a linker or if the linking + /// step has not yet been run. + double? get linkPercentage => null; + + /// The build directory of the respective shorebird project. + Directory get buildDirectory { + return Directory( + p.join(shorebirdEnv.getShorebirdProjectRoot()!.path, 'build'), + ); + } + + /// The path to the output file for the debug info. + File get debugInfoFile { + return File(p.join(buildDirectory.path, 'patch-debug.zip')); + } } diff --git a/packages/shorebird_cli/test/src/commands/patch/ios_framework_patcher_test.dart b/packages/shorebird_cli/test/src/commands/patch/ios_framework_patcher_test.dart index d1b9128c..b08f15aa 100644 --- a/packages/shorebird_cli/test/src/commands/patch/ios_framework_patcher_test.dart +++ b/packages/shorebird_cli/test/src/commands/patch/ios_framework_patcher_test.dart @@ -146,6 +146,26 @@ void main() { }); }); + group('linkPercentage', () { + group('when linking has not occurred', () { + test('returns null', () { + expect(patcher.linkPercentage, isNull); + }); + }); + + group('when linking has occurred', () { + const linkPercentage = 42.1337; + + setUp(() { + patcher.lastBuildLinkPercentage = linkPercentage; + }); + + test('returns correct link percentage', () { + expect(patcher.linkPercentage, equals(linkPercentage)); + }); + }); + }); + group('assertPreconditions', () { setUp(() { when( diff --git a/packages/shorebird_cli/test/src/commands/patch/ios_patcher_test.dart b/packages/shorebird_cli/test/src/commands/patch/ios_patcher_test.dart index 13ec4a7d..427baffc 100644 --- a/packages/shorebird_cli/test/src/commands/patch/ios_patcher_test.dart +++ b/packages/shorebird_cli/test/src/commands/patch/ios_patcher_test.dart @@ -153,6 +153,26 @@ void main() { }); }); + group('linkPercentage', () { + group('when linking has not occurred', () { + test('returns null', () { + expect(patcher.linkPercentage, isNull); + }); + }); + + group('when linking has occurred', () { + const linkPercentage = 42.1337; + + setUp(() { + patcher.lastBuildLinkPercentage = linkPercentage; + }); + + test('returns correct link percentage', () { + expect(patcher.linkPercentage, equals(linkPercentage)); + }); + }); + }); + group('assertPreconditions', () { setUp(() { when( diff --git a/packages/shorebird_cli/test/src/commands/patch/patch_command_test.dart b/packages/shorebird_cli/test/src/commands/patch/patch_command_test.dart index be9da2e2..e32602eb 100644 --- a/packages/shorebird_cli/test/src/commands/patch/patch_command_test.dart +++ b/packages/shorebird_cli/test/src/commands/patch/patch_command_test.dart @@ -34,6 +34,14 @@ void main() { const flutterRevision = '83305b5088e6fe327fb3334a73ff190828d85713'; const releasePlatform = ReleasePlatform.android; const releaseVersion = '1.2.3+1'; + const patchArtifactBundles = { + Arch.arm32: PatchArtifactBundle( + arch: 'arm32', + hash: '#', + size: 42, + path: '', + ), + }; const shorebirdYaml = ShorebirdYaml(appId: appId); final patchMetadata = CreatePatchMetadata.forTest(); @@ -131,6 +139,7 @@ void main() { shorebirdEnv = MockShorebirdEnv(); shorebirdFlutter = MockShorebirdFlutter(); + when(() => argResults['debug-linker']).thenReturn(false); when(() => argResults['dry-run']).thenReturn(false); when(() => argResults['platforms']).thenReturn(['android']); when(() => argResults['release-version']).thenReturn(releaseVersion); @@ -218,16 +227,7 @@ void main() { appId: any(named: 'appId'), releaseId: any(named: 'releaseId'), ), - ).thenAnswer( - (_) async => { - Arch.arm32: const PatchArtifactBundle( - arch: 'arm32', - hash: '#', - size: 42, - path: '', - ), - }, - ); + ).thenAnswer((_) async => patchArtifactBundles); when( () => patcher.createPatchMetadata(any()), ).thenAnswer((_) async => patchMetadata); @@ -284,6 +284,142 @@ void main() { }); }); + group('confirmCreatePatch', () { + group('when has flavors', () { + const flavor = 'development'; + setUp(() { + when(() => argResults['flavor']).thenReturn(flavor); + }); + + test('logs correct summary', () async { + final expectedSummary = [ + '''📱 App: ${lightCyan.wrap(appDisplayName)} ${lightCyan.wrap('($appId)')}''', + '🍧 Flavor: ${lightCyan.wrap(flavor)}', + 'đŸ“Ļ Release Version: ${lightCyan.wrap(releaseVersion)}', + '''🕹ī¸ Platform: ${lightCyan.wrap(patcher.releaseType.releasePlatform.name)} ${lightCyan.wrap('[arm32 (42 B)]')}''', + 'đŸŸĸ Track: ${lightCyan.wrap('Production')}', + ]; + await expectLater( + runWithOverrides( + () => command.confirmCreatePatch( + app: appMetadata, + releaseVersion: releaseVersion, + patcher: patcher, + patchArtifactBundles: patchArtifactBundles, + ), + ), + completes, + ); + verify( + () => logger.info( + any(that: contains(expectedSummary.join('\n'))), + ), + ).called(1); + }); + }); + + group('when is staging', () { + setUp(() { + when(() => argResults['staging']).thenReturn(true); + }); + + test('logs correct summary', () async { + final expectedSummary = [ + '''📱 App: ${lightCyan.wrap(appDisplayName)} ${lightCyan.wrap('($appId)')}''', + 'đŸ“Ļ Release Version: ${lightCyan.wrap(releaseVersion)}', + '''🕹ī¸ Platform: ${lightCyan.wrap(patcher.releaseType.releasePlatform.name)} ${lightCyan.wrap('[arm32 (42 B)]')}''', + '🟠 Track: ${lightCyan.wrap('Staging')}', + ]; + await expectLater( + runWithOverrides( + () => command.confirmCreatePatch( + app: appMetadata, + releaseVersion: releaseVersion, + patcher: patcher, + patchArtifactBundles: patchArtifactBundles, + ), + ), + completes, + ); + verify( + () => logger.info( + any(that: contains(expectedSummary.join('\n'))), + ), + ).called(1); + }); + }); + + group('when has link percentage', () { + const linkPercentage = 42.1337; + + setUp(() { + when(() => patcher.linkPercentage).thenReturn(linkPercentage); + }); + + test('logs correct summary', () async { + final expectedSummary = [ + '''📱 App: ${lightCyan.wrap(appDisplayName)} ${lightCyan.wrap('($appId)')}''', + 'đŸ“Ļ Release Version: ${lightCyan.wrap(releaseVersion)}', + '''🕹ī¸ Platform: ${lightCyan.wrap(patcher.releaseType.releasePlatform.name)} ${lightCyan.wrap('[arm32 (42 B)]')}''', + 'đŸŸĸ Track: ${lightCyan.wrap('Production')}', + '''🔗 Running ${lightCyan.wrap('${patcher.linkPercentage!.toStringAsFixed(1)}%')} on CPU''', + ]; + await expectLater( + runWithOverrides( + () => command.confirmCreatePatch( + app: appMetadata, + releaseVersion: releaseVersion, + patcher: patcher, + patchArtifactBundles: patchArtifactBundles, + ), + ), + completes, + ); + verify( + () => logger.info( + any(that: contains(expectedSummary.join('\n'))), + ), + ).called(1); + }); + + group('when has debug info', () { + final debugInfoFile = File('debug-info.txt'); + + setUp(() { + when(() => argResults['debug-linker']).thenReturn(true); + when(() => patcher.debugInfoFile).thenReturn(debugInfoFile); + }); + + test('logs correct summary', () async { + final expectedSummary = [ + '''📱 App: ${lightCyan.wrap(appDisplayName)} ${lightCyan.wrap('($appId)')}''', + 'đŸ“Ļ Release Version: ${lightCyan.wrap(releaseVersion)}', + '''🕹ī¸ Platform: ${lightCyan.wrap(patcher.releaseType.releasePlatform.name)} ${lightCyan.wrap('[arm32 (42 B)]')}''', + 'đŸŸĸ Track: ${lightCyan.wrap('Production')}', + '''🔗 Running ${lightCyan.wrap('${patcher.linkPercentage!.toStringAsFixed(1)}%')} on CPU''', + '''🔍 Debug Info: ${lightCyan.wrap(patcher.debugInfoFile.path)}''', + ]; + await expectLater( + runWithOverrides( + () => command.confirmCreatePatch( + app: appMetadata, + releaseVersion: releaseVersion, + patcher: patcher, + patchArtifactBundles: patchArtifactBundles, + ), + ), + completes, + ); + verify( + () => logger.info( + any(that: contains(expectedSummary.join('\n'))), + ), + ).called(1); + }); + }); + }); + }); + group('when release version is specified', () { setUp(() { when(() => argResults['release-version']).thenReturn(releaseVersion);