From 75c8c970030dfd26ca08aafbd28041ba2473a9ca Mon Sep 17 00:00:00 2001 From: Eric Seidel Date: Tue, 22 Apr 2025 14:41:44 -0700 Subject: [PATCH] feat: collect linker metadata if available (#3070) Runs aot_tools link_metadata if available. Does not block the build if it is not. Co-authored-by: Felix Angelov --- .../commands/patch/ios_framework_patcher.dart | 16 ++++- .../lib/src/commands/patch/ios_patcher.dart | 10 ++- .../lib/src/executables/aot_tools.dart | 13 ++++ .../src/metadata/create_patch_metadata.dart | 9 +++ .../src/metadata/create_patch_metadata.g.dart | 6 ++ .../shorebird_cli/lib/src/platform/apple.dart | 49 +++++++++++-- .../patch/ios_framework_patcher_test.dart | 25 +++++-- .../src/commands/patch/ios_patcher_test.dart | 25 +++++-- .../test/src/executables/aot_tools_test.dart | 70 +++++++++++++++++++ .../test/src/platform/apple_test.dart | 40 +++++++++++ 10 files changed, 246 insertions(+), 17 deletions(-) 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 564d199e..a6dddf15 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 @@ -58,6 +58,10 @@ class IosFrameworkPatcher extends Patcher { @override ReleaseType get releaseType => ReleaseType.iosFramework; + /// The last build's link metadata. + @visibleForTesting + Map? lastBuildLinkMetadata; + @override double? get linkPercentage => lastBuildLinkPercentage; @@ -175,13 +179,22 @@ class IosFrameworkPatcher extends Patcher { patchSnapshotDir: shorebirdEnv.buildDirectory, ); - await apple.runLinker( + final result = await apple.runLinker( kernelFile: File(_appDillCopyPath), releaseArtifact: releaseArtifactFile, splitDebugInfoArgs: IosPatcher.splitDebugInfoArgs(splitDebugInfoPath), aotOutputFile: File(_aotOutputPath), vmCodeFile: File(_vmcodeOutputPath), ); + final linkPercentage = result.linkPercentage; + final exitCode = result.exitCode; + if (exitCode != ExitCode.success.code) throw ProcessExit(exitCode); + if (linkPercentage != null && + linkPercentage < Patcher.linkPercentageWarningThreshold) { + logger.warn(Patcher.lowLinkPercentageWarning(linkPercentage)); + } + lastBuildLinkPercentage = linkPercentage; + lastBuildLinkMetadata = result.linkMetadata; } final patchBuildFile = @@ -249,6 +262,7 @@ class IosFrameworkPatcher extends Patcher { CreatePatchMetadata metadata, ) async => metadata.copyWith( linkPercentage: lastBuildLinkPercentage, + linkMetadata: lastBuildLinkMetadata, environment: metadata.environment.copyWith( xcodeVersion: await xcodeBuild.version(), ), 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 872f0bf5..4e615ccd 100644 --- a/packages/shorebird_cli/lib/src/commands/patch/ios_patcher.dart +++ b/packages/shorebird_cli/lib/src/commands/patch/ios_patcher.dart @@ -76,6 +76,10 @@ class IosPatcher extends Patcher { @visibleForTesting double? lastBuildLinkPercentage; + /// The last build's link metadata. + @visibleForTesting + Json? lastBuildLinkMetadata; + @override double? get linkPercentage => lastBuildLinkPercentage; @@ -258,19 +262,22 @@ For more information see: ${supportedFlutterVersionsUrl.toLink()}'''); patchSnapshotDir: shorebirdEnv.buildDirectory, ); - final (:exitCode, :linkPercentage) = await apple.runLinker( + final result = await apple.runLinker( kernelFile: File(_appDillCopyPath), releaseArtifact: releaseArtifactFile, splitDebugInfoArgs: splitDebugInfoArgs(splitDebugInfoPath), aotOutputFile: File(_aotOutputPath), vmCodeFile: File(_vmcodeOutputPath), ); + final linkPercentage = result.linkPercentage; + final exitCode = result.exitCode; if (exitCode != ExitCode.success.code) throw ProcessExit(exitCode); if (linkPercentage != null && linkPercentage < Patcher.linkPercentageWarningThreshold) { logger.warn(Patcher.lowLinkPercentageWarning(linkPercentage)); } lastBuildLinkPercentage = linkPercentage; + lastBuildLinkMetadata = result.linkMetadata; } final patchBuildFile = File(useLinker ? _vmcodeOutputPath : _aotOutputPath); @@ -355,6 +362,7 @@ For more information see: ${supportedFlutterVersionsUrl.toLink()}'''); CreatePatchMetadata metadata, ) async => metadata.copyWith( linkPercentage: lastBuildLinkPercentage, + linkMetadata: lastBuildLinkMetadata, environment: metadata.environment.copyWith( xcodeVersion: await xcodeBuild.version(), ), diff --git a/packages/shorebird_cli/lib/src/executables/aot_tools.dart b/packages/shorebird_cli/lib/src/executables/aot_tools.dart index cd65db9d..1cb27468 100644 --- a/packages/shorebird_cli/lib/src/executables/aot_tools.dart +++ b/packages/shorebird_cli/lib/src/executables/aot_tools.dart @@ -233,6 +233,19 @@ class AotTools { return result.stdout.toString().contains('dump-debug-info'); } + /// Dump json metadata from a link debug result. + // Added in Flutter 3.29.3 + Future> getLinkMetadata({ + required String debugDir, + String? workingDirectory, + }) async { + final result = await _exec([ + 'link_metadata', + debugDir, + ], workingDirectory: workingDirectory); + return jsonDecode(result.stdout.toString()) as Map; + } + /// Generate a link vmcode file from two AOT snapshots. Future link({ required String base, diff --git a/packages/shorebird_cli/lib/src/metadata/create_patch_metadata.dart b/packages/shorebird_cli/lib/src/metadata/create_patch_metadata.dart index 62c7cde9..cd5d0c8a 100644 --- a/packages/shorebird_cli/lib/src/metadata/create_patch_metadata.dart +++ b/packages/shorebird_cli/lib/src/metadata/create_patch_metadata.dart @@ -27,6 +27,7 @@ class CreatePatchMetadata extends Equatable { required this.inferredReleaseVersion, required this.environment, this.linkPercentage, + this.linkMetadata, }); // coverage:ignore-start @@ -40,6 +41,7 @@ class CreatePatchMetadata extends Equatable { bool hasNativeChanges = false, bool inferredReleaseVersion = false, double? linkPercentage, + Json? linkMetadata, BuildEnvironmentMetadata? environment, }) => CreatePatchMetadata( releasePlatform: releasePlatform, @@ -49,6 +51,7 @@ class CreatePatchMetadata extends Equatable { hasNativeChanges: hasNativeChanges, inferredReleaseVersion: inferredReleaseVersion, linkPercentage: linkPercentage, + linkMetadata: linkMetadata, environment: environment ?? BuildEnvironmentMetadata.forTest(), ); // coverage:ignore-end @@ -70,6 +73,7 @@ class CreatePatchMetadata extends Equatable { bool? hasNativeChanges, bool? inferredReleaseVersion, double? linkPercentage, + Json? linkMetadata, BuildEnvironmentMetadata? environment, }) => CreatePatchMetadata( releasePlatform: releasePlatform ?? this.releasePlatform, @@ -82,6 +86,7 @@ class CreatePatchMetadata extends Equatable { inferredReleaseVersion: inferredReleaseVersion ?? this.inferredReleaseVersion, linkPercentage: linkPercentage ?? this.linkPercentage, + linkMetadata: linkMetadata ?? this.linkMetadata, environment: environment ?? this.environment, ); @@ -124,6 +129,9 @@ class CreatePatchMetadata extends Equatable { /// Note: link percentage is currently only available for iOS patches. final double? linkPercentage; + /// Metadata from the linker, if available. + final Json? linkMetadata; + /// Properties about the environment in which the patch was created. /// /// Reason: see [BuildEnvironmentMetadata]. @@ -137,6 +145,7 @@ class CreatePatchMetadata extends Equatable { usedIgnoreNativeChangesFlag, hasNativeChanges, linkPercentage, + linkMetadata, inferredReleaseVersion, environment, ]; diff --git a/packages/shorebird_cli/lib/src/metadata/create_patch_metadata.g.dart b/packages/shorebird_cli/lib/src/metadata/create_patch_metadata.g.dart index dda65bea..a42f4056 100644 --- a/packages/shorebird_cli/lib/src/metadata/create_patch_metadata.g.dart +++ b/packages/shorebird_cli/lib/src/metadata/create_patch_metadata.g.dart @@ -41,6 +41,10 @@ CreatePatchMetadata _$CreatePatchMetadataFromJson( 'link_percentage', (v) => (v as num?)?.toDouble(), ), + linkMetadata: $checkedConvert( + 'link_metadata', + (v) => v as Map?, + ), ); return val; }, @@ -52,6 +56,7 @@ CreatePatchMetadata _$CreatePatchMetadataFromJson( 'hasNativeChanges': 'has_native_changes', 'inferredReleaseVersion': 'inferred_release_version', 'linkPercentage': 'link_percentage', + 'linkMetadata': 'link_metadata', }, ); @@ -65,6 +70,7 @@ Map _$CreatePatchMetadataToJson( 'has_native_changes': instance.hasNativeChanges, 'inferred_release_version': instance.inferredReleaseVersion, 'link_percentage': instance.linkPercentage, + 'link_metadata': instance.linkMetadata, 'environment': instance.environment.toJson(), }; diff --git a/packages/shorebird_cli/lib/src/platform/apple.dart b/packages/shorebird_cli/lib/src/platform/apple.dart index a27b2e5a..74c0e3c5 100644 --- a/packages/shorebird_cli/lib/src/platform/apple.dart +++ b/packages/shorebird_cli/lib/src/platform/apple.dart @@ -2,6 +2,7 @@ import 'dart:io'; import 'package:collection/collection.dart'; import 'package:io/io.dart'; +import 'package:meta/meta.dart'; import 'package:path/path.dart' as p; import 'package:pub_semver/pub_semver.dart'; import 'package:scoped_deps/scoped_deps.dart'; @@ -26,7 +27,30 @@ enum ApplePlatform { /// A record containing the exit code and optionally link percentage /// returned by `runLinker`. -typedef LinkResult = ({int exitCode, double? linkPercentage}); +@immutable +class LinkResult { + /// Creates a new [LinkResult] representing failure. + const LinkResult.failure() + : _exitCodeObject = ExitCode.software, + linkPercentage = null, + linkMetadata = null; + + /// Creates a new [LinkResult] representing success. + const LinkResult.success({required this.linkPercentage, this.linkMetadata}) + : _exitCodeObject = ExitCode.success; + + /// ExitCode.code isn't const, so store the actual object. + final ExitCode _exitCodeObject; + + /// The exit code of the linker process. + int get exitCode => _exitCodeObject.code; + + /// The percentage of code that was linked in the patch. + final double? linkPercentage; + + /// Metadata from the linker, if available. + final Map? linkMetadata; +} /// {@template missing_xcode_project_exception} /// Thrown when the Flutter project does not have iOS configured as a platform. @@ -224,7 +248,7 @@ class Apple { if (!patch.existsSync()) { logger.err('Unable to find patch AOT file at ${patch.path}'); - return (exitCode: ExitCode.software.code, linkPercentage: null); + return const LinkResult.failure(); } final analyzeSnapshot = File( @@ -235,7 +259,7 @@ class Apple { if (!analyzeSnapshot.existsSync()) { logger.err('Unable to find analyze_snapshot at ${analyzeSnapshot.path}'); - return (exitCode: ExitCode.software.code, linkPercentage: null); + return const LinkResult.failure(); } final genSnapshot = shorebirdArtifacts.getArtifactPath( @@ -289,12 +313,27 @@ $error'''); ); } on Exception catch (error) { linkProgress.fail('Failed to link AOT files: $error'); - return (exitCode: ExitCode.software.code, linkPercentage: null); + return const LinkResult.failure(); } finally { await dumpDebugInfo(); } + Map? linkMetadata; + try { + if (dumpDebugInfoDir != null) { + linkMetadata = await aotTools.getLinkMetadata( + debugDir: dumpDebugInfoDir.path, + workingDirectory: buildDirectory.path, + ); + } + } on Exception catch (error) { + logger.detail('[aot_tools] Failed to get link metadata: $error'); + } + linkProgress.complete(); - return (exitCode: ExitCode.success.code, linkPercentage: linkPercentage); + return LinkResult.success( + linkPercentage: linkPercentage, + linkMetadata: linkMetadata, + ); } /// Parses the .xcscheme file to determine if it was created for an app 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 09e2d716..32bd5a0c 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 @@ -661,10 +661,8 @@ void main() { vmCodeFile: any(named: 'vmCodeFile'), ), ).thenAnswer( - (_) async => ( - exitCode: ExitCode.success.code, - linkPercentage: linkPercentage, - ), + (_) async => + const LinkResult.success(linkPercentage: linkPercentage), ); when( aotTools.isGeneratePatchDiffBaseSupported, @@ -859,6 +857,19 @@ void main() { }); }); + test('sets link percentage', () async { + expect(patcher.linkPercentage, isNull); + await runWithOverrides( + () => patcher.createPatchArtifacts( + appId: appId, + releaseId: releaseId, + releaseArtifact: releaseArtifactFile, + supplementArtifact: supplementArtifactFile, + ), + ); + expect(patcher.linkPercentage, isNotNull); + }); + group('when code signing the patch', () { setUp(() { final privateKey = File( @@ -1043,9 +1054,12 @@ void main() { group('when linker is enabled', () { const linkPercentage = 100.0; + const linkMetadata = {'link': 'metadata'}; setUp(() { - patcher.lastBuildLinkPercentage = linkPercentage; + patcher + ..lastBuildLinkPercentage = linkPercentage + ..lastBuildLinkMetadata = linkMetadata; }); test('returns correct metadata', () async { @@ -1078,6 +1092,7 @@ void main() { hasNativeChanges: false, inferredReleaseVersion: false, linkPercentage: linkPercentage, + linkMetadata: linkMetadata, environment: BuildEnvironmentMetadata( flutterRevision: flutterRevision, operatingSystem: operatingSystem, 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 ed28588c..267ca8c0 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 @@ -968,10 +968,8 @@ For more information see: ${supportedFlutterVersionsUrl.toLink()}'''), vmCodeFile: any(named: 'vmCodeFile'), ), ).thenAnswer( - (_) async => ( - exitCode: ExitCode.success.code, - linkPercentage: linkPercentage, - ), + (_) async => + const LinkResult.success(linkPercentage: linkPercentage), ); when( aotTools.isGeneratePatchDiffBaseSupported, @@ -1168,6 +1166,19 @@ For more information see: ${supportedFlutterVersionsUrl.toLink()}'''), ), ); }); + + test('sets link percentage', () async { + expect(patcher.linkPercentage, isNull); + await runWithOverrides( + () => patcher.createPatchArtifacts( + appId: appId, + releaseId: releaseId, + releaseArtifact: releaseArtifactFile, + supplementArtifact: supplementArtifactFile, + ), + ); + expect(patcher.linkPercentage, isNotNull); + }); }); group('when code signing the patch', () { @@ -1499,9 +1510,12 @@ For more information see: ${supportedFlutterVersionsUrl.toLink()}'''), group('when linker is enabled', () { const linkPercentage = 100.0; + const linkMetadata = {'link': 'metadata'}; setUp(() { - patcher.lastBuildLinkPercentage = linkPercentage; + patcher + ..lastBuildLinkPercentage = linkPercentage + ..lastBuildLinkMetadata = linkMetadata; }); test('returns correct metadata', () async { @@ -1534,6 +1548,7 @@ For more information see: ${supportedFlutterVersionsUrl.toLink()}'''), hasNativeChanges: false, inferredReleaseVersion: false, linkPercentage: linkPercentage, + linkMetadata: linkMetadata, environment: BuildEnvironmentMetadata( flutterRevision: flutterRevision, operatingSystem: operatingSystem, 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 16ac42c3..867801b2 100644 --- a/packages/shorebird_cli/test/src/executables/aot_tools_test.dart +++ b/packages/shorebird_cli/test/src/executables/aot_tools_test.dart @@ -950,6 +950,76 @@ Run "aot_tools help " for more information about a command. expect(result.existsSync(), isTrue); }); }); + + group('getLinkMetadata', () { + late int exitCode; + late String stdout; + late String stderr; + + setUp(() { + stdout = ''; + stderr = ''; + exitCode = 0; + when( + () => process.start( + any(), + any(), + workingDirectory: any(named: 'workingDirectory'), + ), + ).thenAnswer((_) async { + final mockProcess = MockProcess(); + when(() => mockProcess.exitCode).thenAnswer((_) async => exitCode); + when( + () => mockProcess.stdout, + ).thenAnswer((_) => Stream.value(utf8.encode(stdout))); + when( + () => mockProcess.stderr, + ).thenAnswer((_) => Stream.value(utf8.encode(stderr))); + return mockProcess; + }); + when( + () => process.start( + any(), + any(), + workingDirectory: any(named: 'workingDirectory'), + ), + ).thenAnswer((_) async { + final mockProcess = MockProcess(); + when(() => mockProcess.exitCode).thenAnswer((_) async => exitCode); + when( + () => mockProcess.stdout, + ).thenAnswer((_) => Stream.value(utf8.encode(stdout))); + when( + () => mockProcess.stderr, + ).thenAnswer((_) => Stream.value(utf8.encode(stderr))); + return mockProcess; + }); + }); + + test( + 'returns link metadata when aot_tools executes successfully', + () async { + stdout = '{}'; + final result = await runWithOverrides( + () => aotTools.getLinkMetadata(debugDir: '/debug'), + ); + expect(result, isA>()); + }, + ); + + test( + 'throws FormatException when aot_tools outputs invalid json', + () async { + stdout = 'invalid'; + await expectLater( + () => runWithOverrides( + () => aotTools.getLinkMetadata(debugDir: '/debug'), + ), + throwsFormatException, + ); + }, + ); + }); }); }); } diff --git a/packages/shorebird_cli/test/src/platform/apple_test.dart b/packages/shorebird_cli/test/src/platform/apple_test.dart index 73c67325..822efc83 100644 --- a/packages/shorebird_cli/test/src/platform/apple_test.dart +++ b/packages/shorebird_cli/test/src/platform/apple_test.dart @@ -56,6 +56,13 @@ void main() { when(() => logger.progress(any())).thenReturn(progress); when(() => platform.environment).thenReturn({}); + + when( + () => aotTools.getLinkMetadata( + debugDir: any(named: 'debugDir'), + workingDirectory: any(named: 'workingDirectory'), + ), + ).thenAnswer((_) async => {'key': 'value'}); }); group(MissingXcodeProjectException, () { @@ -632,6 +639,39 @@ To add macOS, run "flutter create . --platforms macos"'''); expect(result.linkPercentage, equals(linkPercentage)); }); }); + + group('when call to aotTools.getLinkMetadata fails', () { + setUp(() { + when( + () => aotTools.isLinkDebugInfoSupported(), + ).thenAnswer((_) async => true); + when( + () => aotTools.getLinkMetadata( + debugDir: any(named: 'debugDir'), + workingDirectory: any(named: 'workingDirectory'), + ), + ).thenThrow(Exception('oops')); + }); + + test('logs error and exits with code 70', () async { + await runWithOverrides( + () => apple.runLinker( + aotOutputFile: aotOutputFile, + kernelFile: File('missing'), + releaseArtifact: File('missing'), + vmCodeFile: File('missing'), + splitDebugInfoArgs: [], + ), + ); + + verify( + () => logger.detail( + '[aot_tools] Failed to get link metadata: Exception: oops', + ), + ).called(1); + verify(() => progress.complete()).called(1); + }); + }); }); }); }