diff --git a/packages/shorebird_cli/lib/src/aab/aab.dart b/packages/shorebird_cli/lib/src/aab/aab.dart deleted file mode 100644 index 29423e7b..00000000 --- a/packages/shorebird_cli/lib/src/aab/aab.dart +++ /dev/null @@ -1 +0,0 @@ -export 'aab_differ.dart'; diff --git a/packages/shorebird_cli/lib/src/aab/aab_differ.dart b/packages/shorebird_cli/lib/src/aab/aab_differ.dart deleted file mode 100644 index 8d473c69..00000000 --- a/packages/shorebird_cli/lib/src/aab/aab_differ.dart +++ /dev/null @@ -1,99 +0,0 @@ -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 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 aabContentDifferences(String aabPath1, String aabPath2) { - final fileDifferences = aabChangedFiles(aabPath1, aabPath2); - - final differences = {}; - 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 == 'META-INF/MANIFEST.MF') - .content as List, - ); - } - - /// Whether any changed files correspond to a change in assets. - bool _hasAssetChanges(Set 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 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 path) { - return path.any((path) => p.extension(path) == '.dex'); - } -} diff --git a/packages/shorebird_cli/lib/src/archive_analysis/aab_differ.dart b/packages/shorebird_cli/lib/src/archive_analysis/aab_differ.dart new file mode 100644 index 00000000..e14ac6e6 --- /dev/null +++ b/packages/shorebird_cli/lib/src/archive_analysis/aab_differ.dart @@ -0,0 +1,52 @@ +import 'dart:convert'; +import 'dart:io'; + +import 'package:archive/archive_io.dart'; +import 'package:shorebird_cli/src/archive_analysis/android_archive_differ.dart'; +import 'package:shorebird_cli/src/archive_analysis/mf_reader.dart'; + +/// 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 extends AndroidArchiveDiffer { + /// Returns a set of file paths whose hashes differ between the AABs at the + /// provided paths. + @override + Set changedFiles(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(); + } + + /// 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 == 'META-INF/MANIFEST.MF') + .content as List, + ); + } +} diff --git a/packages/shorebird_cli/lib/src/archive_analysis/aar_differ.dart b/packages/shorebird_cli/lib/src/archive_analysis/aar_differ.dart new file mode 100644 index 00000000..e81c8ace --- /dev/null +++ b/packages/shorebird_cli/lib/src/archive_analysis/aar_differ.dart @@ -0,0 +1,37 @@ +import 'dart:io'; + +import 'package:archive/archive_io.dart'; +import 'package:crypto/crypto.dart'; +import 'package:shorebird_cli/src/archive_analysis/android_archive_differ.dart'; + +/// Finds differences between two AABs. +/// +/// Types of changes we care about: +/// - Dart code changes +/// - libapp.so will be different +/// - Assets +/// - **/assets/** will be different +/// - AssetManifest.json will have changed if assets have been added or +/// removed +/// +/// See +/// https://developer.android.com/studio/projects/android-library.html#aar-contents +/// for reference. Note that .aars produced by Flutter modules do not contain +/// .jar files, so only asset and dart changes are possible. +class AarDiffer extends AndroidArchiveDiffer { + String _hash(List bytes) => sha256.convert(bytes).toString(); + + @override + Set changedFiles(String archivePath1, String archivePath2) => + _fileHashes(File(archivePath1)) + .difference(_fileHashes(File(archivePath2))) + .map((pair) => pair.$1) + .toSet(); + + Set<(String, String)> _fileHashes(File aar) => ZipDecoder() + .decodeBuffer(InputFileStream(aar.path)) + .files + .where((file) => file.isFile) + .map((file) => (file.name, _hash(file.content as List))) + .toSet(); +} diff --git a/packages/shorebird_cli/lib/src/archive_analysis/android_archive_differ.dart b/packages/shorebird_cli/lib/src/archive_analysis/android_archive_differ.dart new file mode 100644 index 00000000..a5931443 --- /dev/null +++ b/packages/shorebird_cli/lib/src/archive_analysis/android_archive_differ.dart @@ -0,0 +1,49 @@ +import 'package:path/path.dart' as p; +import 'package:shorebird_cli/src/archive_analysis/archive_analysis.dart'; + +abstract class AndroidArchiveDiffer { + /// Files that have changed between the archives at the two provided paths. + Set changedFiles(String archivePath1, String archivePath2); + + /// Whether any changed files correspond to a change in assets. + bool hasAssetChanges(Set 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 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 path) { + return path.any((path) => p.extension(path) == '.dex'); + } + + /// The types of differences between the archives at the two provided paths. + Set contentDifferences( + String archivePath1, + String archivePath2, + ) { + final changedFilePaths = changedFiles(archivePath1, archivePath2); + final differences = {}; + if (hasDartChanges(changedFilePaths)) { + differences.add(ArchiveDifferences.dart); + } + if (hasAssetChanges(changedFilePaths)) { + differences.add(ArchiveDifferences.assets); + } + if (hasNativeChanges(changedFilePaths)) { + differences.add(ArchiveDifferences.native); + } + + return differences; + } +} diff --git a/packages/shorebird_cli/lib/src/archive_analysis/archive_analysis.dart b/packages/shorebird_cli/lib/src/archive_analysis/archive_analysis.dart new file mode 100644 index 00000000..5e126933 --- /dev/null +++ b/packages/shorebird_cli/lib/src/archive_analysis/archive_analysis.dart @@ -0,0 +1,3 @@ +export 'aab_differ.dart'; +export 'aar_differ.dart'; +export 'archive_differences.dart'; diff --git a/packages/shorebird_cli/lib/src/archive_analysis/archive_differences.dart b/packages/shorebird_cli/lib/src/archive_analysis/archive_differences.dart new file mode 100644 index 00000000..431ca767 --- /dev/null +++ b/packages/shorebird_cli/lib/src/archive_analysis/archive_differences.dart @@ -0,0 +1,6 @@ +/// Types of code changes that we care about. +enum ArchiveDifferences { + dart, + native, + assets, +} diff --git a/packages/shorebird_cli/lib/src/aab/mf_reader.dart b/packages/shorebird_cli/lib/src/archive_analysis/mf_reader.dart similarity index 100% rename from packages/shorebird_cli/lib/src/aab/mf_reader.dart rename to packages/shorebird_cli/lib/src/archive_analysis/mf_reader.dart diff --git a/packages/shorebird_cli/lib/src/commands/patch/patch_android_command.dart b/packages/shorebird_cli/lib/src/commands/patch/patch_android_command.dart index efb6f08a..e7e1274e 100644 --- a/packages/shorebird_cli/lib/src/commands/patch/patch_android_command.dart +++ b/packages/shorebird_cli/lib/src/commands/patch/patch_android_command.dart @@ -5,7 +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/archive_analysis/archive_analysis.dart'; import 'package:shorebird_cli/src/command.dart'; import 'package:shorebird_cli/src/config/shorebird_yaml.dart'; import 'package:shorebird_cli/src/formatters/formatters.dart'; @@ -328,15 +328,15 @@ https://github.com/shorebirdtech/shorebird/issues/472 downloadReleaseArtifactProgress.complete(); final contentDiffs = releaseAabPath == null - ? {} - : _aabDiffer.aabContentDifferences( + ? {} + : _aabDiffer.contentDifferences( releaseAabPath, bundlePath, ); logger.detail('aab content differences: $contentDiffs'); - if (contentDiffs.contains(AabDifferences.native)) { + if (contentDiffs.contains(ArchiveDifferences.native)) { logger ..err( '''The Android App Bundle appears to contain Kotlin or Java changes, which cannot be applied via a patch.''', @@ -352,7 +352,7 @@ If you believe you're seeing this in error, please reach out to us for support a return ExitCode.software.code; } - if (contentDiffs.contains(AabDifferences.assets)) { + if (contentDiffs.contains(ArchiveDifferences.assets)) { logger.info( yellow.wrap( '''⚠️ The Android App Bundle contains asset changes, which will not be included in the patch.''', diff --git a/packages/shorebird_cli/test/fixtures/aars/README.md b/packages/shorebird_cli/test/fixtures/aars/README.md new file mode 100644 index 00000000..6b2b2684 --- /dev/null +++ b/packages/shorebird_cli/test/fixtures/aars/README.md @@ -0,0 +1,10 @@ +The aab files in this folder were generated by building a flutter module with +`flutter build aar --no-debug --no-profile`. + +Some of their contents has been removed to reduce the size of the files. Most notably, x86_64 and armeabi-v7a libapp.so files have been removed. + +Files: +- base.aar is meant to represent an aar uploaded as part of a release. +- changed_dart.aar was built from the same code as base.aar with only Dart changes (i.e., no asset changes) +- changed_asset.aar was built from the same code as base.aar with only asset changes (i.e., no Dart changes) +- changed_dart_and_asset.aar was built from the same code as base.aar with both Dart and asset changes diff --git a/packages/shorebird_cli/test/fixtures/aars/base.aar b/packages/shorebird_cli/test/fixtures/aars/base.aar new file mode 100644 index 00000000..b9254e8a Binary files /dev/null and b/packages/shorebird_cli/test/fixtures/aars/base.aar differ diff --git a/packages/shorebird_cli/test/fixtures/aars/changed_asset.aar b/packages/shorebird_cli/test/fixtures/aars/changed_asset.aar new file mode 100644 index 00000000..8d22234c Binary files /dev/null and b/packages/shorebird_cli/test/fixtures/aars/changed_asset.aar differ diff --git a/packages/shorebird_cli/test/fixtures/aars/changed_dart.aar b/packages/shorebird_cli/test/fixtures/aars/changed_dart.aar new file mode 100644 index 00000000..9017a680 Binary files /dev/null and b/packages/shorebird_cli/test/fixtures/aars/changed_dart.aar differ diff --git a/packages/shorebird_cli/test/fixtures/aars/changed_dart_and_asset.aar b/packages/shorebird_cli/test/fixtures/aars/changed_dart_and_asset.aar new file mode 100644 index 00000000..e4eba97a Binary files /dev/null and b/packages/shorebird_cli/test/fixtures/aars/changed_dart_and_asset.aar differ diff --git a/packages/shorebird_cli/test/src/aab/aab_differ_test.dart b/packages/shorebird_cli/test/src/archive_analysis/aab_differ_test.dart similarity index 61% rename from packages/shorebird_cli/test/src/aab/aab_differ_test.dart rename to packages/shorebird_cli/test/src/archive_analysis/aab_differ_test.dart index 9ca13e4f..33b17ea2 100644 --- a/packages/shorebird_cli/test/src/aab/aab_differ_test.dart +++ b/packages/shorebird_cli/test/src/archive_analysis/aab_differ_test.dart @@ -1,5 +1,5 @@ import 'package:path/path.dart' as p; -import 'package:shorebird_cli/src/aab/aab.dart'; +import 'package:shorebird_cli/src/archive_analysis/archive_analysis.dart'; import 'package:test/test.dart'; void main() { @@ -20,14 +20,14 @@ void main() { differ = AabDiffer(); }); - group('aabFileDifferences', () { + group('changedFiles', () { test('finds no differences between the same aab', () { - expect(differ.aabChangedFiles(baseAabPath, baseAabPath), isEmpty); + expect(differ.changedFiles(baseAabPath, baseAabPath), isEmpty); }); - test('finds differences between the two different aabs', () { + test('finds differences between two different aabs', () { expect( - differ.aabChangedFiles(baseAabPath, changedDartAabPath).toSet(), + differ.changedFiles(baseAabPath, changedDartAabPath).toSet(), { 'BUNDLE-METADATA/com.android.tools.build.libraries/dependencies.pb', 'base/lib/arm64-v8a/libapp.so', @@ -38,38 +38,38 @@ void main() { }); }); - group('aabContentDifferences', () { + group('contentDifferences', () { test('detects no differences between the same aab', () { - expect(differ.aabContentDifferences(baseAabPath, baseAabPath), isEmpty); + expect(differ.contentDifferences(baseAabPath, baseAabPath), isEmpty); }); test('detects asset changes', () { expect( - differ.aabContentDifferences(baseAabPath, changedAssetAabPath), - {AabDifferences.assets}, + differ.contentDifferences(baseAabPath, changedAssetAabPath), + {ArchiveDifferences.assets}, ); }); test('detects kotlin changes', () { expect( - differ.aabContentDifferences(baseAabPath, changedKotlinAabPath), - {AabDifferences.native}, + differ.contentDifferences(baseAabPath, changedKotlinAabPath), + {ArchiveDifferences.native}, ); }); test('detects dart changes', () { expect( - differ.aabContentDifferences(baseAabPath, changedDartAabPath), - {AabDifferences.dart}, + differ.contentDifferences(baseAabPath, changedDartAabPath), + {ArchiveDifferences.dart}, ); }); test('detects dart and asset changes', () { expect( - differ.aabContentDifferences(baseAabPath, changedDartAndAssetAabPath), + differ.contentDifferences(baseAabPath, changedDartAndAssetAabPath), { - AabDifferences.assets, - AabDifferences.dart, + ArchiveDifferences.assets, + ArchiveDifferences.dart, }, ); }); diff --git a/packages/shorebird_cli/test/src/archive_analysis/aar_differ_test.dart b/packages/shorebird_cli/test/src/archive_analysis/aar_differ_test.dart new file mode 100644 index 00000000..efc1c4a0 --- /dev/null +++ b/packages/shorebird_cli/test/src/archive_analysis/aar_differ_test.dart @@ -0,0 +1,61 @@ +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 aarFixturesBasePath = p.join('test', 'fixtures', 'aars'); + final baseAarPath = p.join(aarFixturesBasePath, 'base.aar'); + final changedAssetAarPath = p.join(aarFixturesBasePath, 'changed_asset.aar'); + final changedDartAarPath = p.join(aarFixturesBasePath, 'changed_dart.aar'); + final changedDartAndAssetAarPath = + p.join(aarFixturesBasePath, 'changed_dart_and_asset.aar'); + + late AarDiffer differ; + + setUp(() { + differ = AarDiffer(); + }); + + group('changedFiles', () { + test('finds no differences between the same aar', () { + expect(differ.changedFiles(baseAarPath, baseAarPath), isEmpty); + }); + + test('finds differences between two different aars', () { + expect( + differ.changedFiles(baseAarPath, changedDartAarPath).toSet(), + {'jni/arm64-v8a/libapp.so'}, + ); + }); + }); + + group('contentDifferences', () { + test('detects no differences between the same aar', () { + expect(differ.contentDifferences(baseAarPath, baseAarPath), isEmpty); + }); + + test('detects asset changes', () { + expect( + differ.contentDifferences(baseAarPath, changedAssetAarPath), + {ArchiveDifferences.assets}, + ); + }); + + test('detects dart changes', () { + expect( + differ.contentDifferences(baseAarPath, changedDartAarPath), + {ArchiveDifferences.dart}, + ); + }); + + test('detects dart and asset changes', () { + expect( + differ.contentDifferences(baseAarPath, changedDartAndAssetAarPath), + { + ArchiveDifferences.assets, + ArchiveDifferences.dart, + }, + ); + }); + }); +} diff --git a/packages/shorebird_cli/test/src/aab/mf_reader_test.dart b/packages/shorebird_cli/test/src/archive_analysis/mf_reader_test.dart similarity index 97% rename from packages/shorebird_cli/test/src/aab/mf_reader_test.dart rename to packages/shorebird_cli/test/src/archive_analysis/mf_reader_test.dart index 22d9fdc0..5a3c8e71 100644 --- a/packages/shorebird_cli/test/src/aab/mf_reader_test.dart +++ b/packages/shorebird_cli/test/src/archive_analysis/mf_reader_test.dart @@ -1,7 +1,7 @@ import 'dart:io'; import 'package:path/path.dart' as p; -import 'package:shorebird_cli/src/aab/mf_reader.dart'; +import 'package:shorebird_cli/src/archive_analysis/mf_reader.dart'; import 'package:test/test.dart'; void main() { diff --git a/packages/shorebird_cli/test/src/commands/patch/patch_android_command_test.dart b/packages/shorebird_cli/test/src/commands/patch/patch_android_command_test.dart index ffc98a69..db52c4fc 100644 --- a/packages/shorebird_cli/test/src/commands/patch/patch_android_command_test.dart +++ b/packages/shorebird_cli/test/src/commands/patch/patch_android_command_test.dart @@ -6,7 +6,7 @@ import 'package:mason_logger/mason_logger.dart'; import 'package:mocktail/mocktail.dart'; import 'package:path/path.dart' as p; import 'package:platform/platform.dart'; -import 'package:shorebird_cli/src/aab/aab.dart'; +import 'package:shorebird_cli/src/archive_analysis/archive_analysis.dart'; import 'package:shorebird_cli/src/auth/auth.dart'; import 'package:shorebird_cli/src/cache.dart' show Cache; import 'package:shorebird_cli/src/commands/patch/patch_android_command.dart'; @@ -250,7 +250,7 @@ flutter: : releaseVersionNameProcessResult; }); - when(() => aabDiffer.aabContentDifferences(any(), any())).thenReturn({}); + when(() => aabDiffer.contentDifferences(any(), any())).thenReturn({}); when(() => argResults.rest).thenReturn([]); when(() => argResults['arch']).thenReturn(arch); when(() => argResults['channel']).thenReturn(channelName); @@ -661,8 +661,8 @@ Please create a release using "shorebird release" and try again. }); test('throws error when Java/Kotlin code changes are detected', () async { - when(() => aabDiffer.aabContentDifferences(any(), any())).thenReturn( - {AabDifferences.native}, + when(() => aabDiffer.contentDifferences(any(), any())).thenReturn( + {ArchiveDifferences.native}, ); final tempDir = setUpTempDir(); @@ -681,8 +681,8 @@ Please create a release using "shorebird release" and try again. }); test('prompts user to continue when asset changes are detected', () async { - when(() => aabDiffer.aabContentDifferences(any(), any())).thenReturn( - {AabDifferences.assets}, + when(() => aabDiffer.contentDifferences(any(), any())).thenReturn( + {ArchiveDifferences.assets}, ); final tempDir = setUpTempDir(); @@ -708,8 +708,8 @@ Please create a release using "shorebird release" and try again. test( '''does not warn user of asset or code changes if only dart changes are detected''', () async { - when(() => aabDiffer.aabContentDifferences(any(), any())).thenReturn( - {AabDifferences.dart}, + when(() => aabDiffer.contentDifferences(any(), any())).thenReturn( + {ArchiveDifferences.dart}, ); final tempDir = setUpTempDir(); @@ -740,8 +740,8 @@ Please create a release using "shorebird release" and try again. test( '''exits if user decides to not proceed after being warned of non-dart changes''', () async { - when(() => aabDiffer.aabContentDifferences(any(), any())).thenReturn( - {AabDifferences.assets}, + when(() => aabDiffer.contentDifferences(any(), any())).thenReturn( + {ArchiveDifferences.assets}, ); when( () => logger.confirm(any(that: contains('Continue anyways?'))),