flutter/shorebird
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

fix(shorebird_cli): include link % and debug info in patch summary (#2063)

Browse Source
This commit is contained in:
Felix Angelov
2024-05-09 18:19:47 -05:00
committed by GitHub
parent ae3f33e2f0
commit cef8898cb9
7 changed files with 245 additions and 68 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/shorebird_cli/lib/src/commands/patch/ios_framework_patcher.dart
+5 -10
View File
@@ -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');
packages/shorebird_cli/lib/src/commands/patch/ios_patcher.dart
+10 -46
View File
@@ -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');
packages/shorebird_cli/lib/src/commands/patch/patch_command.dart
+7 -2
View File
@@ -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(
packages/shorebird_cli/lib/src/commands/patch/patcher.dart
+37
View File
@@ -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'));
}
}
packages/shorebird_cli/test/src/commands/patch/ios_framework_patcher_test.dart
+20
View File
@@ -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(
packages/shorebird_cli/test/src/commands/patch/ios_patcher_test.dart
+20
View File
@@ -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(
packages/shorebird_cli/test/src/commands/patch/patch_command_test.dart
+146 -10
View File
@@ -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);