diff --git a/packages/shorebird_cli/bin/shorebird.dart b/packages/shorebird_cli/bin/shorebird.dart index e5f3c3d7..058c59a0 100644 --- a/packages/shorebird_cli/bin/shorebird.dart +++ b/packages/shorebird_cli/bin/shorebird.dart @@ -63,6 +63,7 @@ Command: shorebird ${args.join(' ')} codePushClientWrapperRef, codeSignerRef, devicectlRef, + diffRef, dittoRef, doctorRef, engineConfigRef, diff --git a/packages/shorebird_cli/lib/src/archive_analysis/apple_archive_differ.dart b/packages/shorebird_cli/lib/src/archive_analysis/apple_archive_differ.dart index 24ae11c5..4509d7b3 100644 --- a/packages/shorebird_cli/lib/src/archive_analysis/apple_archive_differ.dart +++ b/packages/shorebird_cli/lib/src/archive_analysis/apple_archive_differ.dart @@ -9,6 +9,7 @@ import 'package:crypto/crypto.dart'; import 'package:path/path.dart' as p; import 'package:shorebird_cli/src/archive_analysis/archive_differ.dart'; import 'package:shorebird_cli/src/archive_analysis/file_set_diff.dart'; +import 'package:shorebird_cli/src/executables/executables.dart'; import 'package:shorebird_cli/src/platform/apple/macho.dart'; /// {@template apple_archive_differ} @@ -85,7 +86,7 @@ class AppleArchiveDiffer extends ArchiveDiffer { } for (final file in _carFiles(archivePath)) { - pathHashes[file.name] = await _carFileHash(file); + pathHashes[file.name] = await _sanitizedCarFileHash(file); } return pathHashes; @@ -139,10 +140,11 @@ class AppleArchiveDiffer extends ArchiveDiffer { return _hash(bytes); } - /// Uses assetutil to write a json description of a .car file to disk and - /// diffs the contents of that file, less a timestamp line that changes based - /// on when the .car file was created. - Future _carFileHash(ArchiveFile file) async { + /// Writes a json description of a .car file to a temporary location and + /// returns the [File]. + /// + /// Equivalent of running `xcrun assetutil --info /path/to/Assets.car > outfile.json`. + Future _carJsonFile(ArchiveFile file) async { final tempDir = Directory.systemTemp.createTempSync(); final outPath = p.join(tempDir.path, file.name); final outputStream = OutputFileStream(outPath); @@ -155,14 +157,51 @@ class AppleArchiveDiffer extends ArchiveDiffer { Process.runSync('assetutil', ['--info', outPath, '-o', assetInfoPath]); } - // Remove the timestamp line from the json file - final jsonFile = File(assetInfoPath); + return File(assetInfoPath); + } + + /// Uses assetutil to write a json description of a .car file to disk and + /// diffs the contents of that file, less a timestamp line that changes based + /// on when the .car file was created. + Future _sanitizedCarFileHash(ArchiveFile file) async { + final jsonFile = await _carJsonFile(file); final lines = jsonFile.readAsLinesSync(); final timestampRegex = RegExp(r'^\W+"Timestamp" : \d+$'); final linesToKeep = lines.whereNot(timestampRegex.hasMatch); return _hash(linesToKeep.join('\n').codeUnits); } + @override + Future availableAssetDiffs({ + required FileSetDiff fileSetDiff, + required String oldArchivePath, + required String newArchivePath, + }) async { + final diffs = []; + for (final changedPath in fileSetDiff.changedPaths) { + if (changedPath.endsWith('.car')) { + final oldCarFile = ZipDecoder() + .decodeStream(InputFileStream(oldArchivePath)) + .files + .firstWhere((file) => file.name == changedPath); + final newCarFile = ZipDecoder() + .decodeStream(InputFileStream(newArchivePath)) + .files + .firstWhere((file) => file.name == changedPath); + final oldCarJsonFile = await _carJsonFile(oldCarFile); + final newCarJsonFile = await _carJsonFile(newCarFile); + final diffResult = await diff.run( + oldCarJsonFile.path, + newCarJsonFile.path, + colorMode: DiffColorMode.always, + unified: true, + ); + diffs.add(diffResult.stdout as String); + } + } + return diffs.join('\n'); + } + @override bool isAssetFilePath(String filePath) { /// The flutter_assets directory contains the assets listed in the assets diff --git a/packages/shorebird_cli/lib/src/archive_analysis/archive_differ.dart b/packages/shorebird_cli/lib/src/archive_analysis/archive_differ.dart index d8a02b9a..126ec6e2 100644 --- a/packages/shorebird_cli/lib/src/archive_analysis/archive_differ.dart +++ b/packages/shorebird_cli/lib/src/archive_analysis/archive_differ.dart @@ -107,4 +107,13 @@ abstract class ArchiveDiffer { }; }); } + + /// Prints the diffs of the changed files to the console. + Future availableAssetDiffs({ + required FileSetDiff fileSetDiff, + required String oldArchivePath, + required String newArchivePath, + }) async { + return ''; + } } diff --git a/packages/shorebird_cli/lib/src/executables/diff.dart b/packages/shorebird_cli/lib/src/executables/diff.dart new file mode 100644 index 00000000..444769e2 --- /dev/null +++ b/packages/shorebird_cli/lib/src/executables/diff.dart @@ -0,0 +1,41 @@ +import 'package:scoped_deps/scoped_deps.dart'; +import 'package:shorebird_cli/src/shorebird_process.dart'; + +/// The color mode for the `diff` command. +enum DiffColorMode { + /// Never color the diff. + never, + + /// Always color the diff. + always, + + /// Color the diff automatically based on the output device. + auto, +} + +/// A reference to a [Diff] instance. +final diffRef = create(Diff.new); + +/// The [Diff] instance available in the current zone. +Diff get diff => read(diffRef); + +/// A wrapper around the `diff` command. +class Diff { + /// The name of the `diff` executable. + static const executable = 'diff'; + + /// Runs the `diff` command and returns the result. + Future run( + String fileAPath, + String fileBPath, { + required bool unified, + required DiffColorMode colorMode, + }) async { + return process.run(executable, [ + if (unified) '--unified', + '--color=${colorMode.name}', + fileAPath, + fileBPath, + ]); + } +} diff --git a/packages/shorebird_cli/lib/src/executables/executables.dart b/packages/shorebird_cli/lib/src/executables/executables.dart index 58e9d7e2..38b9bc18 100644 --- a/packages/shorebird_cli/lib/src/executables/executables.dart +++ b/packages/shorebird_cli/lib/src/executables/executables.dart @@ -4,6 +4,7 @@ export 'adb.dart'; export 'aot_tools.dart'; export 'bundletool.dart'; export 'devicectl/devicectl.dart'; +export 'diff.dart'; export 'ditto.dart'; export 'git.dart'; export 'gradlew.dart'; diff --git a/packages/shorebird_cli/lib/src/patch_diff_checker.dart b/packages/shorebird_cli/lib/src/patch_diff_checker.dart index aa0b2aa6..868b7bb8 100644 --- a/packages/shorebird_cli/lib/src/patch_diff_checker.dart +++ b/packages/shorebird_cli/lib/src/patch_diff_checker.dart @@ -109,14 +109,24 @@ If you don't know why you're seeing this error, visit our troubleshooting page a yellow.wrap( archiveDiffer.assetsFileSetDiff(contentDiffs).prettyString, ), - ) - ..info( - yellow.wrap( - ''' + ); + + final diffs = await archiveDiffer.availableAssetDiffs( + fileSetDiff: contentDiffs, + oldArchivePath: releaseArchive.path, + newArchivePath: localArchive.path, + ); + if (diffs.isNotEmpty) { + logger.info(diffs); + } + + logger.info( + yellow.wrap( + ''' If you don't know why you're seeing this error, visit our troubleshooting page at ${assetChangesTroubleshootingUrl.toLink()}''', - ), - ); + ), + ); if (!allowAssetChanges) { if (!shorebirdEnv.canAcceptUserInput) { diff --git a/packages/shorebird_cli/test/fixtures/xcarchives/changed_assets_car.xcarchive.zip b/packages/shorebird_cli/test/fixtures/xcarchives/changed_assets_car.xcarchive.zip new file mode 100644 index 00000000..9c0d191c Binary files /dev/null and b/packages/shorebird_cli/test/fixtures/xcarchives/changed_assets_car.xcarchive.zip differ diff --git a/packages/shorebird_cli/test/src/archive_analysis/apple_archive_differ_test.dart b/packages/shorebird_cli/test/src/archive_analysis/apple_archive_differ_test.dart index 9a843879..85991aa3 100644 --- a/packages/shorebird_cli/test/src/archive_analysis/apple_archive_differ_test.dart +++ b/packages/shorebird_cli/test/src/archive_analysis/apple_archive_differ_test.dart @@ -1,9 +1,15 @@ // cspell:words xcarchive xcarchives xcframeworks xcframework +import 'package:mocktail/mocktail.dart'; import 'package:path/path.dart' as p; +import 'package:scoped_deps/scoped_deps.dart'; import 'package:shorebird_cli/src/archive_analysis/archive_analysis.dart'; +import 'package:shorebird_cli/src/executables/executables.dart'; import 'package:shorebird_cli/src/platform.dart'; +import 'package:shorebird_cli/src/shorebird_process.dart'; import 'package:test/test.dart'; +import '../mocks.dart'; + void main() { final xcarchiveFixturesBasePath = p.join('test', 'fixtures', 'xcarchives'); final baseIpaPath = p.join(xcarchiveFixturesBasePath, 'base.xcarchive.zip'); @@ -15,6 +21,10 @@ void main() { xcarchiveFixturesBasePath, 'changed_asset.xcarchive.zip', ); + final changedCarXcarchivePath = p.join( + xcarchiveFixturesBasePath, + 'changed_assets_car.xcarchive.zip', + ); final changedDartXcarchivePath = p.join( xcarchiveFixturesBasePath, 'changed_dart.xcarchive.zip', @@ -43,9 +53,22 @@ void main() { ); group(AppleArchiveDiffer, () { + late Diff diff; late AppleArchiveDiffer differ; + R runWithOverrides(R Function() body) { + return runScoped( + body, + values: {diffRef.overrideWith(() => diff)}, + ); + } + + setUpAll(() { + registerFallbackValue(DiffColorMode.always); + }); + setUp(() { + diff = MockDiff(); differ = const AppleArchiveDiffer(); }); @@ -191,6 +214,65 @@ void main() { ); }); }); + + group('availableAssetDiffs', () { + group('when a car file has changed', () { + const diffOutput = 'diff output'; + + setUp(() { + when( + () => diff.run( + any(), + any(), + colorMode: any(named: 'colorMode'), + unified: any(named: 'unified'), + ), + ).thenAnswer( + (_) async => const ShorebirdProcessResult( + exitCode: 1, + stdout: diffOutput, + stderr: '', + ), + ); + }); + + test('shows asset diffs', () async { + final fileSetDiff = await differ.changedFiles( + baseIpaPath, + changedCarXcarchivePath, + ); + await runWithOverrides(() async { + expect( + await differ.availableAssetDiffs( + fileSetDiff: fileSetDiff, + oldArchivePath: baseIpaPath, + newArchivePath: changedCarXcarchivePath, + ), + equals(diffOutput), + ); + }); + }); + }); + + group('when no car files have changed', () { + test('shows no asset diffs', () async { + final fileSetDiff = await differ.changedFiles( + baseIpaPath, + changedDartXcarchivePath, + ); + await runWithOverrides(() async { + expect( + await differ.availableAssetDiffs( + fileSetDiff: fileSetDiff, + oldArchivePath: baseIpaPath, + newArchivePath: changedDartXcarchivePath, + ), + isEmpty, + ); + }); + }); + }); + }); }); group('xcframework', () { diff --git a/packages/shorebird_cli/test/src/archive_analysis/archive_differ_test.dart b/packages/shorebird_cli/test/src/archive_analysis/archive_differ_test.dart index 6b52ab67..c2b51c89 100644 --- a/packages/shorebird_cli/test/src/archive_analysis/archive_differ_test.dart +++ b/packages/shorebird_cli/test/src/archive_analysis/archive_differ_test.dart @@ -78,4 +78,11 @@ class TestArchiveDiffer extends ArchiveDiffer { @override bool isNativeFilePath(String filePath) => true; + + @override + Future availableAssetDiffs({ + required FileSetDiff fileSetDiff, + required String oldArchivePath, + required String newArchivePath, + }) async => ''; } diff --git a/packages/shorebird_cli/test/src/executables/diff_test.dart b/packages/shorebird_cli/test/src/executables/diff_test.dart new file mode 100644 index 00000000..76f6774c --- /dev/null +++ b/packages/shorebird_cli/test/src/executables/diff_test.dart @@ -0,0 +1,54 @@ +import 'package:mocktail/mocktail.dart'; +import 'package:scoped_deps/scoped_deps.dart'; +import 'package:shorebird_cli/src/executables/diff.dart'; +import 'package:shorebird_cli/src/shorebird_process.dart'; +import 'package:test/test.dart'; + +import '../mocks.dart'; + +void main() { + group(Diff, () { + late ShorebirdProcess process; + late Diff diff; + + R runWithOverrides(R Function() body) { + return runScoped(body, values: {processRef.overrideWith(() => process)}); + } + + setUp(() { + process = MockShorebirdProcess(); + diff = Diff(); + }); + + group('run', () { + setUp(() { + when(() => process.run(any(), any())).thenAnswer( + (_) async => const ShorebirdProcessResult( + exitCode: 0, + stdout: 'stdout', + stderr: 'stderr', + ), + ); + }); + + test('returns the result of the `diff` command', () async { + await runWithOverrides( + () => diff.run( + 'fileA', + 'fileB', + unified: true, + colorMode: DiffColorMode.always, + ), + ); + verify( + () => process.run(Diff.executable, [ + '--unified', + '--color=always', + 'fileA', + 'fileB', + ]), + ).called(1); + }); + }); + }); +} diff --git a/packages/shorebird_cli/test/src/mocks.dart b/packages/shorebird_cli/test/src/mocks.dart index dc1a2e5c..7c34bab9 100644 --- a/packages/shorebird_cli/test/src/mocks.dart +++ b/packages/shorebird_cli/test/src/mocks.dart @@ -91,6 +91,8 @@ class MockCodeSigner extends Mock implements CodeSigner {} class MockDevicectl extends Mock implements Devicectl {} +class MockDiff extends Mock implements Diff {} + class MockDitto extends Mock implements Ditto {} class MockDirectory extends Mock implements Directory {} diff --git a/packages/shorebird_cli/test/src/patch_diff_checker_test.dart b/packages/shorebird_cli/test/src/patch_diff_checker_test.dart index de173316..eaafef95 100644 --- a/packages/shorebird_cli/test/src/patch_diff_checker_test.dart +++ b/packages/shorebird_cli/test/src/patch_diff_checker_test.dart @@ -72,6 +72,13 @@ void main() { when( () => archiveDiffer.containsPotentiallyBreakingNativeDiffs(any()), ).thenReturn(false); + when( + () => archiveDiffer.availableAssetDiffs( + fileSetDiff: any(named: 'fileSetDiff'), + oldArchivePath: any(named: 'oldArchivePath'), + newArchivePath: any(named: 'newArchivePath'), + ), + ).thenAnswer((_) async => ''); when(() => httpClient.send(any())).thenAnswer( (_) async => http.StreamedResponse(const Stream.empty(), HttpStatus.ok), @@ -221,6 +228,13 @@ void main() { '''Your app contains asset changes, which will not be included in the patch.''', ), ).called(1); + verify( + () => archiveDiffer.availableAssetDiffs( + fileSetDiff: any(named: 'fileSetDiff'), + oldArchivePath: any(named: 'oldArchivePath'), + newArchivePath: any(named: 'newArchivePath'), + ), + ).called(1); verify( () => logger.info(yellow.wrap(assetsDiffPrettyString)), ).called(1);