feat: warn about diffs when patching macos releases

packages/shorebird_cli/lib/src/archive_analysis/ios_archive_differ.dart → packages/shorebird_cli/lib/src/archive_analysis/apple_archive_differ.dart

@@ -11,8 +11,11 @@
 import 'package:shorebird_cli/src/archive_analysis/file_set_diff.dart';
 import 'package:shorebird_cli/src/archive_analysis/macho.dart';
 
-/// {@template ios_archive_differ}
-/// Finds differences between two IPAs or zipped Xcframeworks.
+/// {@template apple_archive_differ}
+/// Finds differences between two IPAs, zipped Xcframeworks, or zipped macOS
+/// .apps. Note that, due to a quirk in how macOS .app zips are handled by
+/// package:archive, they must *not* include the top-level .app directory (i.e.,
+/// they should unzip to a Contents directory).
 ///
 /// 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`
@@ -23,9 +26,9 @@
 ///
 /// Dart changes will appear in the App.framework/App executable.
 /// {@endtemplate}
-class IosArchiveDiffer extends ArchiveDiffer {
-  /// {@macro ios_archive_differ}
-  const IosArchiveDiffer();
+class AppleArchiveDiffer extends ArchiveDiffer {
+  /// {@macro apple_archive_differ}
+  const AppleArchiveDiffer();
 
   String _hash(List<int> bytes) => sha256.convert(bytes).toString();
 
@@ -34,11 +37,16 @@
     RegExp(r'Flutter.framework/Flutter$'),
   };
 
-  /// The regex pattern for identifying app files within an archive.
-  static final RegExp appRegex = RegExp(
+  /// The regex pattern for identifying app files within an xcarchive.
+  static final RegExp xcFrameworkAppRegex = RegExp(
     r'^Products/Applications/[\w\-. ]+.app/[\w\- ]+$',
   );
 
+  /// The regex pattern for identifying executable files within a macOS .app.
+  static final RegExp macosAppRegex = RegExp(
+    r'^Contents/MacOS/.+$',
+  );
+
   /// Files that have been added, removed, or that have changed between the
   /// archives at the two provided paths. This method will also unsign mach-o
   /// binaries in the archives before computing the diff.
@@ -95,7 +103,7 @@
           (file) =>
               _binaryFilePatterns
                   .any((pattern) => pattern.hasMatch(file.name)) ||
-              appRegex.hasMatch(file.name),
+              xcFrameworkAppRegex.hasMatch(file.name),
         )
         .toList();
   }
@@ -170,5 +178,7 @@
       filePath.endsWith('App.framework/App');
 
   @override
-  bool isNativeFilePath(String filePath) => appRegex.hasMatch(filePath);
+  bool isNativeFilePath(String filePath) =>
+      xcFrameworkAppRegex.hasMatch(filePath) ||
+      macosAppRegex.hasMatch(filePath);
 }

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

@@ -1,4 +1,4 @@
 export 'android_archive_differ.dart';
+export 'apple_archive_differ.dart';
 export 'file_set_diff.dart';
-export 'ios_archive_differ.dart';
 export 'plist.dart';

packages/shorebird_cli/lib/src/commands/patch/ios_framework_patcher.dart

@@ -6,7 +6,7 @@ import 'package:meta/meta.dart';
 import 'package:path/path.dart' as p;
 import 'package:platform/platform.dart';
 import 'package:shorebird_cli/src/archive/directory_archive.dart';
-import 'package:shorebird_cli/src/archive_analysis/ios_archive_differ.dart';
+import 'package:shorebird_cli/src/archive_analysis/apple_archive_differ.dart';
 import 'package:shorebird_cli/src/artifact_builder.dart';
 import 'package:shorebird_cli/src/artifact_manager.dart';
 import 'package:shorebird_cli/src/code_push_client_wrapper.dart';
@@ -96,7 +96,7 @@ class IosFrameworkPatcher extends Patcher {
       patchDiffChecker.confirmUnpatchableDiffsIfNecessary(
         localArchive: patchArchive,
         releaseArchive: releaseArchive,
-        archiveDiffer: const IosArchiveDiffer(),
+        archiveDiffer: const AppleArchiveDiffer(),
         allowAssetChanges: allowAssetDiffs,
         allowNativeChanges: allowNativeDiffs,
       );

packages/shorebird_cli/lib/src/commands/patch/ios_patcher.dart

@@ -122,7 +122,7 @@ class IosPatcher extends Patcher {
         await patchDiffChecker.confirmUnpatchableDiffsIfNecessary(
       localArchive: patchArchive,
       releaseArchive: releaseArchive,
-      archiveDiffer: const IosArchiveDiffer(),
+      archiveDiffer: const AppleArchiveDiffer(),
       allowAssetChanges: allowAssetDiffs,
       allowNativeChanges: allowNativeDiffs,
       confirmNativeChanges: false,

packages/shorebird_cli/lib/src/commands/patch/macos_patcher.dart

@@ -6,6 +6,7 @@
 import 'package:path/path.dart' as p;
 import 'package:platform/platform.dart';
 import 'package:shorebird_cli/src/archive/directory_archive.dart';
+import 'package:shorebird_cli/src/archive_analysis/apple_archive_differ.dart';
 import 'package:shorebird_cli/src/archive_analysis/plist.dart';
 import 'package:shorebird_cli/src/artifact_builder.dart';
 import 'package:shorebird_cli/src/artifact_manager.dart';
@@ -24,6 +25,7 @@
 import 'package:shorebird_cli/src/release_type.dart';
 import 'package:shorebird_cli/src/shorebird_artifacts.dart';
 import 'package:shorebird_cli/src/shorebird_documentation.dart';
+import 'package:shorebird_cli/src/shorebird_env.dart';
 import 'package:shorebird_cli/src/shorebird_flutter.dart';
 import 'package:shorebird_cli/src/shorebird_validator.dart';
 import 'package:shorebird_cli/src/third_party/flutter_tools/lib/flutter_tools.dart';
@@ -106,14 +108,61 @@
     }
   }
 
+  // FIXME: this is a direct copy of IosPatcher's implementation. We should
+  // consolidate this and other copied code.
   @override
   Future<DiffStatus> assertUnpatchableDiffs({
     required ReleaseArtifact releaseArtifact,
     required File releaseArchive,
     required File patchArchive,
   }) async {
-    // TODO(bryanoltman): implement assertUnpatchableDiffs
-    return const DiffStatus(hasAssetChanges: false, hasNativeChanges: false);
+    // Check for diffs without warning about native changes, as Xcode builds
+    // can be nondeterministic. So we still have some hope of alerting users of
+    // unpatchable native changes, we compare the Podfile.lock hash between the
+    // patch and the release.
+    final diffStatus =
+        await patchDiffChecker.confirmUnpatchableDiffsIfNecessary(
+      localArchive: patchArchive,
+      releaseArchive: releaseArchive,
+      archiveDiffer: const AppleArchiveDiffer(),
+      allowAssetChanges: allowAssetDiffs,
+      allowNativeChanges: allowNativeDiffs,
+      confirmNativeChanges: false,
+    );
+
+    if (!diffStatus.hasNativeChanges) {
+      return diffStatus;
+    }
+
+    final String? podfileLockHash;
+    if (shorebirdEnv.macosPodfileLockFile.existsSync()) {
+      podfileLockHash = sha256
+          .convert(shorebirdEnv.macosPodfileLockFile.readAsBytesSync())
+          .toString();
+    } else {
+      podfileLockHash = null;
+    }
+
+    if (releaseArtifact.podfileLockHash != null &&
+        podfileLockHash != releaseArtifact.podfileLockHash) {
+      logger.warn(
+        '''
+Your macos/Podfile.lock is different from the one used to build the release.
+This may indicate that the patch contains native changes, which cannot be applied with a patch. Proceeding may result in unexpected behavior or crashes.''',
+      );
+
+      if (!allowNativeDiffs) {
+        if (!shorebirdEnv.canAcceptUserInput) {
+          throw UnpatchableChangeException();
+        }
+
+        if (!logger.confirm('Continue anyways?')) {
+          throw UserCancelledException();
+        }
+      }
+    }
+
+    return diffStatus;
   }
 
   @override
@@ -189,7 +238,9 @@
     await ditto.archive(
       source: appPath,
       destination: zippedApp.path,
-      keepParent: true,
+      // keepParent is false here in order to ensure the directory structure of
+      // this zip file matches what will be provided to the [AppleArchiveDiffer]
+      // (which uses package:archive to stream zip file contents).
     );
     return zippedApp;
   }

packages/shorebird_cli/test/src/archive_analysis/ios_archive_differ_test.dart → packages/shorebird_cli/test/src/archive_analysis/apple_archive_differ_test.dart

@@ -40,18 +40,18 @@ void main() {
       p.join(xcframeworkFixturesBasePath, 'changed_dart.xcframework.zip');
 
   group(
-    IosArchiveDiffer,
+    AppleArchiveDiffer,
     () {
-      late IosArchiveDiffer differ;
+      late AppleArchiveDiffer differ;
 
       setUp(() {
-        differ = const IosArchiveDiffer();
+        differ = const AppleArchiveDiffer();
       });
 
       group('appRegex', () {
         test('identifies Runner.app/Runner as an app file', () {
           expect(
-            IosArchiveDiffer.appRegex.hasMatch(
+            AppleArchiveDiffer.xcFrameworkAppRegex.hasMatch(
               'Products/Applications/Runner.app/Runner',
             ),
             isTrue,
@@ -60,7 +60,7 @@ void main() {
 
         test('does not identify Runner.app/Assets.car as an app file', () {
           expect(
-            IosArchiveDiffer.appRegex.hasMatch(
+            AppleArchiveDiffer.xcFrameworkAppRegex.hasMatch(
               'Products/Applications/Runner.app/Assets.car',
             ),
             isFalse,

packages/shorebird_cli/test/src/commands/patch/ios_framework_patcher_test.dart

@@ -6,7 +6,7 @@ import 'package:mocktail/mocktail.dart';
 import 'package:path/path.dart' as p;
 import 'package:platform/platform.dart';
 import 'package:scoped_deps/scoped_deps.dart';
-import 'package:shorebird_cli/src/archive_analysis/ios_archive_differ.dart';
+import 'package:shorebird_cli/src/archive_analysis/apple_archive_differ.dart';
 import 'package:shorebird_cli/src/artifact_builder.dart';
 import 'package:shorebird_cli/src/artifact_manager.dart';
 import 'package:shorebird_cli/src/code_push_client_wrapper.dart';
@@ -90,7 +90,7 @@ void main() {
       setUpAll(() {
         registerFallbackValue(Directory(''));
         registerFallbackValue(File(''));
-        registerFallbackValue(const IosArchiveDiffer());
+        registerFallbackValue(const AppleArchiveDiffer());
         registerFallbackValue(ReleasePlatform.ios);
         registerFallbackValue(ShorebirdArtifact.genSnapshotIos);
         registerFallbackValue(Uri.parse('https://example.com'));

packages/shorebird_cli/test/src/commands/patch/ios_patcher_test.dart

@@ -9,7 +9,7 @@ import 'package:path/path.dart' as p;
 import 'package:platform/platform.dart';
 import 'package:pub_semver/pub_semver.dart';
 import 'package:scoped_deps/scoped_deps.dart';
-import 'package:shorebird_cli/src/archive_analysis/ios_archive_differ.dart';
+import 'package:shorebird_cli/src/archive_analysis/apple_archive_differ.dart';
 import 'package:shorebird_cli/src/artifact_builder.dart';
 import 'package:shorebird_cli/src/artifact_manager.dart';
 import 'package:shorebird_cli/src/code_push_client_wrapper.dart';
@@ -101,7 +101,7 @@ void main() {
         registerFallbackValue(FakeArgResults());
         registerFallbackValue(Directory(''));
         registerFallbackValue(File(''));
-        registerFallbackValue(const IosArchiveDiffer());
+        registerFallbackValue(const AppleArchiveDiffer());
         registerFallbackValue(ReleasePlatform.ios);
         registerFallbackValue(ShorebirdArtifact.genSnapshotIos);
         registerFallbackValue(Uri.parse('https://example.com'));