Skip to content

Commit

Permalink
Replace rsync when unzipping artifacts on a Mac (flutter#126703)
Browse files Browse the repository at this point in the history 
Instead of using rsync, which has caused errors in the past (flutter#99785), delete the file/directory/link prior to moving it.

Hopefully should let us stop double zipping the FlutterMacOS.framework in the engine: https://github.com/flutter/engine/pull/41306/files

Part of flutter#126016.
  • Loading branch information
@vashworth
vashworth authored and Casey Hillers committed May 24, 2023
1 parent 8320cb4 commit ce841df
Show file tree
Hide file tree
Showing 2 changed files with 108 additions and 52 deletions.
57 changes: 31 additions & 26 deletions packages/flutter_tools/lib/src/base/os.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import 'package:meta/meta.dart';
import 'package:process/process.dart';

import 'common.dart';
import 'error_handling_io.dart';
import 'file_system.dart';
import 'io.dart';
import 'logger.dart';
Expand Down Expand Up @@ -423,43 +424,47 @@ class _MacOSUtils extends _PosixUtils {
return _hostPlatform!;
}

// unzip, then rsync
/// Unzip into a temporary directory.
///
/// For every file/directory/link in the unzipped file, delete the
/// corresponding entity in the [targetDirectory] before moving from the
/// temporary directory to the [targetDirectory].
@override
void unzip(File file, Directory targetDirectory) {
if (!_processManager.canRun('unzip')) {
// unzip is not available. this error message is modeled after the download
// error in bin/internal/update_dart_sdk.sh
throwToolExit('Missing "unzip" tool. Unable to extract ${file.path}.\nConsider running "brew install unzip".');
}
if (_processManager.canRun('rsync')) {
final Directory tempDirectory = _fileSystem.systemTempDirectory.createTempSync('flutter_${file.basename}.');
try {
// Unzip to a temporary directory.
_processUtils.runSync(
<String>['unzip', '-o', '-q', file.path, '-d', tempDirectory.path],
throwOnError: true,
verboseExceptions: true,
);
for (final FileSystemEntity unzippedFile in tempDirectory.listSync(followLinks: false)) {
// rsync --delete the unzipped files so files removed from the archive are also removed from the target.
// Add the '-8' parameter to avoid mangling filenames with encodings that do not match the current locale.
_processUtils.runSync(
<String>['rsync', '-8', '-av', '--delete', unzippedFile.path, targetDirectory.path],
throwOnError: true,
verboseExceptions: true,
);
}
} finally {
tempDirectory.deleteSync(recursive: true);
}
} else {
// Fall back to just unzipping.
_logger.printTrace('Unable to find rsync, falling back to direct unzipping.');
final Directory tempDirectory = _fileSystem.systemTempDirectory.createTempSync('flutter_${file.basename}.');
try {
// Unzip to a temporary directory.
_processUtils.runSync(
<String>['unzip', '-o', '-q', file.path, '-d', targetDirectory.path],
<String>['unzip', '-o', '-q', file.path, '-d', tempDirectory.path],
throwOnError: true,
verboseExceptions: true,
);
for (final FileSystemEntity unzippedFile in tempDirectory.listSync(followLinks: false)) {
final FileSystemEntityType fileType = targetDirectory.fileSystem.typeSync(
targetDirectory.fileSystem.path.join(targetDirectory.path, unzippedFile.basename),
followLinks: false,
);
final FileSystemEntity fileToReplace;
if (fileType == FileSystemEntityType.directory) {
fileToReplace = targetDirectory.childDirectory(unzippedFile.basename);
} else if (fileType == FileSystemEntityType.link) {
fileToReplace = targetDirectory.childLink(unzippedFile.basename);
} else {
fileToReplace = targetDirectory.childFile(unzippedFile.basename);
}
// Delete existing version before moving.
ErrorHandlingFileSystem.deleteIfExists(fileToReplace, recursive: true);
unzippedFile.renameSync(fileToReplace.path);
}
} on FileSystemException catch (e) {
_logger.printTrace('${e.message}: ${e.osError}');
} finally {
tempDirectory.deleteSync(recursive: true);
}
}
}
Expand Down
103 changes: 77 additions & 26 deletions packages/flutter_tools/test/general.shard/base/os_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -626,29 +626,90 @@ void main() {
});

group('unzip on macOS', () {
testWithoutContext('falls back to unzip when rsync cannot run', () {
testWithoutContext('unzip and copy to empty folder', () {
final FileSystem fileSystem = MemoryFileSystem.test();
fakeProcessManager.excludedExecutables.add('rsync');

final BufferLogger logger = BufferLogger.test();
final OperatingSystemUtils macOSUtils = OperatingSystemUtils(
fileSystem: fileSystem,
logger: logger,
logger: BufferLogger.test(),
platform: FakePlatform(operatingSystem: 'macos'),
processManager: fakeProcessManager,
);

final Directory targetDirectory = fileSystem.currentDirectory;
final Directory tempDirectory = fileSystem.systemTempDirectory.childDirectory('flutter_foo.zip.rand0');
fakeProcessManager.addCommands(<FakeCommand>[
FakeCommand(
command: <String>[
'unzip',
'-o',
'-q',
'foo.zip',
'-d',
tempDirectory.path,
],
onRun: () {
expect(tempDirectory, exists);
tempDirectory.childDirectory('dirA').childFile('fileA').createSync(recursive: true);
tempDirectory.childDirectory('dirB').childFile('fileB').createSync(recursive: true);
},
),
]);

macOSUtils.unzip(fileSystem.file('foo.zip'), fileSystem.currentDirectory);
expect(fakeProcessManager, hasNoRemainingExpectations);
expect(tempDirectory, isNot(exists));
expect(targetDirectory.childDirectory('dirA').childFile('fileA').existsSync(), isTrue);
expect(targetDirectory.childDirectory('dirB').childFile('fileB').existsSync(), isTrue);
});

testWithoutContext('unzip and copy to preexisting folder', () {
final FileSystem fileSystem = MemoryFileSystem.test();

final OperatingSystemUtils macOSUtils = OperatingSystemUtils(
fileSystem: fileSystem,
logger: BufferLogger.test(),
platform: FakePlatform(operatingSystem: 'macos'),
processManager: fakeProcessManager,
);

final Directory targetDirectory = fileSystem.currentDirectory;
fakeProcessManager.addCommand(FakeCommand(
command: <String>['unzip', '-o', '-q', 'foo.zip', '-d', targetDirectory.path],
));
final File origFileA = targetDirectory.childDirectory('dirA').childFile('fileA');
origFileA.createSync(recursive: true);
origFileA.writeAsStringSync('old');
expect(targetDirectory.childDirectory('dirA').childFile('fileA').existsSync(), isTrue);
expect(targetDirectory.childDirectory('dirA').childFile('fileA').readAsStringSync(), 'old');

final Directory tempDirectory = fileSystem.systemTempDirectory.childDirectory('flutter_foo.zip.rand0');
fakeProcessManager.addCommands(<FakeCommand>[
FakeCommand(
command: <String>[
'unzip',
'-o',
'-q',
'foo.zip',
'-d',
tempDirectory.path,
],
onRun: () {
expect(tempDirectory, exists);
final File newFileA = tempDirectory.childDirectory('dirA').childFile('fileA');
newFileA.createSync(recursive: true);
newFileA.writeAsStringSync('new');
tempDirectory.childDirectory('dirB').childFile('fileB').createSync(recursive: true);
},
),
]);

macOSUtils.unzip(fileSystem.file('foo.zip'), targetDirectory);
macOSUtils.unzip(fileSystem.file('foo.zip'), fileSystem.currentDirectory);
expect(fakeProcessManager, hasNoRemainingExpectations);
expect(logger.traceText, contains('Unable to find rsync'));
expect(tempDirectory, isNot(exists));
expect(targetDirectory.childDirectory('dirA').childFile('fileA').existsSync(), isTrue);
expect(targetDirectory.childDirectory('dirA').childFile('fileA').readAsStringSync(), 'new');
expect(targetDirectory.childDirectory('dirB').childFile('fileB').existsSync(), isTrue);
});

testWithoutContext('unzip and rsyncs', () {
testWithoutContext('unzip and copy to preexisting folder with type mismatch', () {
final FileSystem fileSystem = MemoryFileSystem.test();

final OperatingSystemUtils macOSUtils = OperatingSystemUtils(
Expand All @@ -659,6 +720,10 @@ void main() {
);

final Directory targetDirectory = fileSystem.currentDirectory;
final Directory origFileA = targetDirectory.childDirectory('dirA').childDirectory('fileA');
origFileA.createSync(recursive: true);
expect(targetDirectory.childDirectory('dirA').childDirectory('fileA').existsSync(), isTrue);

final Directory tempDirectory = fileSystem.systemTempDirectory.childDirectory('flutter_foo.zip.rand0');
fakeProcessManager.addCommands(<FakeCommand>[
FakeCommand(
Expand All @@ -676,27 +741,13 @@ void main() {
tempDirectory.childDirectory('dirB').childFile('fileB').createSync(recursive: true);
},
),
FakeCommand(command: <String>[
'rsync',
'-8',
'-av',
'--delete',
tempDirectory.childDirectory('dirA').path,
targetDirectory.path,
]),
FakeCommand(command: <String>[
'rsync',
'-8',
'-av',
'--delete',
tempDirectory.childDirectory('dirB').path,
targetDirectory.path,
]),
]);

macOSUtils.unzip(fileSystem.file('foo.zip'), fileSystem.currentDirectory);
expect(fakeProcessManager, hasNoRemainingExpectations);
expect(tempDirectory, isNot(exists));
expect(targetDirectory.childDirectory('dirA').childFile('fileA').existsSync(), isTrue);
expect(targetDirectory.childDirectory('dirB').childFile('fileB').existsSync(), isTrue);
});
});

Expand Down

0 comments on commit ce841df

Please sign in to comment.