Skip to content

Commit 48c1c23

Browse files
auto-submit[bot]auto-submit[bot]
authored
Reverts "Reland #128236 "Improve build output for all platforms" (#143166)" (#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
1 parent 2fc76c7 commit 48c1c23

20 files changed

+35
-262
lines changed

‎dev/devicelab/lib/tasks/run_tests.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ class AndroidRunOutputTest extends RunOutputTask {
135135
_findNextMatcherInList(
136136
stdout,
137137
(String line) => line.contains('Built build/app/outputs/flutter-apk/$apk') &&
138-
(!release || line.contains('MB)')),
138+
(!release || line.contains('MB).')),
139139
'Built build/app/outputs/flutter-apk/$apk',
140140
);
141141

‎packages/flutter_tools/lib/src/android/gradle.dart

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -549,15 +549,14 @@ class AndroidGradleBuilder implements AndroidBuilder {
549549
final File bundleFile = findBundleFile(project, buildInfo, _logger, _usage, _analytics);
550550
final String appSize = (buildInfo.mode == BuildMode.debug)
551551
? '' // Don't display the size when building a debug variant.
552-
: ' (${getSizeAsPlatformMB(bundleFile.lengthSync())})';
552+
: ' (${getSizeAsMB(bundleFile.lengthSync())})';
553553

554554
if (buildInfo.codeSizeDirectory != null) {
555555
await _performCodeSizeAnalysis('aab', bundleFile, androidBuildInfo);
556556
}
557557

558558
_logger.printStatus(
559-
'${_logger.terminal.successMark} '
560-
'Built ${_fileSystem.path.relative(bundleFile.path)}$appSize',
559+
'${_logger.terminal.successMark} Built ${_fileSystem.path.relative(bundleFile.path)}$appSize.',
561560
color: TerminalColor.green,
562561
);
563562
return;
@@ -587,10 +586,9 @@ class AndroidGradleBuilder implements AndroidBuilder {
587586

588587
final String appSize = (buildInfo.mode == BuildMode.debug)
589588
? '' // Don't display the size when building a debug variant.
590-
: ' (${getSizeAsPlatformMB(apkFile.lengthSync())})';
589+
: ' (${getSizeAsMB(apkFile.lengthSync())})';
591590
_logger.printStatus(
592-
'${_logger.terminal.successMark} '
593-
'Built ${_fileSystem.path.relative(apkFile.path)}$appSize',
591+
'${_logger.terminal.successMark} Built ${_fileSystem.path.relative(apkFile.path)}$appSize.',
594592
color: TerminalColor.green,
595593
);
596594

@@ -782,8 +780,7 @@ class AndroidGradleBuilder implements AndroidBuilder {
782780
);
783781
}
784782
_logger.printStatus(
785-
'${_logger.terminal.successMark} '
786-
'Built ${_fileSystem.path.relative(repoDirectory.path)}',
783+
'${_logger.terminal.successMark} Built ${_fileSystem.path.relative(repoDirectory.path)}.',
787784
color: TerminalColor.green,
788785
);
789786
}

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

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -105,18 +105,6 @@ abstract class OperatingSystemUtils {
105105
/// Return the File representing a new pipe.
106106
File makePipe(String path);
107107

108-
/// Return a directory's total size in bytes.
109-
int? getDirectorySize(Directory directory) {
110-
int? size;
111-
for (final FileSystemEntity entity in directory.listSync(recursive: true, followLinks: false)) {
112-
if (entity is File) {
113-
size ??= 0;
114-
size += entity.lengthSync();
115-
}
116-
}
117-
return size;
118-
}
119-
120108
void unzip(File file, Directory targetDirectory);
121109

122110
void unpack(File gzippedTarFile, Directory targetDirectory);

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

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,9 @@ import 'dart:math' as math;
77

88
import 'package:file/file.dart';
99
import 'package:intl/intl.dart';
10-
import 'package:meta/meta.dart';
1110
import 'package:path/path.dart' as path; // flutter_ignore: package_path_import
1211

1312
import '../convert.dart';
14-
import 'platform.dart';
1513

1614
/// A path jointer for URL paths.
1715
final path.Context urlContext = path.url;
@@ -90,14 +88,9 @@ String getElapsedAsMilliseconds(Duration duration) {
9088
return '${kMillisecondsFormat.format(duration.inMilliseconds)}ms';
9189
}
9290

93-
/// Return a platform-appropriate [String] representing the size of the given number of bytes.
94-
String getSizeAsPlatformMB(int bytesLength, {
95-
@visibleForTesting Platform platform = const LocalPlatform()
96-
}) {
97-
// Because Windows displays 'MB' but actually reports MiB, we calculate MiB
98-
// accordingly on Windows.
99-
final int bytesInPlatformMB = platform.isWindows ? 1024 * 1024 : 1000 * 1000;
100-
return '${(bytesLength / bytesInPlatformMB).toStringAsFixed(1)}MB';
91+
/// Return a String - with units - for the size in MB of the given number of bytes.
92+
String getSizeAsMB(int bytesLength) {
93+
return '${(bytesLength / (1024 * 1024)).toStringAsFixed(1)}MB';
10194
}
10295

10396
/// A class to maintain a list of items, fire events when items are added or

‎packages/flutter_tools/lib/src/commands/build_ios.dart

Lines changed: 3 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,15 @@
55
import 'dart:typed_data';
66

77
import 'package:crypto/crypto.dart';
8+
import 'package:file/file.dart';
89
import 'package:meta/meta.dart';
910
import 'package:unified_analytics/unified_analytics.dart';
1011

1112
import '../base/analyze_size.dart';
1213
import '../base/common.dart';
1314
import '../base/error_handling_io.dart';
14-
import '../base/file_system.dart';
1515
import '../base/logger.dart';
1616
import '../base/process.dart';
17-
import '../base/terminal.dart';
1817
import '../base/utils.dart';
1918
import '../build_info.dart';
2019
import '../convert.dart';
@@ -524,17 +523,7 @@ class BuildIOSArchiveCommand extends _BuildIOSSubCommand {
524523
return FlutterCommandResult.success();
525524
}
526525

527-
final Directory outputDirectory = globals.fs.directory(absoluteOutputPath);
528-
final int? directorySize = globals.os.getDirectorySize(outputDirectory);
529-
final String appSize = (buildInfo.mode == BuildMode.debug || directorySize == null)
530-
? '' // Don't display the size when building a debug variant.
531-
: ' (${getSizeAsPlatformMB(directorySize)})';
532-
533-
globals.printStatus(
534-
'${globals.terminal.successMark} '
535-
'Built IPA to ${globals.fs.path.relative(outputDirectory.path)}$appSize',
536-
color: TerminalColor.green,
537-
);
526+
globals.printStatus('Built IPA to $absoluteOutputPath.');
538527

539528
if (isAppStoreUpload) {
540529
globals.printStatus('To upload to the App Store either:');
@@ -748,17 +737,7 @@ abstract class _BuildIOSSubCommand extends BuildSubCommand {
748737
}
749738

750739
if (result.output != null) {
751-
final Directory outputDirectory = globals.fs.directory(result.output);
752-
final int? directorySize = globals.os.getDirectorySize(outputDirectory);
753-
final String appSize = (buildInfo.mode == BuildMode.debug || directorySize == null)
754-
? '' // Don't display the size when building a debug variant.
755-
: ' (${getSizeAsPlatformMB(directorySize)})';
756-
757-
globals.printStatus(
758-
'${globals.terminal.successMark} '
759-
'Built ${globals.fs.path.relative(outputDirectory.path)}$appSize',
760-
color: TerminalColor.green,
761-
);
740+
globals.printStatus('Built ${result.output}.');
762741

763742
// When an app is successfully built, record to analytics whether Impeller
764743
// is enabled or disabled.

‎packages/flutter_tools/lib/src/isolated/resident_web_runner.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -577,7 +577,7 @@ Please provide a valid TCP port (an integer between 0 and 65535, inclusive).
577577
shaderCompiler: device!.developmentShaderCompiler,
578578
);
579579
devFSStatus.stop();
580-
_logger.printTrace('Synced ${getSizeAsPlatformMB(report.syncedBytes)}.');
580+
_logger.printTrace('Synced ${getSizeAsMB(report.syncedBytes)}.');
581581
return report;
582582
}
583583

‎packages/flutter_tools/lib/src/linux/build_linux.dart

Lines changed: 3 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import '../base/common.dart';
1010
import '../base/file_system.dart';
1111
import '../base/logger.dart';
1212
import '../base/project_migrator.dart';
13-
import '../base/terminal.dart';
1413
import '../base/utils.dart';
1514
import '../build_info.dart';
1615
import '../cache.dart';
@@ -74,31 +73,16 @@ Future<void> buildLinux(
7473
final Status status = logger.startProgress(
7574
'Building Linux application...',
7675
);
77-
final String buildModeName = buildInfo.mode.cliName;
78-
final Directory platformBuildDirectory = globals.fs.directory(getLinuxBuildDirectory(targetPlatform));
79-
final Directory buildDirectory = platformBuildDirectory.childDirectory(buildModeName);
8076
try {
77+
final String buildModeName = buildInfo.mode.cliName;
78+
final Directory buildDirectory =
79+
globals.fs.directory(getLinuxBuildDirectory(targetPlatform)).childDirectory(buildModeName);
8180
await _runCmake(buildModeName, linuxProject.cmakeFile.parent, buildDirectory,
8281
needCrossBuild, targetPlatform, targetSysroot);
8382
await _runBuild(buildDirectory);
8483
} finally {
8584
status.cancel();
8685
}
87-
88-
final String? binaryName = getCmakeExecutableName(linuxProject);
89-
final File binaryFile = buildDirectory
90-
.childDirectory('bundle')
91-
.childFile('$binaryName');
92-
final FileSystemEntity buildOutput = binaryFile.existsSync() ? binaryFile : binaryFile.parent;
93-
// We don't print a size because the output directory can contain
94-
// optional files not needed by the user and because the binary is not
95-
// self-contained.
96-
globals.printStatus(
97-
'${globals.terminal.successMark} '
98-
'Built ${globals.fs.path.relative(buildOutput.path)}',
99-
color: TerminalColor.green,
100-
);
101-
10286
if (buildInfo.codeSizeDirectory != null && sizeAnalyzer != null) {
10387
final String arch = getNameForTargetPlatform(targetPlatform);
10488
final File codeSizeFile = globals.fs.directory(buildInfo.codeSizeDirectory)

‎packages/flutter_tools/lib/src/macos/build_macos.dart

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@ import '../base/common.dart';
99
import '../base/file_system.dart';
1010
import '../base/logger.dart';
1111
import '../base/project_migrator.dart';
12-
import '../base/terminal.dart';
13-
import '../base/utils.dart';
1412
import '../build_info.dart';
1513
import '../convert.dart';
1614
import '../globals.dart' as globals;
@@ -20,7 +18,6 @@ import '../migrations/xcode_project_object_version_migration.dart';
2018
import '../migrations/xcode_script_build_phase_migration.dart';
2119
import '../migrations/xcode_thin_binary_build_phase_input_paths_migration.dart';
2220
import '../project.dart';
23-
import 'application_package.dart';
2421
import 'cocoapod_utils.dart';
2522
import 'migrations/flutter_application_migration.dart';
2623
import 'migrations/macos_deployment_target_migration.dart';
@@ -161,24 +158,9 @@ Future<void> buildMacOS({
161158
} finally {
162159
status.cancel();
163160
}
164-
165161
if (result != 0) {
166162
throwToolExit('Build process failed');
167163
}
168-
final String? applicationBundle = MacOSApp.fromMacOSProject(flutterProject.macos).applicationBundle(buildInfo);
169-
if (applicationBundle != null) {
170-
final Directory outputDirectory = globals.fs.directory(applicationBundle);
171-
// This output directory is the .app folder itself.
172-
final int? directorySize = globals.os.getDirectorySize(outputDirectory);
173-
final String appSize = (buildInfo.mode == BuildMode.debug || directorySize == null)
174-
? '' // Don't display the size when building a debug variant.
175-
: ' (${getSizeAsPlatformMB(directorySize)})';
176-
globals.printStatus(
177-
'${globals.terminal.successMark} '
178-
'Built ${globals.fs.path.relative(outputDirectory.path)}$appSize',
179-
color: TerminalColor.green,
180-
);
181-
}
182164
await _writeCodeSizeAnalysis(buildInfo, sizeAnalyzer);
183165
final Duration elapsedDuration = sw.elapsed;
184166
globals.flutterUsage.sendTiming('build', 'xcode-macos', elapsedDuration);

‎packages/flutter_tools/lib/src/resident_runner.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -597,7 +597,7 @@ class FlutterDevice {
597597
return UpdateFSReport();
598598
}
599599
devFSStatus.stop();
600-
globals.printTrace('Synced ${getSizeAsPlatformMB(report.syncedBytes)}.');
600+
globals.printTrace('Synced ${getSizeAsMB(report.syncedBytes)}.');
601601
return report;
602602
}
603603

‎packages/flutter_tools/lib/src/web/compile.dart

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import '../base/common.dart';
1010
import '../base/file_system.dart';
1111
import '../base/logger.dart';
1212
import '../base/project_migrator.dart';
13-
import '../base/terminal.dart';
1413
import '../base/utils.dart';
1514
import '../build_info.dart';
1615
import '../build_system/build_system.dart';
@@ -131,14 +130,6 @@ class WebBuilder {
131130
status.stop();
132131
}
133132

134-
// We don't print a size because the output directory can contain
135-
// optional files not needed by the user.
136-
globals.printStatus(
137-
'${globals.terminal.successMark} '
138-
'Built ${globals.fs.path.relative(outputDirectory.path)}',
139-
color: TerminalColor.green,
140-
);
141-
142133
final String buildSettingsString = _buildEventAnalyticsSettings(
143134
configs: compilerConfigs,
144135
);

‎packages/flutter_tools/lib/src/windows/build_windows.dart

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -114,19 +114,20 @@ Future<void> buildWindows(
114114
}
115115

116116
final String? binaryName = getCmakeExecutableName(windowsProject);
117-
final File binaryFile = buildDirectory
117+
final File appFile = buildDirectory
118118
.childDirectory('runner')
119119
.childDirectory(sentenceCase(buildModeName))
120120
.childFile('$binaryName.exe');
121-
final FileSystemEntity buildOutput = binaryFile.existsSync() ? binaryFile : binaryFile.parent;
122-
// We don't print a size because the output directory can contain
123-
// optional files not needed by the user and because the binary is not
124-
// self-contained.
125-
globals.logger.printStatus(
126-
'${globals.logger.terminal.successMark} '
127-
'Built ${globals.fs.path.relative(buildOutput.path)}',
128-
color: TerminalColor.green,
129-
);
121+
if (appFile.existsSync()) {
122+
final String appSize = (buildInfo.mode == BuildMode.debug)
123+
? '' // Don't display the size when building a debug variant.
124+
: ' (${getSizeAsMB(appFile.lengthSync())})';
125+
globals.logger.printStatus(
126+
'${globals.logger.terminal.successMark} '
127+
'Built ${globals.fs.path.relative(appFile.path)}$appSize.',
128+
color: TerminalColor.green,
129+
);
130+
}
130131

131132
if (buildInfo.codeSizeDirectory != null && sizeAnalyzer != null) {
132133
final String arch = getNameForTargetPlatform(targetPlatform);

‎packages/flutter_tools/test/commands.shard/hermetic/build_ios_test.dart

Lines changed: 0 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -282,36 +282,6 @@ void main() {
282282
XcodeProjectInterpreter: () => FakeXcodeProjectInterpreterWithBuildSettings(),
283283
});
284284

285-
286-
testUsingContext('ios build outputs path and size when successful', () async {
287-
final BuildCommand command = BuildCommand(
288-
artifacts: artifacts,
289-
androidSdk: FakeAndroidSdk(),
290-
buildSystem: TestBuildSystem.all(BuildResult(success: true)),
291-
fileSystem: MemoryFileSystem.test(),
292-
logger: BufferLogger.test(),
293-
processUtils: processUtils,
294-
osUtils: FakeOperatingSystemUtils(),
295-
);
296-
createMinimalMockProjectFiles();
297-
298-
await createTestCommandRunner(command).run(
299-
const <String>['build', 'ios', '--no-pub']
300-
);
301-
expect(testLogger.statusText, contains(RegExp(r'✓ Built build/ios/iphoneos/Runner\.app \(\d+\.\d+MB\)')));
302-
}, overrides: <Type, Generator>{
303-
FileSystem: () => fileSystem,
304-
ProcessManager: () => FakeProcessManager.list(<FakeCommand>[
305-
xattrCommand,
306-
setUpFakeXcodeBuildHandler(onRun: (_) {
307-
fileSystem.directory('build/ios/Release-iphoneos/Runner.app').createSync(recursive: true);
308-
}),
309-
setUpRsyncCommand(),
310-
]),
311-
Platform: () => macosPlatform,
312-
XcodeProjectInterpreter: () => FakeXcodeProjectInterpreterWithBuildSettings(),
313-
});
314-
315285
testUsingContext('ios build invokes xcode build', () async {
316286
final BuildCommand command = BuildCommand(
317287
artifacts: artifacts,

‎packages/flutter_tools/test/commands.shard/hermetic/build_ipa_test.dart

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -576,7 +576,7 @@ void main() {
576576

577577
expect(logger.statusText, contains('build/ios/archive/Runner.xcarchive'));
578578
expect(logger.statusText, contains('Building App Store IPA'));
579-
expect(logger.statusText, contains(RegExp(r'Built IPA to build/ios/ipa \(\d+\.\d+MB\)')));
579+
expect(logger.statusText, contains('Built IPA to /build/ios/ipa'));
580580
expect(logger.statusText, contains('To upload to the App Store'));
581581
expect(logger.statusText, contains('Apple Transporter macOS app'));
582582
expect(fakeProcessManager, hasNoRemainingExpectations);
@@ -628,7 +628,7 @@ void main() {
628628

629629
expect(logger.statusText, contains('build/ios/archive/Runner.xcarchive'));
630630
expect(logger.statusText, contains('Building ad-hoc IPA'));
631-
expect(logger.statusText, contains(RegExp(r'Built IPA to build/ios/ipa \(\d+\.\d+MB\)')));
631+
expect(logger.statusText, contains('Built IPA to /build/ios/ipa'));
632632
// Don'ltruct how to upload to the App Store.
633633
expect(logger.statusText, isNot(contains('To upload')));
634634
expect(fakeProcessManager, hasNoRemainingExpectations);
@@ -680,7 +680,7 @@ void main() {
680680

681681
expect(logger.statusText, contains('build/ios/archive/Runner.xcarchive'));
682682
expect(logger.statusText, contains('Building enterprise IPA'));
683-
expect(logger.statusText, contains(RegExp(r'Built IPA to build/ios/ipa \(\d+\.\d+MB\)')));
683+
expect(logger.statusText, contains('Built IPA to /build/ios/ipa'));
684684
// Don'ltruct how to upload to the App Store.
685685
expect(logger.statusText, isNot(contains('To upload')));
686686
expect(fakeProcessManager, hasNoRemainingExpectations);
@@ -889,7 +889,7 @@ void main() {
889889

890890
testUsingContext('ipa build invokes xcode build export archive when passed plist', () async {
891891
final String outputPath =
892-
fileSystem.path.relative(fileSystem.path.join('build', 'ios', 'ipa'));
892+
fileSystem.path.absolute(fileSystem.path.join('build', 'ios', 'ipa'));
893893
final File exportOptions = fileSystem.file('ExportOptions.plist')
894894
..createSync();
895895
final BuildCommand command = BuildCommand(
@@ -918,7 +918,7 @@ void main() {
918918
],
919919
);
920920

921-
expect(logger.statusText, contains(RegExp('Built IPA to $outputPath ' r'\(\d+\.\d+MB\)')));
921+
expect(logger.statusText, contains('Built IPA to $outputPath.'));
922922
expect(fakeProcessManager, hasNoRemainingExpectations);
923923
}, overrides: <Type, Generator>{
924924
FileSystem: () => fileSystem,

0 commit comments

Comments
 (0)