Skip to content

Commit

Permalink
fix(shorebird_cli): ignore Mach-O UUID when checking for iOS patchabi…
Browse files Browse the repository at this point in the history
…lity (#1783)
  • Loading branch information
bryanoltman authored Mar 8, 2024
1 parent 94c4b69 commit 06220c9
Show file tree
Hide file tree
Showing 5 changed files with 105 additions and 15 deletions.
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import 'dart:io';
import 'dart:isolate';
import 'dart:typed_data';

import 'package:archive/archive_io.dart';
import 'package:collection/collection.dart';
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/archive_analysis/macho.dart';

/// Finds differences between two IPAs or zipped Xcframeworks.
///
Expand Down Expand Up @@ -112,7 +114,14 @@ class IosArchiveDiffer extends ArchiveDiffer {
}

final outFile = File(outPath);
return _hash(outFile.readAsBytesSync());
final Uint8List bytes;
if (MachO.isMachOFile(outFile)) {
bytes = MachO.bytesWithZeroedUUID(outFile);
} else {
bytes = outFile.readAsBytesSync();
}

return _hash(bytes);
}

/// Uses assetutil to write a json description of a .car file to disk and
Expand Down
60 changes: 60 additions & 0 deletions packages/shorebird_cli/lib/src/archive_analysis/macho.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
import 'dart:io';
import 'dart:typed_data';

const machOHeaderSize = 32;
const uuidLoadCommandType = 0x1b;

/// Utilities for interacting with Mach-O files.
/// See https://en.wikipedia.org/wiki/Mach-O.
class MachO {
/// Reads a 32-bit integer as a little-endian value from the provided bytes.
static int _readInt32(Uint8List bytes, int offset) {
return bytes[offset + 3] << 24 |
bytes[offset + 2] << 16 |
bytes[offset + 1] << 8 |
bytes[offset];
}

/// Returns `true` if the provided file is a Mach-O file.
static bool isMachOFile(File file) {
final bytes = file.readAsBytesSync();
final magic = _readInt32(bytes, 0);

// These are the magic numbers for Mach-O files.
// See https://en.wikipedia.org/wiki/Mach-O#Mach-O_header
return magic == 0xfeedface || magic == 0xfeedfacf;
}

/// Returns a copy of the provided Mach-O file with the UUID load command
/// zeroed out. This is necessary because the same code built in different
/// locations will generate Mach-O files with different UUIDs as the only
/// difference.
static Uint8List bytesWithZeroedUUID(File file) {
final bytes = file.readAsBytesSync();

// The number of load commands is a 32-bit int at offset 16. We could
// probably write a more robust MachO header parser, but this is all we need
// for now.
final numberOfLoadCommands = _readInt32(bytes, 16);

// The load commands are immediately after the header.
var offset = machOHeaderSize;
for (var i = 0; i < numberOfLoadCommands; i++) {
final commandType = _readInt32(bytes, offset);
final commandLength = _readInt32(bytes, offset + 4);

if (commandType == uuidLoadCommandType) {
// Zero out the UUID bytes.
final loadCommandStart = offset + 8;
for (var j = loadCommandStart; j < offset + commandLength; j++) {
bytes[j] = 0;
}
break;
}

offset += commandLength;
}

return bytes;
}
}
Binary file modified packages/shorebird_cli/test/fixtures/xcarchives/base.xcarchive.zip
Binary file not shown.
Binary file not shown.
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,19 @@ void main() {
xcarchiveFixturesBasePath,
'base.xcarchive.zip',
);
final changedAssetIpaPath = p.join(
final baseChangedUuidPath = p.join(
xcarchiveFixturesBasePath,
'base_changed_uuid.xcarchive.zip',
);
final changedAssetXcarchivePath = p.join(
xcarchiveFixturesBasePath,
'changed_asset.xcarchive.zip',
);
final changedDartIpaPath = p.join(
final changedDartXcarchivePath = p.join(
xcarchiveFixturesBasePath,
'changed_dart.xcarchive.zip',
);
final changedSwiftIpaPath = p.join(
final changedSwiftXcarchivePath = p.join(
xcarchiveFixturesBasePath,
'changed_swift.xcarchive.zip',
);
Expand Down Expand Up @@ -72,10 +76,21 @@ void main() {
);
});

test('finds no differences when only Mach-O UUID differs', () async {
final fileSetDiff = await differ.changedFiles(
baseIpaPath,
baseChangedUuidPath,
);
expect(
fileSetDiff,
isEmpty,
);
});

test('finds differences between two different xcarchives', () async {
final fileSetDiff = await differ.changedFiles(
baseIpaPath,
changedAssetIpaPath,
changedAssetXcarchivePath,
);
if (platform.isMacOS) {
expect(
Expand All @@ -102,8 +117,10 @@ void main() {

group('changedFiles', () {
test('detects asset changes', () async {
final fileSetDiff =
await differ.changedFiles(baseIpaPath, changedAssetIpaPath);
final fileSetDiff = await differ.changedFiles(
baseIpaPath,
changedAssetXcarchivePath,
);
expect(differ.assetsFileSetDiff(fileSetDiff), isNotEmpty);
expect(
differ.dartFileSetDiff(fileSetDiff),
Expand All @@ -113,16 +130,20 @@ void main() {
});

test('detects dart changes', () async {
final fileSetDiff =
await differ.changedFiles(baseIpaPath, changedDartIpaPath);
final fileSetDiff = await differ.changedFiles(
baseIpaPath,
changedDartXcarchivePath,
);
expect(differ.assetsFileSetDiff(fileSetDiff), isEmpty);
expect(differ.dartFileSetDiff(fileSetDiff), isNotEmpty);
expect(differ.nativeFileSetDiff(fileSetDiff), isEmpty);
});

test('detects swift changes', () async {
final fileSetDiff =
await differ.changedFiles(baseIpaPath, changedSwiftIpaPath);
final fileSetDiff = await differ.changedFiles(
baseIpaPath,
changedSwiftXcarchivePath,
);
expect(differ.assetsFileSetDiff(fileSetDiff), isEmpty);
expect(differ.dartFileSetDiff(fileSetDiff), isEmpty);
expect(differ.nativeFileSetDiff(fileSetDiff), isNotEmpty);
Expand All @@ -134,7 +155,7 @@ void main() {
() async {
final fileSetDiff = await differ.changedFiles(
baseIpaPath,
changedAssetIpaPath,
changedAssetXcarchivePath,
);
expect(
differ.containsPotentiallyBreakingAssetDiffs(fileSetDiff),
Expand All @@ -146,7 +167,7 @@ void main() {
() async {
final fileSetDiff = await differ.changedFiles(
baseIpaPath,
changedDartIpaPath,
changedDartXcarchivePath,
);
expect(
differ.containsPotentiallyBreakingAssetDiffs(fileSetDiff),
Expand All @@ -159,7 +180,7 @@ void main() {
test('returns true if Swift files have been changed', () async {
final fileSetDiff = await differ.changedFiles(
baseIpaPath,
changedSwiftIpaPath,
changedSwiftXcarchivePath,
);
expect(
differ.containsPotentiallyBreakingNativeDiffs(fileSetDiff),
Expand All @@ -170,7 +191,7 @@ void main() {
test('returns false if Swift files have not been changed', () async {
final fileSetDiff = await differ.changedFiles(
baseIpaPath,
changedAssetIpaPath,
changedAssetXcarchivePath,
);
expect(
differ.containsPotentiallyBreakingNativeDiffs(fileSetDiff),
Expand Down

0 comments on commit 06220c9

Please sign in to comment.