Skip to content

Commit

Permalink
Improve build time when using SwiftPM (#150052)
Browse files Browse the repository at this point in the history 
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).
  • Loading branch information
@vashworth
vashworth authored Jun 12, 2024
1 parent d68e05b commit e18c5e2
Show file tree
Hide file tree
Showing 7 changed files with 276 additions and 16 deletions.
48 changes: 38 additions & 10 deletions packages/flutter_tools/bin/macos_assemble.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
Expand All @@ -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
Expand All @@ -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
Expand Down
51 changes: 47 additions & 4 deletions packages/flutter_tools/bin/xcode_backend.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> flutterArgs = _generateFlutterArgsForAssemble(buildMode, verbose);
final List<String> 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(
Expand All @@ -385,7 +396,11 @@ class Context {

final String buildMode = parseFlutterBuildMode();

final List<String> flutterArgs = _generateFlutterArgsForAssemble(buildMode, verbose);
final List<String> flutterArgs = _generateFlutterArgsForAssemble(
'build',
buildMode,
verbose,
);

flutterArgs.add('${buildMode}_ios_bundle_flutter_assets');

Expand All @@ -408,7 +423,11 @@ class Context {
echo('Project $projectPath built and packaged successfully.');
}

List<String> _generateFlutterArgsForAssemble(String buildMode, bool verbose) {
List<String> _generateFlutterArgsForAssemble(
String command,
String buildMode,
bool verbose,
) {
String targetPath = 'lib/main.dart';
if (environment['FLUTTER_TARGET'] != null) {
targetPath = environment['FLUTTER_TARGET']!;
Expand Down Expand Up @@ -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(<String>[
'assemble',
'--no-version-check',
Expand All @@ -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'] ?? ''}',
Expand All @@ -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']}');
}
Expand Down
6 changes: 6 additions & 0 deletions packages/flutter_tools/lib/src/build_info.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, String> _defineEncoder = utf8.encoder.fuse(base64.encoder);
final Converter<String, String> _defineDecoder = base64.decoder.fuse(utf8.decoder);

Expand Down
8 changes: 8 additions & 0 deletions packages/flutter_tools/lib/src/build_system/build_system.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -735,6 +736,13 @@ class FlutterBuildSystem extends BuildSystem {
FileSystem fileSystem,
Map<String, File> 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()) {
Expand Down
104 changes: 104 additions & 0 deletions packages/flutter_tools/test/general.shard/xcode_backend_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,7 @@ void main() {
'--ExtraGenSnapshotOptions=',
'--DartDefines=',
'--ExtraFrontEndOptions=',
'-dPreBuildAction=PrepareFramework',
'debug_unpack_ios',
],
),
Expand Down Expand Up @@ -315,6 +316,7 @@ void main() {
'--ExtraGenSnapshotOptions=',
'--DartDefines=',
'--ExtraFrontEndOptions=',
'-dPreBuildAction=PrepareFramework',
'debug_unpack_ios',
],
),
Expand Down Expand Up @@ -386,6 +388,7 @@ void main() {
'--ExtraGenSnapshotOptions=$extraGenSnapshotOptions',
'--DartDefines=$dartDefines',
'--ExtraFrontEndOptions=$extraFrontEndOptions',
'-dPreBuildAction=PrepareFramework',
'-dCodesignIdentity=$expandedCodeSignIdentity',
'release_unpack_ios',
],
Expand All @@ -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(
<String>['prepare'],
<String, String>{
'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>[
FakeCommand(
command: <String>[
'${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(
<String>['prepare'],
<String, String>{
'BUILT_PRODUCTS_DIR': buildDir.path,
'CONFIGURATION': buildMode,
'FLUTTER_ROOT': flutterRoot.path,
'INFOPLIST_PATH': 'Info.plist',
'ARCHS': 'arm64 x86_64',
'NATIVE_ARCH': 'arm64e'
},
commands: <FakeCommand>[
FakeCommand(
command: <String>[
'${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);
});
});
}

Expand Down
Loading

0 comments on commit e18c5e2

Please sign in to comment.