[macOS] framework gets added twice

[knopp]

Reproducible with package ffi.

flutter create --template=package_ffi test_assets
cd test_assets/example
flutter build macos
ls build/macos/Build/Products/Release/test_assets_example.app/Contents/Frameworks

This will contain

test_assets.framework
test_assets1.framework

cc @dcharkes

[knopp]

My guess would be that on macos Uri frameworkUri(String fileName, Set<String> alreadyTakenNames) is called twice for same framework, probably during lipo?

test_assets.framework is arm64
test_assets1.framework is x86_64

Since these are different architectures, that would mean that they are not linked with the executable either. And otool -L seems to confirm that. EDIT: Maybe linking is not necessary? I'm not entirely sure how @Native resolves functions on macOS. I assumed it was DynamicLibrary.process but maybe I have that wrong.

[dcharkes]

Thanks for the report @knopp!

My guess would be that on macos Uri frameworkUri(String fileName, Set<String> alreadyTakenNames) is called twice for same framework, probably during lipo?

test_assets.framework is arm64 test_assets1.framework is x86_64

Since these are different architectures, that would mean that they are not linked with the executable either. And otool -L seems to confirm that. EDIT: Maybe linking is not necessary? I'm not entirely sure how @Native resolves functions on macOS. I assumed it was DynamicLibrary.process but maybe I have that wrong.

Indeed, we do dynamic loading, not dynamic linking. The mapping from arch+assetId -> framework-name is embedded in the Dart compilation and used at runtime to dlopen the dylib in the framework.

I'm a bit surprised we end up with two frameworks for different archs. We should have all archs in a single framework:

https://github.com/flutter/flutter/blob/e73e7e2e56638b3e2d2f50aadd964fa312205b60/packages/flutter_tools/lib/src/isolated/native_assets/macos/native_assets.dart#L264-L271

(P.S. this is an issue with the implementation in flutter_tools, transferring to flutter repo. Edit: apparently I can't do that.)

[knopp]

@dcharkes, there are two things causing the issue:

• _fatAssetTargetLocations will increment the asset name if present in alreadyTakenNames even if the asset has same Id and is just different target.
• KernelAssetAbsolutePath not having operator== and hashCode implementation. The path is used to group artifacts for lipo, and with default instance equality that fails.

[dcharkes]

Thanks for looking into this @knopp!

• KernelAssetAbsolutePath not having operator== and hashCode implementation. The path is used to group artifacts for lipo, and with default instance equality that fails.

Seems like I broke this recently: #964 Apparently no tests failed in either there, nor in flutter_tools. 🙈

• _fatAssetTargetLocations will increment the asset name if present in alreadyTakenNames even if the asset has same Id and is just different target.

Hm, we'd need to do the group before the renaming. Should be a relatively easy refactor for the iOS/MacOS code.

(I think that currently nothing is broken, we should dlopen at runtime with the different framework paths on the different architectures. But it would still be cleaner to ship the them in the same framework, so we should clean this up.)

Feel free to submit a PR to fix it.

[knopp]

@dcharkes, I have possible solution here: https://github.com/flutter/flutter/pull/144688/files. It doesn't require KernelAssetAbsolutePath operator== override because it reuses instance, but that might be something to consider doing anyway.

[dcharkes]

Yeah, feel free to submit a PR on both repos.

I think grouping on assetId is fine.

[knopp]

I think this has been fixed by flutter/flutter#144688.