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 021d7948..cabd9451 100644 --- a/packages/shorebird_cli/lib/src/archive_analysis/archive_differ.dart +++ b/packages/shorebird_cli/lib/src/archive_analysis/archive_differ.dart @@ -76,12 +76,12 @@ abstract class ArchiveDiffer { /// archives at the two provided paths. FileSetDiff changedFiles(String oldArchivePath, String newArchivePath) => FileSetDiff.fromPathHashes( - oldPathHashes: _fileHashes(File(oldArchivePath)), - newPathHashes: _fileHashes(File(newArchivePath)), + oldPathHashes: fileHashes(File(oldArchivePath)), + newPathHashes: fileHashes(File(newArchivePath)), ); - PathHashes _fileHashes(File aar) { - final zipDirectory = ZipDirectory.read(InputFileStream(aar.path)); + PathHashes fileHashes(File archive) { + final zipDirectory = ZipDirectory.read(InputFileStream(archive.path)); return { for (final file in zipDirectory.fileHeaders) // Zip files contain an (optional) crc32 checksum for a file. IPAs and diff --git a/packages/shorebird_cli/lib/src/archive_analysis/ios_archive_differ.dart b/packages/shorebird_cli/lib/src/archive_analysis/ios_archive_differ.dart index ca879848..cc929a00 100644 --- a/packages/shorebird_cli/lib/src/archive_analysis/ios_archive_differ.dart +++ b/packages/shorebird_cli/lib/src/archive_analysis/ios_archive_differ.dart @@ -1,3 +1,7 @@ +import 'dart:io'; + +import 'package:archive/archive_io.dart'; +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'; @@ -13,27 +17,85 @@ import 'package:shorebird_cli/src/archive_analysis/file_set_diff.dart'; /// /// Dart changes will appear in the App.framework/App executable. class IosArchiveDiffer extends ArchiveDiffer { + String _hash(List bytes) => sha256.convert(bytes).toString(); + static const binaryFiles = { 'App.framework/App', 'Flutter.framework/Flutter', }; static RegExp appRegex = RegExp(r'^Payload/[\w\-. ]+.app/[\w\-. ]+$'); + /// Files that have been added, removed, or that have changed between the + /// archives at the two provided paths. This method will also unisgn mach-o + /// binaries in the archives before computing the diff. + @override + FileSetDiff changedFiles(String oldArchivePath, String newArchivePath) { + final oldPathHashes = fileHashes(File(oldArchivePath)); + final newPathHashes = fileHashes(File(newArchivePath)); + + _updateToUnsignedHashes( + archivePath: oldArchivePath, + pathHashes: oldPathHashes, + ); + _updateToUnsignedHashes( + archivePath: newArchivePath, + pathHashes: newPathHashes, + ); + + return FileSetDiff.fromPathHashes( + oldPathHashes: oldPathHashes, + newPathHashes: newPathHashes, + ); + } + + void _updateToUnsignedHashes({ + required String archivePath, + required PathHashes pathHashes, + }) { + for (final file in _filesToUnsign(archivePath)) { + pathHashes[file.name] = _unsignedFileHash(file); + } + } + + List _filesToUnsign(String archivePath) { + return ZipDecoder() + .decodeBuffer(InputFileStream(archivePath)) + .files + .where((file) => file.isFile) + .where( + (file) => + file.name.endsWith('App.framework/App') || + file.name.endsWith('Flutter.framework/Flutter') || + appRegex.hasMatch(file.name), + ) + .toList(); + } + + String _unsignedFileHash(ArchiveFile file) { + final tempDir = Directory.systemTemp.createTempSync(); + final outPath = p.join(tempDir.path, file.name); + final outputStream = OutputFileStream(outPath); + file.writeContent(outputStream); + outputStream.close(); + + if (Platform.isMacOS) { + // coverage:ignore-start + Process.runSync('codesign', ['--remove-signature', outPath]); + // coverage:ignore-end + } + + final outFile = File(outPath); + final hash = _hash(outFile.readAsBytesSync()); + return hash; + } + @override bool containsPotentiallyBreakingAssetDiffs(FileSetDiff fileSetDiff) => assetsFileSetDiff(fileSetDiff).isNotEmpty; @override - bool containsPotentiallyBreakingNativeDiffs(FileSetDiff fileSetDiff) { - // Because the mach-o binaries are signed (we believe with in expiration - // date), they will always have different hashes, even if the code used to - // generate them is identical. - // - // TODO(bryanoltman): support mach-o binary diffing. - // We can do this using the `codesign --remove-signature` command, but this - // is slow and requires a temporary directory to store the unsigned binary. - return false; - } + bool containsPotentiallyBreakingNativeDiffs(FileSetDiff fileSetDiff) => + nativeFileSetDiff(fileSetDiff).isNotEmpty; @override bool isAssetFilePath(String filePath) { diff --git a/packages/shorebird_cli/test/src/archive_analysis/ios_archive_differ_test.dart b/packages/shorebird_cli/test/src/archive_analysis/ios_archive_differ_test.dart index 5f3c5898..9ded77b4 100644 --- a/packages/shorebird_cli/test/src/archive_analysis/ios_archive_differ_test.dart +++ b/packages/shorebird_cli/test/src/archive_analysis/ios_archive_differ_test.dart @@ -1,5 +1,6 @@ import 'package:path/path.dart' as p; import 'package:shorebird_cli/src/archive_analysis/archive_analysis.dart'; +import 'package:shorebird_cli/src/platform.dart'; import 'package:test/test.dart'; void main() { @@ -21,148 +22,213 @@ void main() { final changedDartXcframeworkPath = p.join(xcframeworkFixturesBasePath, 'changed_dart.xcframework.zip'); - group(IosArchiveDiffer, () { - late IosArchiveDiffer differ; + group( + IosArchiveDiffer, + () { + late IosArchiveDiffer differ; - setUp(() { - differ = IosArchiveDiffer(); - }); + setUp(() { + differ = IosArchiveDiffer(); + }); - group('ipa', () { - group('changedPaths', () { - test('finds no differences between the same ipa', () { - expect(differ.changedFiles(baseIpaPath, baseIpaPath), isEmpty); + group('ipa', () { + group('changedPaths', () { + test('finds no differences between the same ipa', () { + expect( + differ.changedFiles(baseIpaPath, baseIpaPath), + isEmpty, + ); + }); + + test('finds differences between two different ipas', () { + final fileSetDiff = differ.changedFiles( + baseIpaPath, + changedAssetIpaPath, + ); + if (platform.isMacOS) { + expect( + fileSetDiff.changedPaths, + { + 'Payload/Runner.app/_CodeSignature/CodeResources', + 'Payload/Runner.app/Frameworks/App.framework/_CodeSignature/CodeResources', + 'Payload/Runner.app/Frameworks/App.framework/flutter_assets/assets/asset.json', + 'Symbols/4C4C4411-5555-3144-A13A-E47369D8ACD5.symbols', + 'Symbols/BC970605-0A53-3457-8736-D7A870AB6E71.symbols', + 'Symbols/0CBBC9EF-0745-3074-81B7-765F5B4515FD.symbols' + }, + ); + } else { + expect( + fileSetDiff.changedPaths, + { + 'Payload/Runner.app/_CodeSignature/CodeResources', + 'Payload/Runner.app/Runner', + 'Payload/Runner.app/Frameworks/Flutter.framework/Flutter', + 'Payload/Runner.app/Frameworks/App.framework/_CodeSignature/CodeResources', + 'Payload/Runner.app/Frameworks/App.framework/App', + 'Payload/Runner.app/Frameworks/App.framework/flutter_assets/assets/asset.json', + 'Symbols/4C4C4411-5555-3144-A13A-E47369D8ACD5.symbols', + 'Symbols/BC970605-0A53-3457-8736-D7A870AB6E71.symbols', + 'Symbols/0CBBC9EF-0745-3074-81B7-765F5B4515FD.symbols' + }, + ); + } + }); }); - test('finds differences between two different ipas', () { - expect( - differ.changedFiles(baseIpaPath, changedAssetIpaPath).changedPaths, - { - 'Payload/Runner.app/_CodeSignature/CodeResources', - 'Payload/Runner.app/Runner', - 'Payload/Runner.app/Frameworks/Flutter.framework/Flutter', - 'Payload/Runner.app/Frameworks/App.framework/_CodeSignature/CodeResources', - 'Payload/Runner.app/Frameworks/App.framework/App', - 'Payload/Runner.app/Frameworks/App.framework/flutter_assets/assets/asset.json', - 'Symbols/4C4C4411-5555-3144-A13A-E47369D8ACD5.symbols', - 'Symbols/BC970605-0A53-3457-8736-D7A870AB6E71.symbols', - 'Symbols/0CBBC9EF-0745-3074-81B7-765F5B4515FD.symbols', - }, - ); + group('changedFiles', () { + test('detects asset changes', () { + final fileSetDiff = + differ.changedFiles(baseIpaPath, changedAssetIpaPath); + expect(differ.assetsFileSetDiff(fileSetDiff), isNotEmpty); + expect( + differ.dartFileSetDiff(fileSetDiff), + platform.isMacOS ? isEmpty : isNotEmpty, + ); + expect( + differ.nativeFileSetDiff(fileSetDiff), + platform.isMacOS ? isEmpty : isNotEmpty, + ); + }); + + test('detects dart changes', () { + final fileSetDiff = + differ.changedFiles(baseIpaPath, changedDartIpaPath); + expect(differ.assetsFileSetDiff(fileSetDiff), isEmpty); + expect(differ.dartFileSetDiff(fileSetDiff), isNotEmpty); + expect( + differ.nativeFileSetDiff(fileSetDiff), + platform.isMacOS ? isEmpty : isNotEmpty, + ); + }); + + test('detects swift changes', () { + final fileSetDiff = + differ.changedFiles(baseIpaPath, changedSwiftIpaPath); + expect(differ.assetsFileSetDiff(fileSetDiff), isEmpty); + expect( + differ.dartFileSetDiff(fileSetDiff), + platform.isMacOS ? isEmpty : isNotEmpty, + ); + expect(differ.nativeFileSetDiff(fileSetDiff), isNotEmpty); + }); + }); + + group('containsPotentiallyBreakingAssetDiffs', () { + test('returns true if a file in flutter_assets has changed', () { + final fileSetDiff = differ.changedFiles( + baseIpaPath, + changedAssetIpaPath, + ); + expect( + differ.containsPotentiallyBreakingAssetDiffs(fileSetDiff), + isTrue, + ); + }); + + test('returns false if no files in flutter_assets has changed', () { + final fileSetDiff = differ.changedFiles( + baseIpaPath, + changedDartIpaPath, + ); + expect( + differ.containsPotentiallyBreakingAssetDiffs(fileSetDiff), + isFalse, + ); + }); + }); + + group('containsPotentiallyBreakingNativeDiffs', () { + test('returns true if Swift files have been changed', () { + final fileSetDiff = differ.changedFiles( + baseIpaPath, + changedSwiftIpaPath, + ); + expect( + differ.containsPotentiallyBreakingNativeDiffs(fileSetDiff), + isTrue, + ); + }); + + test('returns false if Swift files have not been changed', () { + final fileSetDiff = differ.changedFiles( + baseIpaPath, + changedAssetIpaPath, + ); + expect( + differ.containsPotentiallyBreakingNativeDiffs(fileSetDiff), + platform.isMacOS ? isFalse : isTrue, + ); + }); }); }); - group('changedFiles', () { - test('detects asset changes', () { - final fileSetDiff = - differ.changedFiles(baseIpaPath, changedAssetIpaPath); - expect(differ.assetsFileSetDiff(fileSetDiff), isNotEmpty); - expect(differ.dartFileSetDiff(fileSetDiff), isNotEmpty); - expect(differ.nativeFileSetDiff(fileSetDiff), isNotEmpty); + group('xcframework', () { + group('changedPaths', () { + test('finds no differences between the same zipped xcframeworks', () { + expect( + differ.changedFiles(baseXcframeworkPath, baseXcframeworkPath), + isEmpty, + ); + }); + + test('finds differences between two differed zipped xcframeworks', + () { + final fileSetDiff = differ.changedFiles( + baseXcframeworkPath, + changedAssetXcframeworkPath, + ); + if (platform.isMacOS) { + expect( + fileSetDiff.changedPaths, + { + 'ios-arm64_x86_64-simulator/App.framework/_CodeSignature/CodeResources', + 'ios-arm64_x86_64-simulator/App.framework/flutter_assets/assets/asset.json', + 'ios-arm64/App.framework/_CodeSignature/CodeResources', + 'ios-arm64/App.framework/flutter_assets/assets/asset.json' + }, + ); + } else { + expect( + fileSetDiff.changedPaths, + { + 'ios-arm64_x86_64-simulator/App.framework/_CodeSignature/CodeResources', + 'ios-arm64_x86_64-simulator/App.framework/App', + 'ios-arm64_x86_64-simulator/App.framework/flutter_assets/assets/asset.json', + 'ios-arm64/App.framework/_CodeSignature/CodeResources', + 'ios-arm64/App.framework/App', + 'ios-arm64/App.framework/flutter_assets/assets/asset.json', + }, + ); + } + }); }); - test('detects dart changes', () { - final fileSetDiff = - differ.changedFiles(baseIpaPath, changedDartIpaPath); - expect(differ.assetsFileSetDiff(fileSetDiff), isEmpty); - expect(differ.dartFileSetDiff(fileSetDiff), isNotEmpty); - expect(differ.nativeFileSetDiff(fileSetDiff), isNotEmpty); - }); + group('changedFiles', () { + test('detects asset changes', () { + final fileSetDiff = differ.changedFiles( + baseXcframeworkPath, + changedAssetXcframeworkPath, + ); + expect(differ.assetsFileSetDiff(fileSetDiff), isNotEmpty); + expect( + differ.dartFileSetDiff(fileSetDiff), + platform.isMacOS ? isEmpty : isNotEmpty, + ); + expect(differ.nativeFileSetDiff(fileSetDiff), isEmpty); + }); - test('detects swift changes', () { - final fileSetDiff = - differ.changedFiles(baseIpaPath, changedSwiftIpaPath); - expect(differ.assetsFileSetDiff(fileSetDiff), isEmpty); - expect(differ.dartFileSetDiff(fileSetDiff), isNotEmpty); - expect(differ.nativeFileSetDiff(fileSetDiff), isNotEmpty); + test('detects dart changes', () { + final fileSetDiff = differ.changedFiles( + baseXcframeworkPath, + changedDartXcframeworkPath, + ); + expect(differ.assetsFileSetDiff(fileSetDiff), isEmpty); + expect(differ.dartFileSetDiff(fileSetDiff), isNotEmpty); + expect(differ.nativeFileSetDiff(fileSetDiff), isEmpty); + }); }); }); - - group('containsPotentiallyBreakingAssetDiffs', () { - test('returns true if a file in flutter_assets has changed', () { - final fileSetDiff = differ.changedFiles( - baseIpaPath, - changedAssetIpaPath, - ); - expect( - differ.containsPotentiallyBreakingAssetDiffs(fileSetDiff), - isTrue, - ); - }); - - test('returns false if no files in flutter_assets has changed', () { - final fileSetDiff = differ.changedFiles( - baseIpaPath, - changedDartIpaPath, - ); - expect( - differ.containsPotentiallyBreakingAssetDiffs(fileSetDiff), - isFalse, - ); - }); - }); - - group('containsPotentiallyBreakingNativeDiffs', () { - test("always returns false, as we don't check for this yet", () { - final fileSetDiff = differ.changedFiles( - baseIpaPath, - changedSwiftIpaPath, - ); - expect( - differ.containsPotentiallyBreakingNativeDiffs(fileSetDiff), - isFalse, - ); - }); - }); - }); - - group('xcframework', () { - group('changedPaths', () { - test('finds no differences between the same zipped xcframeworks', () { - expect( - differ.changedFiles(baseXcframeworkPath, baseXcframeworkPath), - isEmpty, - ); - }); - - test('finds differences between two differed zipped xcframeworks', () { - expect( - differ - .changedFiles(baseXcframeworkPath, changedAssetXcframeworkPath) - .changedPaths, - { - 'ios-arm64_x86_64-simulator/App.framework/_CodeSignature/CodeResources', - 'ios-arm64_x86_64-simulator/App.framework/App', - 'ios-arm64_x86_64-simulator/App.framework/flutter_assets/assets/asset.json', - 'ios-arm64/App.framework/_CodeSignature/CodeResources', - 'ios-arm64/App.framework/App', - 'ios-arm64/App.framework/flutter_assets/assets/asset.json' - }, - ); - }); - }); - - group('changedFiles', () { - test('detects asset changes', () { - final fileSetDiff = differ.changedFiles( - baseXcframeworkPath, - changedAssetXcframeworkPath, - ); - expect(differ.assetsFileSetDiff(fileSetDiff), isNotEmpty); - expect(differ.dartFileSetDiff(fileSetDiff), isNotEmpty); - expect(differ.nativeFileSetDiff(fileSetDiff), isEmpty); - }); - - test('detects dart changes', () { - final fileSetDiff = differ.changedFiles( - baseXcframeworkPath, - changedDartXcframeworkPath, - ); - expect(differ.assetsFileSetDiff(fileSetDiff), isEmpty); - expect(differ.dartFileSetDiff(fileSetDiff), isNotEmpty); - expect(differ.nativeFileSetDiff(fileSetDiff), isEmpty); - }); - }); - }); - }); + }, + ); }