Replace rsync when unzipping artifacts on a Mac (#126703)

Instead of using rsync, which has caused errors in the past (https://github.com/flutter/flutter/issues/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 https://github.com/flutter/flutter/issues/126016.

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

@@ -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';
@@ -423,7 +424,11 @@ 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')) {
@@ -431,35 +436,35 @@ class _MacOSUtils extends _PosixUtils {
       // 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);
     }
   }
 }

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

@@ -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(
@@ -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(
@@ -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);
     });
   });