From e18c5e209c1b7ba6fb726133cba5cdcdbebf9ebc Mon Sep 17 00:00:00 2001 From: Victoria Ashworth <15619084+vashworth@users.noreply.github.com> Date: Wed, 12 Jun 2024 16:16:07 -0500 Subject: [PATCH] Improve build time when using SwiftPM (#150052) When using SwiftPM, we use `flutter assemble` in an Xcode Pre-action to run the `debug_unpack_macos` (or profile/release) target. This target is also later used in a Run Script build phase. Depending on `ARCHS` build setting, the Flutter/FlutterMacOS binary is thinned. In the Run Script build phase, `ARCHS` is filtered to the active arch. However, in the Pre-action it doesn't always filter to the active arch. As a workaround, assume arm64 if the [`NATIVE_ARCH`](https://developer.apple.com/documentation/xcode/build-settings-reference/#NATIVEARCH) is arm, otherwise assume x86_64. Also, this PR adds a define flag `PreBuildAction`, which gives the Pre-action a [unique configuration of defines](https://github.com/flutter/flutter/blob/fdb74fd3e7e08d99d8e19a0d7c548cf6cab56b31/packages/flutter_tools/lib/src/build_system/build_system.dart#L355-L372) and therefore a separate filecache from the Run Script build phase filecache. This improves caching so the Run Script build phase action doesn't find missing targets in the filecache. It also uses this flag to skip cleaning up the previous build files. Lastly, skip the Pre-action if the build command is `clean`. Note: For iOS, if [`CodesignIdentity`](https://github.com/flutter/flutter/blob/14df7be3f9471a97f34e4601fb7710850373ac3b/packages/flutter_tools/bin/xcode_backend.dart#L470-L473) is set, the Pre-action and Run Script build phase will not match because the Pre-action does not send the `EXPANDED_CODE_SIGN_IDENTITY` and therefore will codesign it with [`-`](https://github.com/flutter/flutter/blob/14df7be3f9471a97f34e4601fb7710850373ac3b/packages/flutter_tools/lib/src/build_system/targets/ios.dart#L695) instead. This will cause `debug_unpack_macos` to invalidate and rerun every time. A potential solution would be to move [codesigning out of the Run Script build phase](https://github.com/flutter/flutter/blob/14df7be3f9471a97f34e4601fb7710850373ac3b/packages/flutter_tools/lib/src/build_system/targets/ios.dart#L299) to the [Thin Binary build phase](https://github.com/flutter/flutter/blob/14df7be3f9471a97f34e4601fb7710850373ac3b/packages/flutter_tools/bin/xcode_backend.dart#L204-L257) after it's copied into the TARGET_BUILD_DIR, like we do with [macOS](https://github.com/flutter/flutter/blob/14df7be3f9471a97f34e4601fb7710850373ac3b/packages/flutter_tools/bin/macos_assemble.sh#L179-L183). --- packages/flutter_tools/bin/macos_assemble.sh | 48 ++++++-- packages/flutter_tools/bin/xcode_backend.dart | 51 ++++++++- .../flutter_tools/lib/src/build_info.dart | 6 + .../lib/src/build_system/build_system.dart | 8 ++ .../general.shard/xcode_backend_test.dart | 104 ++++++++++++++++++ .../swift_package_manager_test.dart | 69 ++++++++++++ .../swift_package_manager_utils.dart | 6 +- 7 files changed, 276 insertions(+), 16 deletions(-) diff --git a/packages/flutter_tools/bin/macos_assemble.sh b/packages/flutter_tools/bin/macos_assemble.sh index 89a00a735e..40c6a5051f 100755 --- a/packages/flutter_tools/bin/macos_assemble.sh +++ b/packages/flutter_tools/bin/macos_assemble.sh @@ -111,11 +111,28 @@ BuildApp() { if [[ -n "$LOCAL_ENGINE_HOST" ]]; then flutter_args+=("--local-engine-host=${LOCAL_ENGINE_HOST}") fi + + local architectures="${ARCHS}" + if [[ -n "$1" && "$1" == "prepare" ]]; then + # The "prepare" command runs in a pre-action script, which doesn't always + # filter the "ARCHS" build setting to only the active arch. To workaround, + # if "ONLY_ACTIVE_ARCH" is true and the "NATIVE_ARCH" is arm, assume the + # active arch is also arm to improve caching. If this assumption is + # incorrect, it will later be corrected by the "build" command. + if [[ -n "$ONLY_ACTIVE_ARCH" && "$ONLY_ACTIVE_ARCH" == "YES" && -n "$NATIVE_ARCH" ]]; then + if [[ "$NATIVE_ARCH" == *"arm"* ]]; then + architectures="arm64" + else + architectures="x86_64" + fi + fi + fi + flutter_args+=( "assemble" "--no-version-check" "-dTargetPlatform=darwin" - "-dDarwinArchs=${ARCHS}" + "-dDarwinArchs=${architectures}" "-dTargetFile=${target_path}" "-dBuildMode=${build_mode}" "-dTreeShakeIcons=${TREE_SHAKE_ICONS}" @@ -132,6 +149,19 @@ BuildApp() { "--output=${BUILT_PRODUCTS_DIR}" ) + local target="${build_mode}_macos_bundle_flutter_assets"; + if [[ -n "$1" && "$1" == "prepare" ]]; then + # The "prepare" command only targets the UnpackMacOS target, which copies the + # FlutterMacOS framework to the BUILT_PRODUCTS_DIR. + target="${build_mode}_unpack_macos" + + # Use the PreBuildAction define flag to force the tool to use a different + # filecache file for the "prepare" command. This will make the environment + # buildPrefix for the "prepare" command unique from the "build" command. + # This will improve caching since the "build" command has more target dependencies. + flutter_args+=("-dPreBuildAction=PrepareFramework") + fi + if [[ -n "$FLAVOR" ]]; then flutter_args+=("-dFlavor=${FLAVOR}") fi @@ -145,20 +175,18 @@ BuildApp() { flutter_args+=("-dCodeSizeDirectory=${CODE_SIZE_DIRECTORY}") fi - # Run flutter assemble with the build mode specific target that was passed in. - # If no target was passed it, default to build mode specific - # macos_bundle_flutter_assets target. - if [[ -n "$1" ]]; then - flutter_args+=("${build_mode}$1") - else - flutter_args+=("${build_mode}_macos_bundle_flutter_assets") - fi + flutter_args+=("${target}") RunCommand "${flutter_args[@]}" } PrepareFramework() { - BuildApp "_unpack_macos" + # The "prepare" command runs in a pre-action script, which also runs when + # using the Xcode/xcodebuild clean command. Skip if cleaning. + if [[ $ACTION == "clean" ]]; then + exit 0 + fi + BuildApp "prepare" } # Adds the App.framework as an embedded binary, the flutter_assets as diff --git a/packages/flutter_tools/bin/xcode_backend.dart b/packages/flutter_tools/bin/xcode_backend.dart index f657defd98..320131a4dd 100644 --- a/packages/flutter_tools/bin/xcode_backend.dart +++ b/packages/flutter_tools/bin/xcode_backend.dart @@ -354,14 +354,25 @@ class Context { } void prepare() { + // The "prepare" command runs in a pre-action script, which also runs when + // using the Xcode/xcodebuild clean command. Skip if cleaning. + if (environment['ACTION'] == 'clean') { + return; + } final bool verbose = (environment['VERBOSE_SCRIPT_LOGGING'] ?? '').isNotEmpty; final String sourceRoot = environment['SOURCE_ROOT'] ?? ''; final String projectPath = environment['FLUTTER_APPLICATION_PATH'] ?? '$sourceRoot/..'; final String buildMode = parseFlutterBuildMode(); - final List flutterArgs = _generateFlutterArgsForAssemble(buildMode, verbose); + final List flutterArgs = _generateFlutterArgsForAssemble( + 'prepare', + buildMode, + verbose, + ); + // The "prepare" command only targets the UnpackIOS target, which copies the + // Flutter framework to the BUILT_PRODUCTS_DIR. flutterArgs.add('${buildMode}_unpack_ios'); final ProcessResult result = runSync( @@ -385,7 +396,11 @@ class Context { final String buildMode = parseFlutterBuildMode(); - final List flutterArgs = _generateFlutterArgsForAssemble(buildMode, verbose); + final List flutterArgs = _generateFlutterArgsForAssemble( + 'build', + buildMode, + verbose, + ); flutterArgs.add('${buildMode}_ios_bundle_flutter_assets'); @@ -408,7 +423,11 @@ class Context { echo('Project $projectPath built and packaged successfully.'); } - List _generateFlutterArgsForAssemble(String buildMode, bool verbose) { + List _generateFlutterArgsForAssemble( + String command, + String buildMode, + bool verbose, + ) { String targetPath = 'lib/main.dart'; if (environment['FLUTTER_TARGET'] != null) { targetPath = environment['FLUTTER_TARGET']!; @@ -442,6 +461,22 @@ class Context { flutterArgs.add('--local-engine-host=${environment['LOCAL_ENGINE_HOST']}'); } + String architectures = environment['ARCHS'] ?? ''; + if (command == 'prepare') { + // The "prepare" command runs in a pre-action script, which doesn't always + // filter the "ARCHS" build setting to only the active arch. To workaround, + // if "ONLY_ACTIVE_ARCH" is true and the "NATIVE_ARCH" is arm, assume the + // active arch is also arm to improve caching. If this assumption is + // incorrect, it will later be corrected by the "build" command. + if (environment['ONLY_ACTIVE_ARCH'] == 'YES' && environment['NATIVE_ARCH'] != null) { + if (environment['NATIVE_ARCH']!.contains('arm')) { + architectures = 'arm64'; + } else { + architectures = 'x86_64'; + } + } + } + flutterArgs.addAll([ 'assemble', '--no-version-check', @@ -450,7 +485,7 @@ class Context { '-dTargetFile=$targetPath', '-dBuildMode=$buildMode', if (environment['FLAVOR'] != null) '-dFlavor=${environment['FLAVOR']}', - '-dIosArchs=${environment['ARCHS'] ?? ''}', + '-dIosArchs=$architectures', '-dSdkRoot=${environment['SDKROOT'] ?? ''}', '-dSplitDebugInfo=${environment['SPLIT_DEBUG_INFO'] ?? ''}', '-dTreeShakeIcons=${environment['TREE_SHAKE_ICONS'] ?? ''}', @@ -463,6 +498,14 @@ class Context { '--ExtraFrontEndOptions=${environment['EXTRA_FRONT_END_OPTIONS'] ?? ''}', ]); + if (command == 'prepare') { + // Use the PreBuildAction define flag to force the tool to use a different + // filecache file for the "prepare" command. This will make the environment + // buildPrefix for the "prepare" command unique from the "build" command. + // This will improve caching since the "build" command has more target dependencies. + flutterArgs.add('-dPreBuildAction=PrepareFramework'); + } + if (environment['PERFORMANCE_MEASUREMENT_FILE'] != null && environment['PERFORMANCE_MEASUREMENT_FILE']!.isNotEmpty) { flutterArgs.add('--performance-measurement-file=${environment['PERFORMANCE_MEASUREMENT_FILE']}'); } diff --git a/packages/flutter_tools/lib/src/build_info.dart b/packages/flutter_tools/lib/src/build_info.dart index 25d9e957a6..e3a903484c 100644 --- a/packages/flutter_tools/lib/src/build_info.dart +++ b/packages/flutter_tools/lib/src/build_info.dart @@ -958,6 +958,12 @@ const String kBuildNumber = 'BuildNumber'; /// Will be "build" when building and "install" when archiving. const String kXcodeAction = 'Action'; +/// The define of the Xcode build Pre-action. +/// +/// Will be "PrepareFramework" when copying the Flutter/FlutterMacOS framework +/// to the BUILT_PRODUCTS_DIR prior to the build. +const String kXcodePreAction = 'PreBuildAction'; + final Converter _defineEncoder = utf8.encoder.fuse(base64.encoder); final Converter _defineDecoder = base64.decoder.fuse(utf8.decoder); diff --git a/packages/flutter_tools/lib/src/build_system/build_system.dart b/packages/flutter_tools/lib/src/build_system/build_system.dart index 4914752ed1..f017e4dcd2 100644 --- a/packages/flutter_tools/lib/src/build_system/build_system.dart +++ b/packages/flutter_tools/lib/src/build_system/build_system.dart @@ -16,6 +16,7 @@ import '../base/file_system.dart'; import '../base/logger.dart'; import '../base/platform.dart'; import '../base/utils.dart'; +import '../build_info.dart'; import '../cache.dart'; import '../convert.dart'; import '../reporting/reporting.dart'; @@ -735,6 +736,13 @@ class FlutterBuildSystem extends BuildSystem { FileSystem fileSystem, Map currentOutputs, ) { + if (environment.defines[kXcodePreAction] == 'PrepareFramework') { + // If the current build is the PrepareFramework Xcode pre-action, skip + // updating the last build identifier and cleaning up the previous build + // since this build is not a complete build. + return; + } + final String currentBuildId = fileSystem.path.basename(environment.buildDir.path); final File lastBuildIdFile = environment.outputDir.childFile('.last_build_id'); if (!lastBuildIdFile.existsSync()) { diff --git a/packages/flutter_tools/test/general.shard/xcode_backend_test.dart b/packages/flutter_tools/test/general.shard/xcode_backend_test.dart index 61722bd8f8..86c5203aaf 100644 --- a/packages/flutter_tools/test/general.shard/xcode_backend_test.dart +++ b/packages/flutter_tools/test/general.shard/xcode_backend_test.dart @@ -262,6 +262,7 @@ void main() { '--ExtraGenSnapshotOptions=', '--DartDefines=', '--ExtraFrontEndOptions=', + '-dPreBuildAction=PrepareFramework', 'debug_unpack_ios', ], ), @@ -315,6 +316,7 @@ void main() { '--ExtraGenSnapshotOptions=', '--DartDefines=', '--ExtraFrontEndOptions=', + '-dPreBuildAction=PrepareFramework', 'debug_unpack_ios', ], ), @@ -386,6 +388,7 @@ void main() { '--ExtraGenSnapshotOptions=$extraGenSnapshotOptions', '--DartDefines=$dartDefines', '--ExtraFrontEndOptions=$extraFrontEndOptions', + '-dPreBuildAction=PrepareFramework', '-dCodesignIdentity=$expandedCodeSignIdentity', 'release_unpack_ios', ], @@ -395,6 +398,107 @@ void main() { )..run(); expect(context.stderr, isEmpty); }); + + test('assumes ARCHS based on NATIVE_ARCH if ONLY_ACTIVE_ARCH is YES', () { + final Directory buildDir = fileSystem.directory('/path/to/builds') + ..createSync(recursive: true); + final Directory flutterRoot = fileSystem.directory('/path/to/flutter') + ..createSync(recursive: true); + final File pipe = fileSystem.file('/tmp/pipe') + ..createSync(recursive: true); + const String buildMode = 'Debug'; + final TestContext context = TestContext( + ['prepare'], + { + 'BUILT_PRODUCTS_DIR': buildDir.path, + 'CONFIGURATION': buildMode, + 'FLUTTER_ROOT': flutterRoot.path, + 'INFOPLIST_PATH': 'Info.plist', + 'ARCHS': 'arm64 x86_64', + 'ONLY_ACTIVE_ARCH': 'YES', + 'NATIVE_ARCH': 'arm64e' + }, + commands: [ + FakeCommand( + command: [ + '${flutterRoot.path}/bin/flutter', + 'assemble', + '--no-version-check', + '--output=${buildDir.path}/', + '-dTargetPlatform=ios', + '-dTargetFile=lib/main.dart', + '-dBuildMode=${buildMode.toLowerCase()}', + '-dIosArchs=arm64', + '-dSdkRoot=', + '-dSplitDebugInfo=', + '-dTreeShakeIcons=', + '-dTrackWidgetCreation=', + '-dDartObfuscation=', + '-dAction=', + '-dFrontendServerStarterPath=', + '--ExtraGenSnapshotOptions=', + '--DartDefines=', + '--ExtraFrontEndOptions=', + '-dPreBuildAction=PrepareFramework', + 'debug_unpack_ios', + ], + ), + ], + fileSystem: fileSystem, + scriptOutputStreamFile: pipe, + )..run(); + expect(context.stderr, isEmpty); + }); + + test('does not assumes ARCHS if ONLY_ACTIVE_ARCH is not YES', () { + final Directory buildDir = fileSystem.directory('/path/to/builds') + ..createSync(recursive: true); + final Directory flutterRoot = fileSystem.directory('/path/to/flutter') + ..createSync(recursive: true); + final File pipe = fileSystem.file('/tmp/pipe') + ..createSync(recursive: true); + const String buildMode = 'Debug'; + final TestContext context = TestContext( + ['prepare'], + { + 'BUILT_PRODUCTS_DIR': buildDir.path, + 'CONFIGURATION': buildMode, + 'FLUTTER_ROOT': flutterRoot.path, + 'INFOPLIST_PATH': 'Info.plist', + 'ARCHS': 'arm64 x86_64', + 'NATIVE_ARCH': 'arm64e' + }, + commands: [ + FakeCommand( + command: [ + '${flutterRoot.path}/bin/flutter', + 'assemble', + '--no-version-check', + '--output=${buildDir.path}/', + '-dTargetPlatform=ios', + '-dTargetFile=lib/main.dart', + '-dBuildMode=${buildMode.toLowerCase()}', + '-dIosArchs=arm64 x86_64', + '-dSdkRoot=', + '-dSplitDebugInfo=', + '-dTreeShakeIcons=', + '-dTrackWidgetCreation=', + '-dDartObfuscation=', + '-dAction=', + '-dFrontendServerStarterPath=', + '--ExtraGenSnapshotOptions=', + '--DartDefines=', + '--ExtraFrontEndOptions=', + '-dPreBuildAction=PrepareFramework', + 'debug_unpack_ios', + ], + ), + ], + fileSystem: fileSystem, + scriptOutputStreamFile: pipe, + )..run(); + expect(context.stderr, isEmpty); + }); }); } diff --git a/packages/flutter_tools/test/integration.shard/swift_package_manager_test.dart b/packages/flutter_tools/test/integration.shard/swift_package_manager_test.dart index 0f6f585a99..3dbc489733 100644 --- a/packages/flutter_tools/test/integration.shard/swift_package_manager_test.dart +++ b/packages/flutter_tools/test/integration.shard/swift_package_manager_test.dart @@ -302,6 +302,75 @@ void main() { ); } }, skip: !platform.isMacOS); // [intended] Swift Package Manager only works on macos. + + test('Caches build targets between builds with Swift Package Manager on $platformName', () async { + final Directory workingDirectory = fileSystem.systemTempDirectory + .createTempSync('swift_package_manager_caching.'); + final String workingDirectoryPath = workingDirectory.path; + try { + // Create and build an app using the Swift Package Manager version of + // integration_test. + await SwiftPackageManagerUtils.enableSwiftPackageManager(flutterBin, workingDirectoryPath); + + final String appDirectoryPath = await SwiftPackageManagerUtils.createApp( + flutterBin, + workingDirectoryPath, + iosLanguage: 'swift', + platform: platformName, + usesSwiftPackageManager: true, + options: ['--platforms=$platformName'], + ); + SwiftPackageManagerUtils.addDependency(appDirectoryPath: appDirectoryPath, plugin: integrationTestPlugin); + + final String unpackTarget = 'debug_unpack_$platformName'; + final String bundleFlutterAssetsTarget = 'debug_${platformName}_bundle_flutter_assets'; + final bool noCodesign = platformName == 'ios'; + await SwiftPackageManagerUtils.buildApp( + flutterBin, + appDirectoryPath, + options: [ + platformName, + '--debug', + '-v', + if (noCodesign) + '--no-codesign', + ], + expectedLines: [ + r'SchemeAction Run\ Prepare\ Flutter\ Framework\ Script', + '$unpackTarget: Starting due to', + '-dPreBuildAction=PrepareFramework $unpackTarget', + ], + unexpectedLines: [], + ); + + await SwiftPackageManagerUtils.buildApp( + flutterBin, + appDirectoryPath, + options: [ + platformName, + '--debug', + '-v', + if (noCodesign) + '--no-codesign', + ], + expectedLines: [ + r'SchemeAction Run\ Prepare\ Flutter\ Framework\ Script', + 'Skipping target: $unpackTarget', + 'Skipping target: $bundleFlutterAssetsTarget', + ], + unexpectedLines: [ + 'Starting due to', + ], + ); + + } finally { + await SwiftPackageManagerUtils.disableSwiftPackageManager(flutterBin, workingDirectoryPath); + ErrorHandlingFileSystem.deleteIfExists( + workingDirectory, + recursive: true, + ); + } + }, skip: !platform.isMacOS); // [intended] Swift Package Manager only works on macos. } test('Build ios-framework with module app uses CocoaPods', () async { diff --git a/packages/flutter_tools/test/integration.shard/swift_package_manager_utils.dart b/packages/flutter_tools/test/integration.shard/swift_package_manager_utils.dart index dd23eaed9f..1dc52e3546 100644 --- a/packages/flutter_tools/test/integration.shard/swift_package_manager_utils.dart +++ b/packages/flutter_tools/test/integration.shard/swift_package_manager_utils.dart @@ -136,8 +136,10 @@ class SwiftPackageManagerUtils { } remainingExpectedLines.remove(trimmedLine); remainingExpectedLines.removeWhere((Pattern expectedLine) => trimmedLine.contains(expectedLine)); - if (unexpectedLines != null && unexpectedLines.contains(trimmedLine)) { - unexpectedLinesFound.add(trimmedLine); + if (unexpectedLines != null) { + if (unexpectedLines.where((String unexpectedLine) => trimmedLine.contains(unexpectedLine)).firstOrNull != null) { + unexpectedLinesFound.add(trimmedLine); + } } } expect(