Skip to content

Conversation

@simolus3
Copy link
Contributor

@simolus3 simolus3 commented Nov 30, 2025

When a Flutter app depends on a package using hooks to add code assets, those get built to build/native_assets/$platform, where $platform is something like ios or macos. Crucially, there's no difference between simulator or release builds here, all native assets for a platform end up in that directory.

To embed those frameworks with the app, the "sign and embed" stage of an XCode build invokes xcode_backend.dart, which then copies all frameworks from build/native_assets/$targetPlatform into $build/Runner.app/Frameworks. This is a problem when a developer runs a simulator build followed by a release build without clearing the build folder in between, since both assets would be in build/native_assets/ios at that point.

This fixes the issue by:

  1. Reading the native_assets.json file emitted by the main build.
  2. Only copying frameworks referenced in that file.

This still needs an integration test.

Closes #178602.

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

@simolus3 simolus3 requested a review from a team as a code owner November 30, 2025 19:25
@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

@github-actions github-actions bot added tool Affects the "flutter" command-line tool. See also t: labels. team-ios Owned by iOS platform team labels Nov 30, 2025
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request changes the logic for embedding native assets in Xcode builds. Instead of copying all assets, it now reads the native_assets.json file to determine which assets are actually referenced by the build, and only copies those. This prevents bundling of unreferenced assets, for example from a previous simulator build when creating a release build. The changes look good and address the issue described. I've added a couple of suggestions to improve the robustness of the JSON and path parsing logic.

Comment on lines +382 to +383
final nativeAssetsSpec = json.decode(nativeAssetsJson.readAsStringSync()) as Map;
for (final Object? perPlatform
in (nativeAssetsSpec['native-assets'] as Map<String, Object?>).values) {
for (final Object? asset in (perPlatform! as Map<String, Object?>).values) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The JSON parsing here could be made more robust. The current implementation uses several unsafe casts (as Map, as Map<String, Object?>) and a null assertion (!) which could lead to runtime errors if native_assets.json has an unexpected structure or is empty/malformed. Consider using type checks and safer access patterns to make this code more resilient against such issues.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better to have a reusable format, I've filed dart-lang/native#2860. For now you could consider simply copy-pasting the validate method from pkg/vm/lib/native_assets/validator.dart from the Dart SDK.

For the error messages we should probably follow: // "error:" prefix makes this show up as an Xcode compilation error..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW I don't think we can import any packages into xcode_backend.dart, so even if there was a Dart package it wouldn't help for this.

But to be honest, I don't really see the value of doing a complete validation on that file. It's generated via hooks_runner in the main build, and we can reasonably expect it to be well-formed. There are plenty of places in xcode_backend.dart relying on the main build emitting files at specific paths that would crash on a mismatch. I don't see why this specifically should be checked more defensively.

I can copy and adapt the validator if you think that improves the logic here. But to me, xcode_backend.dart reads like it's specifically written to do as little as possible and doesn't import any libraries outside of the Dart SDK. Copying the entire validator (which would be around 30% of that file when added) just to be able to generate better error messages when we're unable to extract absolute framework paths just doesn't sound that helpful to me.

for (final Object? asset in (perPlatform! as Map<String, Object?>).values) {
if (asset case ['absolute', final String frameworkPath]) {
// frameworkPath is usually something like sqlite3arm64ios.framework/sqlite3arm64ios
final [String directory, String name] = frameworkPath.split('/');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using pattern destructuring [String directory, String name] here is concise, but it will throw an exception if frameworkPath.split('/') does not produce a list with exactly two elements. It's safer to check the length of the list before destructuring to handle unexpected path formats gracefully.

Suggested change
final [String directory, String name] = frameworkPath.split('/');
final List<String> pathParts = frameworkPath.split('/');
if (pathParts.length != 2) {
throw Exception('Unexpected framework path format: $frameworkPath. Expected "name.framework/name".');
}
final String directory = pathParts[0];
final String name = pathParts[1];

Copy link
Member

@jmagman jmagman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you're operating at the wrong layer here. last_build_id is created and used in service of the build system fingerprinting system, but you're hijacking that fingerprinting one layer too high.
Read about it in the long docs around here:

/// To determine if the action for a target needs to be executed, the
/// [BuildSystem] computes a key of the file contents for both inputs and
/// outputs. This is tracked separately in the [FileStore]. The key may
/// be either an md5 hash of the file contents or a timestamp.
///
/// A Target has both implicit and explicit inputs and outputs. Only the
/// latter are safe to evaluate before invoking the [build] action. For example,
/// a wildcard output pattern requires the outputs to exist before it can
/// glob files correctly.
///
/// - All listed inputs are considered explicit inputs.
/// - Outputs which are provided as [Source.pattern].
/// without wildcards are considered explicit.
/// - The remaining outputs are considered implicit.
///
/// For each target, executing its action creates a corresponding stamp file
/// which records both the input and output files. This file is read by
/// subsequent builds to determine which file hashes need to be checked. If the
/// stamp file is missing, the target's action is always rerun.

I suspect some native asset files (native_assets.json ?) need to be added to the inputs and/or outputs of the build system.

Example:

List<Source> get inputs => const <Source>[
Source.pattern('{FLUTTER_ROOT}/packages/flutter_tools/lib/src/build_system/targets/ios.dart'),
Source.pattern('{BUILD_DIR}/app.dill'),
Source.artifact(Artifact.engineDartBinary),
Source.artifact(Artifact.skyEnginePath),

cc @dcharkes

@dcharkes
Copy link
Contributor

dcharkes commented Dec 2, 2025

When a Flutter app depends on a package using hooks to add code assets, those get built to build/native_assets/$platform, where $platform is something like ios or macos. Crucially, there's no difference between simulator or release builds here, all native assets for a platform end up in that directory.

To embed those frameworks with the app, the "sign and embed" stage of an XCode build invokes xcode_backend.dart, which then copies all frameworks from build/native_assets/$targetPlatform into $build/Runner.app/Frameworks. This is a problem when a developer runs a simulator build followed by a release build without clearing the build folder in between, since both assets would be in build/native_assets/ios at that point.

This fixes the issue by:

  1. Reading the native_assets.json file emitted by the main build.
  2. Only copying frameworks referenced in that file.

Nice catch @simolus3!

(FYI. I'm currently OOO, so I will only have time to review this in 2 weeks when I'm back.)

Copy link
Contributor

@dcharkes dcharkes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @simolus3!

This still needs an integration test.

This could be done in packages/flutter_tools/test/integration.shard/isolated/native_assets_test.dart, with running a flutter run on the simulator with a native asset in a dev dependency. And subsequently running flutter build in release mode. However, that test already runs quite long.

Can we check whether the files not referenced get copied in packages/flutter_tools/test/general.shard/xcode_backend_test.dart or packages/flutter_tools/test/integration.shard/xcode_backend_test.dart instead?

LGTM once covered with test.

Comment on lines +382 to +383
final nativeAssetsSpec = json.decode(nativeAssetsJson.readAsStringSync()) as Map;
for (final Object? perPlatform
in (nativeAssetsSpec['native-assets'] as Map<String, Object?>).values) {
for (final Object? asset in (perPlatform! as Map<String, Object?>).values) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be better to have a reusable format, I've filed dart-lang/native#2860. For now you could consider simply copy-pasting the validate method from pkg/vm/lib/native_assets/validator.dart from the Dart SDK.

For the error messages we should probably follow: // "error:" prefix makes this show up as an Xcode compilation error..

@simolus3 simolus3 force-pushed the only-copy-referenced-native-assets branch from d78ee30 to c3dca22 Compare December 16, 2025 21:44
@github-actions github-actions bot added the engine flutter/engine related. See also e: labels. label Dec 16, 2025
@simolus3
Copy link
Contributor Author

simolus3 commented Dec 16, 2025

Can we check whether the files not referenced get copied in packages/flutter_tools/test/general.shard/xcode_backend_test.dart or packages/flutter_tools/test/integration.shard/xcode_backend_test.dart instead?

I've adapted existing tests in packages/flutter_tools/test/general.shard/xcode_backend_test.dart to ensure we're not copying additional frameworks for macOS or iOS builds.

Since the existing integration tests pas (edit: whops, actually they don't), that kind of serves as a validation that we are able to parse real NativeAssetsManifest.json files correctly (since frameworks would not get copied otherwise).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

engine flutter/engine related. See also e: labels. team-ios Owned by iOS platform team tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flutter 3.38.1: Failed to Verify when Submitting to App Store Connect

3 participants