From cfd68095f4e1f47ac83773d64ee6aff9b4bf9e39 Mon Sep 17 00:00:00 2001 From: Felix Angelov Date: Thu, 9 May 2024 18:44:55 -0500 Subject: [PATCH] chore(shorebird_cli): remove `--debug-linker` flag from `shorebird patch` (#2067) --- .../lib/src/commands/patch/ios_patcher.dart | 11 ++--- .../lib/src/commands/patch/patch_command.dart | 11 +---- .../src/commands/patch/ios_patcher_test.dart | 3 +- .../commands/patch/patch_command_test.dart | 46 ++++--------------- 4 files changed, 17 insertions(+), 54 deletions(-) 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 6bd95395..96b5cb78 100644 --- a/packages/shorebird_cli/lib/src/commands/patch/ios_patcher.dart +++ b/packages/shorebird_cli/lib/src/commands/patch/ios_patcher.dart @@ -286,8 +286,7 @@ class IosPatcher extends Patcher { required File releaseArtifact, }) async { final patch = File(_aotOutputPath); - final dumpDebugInfo = argResults['debug-linker'] == true && - (await aotTools.isLinkDebugInfoSupported()); + final dumpDebugInfo = await aotTools.isLinkDebugInfoSupported(); if (!patch.existsSync()) { logger.err('Unable to find patch AOT file at ${patch.path}'); @@ -312,9 +311,7 @@ class IosPatcher extends Patcher { final linkProgress = logger.progress('Linking AOT files'); double? linkPercentage; try { - final dumpDebugInfoDir = - dumpDebugInfo ? Directory.systemTemp.createTempSync() : null; - + final dumpDebugInfoDir = Directory.systemTemp.createTempSync(); linkPercentage = await aotTools.link( base: releaseArtifact.path, patch: patch.path, @@ -323,10 +320,10 @@ class IosPatcher extends Patcher { outputPath: _vmcodeOutputPath, workingDirectory: buildDirectory.path, kernel: artifactManager.newestAppDill().path, - dumpDebugInfoPath: dumpDebugInfoDir?.path, + dumpDebugInfoPath: dumpDebugInfoDir.path, ); - if (dumpDebugInfo && dumpDebugInfoDir != null) { + if (dumpDebugInfo) { final debugInfoZip = await dumpDebugInfoDir.zipToTempFile(); debugInfoZip.copySync(p.join('build', debugInfoFile.path)); } 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 acaa4d71..3579d6da 100644 --- a/packages/shorebird_cli/lib/src/commands/patch/patch_command.dart +++ b/packages/shorebird_cli/lib/src/commands/patch/patch_command.dart @@ -85,12 +85,6 @@ of the iOS app that is using this module.''', help: 'Codesign the application bundle (iOS only).', defaultsTo: true, ) - ..addFlag( - 'debug-linker', - defaultsTo: true, - help: 'Collects linker diagnostic information to help troubleshoot low ' - 'link percentages (iOS only.)', - ) ..addFlag( 'dry-run', abbr: 'n', @@ -299,9 +293,8 @@ NOTE: this is ${styleBold.wrap('not')} recommended. Asset changes cannot be incl 'đŸŸĸ 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)) + if (patcher.linkPercentage != null && + patcher.linkPercentage! < Patcher.minLinkPercentage) '''🔍 Debug Info: ${lightCyan.wrap(patcher.debugInfoFile.path)}''', ]; 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 427baffc..7409b5bf 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 @@ -128,6 +128,8 @@ void main() { when(() => ios.exportOptionsPlistFromArgs(any())).thenReturn(File('')); + when(aotTools.isLinkDebugInfoSupported).thenAnswer((_) async => false); + patcher = IosPatcher( argResults: argResults, flavor: null, @@ -875,7 +877,6 @@ void main() { group('when isLinkDebugInfoSupported', () { setUp(() { - when(() => argResults['debug-linker']).thenReturn(true); when( aotTools.isLinkDebugInfoSupported, ).thenAnswer((_) async => true); 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 e32602eb..19689765 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 @@ -11,6 +11,7 @@ import 'package:shorebird_cli/src/code_push_client_wrapper.dart'; import 'package:shorebird_cli/src/commands/patch/patch.dart'; import 'package:shorebird_cli/src/config/config.dart'; import 'package:shorebird_cli/src/deployment_track.dart'; +import 'package:shorebird_cli/src/executables/executables.dart'; import 'package:shorebird_cli/src/logger.dart'; import 'package:shorebird_cli/src/patch_diff_checker.dart'; import 'package:shorebird_cli/src/platform/platform.dart'; @@ -80,6 +81,7 @@ void main() { url: 'https://example.com/release.aab', ); + late AotTools aotTools; late ArchiveDiffer archiveDiffer; late ArgResults argResults; late ArtifactBuilder artifactBuilder; @@ -99,6 +101,7 @@ void main() { return runScoped( body, values: { + aotToolsRef.overrideWith(() => aotTools), artifactBuilderRef.overrideWith(() => artifactBuilder), artifactManagerRef.overrideWith(() => artifactManager), cacheRef.overrideWith(() => cache), @@ -126,6 +129,7 @@ void main() { tearDownAll(restoreExitFunction); setUp(() { + aotTools = MockAotTools(); archiveDiffer = MockAndroidArchiveDiffer(); argResults = MockArgResults(); artifactBuilder = MockArtifactBuilder(); @@ -139,12 +143,13 @@ 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); when(() => argResults.wasParsed(any())).thenReturn(true); + when(aotTools.isLinkDebugInfoSupported).thenAnswer((_) async => true); + when( () => artifactManager.downloadFile(any()), ).thenAnswer((_) async => File('')); @@ -351,9 +356,11 @@ void main() { group('when has link percentage', () { const linkPercentage = 42.1337; + final debugInfoFile = File('debug-info.txt'); setUp(() { when(() => patcher.linkPercentage).thenReturn(linkPercentage); + when(() => patcher.debugInfoFile).thenReturn(debugInfoFile); }); test('logs correct summary', () async { @@ -363,6 +370,7 @@ void main() { '''🕹ī¸ 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( @@ -381,42 +389,6 @@ void main() { ), ).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); - }); - }); }); });