From 1636fbd4cbdf09ba3638de4d7c2eabc84bc933e2 Mon Sep 17 00:00:00 2001 From: Martin Kustermann Date: Fri, 15 Nov 2024 16:23:23 +0100 Subject: [PATCH] Fix code asset copying logic in native asset code (#158984) After the dart build is done, the flutter tool has to bundle the produced shared libraries, which it does that by copying them around. Though the code assumed that all code assets are shared libraries to be bundled, whereas in fact one can have code assets without any actual code (ones that are installed on the target system already or artificial code assets whose symbols get resolved from executable / process). => Using non-bundled code assets currently results in null pointer exceptions and/or cast errors. => We update the copy code to only operate on code assets that have a shared library to bundle. We also update the copy routines by removing copy&past'ed - but slightly different - printing code into the shared caller function. --- .../native_assets/android/native_assets.dart | 45 ++--- .../native_assets/ios/native_assets.dart | 69 +++---- .../native_assets/macos/native_assets.dart | 183 ++++++++---------- .../isolated/native_assets/native_assets.dart | 44 +++-- .../isolated/native_assets_test.dart | 48 +++++ 5 files changed, 212 insertions(+), 177 deletions(-) diff --git a/packages/flutter_tools/lib/src/isolated/native_assets/android/native_assets.dart b/packages/flutter_tools/lib/src/isolated/native_assets/android/native_assets.dart index bb88f52afdb..fb3634365dc 100644 --- a/packages/flutter_tools/lib/src/isolated/native_assets/android/native_assets.dart +++ b/packages/flutter_tools/lib/src/isolated/native_assets/android/native_assets.dart @@ -10,7 +10,6 @@ import '../../../android/gradle_utils.dart'; import '../../../base/common.dart'; import '../../../base/file_system.dart'; import '../../../build_info.dart' hide BuildMode; -import '../../../globals.dart' as globals; int targetAndroidNdkApi(Map environmentDefines) { return int.parse(environmentDefines[kMinSdkVersion] ?? minSdkVersion); @@ -21,30 +20,26 @@ Future copyNativeCodeAssetsAndroid( Map assetTargetLocations, FileSystem fileSystem, ) async { - if (assetTargetLocations.isNotEmpty) { - globals.logger - .printTrace('Copying native assets to ${buildUri.toFilePath()}.'); - final List jniArchDirs = [ - for (final AndroidArch androidArch in AndroidArch.values) - androidArch.archName, - ]; - for (final String jniArchDir in jniArchDirs) { - final Uri archUri = buildUri.resolve('jniLibs/lib/$jniArchDir/'); - await fileSystem.directory(archUri).create(recursive: true); - } - for (final MapEntry assetMapping - in assetTargetLocations.entries) { - final Uri source = assetMapping.key.file!; - final Uri target = (assetMapping.value.path as KernelAssetAbsolutePath).uri; - final AndroidArch androidArch = - _getAndroidArch(assetMapping.value.target); - final String jniArchDir = androidArch.archName; - final Uri archUri = buildUri.resolve('jniLibs/lib/$jniArchDir/'); - final Uri targetUri = archUri.resolveUri(target); - final String targetFullPath = targetUri.toFilePath(); - await fileSystem.file(source).copy(targetFullPath); - } - globals.logger.printTrace('Copying native assets done.'); + assert(assetTargetLocations.isNotEmpty); + final List jniArchDirs = [ + for (final AndroidArch androidArch in AndroidArch.values) + androidArch.archName, + ]; + for (final String jniArchDir in jniArchDirs) { + final Uri archUri = buildUri.resolve('jniLibs/lib/$jniArchDir/'); + await fileSystem.directory(archUri).create(recursive: true); + } + for (final MapEntry assetMapping + in assetTargetLocations.entries) { + final Uri source = assetMapping.key.file!; + final Uri target = (assetMapping.value.path as KernelAssetAbsolutePath).uri; + final AndroidArch androidArch = + _getAndroidArch(assetMapping.value.target); + final String jniArchDir = androidArch.archName; + final Uri archUri = buildUri.resolve('jniLibs/lib/$jniArchDir/'); + final Uri targetUri = archUri.resolveUri(target); + final String targetFullPath = targetUri.toFilePath(); + await fileSystem.file(source).copy(targetFullPath); } } diff --git a/packages/flutter_tools/lib/src/isolated/native_assets/ios/native_assets.dart b/packages/flutter_tools/lib/src/isolated/native_assets/ios/native_assets.dart index a3df69900bf..76842bcd8fb 100644 --- a/packages/flutter_tools/lib/src/isolated/native_assets/ios/native_assets.dart +++ b/packages/flutter_tools/lib/src/isolated/native_assets/ios/native_assets.dart @@ -8,7 +8,6 @@ import 'package:native_assets_cli/code_assets_builder.dart'; import '../../../base/file_system.dart'; import '../../../build_info.dart' hide BuildMode; import '../../../build_info.dart' as build_info; -import '../../../globals.dart' as globals; import '../macos/native_assets_host.dart'; // TODO(dcharkes): Fetch minimum iOS version from somewhere. https://github.com/flutter/flutter/issues/145104 @@ -115,47 +114,41 @@ Future copyNativeCodeAssetsIOS( build_info.BuildMode buildMode, FileSystem fileSystem, ) async { - if (assetTargetLocations.isNotEmpty) { - globals.logger - .printTrace('Copying native assets to ${buildUri.toFilePath()}.'); + assert(assetTargetLocations.isNotEmpty); + final Map oldToNewInstallNames = {}; + final List<(File, String, Directory)> dylibs = <(File, String, Directory)>[]; - final Map oldToNewInstallNames = {}; - final List<(File, String, Directory)> dylibs = <(File, String, Directory)>[]; - - for (final MapEntry> assetMapping - in assetTargetLocations.entries) { - final Uri target = (assetMapping.key as KernelAssetAbsolutePath).uri; - final List sources = [ - for (final CodeAsset source in assetMapping.value) - fileSystem.file(source.file) - ]; - final Uri targetUri = buildUri.resolveUri(target); - final File dylibFile = fileSystem.file(targetUri); - final Directory frameworkDir = dylibFile.parent; - if (!await frameworkDir.exists()) { - await frameworkDir.create(recursive: true); - } - await lipoDylibs(dylibFile, sources); - - final String dylibFileName = dylibFile.basename; - final String newInstallName = - '@rpath/$dylibFileName.framework/$dylibFileName'; - final Set oldInstallNames = await getInstallNamesDylib(dylibFile); - for (final String oldInstallName in oldInstallNames) { - oldToNewInstallNames[oldInstallName] = newInstallName; - } - dylibs.add((dylibFile, newInstallName, frameworkDir)); - - // TODO(knopp): Wire the value once there is a way to configure that in the hook. - // https://github.com/dart-lang/native/issues/1133 - await createInfoPlist(targetUri.pathSegments.last, frameworkDir, minimumIOSVersion: '12.0'); + for (final MapEntry> assetMapping + in assetTargetLocations.entries) { + final Uri target = (assetMapping.key as KernelAssetAbsolutePath).uri; + final List sources = [ + for (final CodeAsset source in assetMapping.value) + fileSystem.file(source.file) + ]; + final Uri targetUri = buildUri.resolveUri(target); + final File dylibFile = fileSystem.file(targetUri); + final Directory frameworkDir = dylibFile.parent; + if (!await frameworkDir.exists()) { + await frameworkDir.create(recursive: true); } + await lipoDylibs(dylibFile, sources); - for (final (File dylibFile, String newInstallName, Directory frameworkDir) in dylibs) { - await setInstallNamesDylib(dylibFile, newInstallName, oldToNewInstallNames); - await codesignDylib(codesignIdentity, buildMode, frameworkDir); + final String dylibFileName = dylibFile.basename; + final String newInstallName = + '@rpath/$dylibFileName.framework/$dylibFileName'; + final Set oldInstallNames = await getInstallNamesDylib(dylibFile); + for (final String oldInstallName in oldInstallNames) { + oldToNewInstallNames[oldInstallName] = newInstallName; } + dylibs.add((dylibFile, newInstallName, frameworkDir)); - globals.logger.printTrace('Copying native assets done.'); + // TODO(knopp): Wire the value once there is a way to configure that in the hook. + // https://github.com/dart-lang/native/issues/1133 + await createInfoPlist(targetUri.pathSegments.last, frameworkDir, minimumIOSVersion: '12.0'); + } + + for (final (File dylibFile, String newInstallName, Directory frameworkDir) in dylibs) { + await setInstallNamesDylib(dylibFile, newInstallName, oldToNewInstallNames); + await codesignDylib(codesignIdentity, buildMode, frameworkDir); } } diff --git a/packages/flutter_tools/lib/src/isolated/native_assets/macos/native_assets.dart b/packages/flutter_tools/lib/src/isolated/native_assets/macos/native_assets.dart index ff7d6305cd6..700157416fa 100644 --- a/packages/flutter_tools/lib/src/isolated/native_assets/macos/native_assets.dart +++ b/packages/flutter_tools/lib/src/isolated/native_assets/macos/native_assets.dart @@ -8,7 +8,6 @@ import 'package:native_assets_cli/code_assets_builder.dart'; import '../../../base/file_system.dart'; import '../../../build_info.dart' hide BuildMode; import '../../../build_info.dart' as build_info; -import '../../../globals.dart' as globals; import 'native_assets_host.dart'; // TODO(dcharkes): Fetch minimum MacOS version from somewhere. https://github.com/flutter/flutter/issues/145104 @@ -127,79 +126,73 @@ Future copyNativeCodeAssetsMacOS( build_info.BuildMode buildMode, FileSystem fileSystem, ) async { - if (assetTargetLocations.isNotEmpty) { - globals.logger.printTrace( - 'Copying native assets to ${buildUri.toFilePath()}.', - ); + assert(assetTargetLocations.isNotEmpty); - final Map oldToNewInstallNames = {}; - final List<(File, String, Directory)> dylibs = <(File, String, Directory)>[]; + final Map oldToNewInstallNames = {}; + final List<(File, String, Directory)> dylibs = <(File, String, Directory)>[]; - for (final MapEntry> assetMapping - in assetTargetLocations.entries) { - final Uri target = (assetMapping.key as KernelAssetAbsolutePath).uri; - final List sources = [ - for (final CodeAsset source in assetMapping.value) fileSystem.file(source.file), - ]; - final Uri targetUri = buildUri.resolveUri(target); - final String name = targetUri.pathSegments.last; - final Directory frameworkDir = fileSystem.file(targetUri).parent; - if (await frameworkDir.exists()) { - await frameworkDir.delete(recursive: true); - } - // MyFramework.framework/ frameworkDir - // MyFramework -> Versions/Current/MyFramework dylibLink - // Resources -> Versions/Current/Resources resourcesLink - // Versions/ versionsDir - // A/ versionADir - // MyFramework dylibFile - // Resources/ resourcesDir - // Info.plist - // Current -> A currentLink - final Directory versionsDir = frameworkDir.childDirectory('Versions'); - final Directory versionADir = versionsDir.childDirectory('A'); - final Directory resourcesDir = versionADir.childDirectory('Resources'); - await resourcesDir.create(recursive: true); - final File dylibFile = versionADir.childFile(name); - final Link currentLink = versionsDir.childLink('Current'); - await currentLink.create(fileSystem.path.relative( - versionADir.path, - from: currentLink.parent.path, - )); - final Link resourcesLink = frameworkDir.childLink('Resources'); - await resourcesLink.create(fileSystem.path.relative( - resourcesDir.path, - from: resourcesLink.parent.path, - )); - await lipoDylibs(dylibFile, sources); - final Link dylibLink = frameworkDir.childLink(name); - await dylibLink.create(fileSystem.path.relative( - versionsDir.childDirectory('Current').childFile(name).path, - from: dylibLink.parent.path, - )); - - final String dylibFileName = dylibFile.basename; - final String newInstallName = '@rpath/$dylibFileName.framework/$dylibFileName'; - final Set oldInstallNames = await getInstallNamesDylib(dylibFile); - for (final String oldInstallName in oldInstallNames) { - oldToNewInstallNames[oldInstallName] = newInstallName; - } - dylibs.add((dylibFile, newInstallName, frameworkDir)); - - await createInfoPlist(name, resourcesDir); + for (final MapEntry> assetMapping + in assetTargetLocations.entries) { + final Uri target = (assetMapping.key as KernelAssetAbsolutePath).uri; + final List sources = [ + for (final CodeAsset source in assetMapping.value) fileSystem.file(source.file), + ]; + final Uri targetUri = buildUri.resolveUri(target); + final String name = targetUri.pathSegments.last; + final Directory frameworkDir = fileSystem.file(targetUri).parent; + if (await frameworkDir.exists()) { + await frameworkDir.delete(recursive: true); } + // MyFramework.framework/ frameworkDir + // MyFramework -> Versions/Current/MyFramework dylibLink + // Resources -> Versions/Current/Resources resourcesLink + // Versions/ versionsDir + // A/ versionADir + // MyFramework dylibFile + // Resources/ resourcesDir + // Info.plist + // Current -> A currentLink + final Directory versionsDir = frameworkDir.childDirectory('Versions'); + final Directory versionADir = versionsDir.childDirectory('A'); + final Directory resourcesDir = versionADir.childDirectory('Resources'); + await resourcesDir.create(recursive: true); + final File dylibFile = versionADir.childFile(name); + final Link currentLink = versionsDir.childLink('Current'); + await currentLink.create(fileSystem.path.relative( + versionADir.path, + from: currentLink.parent.path, + )); + final Link resourcesLink = frameworkDir.childLink('Resources'); + await resourcesLink.create(fileSystem.path.relative( + resourcesDir.path, + from: resourcesLink.parent.path, + )); + await lipoDylibs(dylibFile, sources); + final Link dylibLink = frameworkDir.childLink(name); + await dylibLink.create(fileSystem.path.relative( + versionsDir.childDirectory('Current').childFile(name).path, + from: dylibLink.parent.path, + )); - for (final (File dylibFile, String newInstallName, Directory frameworkDir) in dylibs) { - await setInstallNamesDylib(dylibFile, newInstallName, oldToNewInstallNames); - // Do not code-sign the libraries here with identity. Code-signing - // for bundled dylibs is done in `macos_assemble.sh embed` because the - // "Flutter Assemble" target does not have access to the signing identity. - if (codesignIdentity != null) { - await codesignDylib(codesignIdentity, buildMode, frameworkDir); - } + final String dylibFileName = dylibFile.basename; + final String newInstallName = '@rpath/$dylibFileName.framework/$dylibFileName'; + final Set oldInstallNames = await getInstallNamesDylib(dylibFile); + for (final String oldInstallName in oldInstallNames) { + oldToNewInstallNames[oldInstallName] = newInstallName; } + dylibs.add((dylibFile, newInstallName, frameworkDir)); - globals.logger.printTrace('Copying native assets done.'); + await createInfoPlist(name, resourcesDir); + } + + for (final (File dylibFile, String newInstallName, Directory frameworkDir) in dylibs) { + await setInstallNamesDylib(dylibFile, newInstallName, oldToNewInstallNames); + // Do not code-sign the libraries here with identity. Code-signing + // for bundled dylibs is done in `macos_assemble.sh embed` because the + // "Flutter Assemble" target does not have access to the signing identity. + if (codesignIdentity != null) { + await codesignDylib(codesignIdentity, buildMode, frameworkDir); + } } } @@ -221,40 +214,34 @@ Future copyNativeCodeAssetsMacOSFlutterTester( build_info.BuildMode buildMode, FileSystem fileSystem, ) async { - if (assetTargetLocations.isNotEmpty) { - globals.logger.printTrace( - 'Copying native assets to ${buildUri.toFilePath()}.', - ); + assert(assetTargetLocations.isNotEmpty); - final Map oldToNewInstallNames = {}; - final List<(File, String)> dylibs = <(File, String)>[]; + final Map oldToNewInstallNames = {}; + final List<(File, String)> dylibs = <(File, String)>[]; - for (final MapEntry> assetMapping - in assetTargetLocations.entries) { - final Uri target = (assetMapping.key as KernelAssetAbsolutePath).uri; - final List sources = [ - for (final CodeAsset source in assetMapping.value) fileSystem.file(source.file), - ]; - final Uri targetUri = buildUri.resolveUri(target); - final File dylibFile = fileSystem.file(targetUri); - final Directory targetParent = dylibFile.parent; - if (!await targetParent.exists()) { - await targetParent.create(recursive: true); - } - await lipoDylibs(dylibFile, sources); - final String newInstallName = dylibFile.path; - final Set oldInstallNames = await getInstallNamesDylib(dylibFile); - for (final String oldInstallName in oldInstallNames) { - oldToNewInstallNames[oldInstallName] = newInstallName; - } - dylibs.add((dylibFile, newInstallName)); + for (final MapEntry> assetMapping + in assetTargetLocations.entries) { + final Uri target = (assetMapping.key as KernelAssetAbsolutePath).uri; + final List sources = [ + for (final CodeAsset source in assetMapping.value) fileSystem.file(source.file), + ]; + final Uri targetUri = buildUri.resolveUri(target); + final File dylibFile = fileSystem.file(targetUri); + final Directory targetParent = dylibFile.parent; + if (!await targetParent.exists()) { + await targetParent.create(recursive: true); } - - for (final (File dylibFile, String newInstallName) in dylibs) { - await setInstallNamesDylib(dylibFile, newInstallName, oldToNewInstallNames); - await codesignDylib(codesignIdentity, buildMode, dylibFile); + await lipoDylibs(dylibFile, sources); + final String newInstallName = dylibFile.path; + final Set oldInstallNames = await getInstallNamesDylib(dylibFile); + for (final String oldInstallName in oldInstallNames) { + oldToNewInstallNames[oldInstallName] = newInstallName; } + dylibs.add((dylibFile, newInstallName)); + } - globals.logger.printTrace('Copying native assets done.'); + for (final (File dylibFile, String newInstallName) in dylibs) { + await setInstallNamesDylib(dylibFile, newInstallName, oldToNewInstallNames); + await codesignDylib(codesignIdentity, buildMode, dylibFile); } } diff --git a/packages/flutter_tools/lib/src/isolated/native_assets/native_assets.dart b/packages/flutter_tools/lib/src/isolated/native_assets/native_assets.dart index a6b36b00e01..f50fb43389d 100644 --- a/packages/flutter_tools/lib/src/isolated/native_assets/native_assets.dart +++ b/packages/flutter_tools/lib/src/isolated/native_assets/native_assets.dart @@ -629,6 +629,21 @@ Future _copyNativeCodeAssetsForOS( Map assetTargetLocations, String? codesignIdentity, bool flutterTester) async { + // We only have to copy code assets that are bundled within the app. + // If a code asset that use a linking mode of [LookupInProcess], + // [LookupInExecutable] or [DynamicLoadingSystem] do not have anything to + // bundle as part of the app. + assetTargetLocations = { + for (final CodeAsset codeAsset in assetTargetLocations.keys) + if (codeAsset.linkMode is DynamicLoadingBundled) + codeAsset: assetTargetLocations[codeAsset]!, + }; + + if (assetTargetLocations.isEmpty) { + return; + } + + globals.logger.printTrace('Copying native assets to ${buildUri.toFilePath()}.'); final List codeAssets = assetTargetLocations.keys.toList(); switch (targetOS) { case OS.windows: @@ -673,6 +688,7 @@ Future _copyNativeCodeAssetsForOS( default: throw StateError('This should be unreachable.'); } + globals.logger.printTrace('Copying native assets done.'); } /// Invokes the build of all transitive Dart packages. @@ -908,22 +924,18 @@ Future _copyNativeCodeAssetsToBundleOnWindowsLinux( build_info.BuildMode buildMode, FileSystem fileSystem, ) async { - globals.logger.printTrace('copyNativeCodeAssetsToBundleOnWindowsLinux()'); - if (assetTargetLocations.isNotEmpty) { - globals.logger.printTrace('Copying native assets to ${buildUri.toFilePath()}.'); - final Directory buildDir = fileSystem.directory(buildUri.toFilePath()); - if (!buildDir.existsSync()) { - buildDir.createSync(recursive: true); - } - for (final MapEntry assetMapping in assetTargetLocations.entries) { - final Uri source = assetMapping.key.file!; - final Uri target = (assetMapping.value.path as KernelAssetAbsolutePath).uri; - final Uri targetUri = buildUri.resolveUri(target); - final String targetFullPath = targetUri.toFilePath(); - await fileSystem.file(source).copy(targetFullPath); - globals.logger.printTrace('copyNativeCodeAssetsToBundleOnWindowsLinux(): copied $source to $targetFullPath'); - } - globals.logger.printTrace('Copying native assets done.'); + assert(assetTargetLocations.isNotEmpty); + + final Directory buildDir = fileSystem.directory(buildUri.toFilePath()); + if (!buildDir.existsSync()) { + buildDir.createSync(recursive: true); + } + for (final MapEntry assetMapping in assetTargetLocations.entries) { + final Uri source = assetMapping.key.file!; + final Uri target = (assetMapping.value.path as KernelAssetAbsolutePath).uri; + final Uri targetUri = buildUri.resolveUri(target); + final String targetFullPath = targetUri.toFilePath(); + await fileSystem.file(source).copy(targetFullPath); } } diff --git a/packages/flutter_tools/test/general.shard/isolated/native_assets_test.dart b/packages/flutter_tools/test/general.shard/isolated/native_assets_test.dart index dbb720d5ca6..dce1d3187f4 100644 --- a/packages/flutter_tools/test/general.shard/isolated/native_assets_test.dart +++ b/packages/flutter_tools/test/general.shard/isolated/native_assets_test.dart @@ -180,6 +180,54 @@ void main() { expect(buildRunner.buildDryRunInvocations, 1); }); + testUsingContext('Native assets: non-bundled libraries require no copying', overrides: { + FeatureFlags: () => TestFeatureFlags(isNativeAssetsEnabled: true), + ProcessManager: () => FakeProcessManager.empty(), + }, () async { + final File packageConfig = + environment.projectDir.childFile('.dart_tool/package_config.json'); + final Uri nonFlutterTesterAssetUri = environment.buildDir.childFile('native_assets.yaml').uri; + await packageConfig.parent.create(); + await packageConfig.create(); + + final File directSoFile = environment.projectDir.childFile('direct.so'); + directSoFile.writeAsBytesSync([]); + + CodeAsset makeCodeAsset(String name, LinkMode linkMode, [Uri? file]) + => CodeAsset( + package: 'bar', + name: name, + linkMode: linkMode, + os: OS.linux, + architecture: Architecture.x64, + file: file, + ); + + final List codeAssets = [ + makeCodeAsset('malloc', LookupInProcess()), + makeCodeAsset('free', LookupInExecutable()), + makeCodeAsset('draw', DynamicLoadingSystem(Uri.file('/usr/lib/skia.so'))), + ]; + + await runFlutterSpecificDartBuild( + environmentDefines: { + kBuildMode: BuildMode.release.cliName, + }, + targetPlatform: TargetPlatform.linux_x64, + projectUri: projectUri, + nativeAssetsYamlUri: nonFlutterTesterAssetUri, + fileSystem: fileSystem, + buildRunner: FakeFlutterNativeAssetsBuildRunner( + packagesWithNativeAssetsResult: [ + Package('bar', projectUri), + ], + buildResult: FakeFlutterNativeAssetsBuilderResult.fromAssets(codeAssets: codeAssets), + linkResult: FakeFlutterNativeAssetsBuilderResult.fromAssets(codeAssets: codeAssets), + ), + ); + expect(testLogger.traceText, isNot(contains('Copying native assets to'))); + }); + testUsingContext('build with assets but not enabled', overrides: { ProcessManager: () => FakeProcessManager.empty(), }, () async {