Skip to content

Commit

Permalink
fix: remove faulty native change detection from Windows (#2796)
Browse files Browse the repository at this point in the history
  • Loading branch information
bryanoltman authored Jan 23, 2025
1 parent 794a1c6 commit 1bdcf05
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 78 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@ import 'dart:typed_data';
import 'package:shorebird_cli/src/archive_analysis/byte_utils.dart';

/// Utilities for interacting with Windows Portable Executable files.
///
/// .exe and .dll files are examples of PE files. .dll files specifically "are
/// considered executable files for almost all purposes, although they cannot be
/// directly run."
/// (https://learn.microsoft.com/en-us/windows/win32/debug/pe-format#coff-file-header-object-and-image)
class PortableExecutable {
/// Zeroes out the timestamps in the provided PE file to enable comparison of
/// binaries with different build times.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,8 @@
import 'dart:io';
import 'dart:isolate';

import 'package:archive/archive_io.dart';
import 'package:crypto/crypto.dart';
import 'package:path/path.dart' as p;
import 'package:shorebird_cli/src/archive_analysis/archive_analysis.dart';
import 'package:shorebird_cli/src/archive_analysis/archive_differ.dart';
import 'package:shorebird_cli/src/archive_analysis/portable_executable.dart';

/// {@template windows_archive_differ}
/// Finds differences between two Windows app packages.
Expand All @@ -15,8 +11,6 @@ class WindowsArchiveDiffer extends ArchiveDiffer {
/// {@macro windows_archive_differ}
const WindowsArchiveDiffer();

String _hash(List<int> bytes) => sha256.convert(bytes).toString();

bool _isDirectoryPath(String path) {
return path.endsWith('/');
}
Expand All @@ -36,67 +30,30 @@ class WindowsArchiveDiffer extends ArchiveDiffer {

@override
bool isNativeFilePath(String filePath) {
const nativeFileExtensions = ['.dll', '.exe'];
return nativeFileExtensions.contains(p.extension(filePath));
// We can't reliably detect native changes in Windows patches, so we don't
// attempt to diff them.
//
// Creating a release on one Windows machine and then attempting to patch
// on another results in a large number of changes to the exe and any dll
// files it requires that are spread throughout the file and very noisy.
//
// Otherwise, this function would return true if the file has a .dll or .exe
// extension.
//
// See https://github.com/shorebirdtech/shorebird/issues/2794
return false;
}

@override
Future<FileSetDiff> changedFiles(
String oldArchivePath,
String newArchivePath,
) async {
var oldPathHashes = await fileHashes(File(oldArchivePath));
var newPathHashes = await fileHashes(File(newArchivePath));

oldPathHashes = await _updateHashes(
archivePath: oldArchivePath,
pathHashes: oldPathHashes,
);
newPathHashes = await _updateHashes(
archivePath: newArchivePath,
pathHashes: newPathHashes,
);

final oldPathHashes = await fileHashes(File(oldArchivePath));
final newPathHashes = await fileHashes(File(newArchivePath));
return FileSetDiff.fromPathHashes(
oldPathHashes: oldPathHashes,
newPathHashes: newPathHashes,
);
}

/// Removes the timestamps from exe headers
Future<PathHashes> _updateHashes({
required String archivePath,
required PathHashes pathHashes,
}) async {
return Isolate.run(() async {
for (final file in _exeFiles(archivePath)) {
pathHashes[file.name] = await _sanitizedFileHash(file);
}

return pathHashes;
});
}

Future<String> _sanitizedFileHash(ArchiveFile file) async {
final tempDir = Directory.systemTemp.createTempSync();
final outPath = p.join(tempDir.path, file.name);
final outputStream = OutputFileStream(outPath);
file.writeContent(outputStream);
await outputStream.close();

final outFile = File(outPath);
final bytes = PortableExecutable.bytesWithZeroedTimestamps(outFile);
return _hash(bytes);
}

List<ArchiveFile> _exeFiles(String archivePath) {
return ZipDecoder()
.decodeStream(InputFileStream(archivePath))
.files
.where((file) => file.isFile)
.where(
(file) => p.extension(file.name) == '.exe',
)
.toList();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,25 +43,8 @@ void main() {
});

group('isNativeFilePath', () {
group('when file extension is .dll', () {
test('returns true', () {
final result = differ.isNativeFilePath('foo.dll');
expect(result, isTrue);
});
});

group('when file extension is .exe', () {
test('returns true', () {
final result = differ.isNativeFilePath('foo.exe');
expect(result, isTrue);
});
});

group('when file extension is not .dll or .exe', () {
test('returns false', () {
final result = differ.isNativeFilePath('foo.so');
expect(result, isFalse);
});
test('returns false', () {
expect(differ.isNativeFilePath(r'C:\path\to\file.exe'), isFalse);
});
});

Expand All @@ -77,9 +60,18 @@ void main() {
'patch.zip',
);

test('returns an empty FileSetDiff', () async {
test('returns a FileSetDiff containing only the .exe', () async {
final result = await differ.changedFiles(releasePath, patchPath);
expect(result, equals(FileSetDiff.empty()));
expect(
result,
equals(
const FileSetDiff(
addedPaths: {},
removedPaths: {},
changedPaths: {'hello_windows.exe'},
),
),
);
});
});
});
Expand Down

0 comments on commit 1bdcf05

Please sign in to comment.