Skip to content

Commit 2b3cd7f

Browse files
vashworthvashworth
authored
Replace rsync when unzipping artifacts on a Mac (#126703)
Instead of using rsync, which has caused errors in the past (flutter/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/flutter#126016.
1 parent a3b38aa commit 2b3cd7f

File tree

2 files changed

+108
-52
lines changed

2 files changed

+108
-52
lines changed

‎packages/flutter_tools/lib/src/base/os.dart‎

Lines changed: 31 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import 'package:meta/meta.dart';
88
import 'package:process/process.dart';
99

1010
import 'common.dart';
11+
import 'error_handling_io.dart';
1112
import 'file_system.dart';
1213
import 'io.dart';
1314
import 'logger.dart';
@@ -423,43 +424,47 @@ class _MacOSUtils extends _PosixUtils {
423424
return _hostPlatform!;
424425
}
425426

426-
// unzip, then rsync
427+
/// Unzip into a temporary directory.
428+
///
429+
/// For every file/directory/link in the unzipped file, delete the
430+
/// corresponding entity in the [targetDirectory] before moving from the
431+
/// temporary directory to the [targetDirectory].
427432
@override
428433
void unzip(File file, Directory targetDirectory) {
429434
if (!_processManager.canRun('unzip')) {
430435
// unzip is not available. this error message is modeled after the download
431436
// error in bin/internal/update_dart_sdk.sh
432437
throwToolExit('Missing "unzip" tool. Unable to extract ${file.path}.\nConsider running "brew install unzip".');
433438
}
434-
if (_processManager.canRun('rsync')) {
435-
final Directory tempDirectory = _fileSystem.systemTempDirectory.createTempSync('flutter_${file.basename}.');
436-
try {
437-
// Unzip to a temporary directory.
438-
_processUtils.runSync(
439-
<String>['unzip', '-o', '-q', file.path, '-d', tempDirectory.path],
440-
throwOnError: true,
441-
verboseExceptions: true,
442-
);
443-
for (final FileSystemEntity unzippedFile in tempDirectory.listSync(followLinks: false)) {
444-
// rsync --delete the unzipped files so files removed from the archive are also removed from the target.
445-
// Add the '-8' parameter to avoid mangling filenames with encodings that do not match the current locale.
446-
_processUtils.runSync(
447-
<String>['rsync', '-8', '-av', '--delete', unzippedFile.path, targetDirectory.path],
448-
throwOnError: true,
449-
verboseExceptions: true,
450-
);
451-
}
452-
} finally {
453-
tempDirectory.deleteSync(recursive: true);
454-
}
455-
} else {
456-
// Fall back to just unzipping.
457-
_logger.printTrace('Unable to find rsync, falling back to direct unzipping.');
439+
final Directory tempDirectory = _fileSystem.systemTempDirectory.createTempSync('flutter_${file.basename}.');
440+
try {
441+
// Unzip to a temporary directory.
458442
_processUtils.runSync(
459-
<String>['unzip', '-o', '-q', file.path, '-d', targetDirectory.path],
443+
<String>['unzip', '-o', '-q', file.path, '-d', tempDirectory.path],
460444
throwOnError: true,
461445
verboseExceptions: true,
462446
);
447+
for (final FileSystemEntity unzippedFile in tempDirectory.listSync(followLinks: false)) {
448+
final FileSystemEntityType fileType = targetDirectory.fileSystem.typeSync(
449+
targetDirectory.fileSystem.path.join(targetDirectory.path, unzippedFile.basename),
450+
followLinks: false,
451+
);
452+
final FileSystemEntity fileToReplace;
453+
if (fileType == FileSystemEntityType.directory) {
454+
fileToReplace = targetDirectory.childDirectory(unzippedFile.basename);
455+
} else if (fileType == FileSystemEntityType.link) {
456+
fileToReplace = targetDirectory.childLink(unzippedFile.basename);
457+
} else {
458+
fileToReplace = targetDirectory.childFile(unzippedFile.basename);
459+
}
460+
// Delete existing version before moving.
461+
ErrorHandlingFileSystem.deleteIfExists(fileToReplace, recursive: true);
462+
unzippedFile.renameSync(fileToReplace.path);
463+
}
464+
} on FileSystemException catch (e) {
465+
_logger.printTrace('${e.message}: ${e.osError}');
466+
} finally {
467+
tempDirectory.deleteSync(recursive: true);
463468
}
464469
}
465470
}

‎packages/flutter_tools/test/general.shard/base/os_test.dart‎

Lines changed: 77 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -626,29 +626,90 @@ void main() {
626626
});
627627

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

633-
final BufferLogger logger = BufferLogger.test();
634632
final OperatingSystemUtils macOSUtils = OperatingSystemUtils(
635633
fileSystem: fileSystem,
636-
logger: logger,
634+
logger: BufferLogger.test(),
635+
platform: FakePlatform(operatingSystem: 'macos'),
636+
processManager: fakeProcessManager,
637+
);
638+
639+
final Directory targetDirectory = fileSystem.currentDirectory;
640+
final Directory tempDirectory = fileSystem.systemTempDirectory.childDirectory('flutter_foo.zip.rand0');
641+
fakeProcessManager.addCommands(<FakeCommand>[
642+
FakeCommand(
643+
command: <String>[
644+
'unzip',
645+
'-o',
646+
'-q',
647+
'foo.zip',
648+
'-d',
649+
tempDirectory.path,
650+
],
651+
onRun: () {
652+
expect(tempDirectory, exists);
653+
tempDirectory.childDirectory('dirA').childFile('fileA').createSync(recursive: true);
654+
tempDirectory.childDirectory('dirB').childFile('fileB').createSync(recursive: true);
655+
},
656+
),
657+
]);
658+
659+
macOSUtils.unzip(fileSystem.file('foo.zip'), fileSystem.currentDirectory);
660+
expect(fakeProcessManager, hasNoRemainingExpectations);
661+
expect(tempDirectory, isNot(exists));
662+
expect(targetDirectory.childDirectory('dirA').childFile('fileA').existsSync(), isTrue);
663+
expect(targetDirectory.childDirectory('dirB').childFile('fileB').existsSync(), isTrue);
664+
});
665+
666+
testWithoutContext('unzip and copy to preexisting folder', () {
667+
final FileSystem fileSystem = MemoryFileSystem.test();
668+
669+
final OperatingSystemUtils macOSUtils = OperatingSystemUtils(
670+
fileSystem: fileSystem,
671+
logger: BufferLogger.test(),
637672
platform: FakePlatform(operatingSystem: 'macos'),
638673
processManager: fakeProcessManager,
639674
);
640675

641676
final Directory targetDirectory = fileSystem.currentDirectory;
642-
fakeProcessManager.addCommand(FakeCommand(
643-
command: <String>['unzip', '-o', '-q', 'foo.zip', '-d', targetDirectory.path],
644-
));
677+
final File origFileA = targetDirectory.childDirectory('dirA').childFile('fileA');
678+
origFileA.createSync(recursive: true);
679+
origFileA.writeAsStringSync('old');
680+
expect(targetDirectory.childDirectory('dirA').childFile('fileA').existsSync(), isTrue);
681+
expect(targetDirectory.childDirectory('dirA').childFile('fileA').readAsStringSync(), 'old');
682+
683+
final Directory tempDirectory = fileSystem.systemTempDirectory.childDirectory('flutter_foo.zip.rand0');
684+
fakeProcessManager.addCommands(<FakeCommand>[
685+
FakeCommand(
686+
command: <String>[
687+
'unzip',
688+
'-o',
689+
'-q',
690+
'foo.zip',
691+
'-d',
692+
tempDirectory.path,
693+
],
694+
onRun: () {
695+
expect(tempDirectory, exists);
696+
final File newFileA = tempDirectory.childDirectory('dirA').childFile('fileA');
697+
newFileA.createSync(recursive: true);
698+
newFileA.writeAsStringSync('new');
699+
tempDirectory.childDirectory('dirB').childFile('fileB').createSync(recursive: true);
700+
},
701+
),
702+
]);
645703

646-
macOSUtils.unzip(fileSystem.file('foo.zip'), targetDirectory);
704+
macOSUtils.unzip(fileSystem.file('foo.zip'), fileSystem.currentDirectory);
647705
expect(fakeProcessManager, hasNoRemainingExpectations);
648-
expect(logger.traceText, contains('Unable to find rsync'));
706+
expect(tempDirectory, isNot(exists));
707+
expect(targetDirectory.childDirectory('dirA').childFile('fileA').existsSync(), isTrue);
708+
expect(targetDirectory.childDirectory('dirA').childFile('fileA').readAsStringSync(), 'new');
709+
expect(targetDirectory.childDirectory('dirB').childFile('fileB').existsSync(), isTrue);
649710
});
650711

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

654715
final OperatingSystemUtils macOSUtils = OperatingSystemUtils(
@@ -659,6 +720,10 @@ void main() {
659720
);
660721

661722
final Directory targetDirectory = fileSystem.currentDirectory;
723+
final Directory origFileA = targetDirectory.childDirectory('dirA').childDirectory('fileA');
724+
origFileA.createSync(recursive: true);
725+
expect(targetDirectory.childDirectory('dirA').childDirectory('fileA').existsSync(), isTrue);
726+
662727
final Directory tempDirectory = fileSystem.systemTempDirectory.childDirectory('flutter_foo.zip.rand0');
663728
fakeProcessManager.addCommands(<FakeCommand>[
664729
FakeCommand(
@@ -676,27 +741,13 @@ void main() {
676741
tempDirectory.childDirectory('dirB').childFile('fileB').createSync(recursive: true);
677742
},
678743
),
679-
FakeCommand(command: <String>[
680-
'rsync',
681-
'-8',
682-
'-av',
683-
'--delete',
684-
tempDirectory.childDirectory('dirA').path,
685-
targetDirectory.path,
686-
]),
687-
FakeCommand(command: <String>[
688-
'rsync',
689-
'-8',
690-
'-av',
691-
'--delete',
692-
tempDirectory.childDirectory('dirB').path,
693-
targetDirectory.path,
694-
]),
695744
]);
696745

697746
macOSUtils.unzip(fileSystem.file('foo.zip'), fileSystem.currentDirectory);
698747
expect(fakeProcessManager, hasNoRemainingExpectations);
699748
expect(tempDirectory, isNot(exists));
749+
expect(targetDirectory.childDirectory('dirA').childFile('fileA').existsSync(), isTrue);
750+
expect(targetDirectory.childDirectory('dirB').childFile('fileB').existsSync(), isTrue);
700751
});
701752
});
702753

0 commit comments

Comments
 (0)