From 7652dc682cafbbe20988364aac3d5f845862ff3c Mon Sep 17 00:00:00 2001 From: Erick Date: Mon, 22 Apr 2024 15:15:55 -0300 Subject: [PATCH] feat: Adding debug-linker flag to patch ios command (#1934) Co-authored-by: Felix Angelov --- .../src/commands/patch/patch_ios_command.dart | 35 ++++++++++- .../lib/src/executables/aot_tools.dart | 2 + .../patch/patch_ios_command_test.dart | 26 ++++++++ .../test/src/executables/aot_tools_test.dart | 61 +++++++++++++++++++ 4 files changed, 122 insertions(+), 2 deletions(-) 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 9fec24aa..eed033fd 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 @@ -6,6 +6,7 @@ import 'package:mason_logger/mason_logger.dart'; import 'package:path/path.dart' as p; import 'package:platform/platform.dart'; import 'package:scoped/scoped.dart'; +import 'package:shorebird_cli/src/archive/directory_archive.dart'; import 'package:shorebird_cli/src/archive_analysis/archive_analysis.dart'; import 'package:shorebird_cli/src/artifact_manager.dart'; import 'package:shorebird_cli/src/code_push_client_wrapper.dart'; @@ -55,7 +56,7 @@ class PatchIosCommand extends ShorebirdCommand 'release-version', help: ''' The version of the release being patched (e.g. "1.0.0+1"). - + If this option is not provided, the version number will be determined from the patch artifact.''', ) ..addFlag( @@ -88,6 +89,12 @@ If this option is not provided, the version number will be determined from the p 'staging', negatable: false, help: 'Whether to publish the patch to the staging environment.', + ) + ..addFlag( + 'debug-linker', + negatable: false, + help: 'Collects linker diagnostic information to help troubleshoot low ' + 'link percentages.', ); } @@ -371,6 +378,8 @@ Please re-run the release command for this version or create a new release.'''); '🟢 Track: ${lightCyan.wrap('Production')}', if (percentLinked != null) '''🔗 Running ${lightCyan.wrap('${percentLinked.toStringAsFixed(1)}%')} on CPU''', + if (results['debug-linker'] == true) + '''🔍 Debug Info: ${lightCyan.wrap(_debugInfoOutpath)}''', ]; logger.info( @@ -444,6 +453,11 @@ ${summary.join('\n')} 'out.vmcode', ); + String get _debugInfoOutpath => p.join( + _buildDirectory, + 'linker_diagnostic.zip', + ); + String _readVersionFromPlist() { final archivePath = getXcarchiveDirectory()?.path; if (archivePath == null) { @@ -509,8 +523,11 @@ ${summary.join('\n')} buildProgress.complete(); } - Future<_LinkResult> _runLinker({required File releaseArtifact}) async { + Future<_LinkResult> _runLinker({ + required File releaseArtifact, + }) async { final patch = File(_aotOutputPath); + final dumpDebugInfo = results['debug-linker'] == true; if (!patch.existsSync()) { logger.err('Unable to find patch AOT file at ${patch.path}'); @@ -535,6 +552,9 @@ ${summary.join('\n')} final linkProgress = logger.progress('Linking AOT files'); double? linkPercentage; try { + final dumpDebugInfoDir = + dumpDebugInfo ? Directory.systemTemp.createTempSync() : null; + linkPercentage = await aotTools.link( base: releaseArtifact.path, patch: patch.path, @@ -543,7 +563,18 @@ ${summary.join('\n')} outputPath: _vmcodeOutputPath, workingDirectory: _buildDirectory, kernel: newestAppDill().path, + dumpDebugInfoPath: dumpDebugInfoDir?.path, ); + + if (dumpDebugInfo && dumpDebugInfoDir != null) { + final debugInfoZip = await dumpDebugInfoDir.zipToTempFile(); + debugInfoZip.copySync( + p.join( + 'build', + _debugInfoOutpath, + ), + ); + } } catch (error) { linkProgress.fail('Failed to link AOT files: $error'); return (exitCode: ExitCode.software.code, linkPercentage: null); diff --git a/packages/shorebird_cli/lib/src/executables/aot_tools.dart b/packages/shorebird_cli/lib/src/executables/aot_tools.dart index fbc26ec8..cd910e66 100644 --- a/packages/shorebird_cli/lib/src/executables/aot_tools.dart +++ b/packages/shorebird_cli/lib/src/executables/aot_tools.dart @@ -140,6 +140,7 @@ class AotTools { required String kernel, required String outputPath, String? workingDirectory, + String? dumpDebugInfoPath, }) async { // We use the json lines format. https://jsonlines.org const linkJson = 'link.jsonl'; @@ -158,6 +159,7 @@ class AotTools { '--reporter=json', '--redirect-to=${p.join(outputDir, linkJson)}', ], + if (dumpDebugInfoPath != null) '--dump-debug-info=$dumpDebugInfoPath', ], 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 82529d87..23121b54 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 @@ -351,6 +351,7 @@ flutter: kernel: any(named: 'kernel'), workingDirectory: any(named: 'workingDirectory'), outputPath: any(named: 'outputPath'), + dumpDebugInfoPath: any(named: 'dumpDebugInfoPath'), ), ).thenAnswer((_) async => null); when(() => aotTools.isGeneratePatchDiffBaseSupported()) @@ -1296,6 +1297,31 @@ Please re-run the release command for this version or create a new release.'''), ).called(1); }); }); + + group('when debug-linker is true', () { + test( + 'succeeds and create debug info zip', + () async { + when(() => argResults['debug-linker']).thenReturn(true); + setUpProjectRoot(); + setUpProjectRootArtifacts(); + final exitCode = await runWithOverrides(command.run); + expect(exitCode, ExitCode.success.code); + verify( + () => aotTools.link( + 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'), + dumpDebugInfoPath: any(named: 'dumpDebugInfoPath'), + ), + ).called(1); + }, + ); + }); }); group('when aot-tools supports generating patch diff base', () { 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 092ee265..cdb343f3 100644 --- a/packages/shorebird_cli/test/src/executables/aot_tools_test.dart +++ b/packages/shorebird_cli/test/src/executables/aot_tools_test.dart @@ -157,6 +157,67 @@ void main() { }); }); + group( + 'when --dump-debug-info is provided', + () { + const aotToolsPath = 'aot_tools'; + + setUp(() { + when( + () => shorebirdArtifacts.getArtifactPath( + artifact: ShorebirdArtifact.aotTools, + ), + ).thenReturn(aotToolsPath); + }); + + test('forwards the option to aot_tools', () async { + const debugPath = 'my_debug_path'; + when( + () => process.run( + aotToolsPath, + any(), + 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, + dumpDebugInfoPath: debugPath, + ), + ), + completes, + ); + verify( + () => process.run( + aotToolsPath, + [ + 'link', + '--base=$base', + '--patch=$patch', + '--analyze-snapshot=$analyzeSnapshot', + '--output=$outputPath', + '--dump-debug-info=$debugPath', + ], + workingDirectory: any(named: 'workingDirectory'), + ), + ).called(1); + }); + }, + ); + group('when aot-tools is a kernel file', () { const aotToolsPath = 'aot_tools.dill';