Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 13 commits into from
May 24, 2023
1 change: 1 addition & 0 deletions packages/shorebird_cli/lib/src/aab/aab.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export 'aab_differ.dart';
99 changes: 99 additions & 0 deletions packages/shorebird_cli/lib/src/aab/aab_differ.dart
Original file line number Diff line number Diff line change
@@ -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');
}
}
78 changes: 78 additions & 0 deletions packages/shorebird_cli/lib/src/aab/mf_reader.dart
Original file line number Diff line number Diff line change
@@ -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;
}
}
70 changes: 69 additions & 1 deletion packages/shorebird_cli/lib/src/commands/patch_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -111,6 +114,7 @@ class PatchCommand extends ShorebirdCommand
@override
String get name => 'patch';

final AabDiffer _aabDiffer;
final HashFunction _hashFn;
final http.Client _httpClient;

Expand Down Expand Up @@ -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
bryanoltman marked this conversation as resolved.
Show resolved Hide resolved
// artifact.
// TODO(bryanoltman): Treat this as an error once all releases have an aab
}

fetchReleaseArtifactProgress.complete();

final releaseArtifactPaths = <Arch, String>{};
Expand All @@ -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>{};
Expand Down
10 changes: 10 additions & 0 deletions packages/shorebird_cli/test/fixtures/aabs/README.md
Original file line number Diff line number Diff line change
@@ -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)
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
Binary file not shown.
78 changes: 78 additions & 0 deletions packages/shorebird_cli/test/src/aab/aab_differ_test.dart
Original file line number Diff line number Diff line change
@@ -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,
},
);
});
});
});
}
Loading