Reverts "Reland #128236 "Improve build output for all platforms" (#14…

…3166)" (#145261) Reverts: flutter/flutter#143166 Initiated by: guidezpl Reason for reverting: breaks devicelab windows tests Original PR Author: guidezpl Reviewed By: {loic-sharma} This change reverts the following previous change: Reland #128236, reverted in flutter/flutter#143125. This PR contains [one additional commit](flutter/flutter@199baea), fixing the 2 failed tests. ## Original description Improves the build output: 1. Gives confirmation that the build succeeded, in green 1. Gives the path to the built executable, without a trailing period to make it slightly easier to cmd/ctrl+open 1. Gives the size of the built executable (when the built executable is self contained) ### `apk`, `appbundle` <img width="607" alt="image" src="https://github.com/flutter/flutter/assets/6655696/ecc52abe-cd2e-4116-b22a-8385ae3e980d"> <img width="634" alt="image" src="https://github.com/flutter/flutter/assets/6655696/8af8bd33-c0bd-4215-9a06-9652ee019436"> ### `macos`, `ios`, `ipa` Build executables are self-contained and use a newly introduced `OperatingSystemUtils.getDirectorySize`. <img width="514" alt="image" src="https://github.com/flutter/flutter/assets/6655696/b5918a69-3959-4417-9205-4f501d185257"> <img width="581" alt="image" src="https://github.com/flutter/flutter/assets/6655696/d72fd420-18cf-4470-9e4b-b6ac10fbcd50"> <img width="616" alt="image" src="https://github.com/flutter/flutter/assets/6655696/5f235ce1-252a-4c13-898f-139f6c7bc698"> ### `windows`, `linux`, and `web` Build executables aren't self-contained, and folder size can sometimes overestimate distribution size, therefore their size isn't mentioned (see discussion below). <img width="647" alt="image" src="https://github.com/flutter/flutter/assets/6655696/7179e771-1eb7-48f6-b770-975bc073437b"> <img width="658" alt="image" src="https://github.com/flutter/flutter/assets/6655696/a6801cab-7b5a-4975-a406-f4c9fa44d7a2"> <img width="608" alt="image" src="https://github.com/flutter/flutter/assets/6655696/ee7c4125-a273-4a65-95d7-ab441edf8ac5"> ### Size reporting When applicable, the printed size matches the OS reported size. - macOS <img width="391" alt="image" src="https://github.com/flutter/flutter/assets/6655696/881cbfb1-d355-444b-ab44-c1a6343190ce"> - Windows <img width="338" alt="image" src="https://github.com/flutter/flutter/assets/6655696/3b806def-3d15-48a9-8a25-df200d6feef7"> - Linux <img width="320" alt="image" src="https://github.com/flutter/flutter/assets/6655696/89a4aa3d-2148-4f3b-b231-f93a057fee2b"> ## Related issues Part of #120127 Fixes flutter/flutter#121401
engine-flutter-autoroll · Mar 16, 2024 · 48c1c23 · 48c1c23
1 parent 2fc76c7
commit 48c1c23
Show file tree

Hide file tree

Showing 20 changed files with 35 additions and 262 deletions.
diff --git a/dev/devicelab/lib/tasks/run_tests.dart b/dev/devicelab/lib/tasks/run_tests.dart
@@ -135,7 +135,7 @@ class AndroidRunOutputTest extends RunOutputTask {
     _findNextMatcherInList(
       stdout,
       (String line) => line.contains('Built build/app/outputs/flutter-apk/$apk') &&
-        (!release || line.contains('MB)')),
+        (!release || line.contains('MB).')),
       'Built build/app/outputs/flutter-apk/$apk',
     );
 

diff --git a/packages/flutter_tools/lib/src/android/gradle.dart b/packages/flutter_tools/lib/src/android/gradle.dart
@@ -549,15 +549,14 @@ class AndroidGradleBuilder implements AndroidBuilder {
       final File bundleFile = findBundleFile(project, buildInfo, _logger, _usage, _analytics);
       final String appSize = (buildInfo.mode == BuildMode.debug)
           ? '' // Don't display the size when building a debug variant.
-          : ' (${getSizeAsPlatformMB(bundleFile.lengthSync())})';
+          : ' (${getSizeAsMB(bundleFile.lengthSync())})';
 
       if (buildInfo.codeSizeDirectory != null) {
         await _performCodeSizeAnalysis('aab', bundleFile, androidBuildInfo);
       }
 
       _logger.printStatus(
-        '${_logger.terminal.successMark} '
-        'Built ${_fileSystem.path.relative(bundleFile.path)}$appSize',
+        '${_logger.terminal.successMark} Built ${_fileSystem.path.relative(bundleFile.path)}$appSize.',
         color: TerminalColor.green,
       );
       return;
@@ -587,10 +586,9 @@ class AndroidGradleBuilder implements AndroidBuilder {
 
       final String appSize = (buildInfo.mode == BuildMode.debug)
           ? '' // Don't display the size when building a debug variant.
-          : ' (${getSizeAsPlatformMB(apkFile.lengthSync())})';
+          : ' (${getSizeAsMB(apkFile.lengthSync())})';
       _logger.printStatus(
-        '${_logger.terminal.successMark} '
-        'Built ${_fileSystem.path.relative(apkFile.path)}$appSize',
+        '${_logger.terminal.successMark}  Built ${_fileSystem.path.relative(apkFile.path)}$appSize.',
         color: TerminalColor.green,
       );
 
@@ -782,8 +780,7 @@ class AndroidGradleBuilder implements AndroidBuilder {
       );
     }
     _logger.printStatus(
-      '${_logger.terminal.successMark} '
-      'Built ${_fileSystem.path.relative(repoDirectory.path)}',
+      '${_logger.terminal.successMark} Built ${_fileSystem.path.relative(repoDirectory.path)}.',
       color: TerminalColor.green,
     );
   }

diff --git a/packages/flutter_tools/lib/src/base/os.dart b/packages/flutter_tools/lib/src/base/os.dart
@@ -105,18 +105,6 @@ abstract class OperatingSystemUtils {
   /// Return the File representing a new pipe.
   File makePipe(String path);
 
-  /// Return a directory's total size in bytes.
-  int? getDirectorySize(Directory directory) {
-    int? size;
-    for (final FileSystemEntity entity in directory.listSync(recursive: true, followLinks: false)) {
-      if (entity is File) {
-        size ??= 0;
-        size += entity.lengthSync();
-      }
-    }
-    return size;
-  }
-
   void unzip(File file, Directory targetDirectory);
 
   void unpack(File gzippedTarFile, Directory targetDirectory);

diff --git a/packages/flutter_tools/lib/src/base/utils.dart b/packages/flutter_tools/lib/src/base/utils.dart
@@ -7,11 +7,9 @@ import 'dart:math' as math;
 
 import 'package:file/file.dart';
 import 'package:intl/intl.dart';
-import 'package:meta/meta.dart';
 import 'package:path/path.dart' as path; // flutter_ignore: package_path_import
 
 import '../convert.dart';
-import 'platform.dart';
 
 /// A path jointer for URL paths.
 final path.Context urlContext = path.url;
@@ -90,14 +88,9 @@ String getElapsedAsMilliseconds(Duration duration) {
   return '${kMillisecondsFormat.format(duration.inMilliseconds)}ms';
 }
 
-/// Return a platform-appropriate [String] representing the size of the given number of bytes.
-String getSizeAsPlatformMB(int bytesLength, {
-    @visibleForTesting Platform platform = const LocalPlatform()
-  }) {
-  // Because Windows displays 'MB' but actually reports MiB, we calculate MiB
-  // accordingly on Windows.
-  final int bytesInPlatformMB = platform.isWindows ? 1024 * 1024 : 1000 * 1000;
-  return '${(bytesLength / bytesInPlatformMB).toStringAsFixed(1)}MB';
+/// Return a String - with units - for the size in MB of the given number of bytes.
+String getSizeAsMB(int bytesLength) {
+  return '${(bytesLength / (1024 * 1024)).toStringAsFixed(1)}MB';
 }
 
 /// A class to maintain a list of items, fire events when items are added or

diff --git a/packages/flutter_tools/lib/src/commands/build_ios.dart b/packages/flutter_tools/lib/src/commands/build_ios.dart
@@ -5,16 +5,15 @@
 import 'dart:typed_data';
 
 import 'package:crypto/crypto.dart';
+import 'package:file/file.dart';
 import 'package:meta/meta.dart';
 import 'package:unified_analytics/unified_analytics.dart';
 
 import '../base/analyze_size.dart';
 import '../base/common.dart';
 import '../base/error_handling_io.dart';
-import '../base/file_system.dart';
 import '../base/logger.dart';
 import '../base/process.dart';
-import '../base/terminal.dart';
 import '../base/utils.dart';
 import '../build_info.dart';
 import '../convert.dart';
@@ -524,17 +523,7 @@ class BuildIOSArchiveCommand extends _BuildIOSSubCommand {
       return FlutterCommandResult.success();
     }
 
-    final Directory outputDirectory = globals.fs.directory(absoluteOutputPath);
-    final int? directorySize = globals.os.getDirectorySize(outputDirectory);
-    final String appSize = (buildInfo.mode == BuildMode.debug || directorySize == null)
-      ? '' // Don't display the size when building a debug variant.
-        : ' (${getSizeAsPlatformMB(directorySize)})';
-
-    globals.printStatus(
-      '${globals.terminal.successMark} '
-      'Built IPA to ${globals.fs.path.relative(outputDirectory.path)}$appSize',
-      color: TerminalColor.green,
-    );
+    globals.printStatus('Built IPA to $absoluteOutputPath.');
 
     if (isAppStoreUpload) {
       globals.printStatus('To upload to the App Store either:');
@@ -748,17 +737,7 @@ abstract class _BuildIOSSubCommand extends BuildSubCommand {
     }
 
     if (result.output != null) {
-      final Directory outputDirectory = globals.fs.directory(result.output);
-      final int? directorySize = globals.os.getDirectorySize(outputDirectory);
-      final String appSize = (buildInfo.mode == BuildMode.debug || directorySize == null)
-        ? '' // Don't display the size when building a debug variant.
-          : ' (${getSizeAsPlatformMB(directorySize)})';
-
-      globals.printStatus(
-        '${globals.terminal.successMark} '
-        'Built ${globals.fs.path.relative(outputDirectory.path)}$appSize',
-        color: TerminalColor.green,
-      );
+      globals.printStatus('Built ${result.output}.');
 
       // When an app is successfully built, record to analytics whether Impeller
       // is enabled or disabled.

diff --git a/packages/flutter_tools/lib/src/isolated/resident_web_runner.dart b/packages/flutter_tools/lib/src/isolated/resident_web_runner.dart
@@ -577,7 +577,7 @@ Please provide a valid TCP port (an integer between 0 and 65535, inclusive).
       shaderCompiler: device!.developmentShaderCompiler,
     );
     devFSStatus.stop();
-    _logger.printTrace('Synced ${getSizeAsPlatformMB(report.syncedBytes)}.');
+    _logger.printTrace('Synced ${getSizeAsMB(report.syncedBytes)}.');
     return report;
   }
 

diff --git a/packages/flutter_tools/lib/src/linux/build_linux.dart b/packages/flutter_tools/lib/src/linux/build_linux.dart
@@ -10,7 +10,6 @@ import '../base/common.dart';
 import '../base/file_system.dart';
 import '../base/logger.dart';
 import '../base/project_migrator.dart';
-import '../base/terminal.dart';
 import '../base/utils.dart';
 import '../build_info.dart';
 import '../cache.dart';
@@ -74,31 +73,16 @@ Future<void> buildLinux(
   final Status status = logger.startProgress(
     'Building Linux application...',
   );
-  final String buildModeName = buildInfo.mode.cliName;
-  final Directory platformBuildDirectory = globals.fs.directory(getLinuxBuildDirectory(targetPlatform));
-  final Directory buildDirectory = platformBuildDirectory.childDirectory(buildModeName);
   try {
+    final String buildModeName = buildInfo.mode.cliName;
+    final Directory buildDirectory =
+        globals.fs.directory(getLinuxBuildDirectory(targetPlatform)).childDirectory(buildModeName);
     await _runCmake(buildModeName, linuxProject.cmakeFile.parent, buildDirectory,
                     needCrossBuild, targetPlatform, targetSysroot);
     await _runBuild(buildDirectory);
   } finally {
     status.cancel();
   }
-
-  final String? binaryName = getCmakeExecutableName(linuxProject);
-  final File binaryFile = buildDirectory
-    .childDirectory('bundle')
-    .childFile('$binaryName');
-  final FileSystemEntity buildOutput =  binaryFile.existsSync() ? binaryFile : binaryFile.parent;
-  // We don't print a size because the output directory can contain
-  // optional files not needed by the user and because the binary is not
-  // self-contained.
-  globals.printStatus(
-    '${globals.terminal.successMark} '
-    'Built ${globals.fs.path.relative(buildOutput.path)}',
-    color: TerminalColor.green,
-  );
-
   if (buildInfo.codeSizeDirectory != null && sizeAnalyzer != null) {
     final String arch = getNameForTargetPlatform(targetPlatform);
     final File codeSizeFile = globals.fs.directory(buildInfo.codeSizeDirectory)

diff --git a/packages/flutter_tools/lib/src/macos/build_macos.dart b/packages/flutter_tools/lib/src/macos/build_macos.dart
@@ -9,8 +9,6 @@ import '../base/common.dart';
 import '../base/file_system.dart';
 import '../base/logger.dart';
 import '../base/project_migrator.dart';
-import '../base/terminal.dart';
-import '../base/utils.dart';
 import '../build_info.dart';
 import '../convert.dart';
 import '../globals.dart' as globals;
@@ -20,7 +18,6 @@ import '../migrations/xcode_project_object_version_migration.dart';
 import '../migrations/xcode_script_build_phase_migration.dart';
 import '../migrations/xcode_thin_binary_build_phase_input_paths_migration.dart';
 import '../project.dart';
-import 'application_package.dart';
 import 'cocoapod_utils.dart';
 import 'migrations/flutter_application_migration.dart';
 import 'migrations/macos_deployment_target_migration.dart';
@@ -161,24 +158,9 @@ Future<void> buildMacOS({
   } finally {
     status.cancel();
   }
-
   if (result != 0) {
     throwToolExit('Build process failed');
   }
-  final String? applicationBundle = MacOSApp.fromMacOSProject(flutterProject.macos).applicationBundle(buildInfo);
-  if (applicationBundle != null) {
-    final Directory outputDirectory = globals.fs.directory(applicationBundle);
-    // This output directory is the .app folder itself.
-    final int? directorySize = globals.os.getDirectorySize(outputDirectory);
-    final String appSize = (buildInfo.mode == BuildMode.debug || directorySize == null)
-        ? '' // Don't display the size when building a debug variant.
-        : ' (${getSizeAsPlatformMB(directorySize)})';
-    globals.printStatus(
-      '${globals.terminal.successMark} '
-      'Built ${globals.fs.path.relative(outputDirectory.path)}$appSize',
-      color: TerminalColor.green,
-    );
-  }
   await _writeCodeSizeAnalysis(buildInfo, sizeAnalyzer);
   final Duration elapsedDuration = sw.elapsed;
   globals.flutterUsage.sendTiming('build', 'xcode-macos', elapsedDuration);

diff --git a/packages/flutter_tools/lib/src/resident_runner.dart b/packages/flutter_tools/lib/src/resident_runner.dart
@@ -597,7 +597,7 @@ class FlutterDevice {
       return UpdateFSReport();
     }
     devFSStatus.stop();
-    globals.printTrace('Synced ${getSizeAsPlatformMB(report.syncedBytes)}.');
+    globals.printTrace('Synced ${getSizeAsMB(report.syncedBytes)}.');
     return report;
   }
 

diff --git a/packages/flutter_tools/lib/src/web/compile.dart b/packages/flutter_tools/lib/src/web/compile.dart
@@ -10,7 +10,6 @@ import '../base/common.dart';
 import '../base/file_system.dart';
 import '../base/logger.dart';
 import '../base/project_migrator.dart';
-import '../base/terminal.dart';
 import '../base/utils.dart';
 import '../build_info.dart';
 import '../build_system/build_system.dart';
@@ -131,14 +130,6 @@ class WebBuilder {
       status.stop();
     }
 
-    // We don't print a size because the output directory can contain
-    // optional files not needed by the user.
-    globals.printStatus(
-      '${globals.terminal.successMark} '
-      'Built ${globals.fs.path.relative(outputDirectory.path)}',
-      color: TerminalColor.green,
-    );
-
     final String buildSettingsString = _buildEventAnalyticsSettings(
       configs: compilerConfigs,
     );

diff --git a/packages/flutter_tools/lib/src/windows/build_windows.dart b/packages/flutter_tools/lib/src/windows/build_windows.dart
@@ -114,19 +114,20 @@ Future<void> buildWindows(
   }
 
   final String? binaryName = getCmakeExecutableName(windowsProject);
-  final File binaryFile = buildDirectory
+  final File appFile = buildDirectory
     .childDirectory('runner')
     .childDirectory(sentenceCase(buildModeName))
     .childFile('$binaryName.exe');
-  final FileSystemEntity buildOutput =  binaryFile.existsSync() ? binaryFile : binaryFile.parent;
-  // We don't print a size because the output directory can contain
-  // optional files not needed by the user and because the binary is not
-  // self-contained.
-  globals.logger.printStatus(
-    '${globals.logger.terminal.successMark} '
-    'Built ${globals.fs.path.relative(buildOutput.path)}',
-    color: TerminalColor.green,
-  );
+  if (appFile.existsSync()) {
+    final String appSize = (buildInfo.mode == BuildMode.debug)
+        ? '' // Don't display the size when building a debug variant.
+        : ' (${getSizeAsMB(appFile.lengthSync())})';
+    globals.logger.printStatus(
+      '${globals.logger.terminal.successMark}  '
+      'Built ${globals.fs.path.relative(appFile.path)}$appSize.',
+      color: TerminalColor.green,
+    );
+  }
 
   if (buildInfo.codeSizeDirectory != null && sizeAnalyzer != null) {
     final String arch = getNameForTargetPlatform(targetPlatform);

diff --git a/packages/flutter_tools/test/commands.shard/hermetic/build_ios_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/build_ios_test.dart
@@ -282,36 +282,6 @@ void main() {
     XcodeProjectInterpreter: () => FakeXcodeProjectInterpreterWithBuildSettings(),
   });
 
-
-  testUsingContext('ios build outputs path and size when successful', () async {
-    final BuildCommand command = BuildCommand(
-      artifacts: artifacts,
-      androidSdk: FakeAndroidSdk(),
-      buildSystem: TestBuildSystem.all(BuildResult(success: true)),
-      fileSystem: MemoryFileSystem.test(),
-      logger: BufferLogger.test(),
-      processUtils: processUtils,
-      osUtils: FakeOperatingSystemUtils(),
-    );
-    createMinimalMockProjectFiles();
-
-    await createTestCommandRunner(command).run(
-      const <String>['build', 'ios', '--no-pub']
-    );
-    expect(testLogger.statusText, contains(RegExp(r'✓ Built build/ios/iphoneos/Runner\.app \(\d+\.\d+MB\)')));
-  }, overrides: <Type, Generator>{
-    FileSystem: () => fileSystem,
-    ProcessManager: () => FakeProcessManager.list(<FakeCommand>[
-      xattrCommand,
-      setUpFakeXcodeBuildHandler(onRun: (_) {
-        fileSystem.directory('build/ios/Release-iphoneos/Runner.app').createSync(recursive: true);
-      }),
-      setUpRsyncCommand(),
-    ]),
-    Platform: () => macosPlatform,
-    XcodeProjectInterpreter: () => FakeXcodeProjectInterpreterWithBuildSettings(),
-  });
-
   testUsingContext('ios build invokes xcode build', () async {
     final BuildCommand command = BuildCommand(
       artifacts: artifacts,

diff --git a/packages/flutter_tools/test/commands.shard/hermetic/build_ipa_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/build_ipa_test.dart
@@ -576,7 +576,7 @@ void main() {
 
     expect(logger.statusText, contains('build/ios/archive/Runner.xcarchive'));
     expect(logger.statusText, contains('Building App Store IPA'));
-    expect(logger.statusText, contains(RegExp(r'Built IPA to build/ios/ipa \(\d+\.\d+MB\)')));
+    expect(logger.statusText, contains('Built IPA to /build/ios/ipa'));
     expect(logger.statusText, contains('To upload to the App Store'));
     expect(logger.statusText, contains('Apple Transporter macOS app'));
     expect(fakeProcessManager, hasNoRemainingExpectations);
@@ -628,7 +628,7 @@ void main() {
 
     expect(logger.statusText, contains('build/ios/archive/Runner.xcarchive'));
     expect(logger.statusText, contains('Building ad-hoc IPA'));
-    expect(logger.statusText, contains(RegExp(r'Built IPA to build/ios/ipa \(\d+\.\d+MB\)')));
+    expect(logger.statusText, contains('Built IPA to /build/ios/ipa'));
     // Don'ltruct how to upload to the App Store.
     expect(logger.statusText, isNot(contains('To upload')));
     expect(fakeProcessManager, hasNoRemainingExpectations);
@@ -680,7 +680,7 @@ void main() {
 
     expect(logger.statusText, contains('build/ios/archive/Runner.xcarchive'));
     expect(logger.statusText, contains('Building enterprise IPA'));
-    expect(logger.statusText, contains(RegExp(r'Built IPA to build/ios/ipa \(\d+\.\d+MB\)')));
+    expect(logger.statusText, contains('Built IPA to /build/ios/ipa'));
     // Don'ltruct how to upload to the App Store.
     expect(logger.statusText, isNot(contains('To upload')));
     expect(fakeProcessManager, hasNoRemainingExpectations);
@@ -889,7 +889,7 @@ void main() {
 
   testUsingContext('ipa build invokes xcode build export archive when passed plist', () async {
     final String outputPath =
-        fileSystem.path.relative(fileSystem.path.join('build', 'ios', 'ipa'));
+        fileSystem.path.absolute(fileSystem.path.join('build', 'ios', 'ipa'));
     final File exportOptions = fileSystem.file('ExportOptions.plist')
       ..createSync();
     final BuildCommand command = BuildCommand(
@@ -918,7 +918,7 @@ void main() {
       ],
     );
 
-    expect(logger.statusText, contains(RegExp('Built IPA to $outputPath ' r'\(\d+\.\d+MB\)')));
+    expect(logger.statusText, contains('Built IPA to $outputPath.'));
     expect(fakeProcessManager, hasNoRemainingExpectations);
   }, overrides: <Type, Generator>{
     FileSystem: () => fileSystem,