feat(shorebird_cli): add IpaDiffer to get changes between to ipa files (#1004)

packages/shorebird_cli/lib/src/archive_analysis/android_archive_differ.dart

@@ -6,14 +6,6 @@ import 'package:shorebird_cli/src/archive_analysis/archive_differ.dart';
 abstract class AndroidArchiveDiffer extends ArchiveDiffer {
   @override
   bool containsPotentiallyBreakingAssetDiffs(FileSetDiff fileSetDiff) {
-    // If only files in this set have changed, we don't need to warn the user
-    // about asset differences.
-    const assetFileNamesToIgnore = {
-      'AssetManifest.bin',
-      'AssetManifest.json',
-      'NOTICES.Z',
-    };
-
     final assetsDiff = assetsFileSetDiff(fileSetDiff);
 
     // If assets were added, we need to warn the user about asset differences.
@@ -22,7 +14,10 @@ abstract class AndroidArchiveDiffer extends ArchiveDiffer {
     }
 
     return assetsDiff.changedPaths
-        .whereNot((path) => assetFileNamesToIgnore.contains(p.basename(path)))
+        .whereNot(
+          (path) =>
+              ArchiveDiffer.assetFileNamesToIgnore.contains(p.basename(path)),
+        )
         .isNotEmpty;
   }
 
@@ -30,25 +25,8 @@ abstract class AndroidArchiveDiffer extends ArchiveDiffer {
   bool containsPotentiallyBreakingNativeDiffs(FileSetDiff fileSetDiff) =>
       nativeFileSetDiff(fileSetDiff).isNotEmpty;
 
-  FileSetDiff assetsFileSetDiff(FileSetDiff fileSetDiff) => FileSetDiff(
-        addedPaths: fileSetDiff.addedPaths.where(_isAssetFilePath).toSet(),
-        removedPaths: fileSetDiff.removedPaths.where(_isAssetFilePath).toSet(),
-        changedPaths: fileSetDiff.changedPaths.where(_isAssetFilePath).toSet(),
-      );
-
-  FileSetDiff dartFileSetDiff(FileSetDiff fileSetDiff) => FileSetDiff(
-        addedPaths: fileSetDiff.addedPaths.where(_isDartFilePath).toSet(),
-        removedPaths: fileSetDiff.removedPaths.where(_isDartFilePath).toSet(),
-        changedPaths: fileSetDiff.changedPaths.where(_isDartFilePath).toSet(),
-      );
-
-  FileSetDiff nativeFileSetDiff(FileSetDiff fileSetDiff) => FileSetDiff(
-        addedPaths: fileSetDiff.addedPaths.where(_isNativeFilePath).toSet(),
-        removedPaths: fileSetDiff.removedPaths.where(_isNativeFilePath).toSet(),
-        changedPaths: fileSetDiff.changedPaths.where(_isNativeFilePath).toSet(),
-      );
-
-  static bool _isAssetFilePath(String filePath) {
+  @override
+  bool isAssetFilePath(String filePath) {
     const assetDirNames = ['assets', 'res'];
     const assetFileNames = ['AssetManifest.json'];
 
@@ -58,11 +36,12 @@ abstract class AndroidArchiveDiffer extends ArchiveDiffer {
         assetFileNames.contains(p.basename(filePath));
   }
 
-  static bool _isDartFilePath(String filePath) {
+  @override
+  bool isDartFilePath(String filePath) {
     const dartFileNames = ['libapp.so', 'libflutter.so'];
     return dartFileNames.contains(p.basename(filePath));
   }
 
-  static bool _isNativeFilePath(String filePath) =>
-      p.extension(filePath) == '.dex';
+  @override
+  bool isNativeFilePath(String filePath) => p.extension(filePath) == '.dex';
 }

packages/shorebird_cli/lib/src/archive_analysis/archive_analysis.dart

@@ -2,3 +2,4 @@ export 'aab_differ.dart';
 export 'aar_differ.dart';
 export 'file_set_diff.dart';
 export 'ipa.dart';
+export 'ipa_differ.dart';

packages/shorebird_cli/lib/src/archive_analysis/archive_differ.dart

@@ -1,3 +1,5 @@
+import 'package:collection/collection.dart';
+import 'package:path/path.dart' as p;
 import 'package:shorebird_cli/src/archive_analysis/archive_analysis.dart';
 
 /// Thrown when an [ArchiveDiffer] fails to generate a [FileSetDiff].
@@ -5,15 +7,69 @@ class DiffFailedException implements Exception {}
 
 /// Computes content differences between two archives.
 abstract class ArchiveDiffer {
+  /// Asset files that are not considered to be breaking changes.
+  static const assetFileNamesToIgnore = {
+    'AssetManifest.bin',
+    'AssetManifest.json',
+    'NOTICES.Z',
+  };
+
   /// Files that have been added, removed, or that have changed between the
   /// archives at the two provided paths.
   FileSetDiff changedFiles(String oldArchivePath, String newArchivePath);
 
   /// Whether there are asset differences between the archives that may cause
   /// issues when patching a release.
-  bool containsPotentiallyBreakingAssetDiffs(FileSetDiff fileSetDiff);
+  bool containsPotentiallyBreakingAssetDiffs(FileSetDiff fileSetDiff) {
+    final assetsDiff = assetsFileSetDiff(fileSetDiff);
+
+    // If assets were added, we need to warn the user about asset differences.
+    if (assetsDiff.addedPaths.isNotEmpty) {
+      return true;
+    }
+
+    return assetsDiff.changedPaths
+        .whereNot(
+          (path) =>
+              ArchiveDiffer.assetFileNamesToIgnore.contains(p.basename(path)),
+        )
+        .isNotEmpty;
+  }
 
   /// Whether there are native code differences between the archives that may
   /// cause issues when patching a release.
   bool containsPotentiallyBreakingNativeDiffs(FileSetDiff fileSetDiff);
+
+  /// Whether the provided file path represents a changed asset.
+  bool isAssetFilePath(String filePath);
+
+  /// Whether the provided file path represents changed Dart code.
+  bool isDartFilePath(String filePath);
+
+  /// Whether the provided file path represents changed Native code.
+  bool isNativeFilePath(String filePath);
+
+  /// The subset of [fileSetDiff] that contains only changes that result from
+  /// edited assets.
+  FileSetDiff assetsFileSetDiff(FileSetDiff fileSetDiff) => FileSetDiff(
+        addedPaths: fileSetDiff.addedPaths.where(isAssetFilePath).toSet(),
+        removedPaths: fileSetDiff.removedPaths.where(isAssetFilePath).toSet(),
+        changedPaths: fileSetDiff.changedPaths.where(isAssetFilePath).toSet(),
+      );
+
+  /// The subset of [fileSetDiff] that contains only changes that result from
+  /// edited Dart code.
+  FileSetDiff dartFileSetDiff(FileSetDiff fileSetDiff) => FileSetDiff(
+        addedPaths: fileSetDiff.addedPaths.where(isDartFilePath).toSet(),
+        removedPaths: fileSetDiff.removedPaths.where(isDartFilePath).toSet(),
+        changedPaths: fileSetDiff.changedPaths.where(isDartFilePath).toSet(),
+      );
+
+  /// The subset of [fileSetDiff] that contains only changes that result from
+  /// edited native code.
+  FileSetDiff nativeFileSetDiff(FileSetDiff fileSetDiff) => FileSetDiff(
+        addedPaths: fileSetDiff.addedPaths.where(isNativeFilePath).toSet(),
+        removedPaths: fileSetDiff.removedPaths.where(isNativeFilePath).toSet(),
+        changedPaths: fileSetDiff.changedPaths.where(isNativeFilePath).toSet(),
+      );
 }

packages/shorebird_cli/lib/src/archive_analysis/ipa_differ.dart

@@ -0,0 +1,74 @@
+import 'dart:io';
+
+import 'package:archive/archive_io.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';
+
+/// Finds differences between two IPAs.
+///
+/// Asset changes will be in the `Assets.car` file (which is a combination of
+/// the `.xcasset` catalogs in the Xcode project) and the `flutter_assets`
+/// directory.
+///
+/// Native changes will appear in the Runner.app/Runner executable and non
+///   Flutter.framework or App.framework files.
+///
+/// Dart changes will appear in the App.framework/App executable.
+class IpaDiffer extends ArchiveDiffer {
+  static const binaryFiles = {
+    'App.framework/App',
+    'Flutter.framework/Flutter',
+  };
+  static RegExp appRegex = RegExp(r'^Payload/[\w\-. ]+.app/[\w\-. ]+$');
+
+  @override
+  FileSetDiff changedFiles(String oldArchivePath, String newArchivePath) =>
+      FileSetDiff.fromPathHashes(
+        oldPathHashes: _fileHashes(File(oldArchivePath)),
+        newPathHashes: _fileHashes(File(newArchivePath)),
+      );
+
+  @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;
+  }
+
+  @override
+  bool isAssetFilePath(String filePath) {
+    /// The flutter_assets directory contains the assets listed in the assets
+    ///   section of the pubspec.yaml file.
+    /// Assets.car is the compiled asset catalog(s) (.xcassets files).
+    return p.basename(filePath) == 'Assets.car' ||
+        p.split(filePath).contains('flutter_assets');
+  }
+
+  @override
+  bool isDartFilePath(String filePath) =>
+      filePath.endsWith('App.framework/App');
+
+  @override
+  bool isNativeFilePath(String filePath) => appRegex.hasMatch(filePath);
+
+  PathHashes _fileHashes(File ipa) {
+    final zipDirectory = ZipDirectory.read(InputFileStream(ipa.path));
+    return {
+      for (final file in zipDirectory.fileHeaders)
+        // Zip files contain an (optional) crc32 checksum for a file. IPAs seem
+        // to always include this for files, so a quick way for us to tell if
+        // file contents differ is if their checksums differ.
+        file.filename: file.crc32!.toString()
+    };
+  }
+}

packages/shorebird_cli/test/fixtures/ipas/README.md

@@ -2,7 +2,11 @@ The ipa files in this folder were generated by building the stock Flutter counte
 
 Some of their contents has been removed to reduce the size of the files. All .dylib files have been removed.
 
-Files: 
-- base.ipa is meant to represent an ipa uploaded as part of a release.
-- no_version.ipa is base.ipa with the version info removed from the Info.plist, along with several other files to save space.
-- no_plist.ipa is base.ipa with the Info.plist (and many other parts) removed.
+Files:
+
+- base.ipa is meant to represent an ipa uploaded as part of a release. App.framework/App and Flutter.framework/Flutter have been removed to save space.
+- no_version.ipa an .ipa with the version info removed from the Info.plist, along with several other files to save space.
+- no_plist.ipa an .ipa with the Info.plist (and many other parts) removed.
+- app_file_space.ipa is an .ipa with a space in the .app file name.
+- asset_changes.ipa is built from the same codebase as base.ipa with a change made to one of the assets. App.framework/App and Flutter.framework/Flutter have been removed to save space.
+- dart_changes.ipa is built from the same codebase as base.ipa with a change made to the dart code. App.framework/App and Flutter.framework/Flutter have been removed to save space.

packages/shorebird_cli/test/src/archive_analysis/aab_differ_test.dart

@@ -22,7 +22,7 @@ void main() {
       differ = AabDiffer();
     });
 
-    group('changedPaths', () {
+    group('changedFiles', () {
       test('finds no differences between the same aab', () {
         expect(differ.changedFiles(baseAabPath, baseAabPath), isEmpty);
       });

packages/shorebird_cli/test/src/archive_analysis/aar_differ_test.dart

@@ -16,7 +16,7 @@ void main() {
     differ = AarDiffer();
   });
 
-  group('changedPaths', () {
+  group('changedFiles', () {
     test('finds no differences between the same aar', () {
       expect(differ.changedFiles(baseAarPath, baseAarPath), isEmpty);
     });

packages/shorebird_cli/test/src/archive_analysis/archive_differ_test.dart

@@ -0,0 +1,79 @@
+import 'package:shorebird_cli/src/archive_analysis/archive_differ.dart';
+import 'package:shorebird_cli/src/archive_analysis/file_set_diff.dart';
+import 'package:test/test.dart';
+
+const assetFilePath = 'some/assets/file';
+
+void main() {
+  group(ArchiveDiffer, () {
+    late TestArchiveDiffer archiveDiffer;
+
+    setUp(() {
+      archiveDiffer = TestArchiveDiffer();
+    });
+
+    group('containsPotentiallyBreakingAssetDiffs', () {
+      test('returns true if any assets were added', () {
+        archiveDiffer.changedFileSetDiff = FileSetDiff(
+          addedPaths: {assetFilePath},
+          removedPaths: {},
+          changedPaths: {},
+        );
+        expect(
+          archiveDiffer.containsPotentiallyBreakingAssetDiffs(
+            archiveDiffer.changedFileSetDiff,
+          ),
+          isTrue,
+        );
+      });
+
+      test('returns false if changed assets are all in the ignore list', () {
+        archiveDiffer.changedFileSetDiff = FileSetDiff(
+          addedPaths: {},
+          removedPaths: {},
+          changedPaths: {'AssetManifest.bin'},
+        );
+        expect(
+          archiveDiffer.containsPotentiallyBreakingAssetDiffs(
+            archiveDiffer.changedFileSetDiff,
+          ),
+          isFalse,
+        );
+      });
+
+      test('returns true if changed assets are not all in the ignore list', () {
+        archiveDiffer.changedFileSetDiff = FileSetDiff(
+          addedPaths: {},
+          removedPaths: {},
+          changedPaths: {assetFilePath},
+        );
+        expect(
+          archiveDiffer.containsPotentiallyBreakingAssetDiffs(
+            archiveDiffer.changedFileSetDiff,
+          ),
+          isTrue,
+        );
+      });
+    });
+  });
+}
+
+class TestArchiveDiffer extends ArchiveDiffer {
+  FileSetDiff changedFileSetDiff = FileSetDiff.empty();
+
+  @override
+  FileSetDiff changedFiles(String oldArchivePath, String newArchivePath) =>
+      changedFileSetDiff;
+
+  @override
+  bool containsPotentiallyBreakingNativeDiffs(FileSetDiff fileSetDiff) => false;
+
+  @override
+  bool isAssetFilePath(String filePath) => filePath == assetFilePath;
+
+  @override
+  bool isDartFilePath(String filePath) => true;
+
+  @override
+  bool isNativeFilePath(String filePath) => true;
+}

packages/shorebird_cli/test/src/archive_analysis/ipa_differ_test.dart

@@ -0,0 +1,105 @@
+import 'package:path/path.dart' as p;
+import 'package:shorebird_cli/src/archive_analysis/archive_analysis.dart';
+import 'package:test/test.dart';
+
+void main() {
+  final ipaFixturesBasePath = p.join('test', 'fixtures', 'ipas');
+  final baseIpaPath = p.join(ipaFixturesBasePath, 'base.ipa');
+  final changedAssetIpaPath = p.join(ipaFixturesBasePath, 'asset_changes.ipa');
+  final changedDartIpaPath = p.join(ipaFixturesBasePath, 'dart_changes.ipa');
+  final changedSwiftIpaPath = p.join(ipaFixturesBasePath, 'swift_changes.ipa');
+
+  late IpaDiffer differ;
+
+  setUp(() {
+    differ = IpaDiffer();
+  });
+
+  group(IpaDiffer, () {
+    group('changedPaths', () {
+      test('finds no differences between the same ipa', () {
+        expect(differ.changedFiles(baseIpaPath, baseIpaPath), isEmpty);
+      });
+
+      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), isNotEmpty);
+        expect(differ.nativeFileSetDiff(fileSetDiff), 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), isNotEmpty);
+      });
+
+      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);
+      });
+    });
+
+    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,
+        );
+      });
+    });
+  });
+}