feat(shorebird_cli): alert user of non-patchable changes (#538)

packages/shorebird_cli/lib/src/aab/aab.dart

@@ -0,0 +1 @@
+export 'aab_differ.dart';

packages/shorebird_cli/lib/src/aab/aab_differ.dart

@@ -0,0 +1,99 @@
+import 'dart:convert';
+import 'dart:io';
+
+import 'package:archive/archive_io.dart';
+import 'package:path/path.dart' as p;
+import 'package:shorebird_cli/src/aab/mf_reader.dart';
+
+/// Types of code changes that we care about.
+enum AabDifferences {
+  dart,
+  native,
+  assets,
+}
+
+/// Finds differences between two AABs.
+///
+/// Types of changes we care about:
+///   - Dart code changes
+///      - libapp.so will be different
+///   - Java/Kotlin code changes
+///      - .dex files will be different
+///   - Assets
+///      - **/assets/** will be different
+///      - AssetManifest.json will have changed if assets have been added or
+///        removed
+///
+/// Changes we don't care about:
+///   - Anything in META-INF
+///   - BUNDLE-METADATA/com.android.tools.build.libraries/dependencies.pb
+///      - This seems to change with every build, regardless of whether any code
+///        or assets were changed.
+///
+/// See https://developer.android.com/guide/app-bundle/app-bundle-format for
+/// reference.
+class AabDiffer {
+  /// Returns a set of file paths whose hashes differ between the AABs at the
+  /// provided paths.
+  Set<String> aabChangedFiles(String aabPath1, String aabPath2) {
+    final mfContents1 = _metaInfMfContent(File(aabPath1));
+    final mfContents2 = _metaInfMfContent(File(aabPath2));
+    final mfEntries1 = MfReader.parse(mfContents1).toSet();
+    final mfEntries2 = MfReader.parse(mfContents2).toSet();
+    return mfEntries1.difference(mfEntries2).map((entry) => entry.name).toSet();
+  }
+
+  /// Returns a set of difference types detected between the aabs at [aabPath1]
+  /// and [aabPath2].
+  Set<AabDifferences> aabContentDifferences(String aabPath1, String aabPath2) {
+    final fileDifferences = aabChangedFiles(aabPath1, aabPath2);
+
+    final differences = <AabDifferences>{};
+    if (_hasAssetChanges(fileDifferences)) {
+      differences.add(AabDifferences.assets);
+    }
+    if (_hasDartChanges(fileDifferences)) {
+      differences.add(AabDifferences.dart);
+    }
+    if (_hasNativeChanges(fileDifferences)) {
+      differences.add(AabDifferences.native);
+    }
+
+    return differences;
+  }
+
+  /// Reads the contents of META-INF/MANIFEST.MF from an AAB.
+  ///
+  /// This file contains a list of file paths and their SHA-256 hashes.
+  String _metaInfMfContent(File aab) {
+    final inputStream = InputFileStream(aab.path);
+    final archive = ZipDecoder().decodeBuffer(inputStream);
+    return utf8.decode(
+      archive.files
+          .firstWhere((file) => file.name == p.join('META-INF', 'MANIFEST.MF'))
+          .content as List<int>,
+    );
+  }
+
+  /// Whether any changed files correspond to a change in assets.
+  bool _hasAssetChanges(Set<String> paths) {
+    const assetDirNames = ['assets', 'res'];
+    const assetFileNames = ['AssetManifest.json'];
+    return paths.any(
+      (path) =>
+          p.split(path).any((component) => assetDirNames.contains(component)) ||
+          assetFileNames.contains(p.basename(path)),
+    );
+  }
+
+  /// Whether any changed files correspond to a change in Dart code.
+  bool _hasDartChanges(Set<String> paths) {
+    const dartFileNames = ['libapp.so', 'libflutter.so'];
+    return paths.any((path) => dartFileNames.contains(p.basename(path)));
+  }
+
+  /// Whether changed files correspond to a change in Java or Kotlin code.
+  bool _hasNativeChanges(Set<String> path) {
+    return path.any((path) => p.extension(path) == '.dex');
+  }
+}

packages/shorebird_cli/lib/src/aab/mf_reader.dart

@@ -0,0 +1,78 @@
+import 'dart:io';
+
+import 'package:meta/meta.dart';
+
+/// {@template mf_entry}
+/// A single entry from an .MF file.
+/// {@endtemplate}
+@immutable
+class MfEntry {
+  /// {@macro mf_entry}
+  const MfEntry({
+    required this.name,
+    required this.sha256Digest,
+  });
+
+  /// Contents of the `Name` field.
+  final String name;
+
+  /// Contents of the `SHA-256-Digest` field.
+  final String sha256Digest;
+
+  @override
+  String toString() => 'MfEntry(name: $name, sha256Digest: $sha256Digest)';
+
+  @override
+  bool operator ==(Object other) =>
+      identical(this, other) ||
+      other is MfEntry &&
+          runtimeType == other.runtimeType &&
+          name == other.name &&
+          sha256Digest == other.sha256Digest;
+
+  @override
+  int get hashCode => Object.hashAll([name, sha256Digest]);
+}
+
+/// Parses a .MF file into a list of [MfEntry]s.
+class MfReader {
+  static final nameRegex = RegExp(r'^Name: (.+)$');
+  static final nameContinuedRegex = RegExp(r'^ (.+)$');
+  static final shaDigestRegex = RegExp(r'^SHA-256-Digest: (.+)$');
+
+  /// Parses the content of [mfFile] into a list of [MfEntry]s.
+  ///
+  /// [mfFile] should be a JAR manifest file, as described in
+  /// https://docs.oracle.com/javase/tutorial/deployment/jar/manifestindex.html.
+  static List<MfEntry> read(File mfFile) => parse(mfFile.readAsStringSync());
+
+  /// Parses the contents [mfContents] file into a list of [MfEntry]s.
+  ///
+  /// [mfContents] should be a JAR manifest file, as described in
+  /// https://docs.oracle.com/javase/tutorial/deployment/jar/manifestindex.html.
+  static List<MfEntry> parse(String mfContents) {
+    final lines = mfContents.split('\n').map((line) => line.trimRight());
+    final entries = <MfEntry>[];
+    var currentHash = '';
+    var currentName = '';
+    for (final line in lines) {
+      if (line.isEmpty && currentName.isNotEmpty && currentHash.isNotEmpty) {
+        entries.add(MfEntry(name: currentName, sha256Digest: currentHash));
+        currentHash = '';
+        currentName = '';
+      } else if (nameRegex.hasMatch(line)) {
+        currentName = nameRegex.firstMatch(line)!.group(1)!;
+      } else if (nameContinuedRegex.hasMatch(line)) {
+        currentName += nameContinuedRegex.firstMatch(line)!.group(1)!;
+      } else if (shaDigestRegex.hasMatch(line)) {
+        currentHash = shaDigestRegex.firstMatch(line)!.group(1)!;
+      }
+    }
+
+    if (currentName.isNotEmpty && currentHash.isNotEmpty) {
+      entries.add(MfEntry(name: currentName, sha256Digest: currentHash));
+    }
+
+    return entries;
+  }
+}

packages/shorebird_cli/lib/src/commands/patch_command.dart

@@ -5,6 +5,7 @@ import 'package:crypto/crypto.dart';
 import 'package:http/http.dart' as http;
 import 'package:mason_logger/mason_logger.dart';
 import 'package:path/path.dart' as p;
+import 'package:shorebird_cli/src/aab/aab.dart';
 import 'package:shorebird_cli/src/auth_logger_mixin.dart';
 import 'package:shorebird_cli/src/command.dart';
 import 'package:shorebird_cli/src/config/shorebird_yaml.dart';
@@ -58,7 +59,9 @@ class PatchCommand extends ShorebirdCommand
     super.validators,
     HashFunction? hashFn,
     http.Client? httpClient,
-  })  : _hashFn = hashFn ?? ((m) => sha256.convert(m).toString()),
+    AabDiffer? aabDiffer,
+  })  : _aabDiffer = aabDiffer ?? AabDiffer(),
+        _hashFn = hashFn ?? ((m) => sha256.convert(m).toString()),
         _httpClient = httpClient ?? http.Client() {
     argParser
       ..addOption(
@@ -111,6 +114,7 @@ class PatchCommand extends ShorebirdCommand
   @override
   String get name => 'patch';
 
+  final AabDiffer _aabDiffer;
   final HashFunction _hashFn;
   final http.Client _httpClient;
 
@@ -293,6 +297,20 @@ https://github.com/shorebirdtech/shorebird/issues/472
         return ExitCode.software.code;
       }
     }
+
+    ReleaseArtifact? releaseAabArtifact;
+    try {
+      releaseAabArtifact = await codePushClient.getReleaseArtifact(
+        releaseId: release.id,
+        arch: 'aab',
+        platform: 'android',
+      );
+    } catch (error) {
+      // Do nothing for now, not all releases will have an associated aab
+      // artifact.
+      // TODO(bryanoltman): Treat this as an error once all releases have an aab
+    }
+
     fetchReleaseArtifactProgress.complete();
 
     final releaseArtifactPaths = <Arch, String>{};
@@ -310,8 +328,58 @@ https://github.com/shorebirdtech/shorebird/issues/472
         return ExitCode.software.code;
       }
     }
+
+    String? releaseAabPath;
+    try {
+      if (releaseAabArtifact != null) {
+        releaseAabPath = await _downloadReleaseArtifact(
+          Uri.parse(releaseAabArtifact.url),
+        );
+      }
+    } catch (error) {
+      downloadReleaseArtifactProgress.fail('$error');
+      return ExitCode.software.code;
+    }
+
     downloadReleaseArtifactProgress.complete();
 
+    final contentDiffs = releaseAabPath == null
+        ? <AabDifferences>{}
+        : _aabDiffer.aabContentDifferences(
+            releaseAabPath,
+            bundlePath,
+          );
+
+    logger.detail('content diffs detected: $contentDiffs');
+
+    if (contentDiffs.contains(AabDifferences.native)) {
+      logger
+        ..err(
+          '''The Android App Bundle appears to contain Kotlin or Java changes, which cannot be applied via a patch.''',
+        )
+        ..info(
+          yellow.wrap(
+            '''
+Please create a new release or revert those changes to create a patch.
+
+If you believe you're seeing this in error, please reach out to us for support at https://shorebird.dev/support''',
+          ),
+        );
+      return ExitCode.software.code;
+    }
+
+    if (contentDiffs.contains(AabDifferences.assets)) {
+      logger.info(
+        yellow.wrap(
+          '''⚠️ The Android App Bundle contains asset changes, which will not be included in the patch.''',
+        ),
+      );
+      final shouldContinue = logger.confirm('Continue anyways?');
+      if (!shouldContinue) {
+        return ExitCode.success.code;
+      }
+    }
+
     final patchArtifactBundles = <Arch, PatchArtifactBundle>{};
     final createDiffProgress = logger.progress('Creating artifacts');
     final sizes = <Arch, int>{};

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

@@ -0,0 +1,10 @@
+The aab files in this folder were generated by building our time_shift app.
+
+Some of their contents has been removed to reduce the size of the files. Most notably, all libflutter.so and libapp.so files have been removed.
+
+Files: 
+- base.aab is meant to represent an aab uploaded as part of a release.
+- changed_dart.aab was built from the same code as base.aab with only Dart changes (i.e., no asset or Java/Kotlin changes)
+- changed_kotlin.aab was built from the same code as base.aab with only Kotlin changes (i.e., no asset or Dart changes)
+- changed_asset.aab was built from the same code as base.aab with only asset changes (i.e., no Dart or Java/Kotlin changes)
+- changed_dart_and_asset.aab was built from the same code as base.aab with only Dart and asset changes (i.e., no Java/Kotlin changes)

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

@@ -0,0 +1,78 @@
+import 'package:path/path.dart' as p;
+import 'package:shorebird_cli/src/aab/aab.dart';
+import 'package:test/test.dart';
+
+void main() {
+  group(AabDiffer, () {
+    final aabFixturesBasePath = p.join('test', 'fixtures', 'aabs');
+    final baseAabPath = p.join(aabFixturesBasePath, 'base.aab');
+    final changedAssetAabPath =
+        p.join(aabFixturesBasePath, 'changed_asset.aab');
+    final changedDartAabPath = p.join(aabFixturesBasePath, 'changed_dart.aab');
+    final changedKotlinAabPath =
+        p.join(aabFixturesBasePath, 'changed_kotlin.aab');
+    final changedDartAndAssetAabPath =
+        p.join(aabFixturesBasePath, 'changed_dart_and_asset.aab');
+
+    late AabDiffer differ;
+
+    setUp(() {
+      differ = AabDiffer();
+    });
+
+    group('aabFileDifferences', () {
+      test('finds no differences between the same aab', () {
+        expect(differ.aabChangedFiles(baseAabPath, baseAabPath), isEmpty);
+      });
+
+      test('finds differences between the two different aabs', () {
+        expect(
+          differ.aabChangedFiles(baseAabPath, changedDartAabPath).toSet(),
+          {
+            'BUNDLE-METADATA/com.android.tools.build.libraries/dependencies.pb',
+            'base/lib/arm64-v8a/libapp.so',
+            'base/lib/armeabi-v7a/libapp.so',
+            'base/lib/x86_64/libapp.so',
+          },
+        );
+      });
+    });
+
+    group('aabContentDifferences', () {
+      test('detects no differences between the same aab', () {
+        expect(differ.aabContentDifferences(baseAabPath, baseAabPath), isEmpty);
+      });
+
+      test('detects asset changes', () {
+        expect(
+          differ.aabContentDifferences(baseAabPath, changedAssetAabPath),
+          {AabDifferences.assets},
+        );
+      });
+
+      test('detects kotlin changes', () {
+        expect(
+          differ.aabContentDifferences(baseAabPath, changedKotlinAabPath),
+          {AabDifferences.native},
+        );
+      });
+
+      test('detects dart changes', () {
+        expect(
+          differ.aabContentDifferences(baseAabPath, changedDartAabPath),
+          {AabDifferences.dart},
+        );
+      });
+
+      test('detects dart and asset changes', () {
+        expect(
+          differ.aabContentDifferences(baseAabPath, changedDartAndAssetAabPath),
+          {
+            AabDifferences.assets,
+            AabDifferences.dart,
+          },
+        );
+      });
+    });
+  });
+}