feat(shorebird_cli): show Assets.car diff if change is detected in iOS patch (#3451)
This commit is contained in:
@@ -63,6 +63,7 @@ Command: shorebird ${args.join(' ')}
|
||||
codePushClientWrapperRef,
|
||||
codeSignerRef,
|
||||
devicectlRef,
|
||||
diffRef,
|
||||
dittoRef,
|
||||
doctorRef,
|
||||
engineConfigRef,
|
||||
|
||||
@@ -9,6 +9,7 @@ import 'package:crypto/crypto.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';
|
||||
import 'package:shorebird_cli/src/executables/executables.dart';
|
||||
import 'package:shorebird_cli/src/platform/apple/macho.dart';
|
||||
|
||||
/// {@template apple_archive_differ}
|
||||
@@ -85,7 +86,7 @@ class AppleArchiveDiffer extends ArchiveDiffer {
|
||||
}
|
||||
|
||||
for (final file in _carFiles(archivePath)) {
|
||||
pathHashes[file.name] = await _carFileHash(file);
|
||||
pathHashes[file.name] = await _sanitizedCarFileHash(file);
|
||||
}
|
||||
|
||||
return pathHashes;
|
||||
@@ -139,10 +140,11 @@ class AppleArchiveDiffer extends ArchiveDiffer {
|
||||
return _hash(bytes);
|
||||
}
|
||||
|
||||
/// Uses assetutil to write a json description of a .car file to disk and
|
||||
/// diffs the contents of that file, less a timestamp line that changes based
|
||||
/// on when the .car file was created.
|
||||
Future<String> _carFileHash(ArchiveFile file) async {
|
||||
/// Writes a json description of a .car file to a temporary location and
|
||||
/// returns the [File].
|
||||
///
|
||||
/// Equivalent of running `xcrun assetutil --info /path/to/Assets.car > outfile.json`.
|
||||
Future<File> _carJsonFile(ArchiveFile file) async {
|
||||
final tempDir = Directory.systemTemp.createTempSync();
|
||||
final outPath = p.join(tempDir.path, file.name);
|
||||
final outputStream = OutputFileStream(outPath);
|
||||
@@ -155,14 +157,51 @@ class AppleArchiveDiffer extends ArchiveDiffer {
|
||||
Process.runSync('assetutil', ['--info', outPath, '-o', assetInfoPath]);
|
||||
}
|
||||
|
||||
// Remove the timestamp line from the json file
|
||||
final jsonFile = File(assetInfoPath);
|
||||
return File(assetInfoPath);
|
||||
}
|
||||
|
||||
/// Uses assetutil to write a json description of a .car file to disk and
|
||||
/// diffs the contents of that file, less a timestamp line that changes based
|
||||
/// on when the .car file was created.
|
||||
Future<String> _sanitizedCarFileHash(ArchiveFile file) async {
|
||||
final jsonFile = await _carJsonFile(file);
|
||||
final lines = jsonFile.readAsLinesSync();
|
||||
final timestampRegex = RegExp(r'^\W+"Timestamp" : \d+$');
|
||||
final linesToKeep = lines.whereNot(timestampRegex.hasMatch);
|
||||
return _hash(linesToKeep.join('\n').codeUnits);
|
||||
}
|
||||
|
||||
@override
|
||||
Future<String> availableAssetDiffs({
|
||||
required FileSetDiff fileSetDiff,
|
||||
required String oldArchivePath,
|
||||
required String newArchivePath,
|
||||
}) async {
|
||||
final diffs = <String>[];
|
||||
for (final changedPath in fileSetDiff.changedPaths) {
|
||||
if (changedPath.endsWith('.car')) {
|
||||
final oldCarFile = ZipDecoder()
|
||||
.decodeStream(InputFileStream(oldArchivePath))
|
||||
.files
|
||||
.firstWhere((file) => file.name == changedPath);
|
||||
final newCarFile = ZipDecoder()
|
||||
.decodeStream(InputFileStream(newArchivePath))
|
||||
.files
|
||||
.firstWhere((file) => file.name == changedPath);
|
||||
final oldCarJsonFile = await _carJsonFile(oldCarFile);
|
||||
final newCarJsonFile = await _carJsonFile(newCarFile);
|
||||
final diffResult = await diff.run(
|
||||
oldCarJsonFile.path,
|
||||
newCarJsonFile.path,
|
||||
colorMode: DiffColorMode.always,
|
||||
unified: true,
|
||||
);
|
||||
diffs.add(diffResult.stdout as String);
|
||||
}
|
||||
}
|
||||
return diffs.join('\n');
|
||||
}
|
||||
|
||||
@override
|
||||
bool isAssetFilePath(String filePath) {
|
||||
/// The flutter_assets directory contains the assets listed in the assets
|
||||
|
||||
@@ -107,4 +107,13 @@ abstract class ArchiveDiffer {
|
||||
};
|
||||
});
|
||||
}
|
||||
|
||||
/// Prints the diffs of the changed files to the console.
|
||||
Future<String> availableAssetDiffs({
|
||||
required FileSetDiff fileSetDiff,
|
||||
required String oldArchivePath,
|
||||
required String newArchivePath,
|
||||
}) async {
|
||||
return '';
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,41 @@
|
||||
import 'package:scoped_deps/scoped_deps.dart';
|
||||
import 'package:shorebird_cli/src/shorebird_process.dart';
|
||||
|
||||
/// The color mode for the `diff` command.
|
||||
enum DiffColorMode {
|
||||
/// Never color the diff.
|
||||
never,
|
||||
|
||||
/// Always color the diff.
|
||||
always,
|
||||
|
||||
/// Color the diff automatically based on the output device.
|
||||
auto,
|
||||
}
|
||||
|
||||
/// A reference to a [Diff] instance.
|
||||
final diffRef = create(Diff.new);
|
||||
|
||||
/// The [Diff] instance available in the current zone.
|
||||
Diff get diff => read(diffRef);
|
||||
|
||||
/// A wrapper around the `diff` command.
|
||||
class Diff {
|
||||
/// The name of the `diff` executable.
|
||||
static const executable = 'diff';
|
||||
|
||||
/// Runs the `diff` command and returns the result.
|
||||
Future<ShorebirdProcessResult> run(
|
||||
String fileAPath,
|
||||
String fileBPath, {
|
||||
required bool unified,
|
||||
required DiffColorMode colorMode,
|
||||
}) async {
|
||||
return process.run(executable, [
|
||||
if (unified) '--unified',
|
||||
'--color=${colorMode.name}',
|
||||
fileAPath,
|
||||
fileBPath,
|
||||
]);
|
||||
}
|
||||
}
|
||||
@@ -4,6 +4,7 @@ export 'adb.dart';
|
||||
export 'aot_tools.dart';
|
||||
export 'bundletool.dart';
|
||||
export 'devicectl/devicectl.dart';
|
||||
export 'diff.dart';
|
||||
export 'ditto.dart';
|
||||
export 'git.dart';
|
||||
export 'gradlew.dart';
|
||||
|
||||
@@ -109,14 +109,24 @@ If you don't know why you're seeing this error, visit our troubleshooting page a
|
||||
yellow.wrap(
|
||||
archiveDiffer.assetsFileSetDiff(contentDiffs).prettyString,
|
||||
),
|
||||
)
|
||||
..info(
|
||||
yellow.wrap(
|
||||
'''
|
||||
);
|
||||
|
||||
final diffs = await archiveDiffer.availableAssetDiffs(
|
||||
fileSetDiff: contentDiffs,
|
||||
oldArchivePath: releaseArchive.path,
|
||||
newArchivePath: localArchive.path,
|
||||
);
|
||||
if (diffs.isNotEmpty) {
|
||||
logger.info(diffs);
|
||||
}
|
||||
|
||||
logger.info(
|
||||
yellow.wrap(
|
||||
'''
|
||||
|
||||
If you don't know why you're seeing this error, visit our troubleshooting page at ${assetChangesTroubleshootingUrl.toLink()}''',
|
||||
),
|
||||
);
|
||||
),
|
||||
);
|
||||
|
||||
if (!allowAssetChanges) {
|
||||
if (!shorebirdEnv.canAcceptUserInput) {
|
||||
|
||||
BIN
Binary file not shown.
@@ -1,9 +1,15 @@
|
||||
// cspell:words xcarchive xcarchives xcframeworks xcframework
|
||||
import 'package:mocktail/mocktail.dart';
|
||||
import 'package:path/path.dart' as p;
|
||||
import 'package:scoped_deps/scoped_deps.dart';
|
||||
import 'package:shorebird_cli/src/archive_analysis/archive_analysis.dart';
|
||||
import 'package:shorebird_cli/src/executables/executables.dart';
|
||||
import 'package:shorebird_cli/src/platform.dart';
|
||||
import 'package:shorebird_cli/src/shorebird_process.dart';
|
||||
import 'package:test/test.dart';
|
||||
|
||||
import '../mocks.dart';
|
||||
|
||||
void main() {
|
||||
final xcarchiveFixturesBasePath = p.join('test', 'fixtures', 'xcarchives');
|
||||
final baseIpaPath = p.join(xcarchiveFixturesBasePath, 'base.xcarchive.zip');
|
||||
@@ -15,6 +21,10 @@ void main() {
|
||||
xcarchiveFixturesBasePath,
|
||||
'changed_asset.xcarchive.zip',
|
||||
);
|
||||
final changedCarXcarchivePath = p.join(
|
||||
xcarchiveFixturesBasePath,
|
||||
'changed_assets_car.xcarchive.zip',
|
||||
);
|
||||
final changedDartXcarchivePath = p.join(
|
||||
xcarchiveFixturesBasePath,
|
||||
'changed_dart.xcarchive.zip',
|
||||
@@ -43,9 +53,22 @@ void main() {
|
||||
);
|
||||
|
||||
group(AppleArchiveDiffer, () {
|
||||
late Diff diff;
|
||||
late AppleArchiveDiffer differ;
|
||||
|
||||
R runWithOverrides<R>(R Function() body) {
|
||||
return runScoped(
|
||||
body,
|
||||
values: {diffRef.overrideWith(() => diff)},
|
||||
);
|
||||
}
|
||||
|
||||
setUpAll(() {
|
||||
registerFallbackValue(DiffColorMode.always);
|
||||
});
|
||||
|
||||
setUp(() {
|
||||
diff = MockDiff();
|
||||
differ = const AppleArchiveDiffer();
|
||||
});
|
||||
|
||||
@@ -191,6 +214,65 @@ void main() {
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
group('availableAssetDiffs', () {
|
||||
group('when a car file has changed', () {
|
||||
const diffOutput = 'diff output';
|
||||
|
||||
setUp(() {
|
||||
when(
|
||||
() => diff.run(
|
||||
any(),
|
||||
any(),
|
||||
colorMode: any(named: 'colorMode'),
|
||||
unified: any(named: 'unified'),
|
||||
),
|
||||
).thenAnswer(
|
||||
(_) async => const ShorebirdProcessResult(
|
||||
exitCode: 1,
|
||||
stdout: diffOutput,
|
||||
stderr: '',
|
||||
),
|
||||
);
|
||||
});
|
||||
|
||||
test('shows asset diffs', () async {
|
||||
final fileSetDiff = await differ.changedFiles(
|
||||
baseIpaPath,
|
||||
changedCarXcarchivePath,
|
||||
);
|
||||
await runWithOverrides(() async {
|
||||
expect(
|
||||
await differ.availableAssetDiffs(
|
||||
fileSetDiff: fileSetDiff,
|
||||
oldArchivePath: baseIpaPath,
|
||||
newArchivePath: changedCarXcarchivePath,
|
||||
),
|
||||
equals(diffOutput),
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
group('when no car files have changed', () {
|
||||
test('shows no asset diffs', () async {
|
||||
final fileSetDiff = await differ.changedFiles(
|
||||
baseIpaPath,
|
||||
changedDartXcarchivePath,
|
||||
);
|
||||
await runWithOverrides(() async {
|
||||
expect(
|
||||
await differ.availableAssetDiffs(
|
||||
fileSetDiff: fileSetDiff,
|
||||
oldArchivePath: baseIpaPath,
|
||||
newArchivePath: changedDartXcarchivePath,
|
||||
),
|
||||
isEmpty,
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
group('xcframework', () {
|
||||
|
||||
@@ -78,4 +78,11 @@ class TestArchiveDiffer extends ArchiveDiffer {
|
||||
|
||||
@override
|
||||
bool isNativeFilePath(String filePath) => true;
|
||||
|
||||
@override
|
||||
Future<String> availableAssetDiffs({
|
||||
required FileSetDiff fileSetDiff,
|
||||
required String oldArchivePath,
|
||||
required String newArchivePath,
|
||||
}) async => '';
|
||||
}
|
||||
|
||||
@@ -0,0 +1,54 @@
|
||||
import 'package:mocktail/mocktail.dart';
|
||||
import 'package:scoped_deps/scoped_deps.dart';
|
||||
import 'package:shorebird_cli/src/executables/diff.dart';
|
||||
import 'package:shorebird_cli/src/shorebird_process.dart';
|
||||
import 'package:test/test.dart';
|
||||
|
||||
import '../mocks.dart';
|
||||
|
||||
void main() {
|
||||
group(Diff, () {
|
||||
late ShorebirdProcess process;
|
||||
late Diff diff;
|
||||
|
||||
R runWithOverrides<R>(R Function() body) {
|
||||
return runScoped(body, values: {processRef.overrideWith(() => process)});
|
||||
}
|
||||
|
||||
setUp(() {
|
||||
process = MockShorebirdProcess();
|
||||
diff = Diff();
|
||||
});
|
||||
|
||||
group('run', () {
|
||||
setUp(() {
|
||||
when(() => process.run(any(), any())).thenAnswer(
|
||||
(_) async => const ShorebirdProcessResult(
|
||||
exitCode: 0,
|
||||
stdout: 'stdout',
|
||||
stderr: 'stderr',
|
||||
),
|
||||
);
|
||||
});
|
||||
|
||||
test('returns the result of the `diff` command', () async {
|
||||
await runWithOverrides(
|
||||
() => diff.run(
|
||||
'fileA',
|
||||
'fileB',
|
||||
unified: true,
|
||||
colorMode: DiffColorMode.always,
|
||||
),
|
||||
);
|
||||
verify(
|
||||
() => process.run(Diff.executable, [
|
||||
'--unified',
|
||||
'--color=always',
|
||||
'fileA',
|
||||
'fileB',
|
||||
]),
|
||||
).called(1);
|
||||
});
|
||||
});
|
||||
});
|
||||
}
|
||||
@@ -91,6 +91,8 @@ class MockCodeSigner extends Mock implements CodeSigner {}
|
||||
|
||||
class MockDevicectl extends Mock implements Devicectl {}
|
||||
|
||||
class MockDiff extends Mock implements Diff {}
|
||||
|
||||
class MockDitto extends Mock implements Ditto {}
|
||||
|
||||
class MockDirectory extends Mock implements Directory {}
|
||||
|
||||
@@ -72,6 +72,13 @@ void main() {
|
||||
when(
|
||||
() => archiveDiffer.containsPotentiallyBreakingNativeDiffs(any()),
|
||||
).thenReturn(false);
|
||||
when(
|
||||
() => archiveDiffer.availableAssetDiffs(
|
||||
fileSetDiff: any(named: 'fileSetDiff'),
|
||||
oldArchivePath: any(named: 'oldArchivePath'),
|
||||
newArchivePath: any(named: 'newArchivePath'),
|
||||
),
|
||||
).thenAnswer((_) async => '');
|
||||
|
||||
when(() => httpClient.send(any())).thenAnswer(
|
||||
(_) async => http.StreamedResponse(const Stream.empty(), HttpStatus.ok),
|
||||
@@ -221,6 +228,13 @@ void main() {
|
||||
'''Your app contains asset changes, which will not be included in the patch.''',
|
||||
),
|
||||
).called(1);
|
||||
verify(
|
||||
() => archiveDiffer.availableAssetDiffs(
|
||||
fileSetDiff: any(named: 'fileSetDiff'),
|
||||
oldArchivePath: any(named: 'oldArchivePath'),
|
||||
newArchivePath: any(named: 'newArchivePath'),
|
||||
),
|
||||
).called(1);
|
||||
verify(
|
||||
() => logger.info(yellow.wrap(assetsDiffPrettyString)),
|
||||
).called(1);
|
||||
|
||||
Reference in New Issue
Block a user