diff --git a/pkgs/native_assets_builder/lib/native_assets_builder.dart b/pkgs/native_assets_builder/lib/native_assets_builder.dart index f56216ddc4..5c1d55a75a 100644 --- a/pkgs/native_assets_builder/lib/native_assets_builder.dart +++ b/pkgs/native_assets_builder/lib/native_assets_builder.dart @@ -12,7 +12,10 @@ export 'package:native_assets_builder/src/build_runner/build_runner.dart' LinkInputValidator, LinkValidator, NativeAssetsBuildRunner; -export 'package:native_assets_builder/src/model/build_result.dart'; +export 'package:native_assets_builder/src/model/build_result.dart' + show BuildResult; export 'package:native_assets_builder/src/model/kernel_assets.dart'; -export 'package:native_assets_builder/src/model/link_result.dart'; -export 'package:native_assets_builder/src/package_layout/package_layout.dart'; +export 'package:native_assets_builder/src/model/link_result.dart' + show LinkResult; +export 'package:native_assets_builder/src/package_layout/package_layout.dart' + show PackageLayout; diff --git a/pkgs/native_assets_builder/lib/src/build_runner/build_planner.dart b/pkgs/native_assets_builder/lib/src/build_runner/build_planner.dart index 7b01456a1b..14d7e8bee9 100644 --- a/pkgs/native_assets_builder/lib/src/build_runner/build_planner.dart +++ b/pkgs/native_assets_builder/lib/src/build_runner/build_planner.dart @@ -5,28 +5,37 @@ import 'dart:convert'; import 'dart:io' show Process; +import 'package:file/file.dart'; import 'package:graphs/graphs.dart' as graphs; import 'package:logging/logging.dart'; +import 'package:meta/meta.dart'; +import 'package:native_assets_cli/native_assets_cli_internal.dart'; import 'package:package_config/package_config.dart'; +import '../package_layout/package_layout.dart'; + +@internal class NativeAssetsBuildPlanner { final PackageGraph packageGraph; - final List packagesWithNativeAssets; final Uri dartExecutable; final Logger logger; + final PackageLayout packageLayout; + final FileSystem fileSystem; - NativeAssetsBuildPlanner({ + NativeAssetsBuildPlanner._({ required this.packageGraph, - required this.packagesWithNativeAssets, required this.dartExecutable, required this.logger, + required this.packageLayout, + required this.fileSystem, }); static Future fromPackageConfigUri({ required Uri packageConfigUri, - required List packagesWithNativeAssets, required Uri dartExecutable, required Logger logger, + required PackageLayout packageLayout, + required FileSystem fileSystem, }) async { final workingDirectory = packageConfigUri.resolve('../'); final result = await Process.run( @@ -40,21 +49,67 @@ class NativeAssetsBuildPlanner { ); final packageGraph = PackageGraph.fromPubDepsJsonString(result.stdout as String); - return NativeAssetsBuildPlanner( - packageGraph: packageGraph, - packagesWithNativeAssets: packagesWithNativeAssets, + final packageGraphFromRunPackage = + packageGraph.subGraph(packageLayout.runPackageName); + return NativeAssetsBuildPlanner._( + packageGraph: packageGraphFromRunPackage, dartExecutable: dartExecutable, logger: logger, + packageLayout: packageLayout, + fileSystem: fileSystem, ); } + /// All packages in [PackageLayout.packageConfig] with native assets. + /// + /// Whether a package has native assets is defined by whether it contains + /// a `hook/build.dart` or `hook/link.dart`. + /// + /// For backwards compatibility, a toplevel `build.dart` is also supported. + // TODO(https://github.com/dart-lang/native/issues/823): Remove fallback when + // everyone has migrated. (Probably once we stop backwards compatibility of + // the protocol version pre 1.2.0 on some future version.) + Future> packagesWithHook(Hook hook) async => switch (hook) { + Hook.build => _packagesWithBuildHook ??= + await _runPackagesWithHook(hook), + Hook.link => _packagesWithLinkHook ??= await _runPackagesWithHook(hook), + }; + + List? _packagesWithBuildHook; + List? _packagesWithLinkHook; + + Future> _runPackagesWithHook(Hook hook) async { + final packageNamesInDependencies = packageGraph.vertices.toSet(); + final result = []; + for (final package in packageLayout.packageConfig.packages) { + if (!packageNamesInDependencies.contains(package.name)) { + continue; + } + final packageRoot = package.root; + if (packageRoot.scheme == 'file') { + if (await fileSystem + .file(packageRoot.resolve('hook/').resolve(hook.scriptName)) + .exists() || + await fileSystem + .file(packageRoot.resolve(hook.scriptName)) + .exists()) { + result.add(package); + } + } + } + return result; + } + + List? _buildHookPlan; + /// Plans in what order to run build hooks. /// - /// [runPackageName] provides the entry-point in the graph. The hooks of - /// packages not in the transitive dependencies of [runPackageName] will not - /// be run. - List? plan(String runPackageName) { - final packageGraph = this.packageGraph.subGraph(runPackageName); + /// [PackageLayout.runPackageName] provides the entry-point in the graph. The + /// hooks of packages not in the transitive dependencies of + /// [PackageLayout.runPackageName] will not be run. + Future?> makeBuildHookPlan() async { + if (_buildHookPlan != null) return _buildHookPlan; + final packagesWithNativeAssets = await packagesWithHook(Hook.build); final packageMap = { for (final package in packagesWithNativeAssets) package.name: package }; @@ -77,6 +132,7 @@ class NativeAssetsBuildPlanner { packageMap[stronglyConnectedComponentWithNativeAssets.single]!); } } + _buildHookPlan = result; return result; } } diff --git a/pkgs/native_assets_builder/lib/src/build_runner/build_runner.dart b/pkgs/native_assets_builder/lib/src/build_runner/build_runner.dart index 9b5bd85227..4f9116ff87 100644 --- a/pkgs/native_assets_builder/lib/src/build_runner/build_runner.dart +++ b/pkgs/native_assets_builder/lib/src/build_runner/build_runner.dart @@ -8,6 +8,7 @@ import 'dart:io' show Platform; import 'package:file/file.dart'; import 'package:logging/logging.dart'; +import 'package:meta/meta.dart'; import 'package:native_assets_cli/native_assets_cli_internal.dart'; import 'package:package_config/package_config.dart'; @@ -82,6 +83,16 @@ class NativeAssetsBuildRunner { hookEnvironment = hookEnvironment ?? filteredEnvironment(hookEnvironmentVariablesFilter); + /// Checks whether any hooks need to be run. + /// + /// This method is invoked by launchers such as dartdev (for `dart run`) and + /// flutter_tools (for `flutter run` and `flutter build`). + Future hasBuildHooks() async { + final planner = await _planner; + final packagesWithHook = await planner.packagesWithHook(Hook.build); + return packagesWithHook.isNotEmpty; + } + /// This method is invoked by launchers such as dartdev (for `dart run`) and /// flutter_tools (for `flutter run` and `flutter build`). /// @@ -712,9 +723,9 @@ ${compileResult.stdout} errors.addAll(await validator(input, output)); if (input is BuildInput) { + final planner = await _planner; final packagesWithLink = - (await packageLayout.packagesWithAssets(Hook.link)) - .map((p) => p.name); + (await planner.packagesWithHook(Hook.link)).map((p) => p.name); for (final targetPackage in (output as BuildOutput).assets.encodedAssetsForLinking.keys) { if (!packagesWithLink.contains(targetPackage)) { @@ -731,23 +742,28 @@ ${compileResult.stdout} return errors; } + late final _planner = () async { + final planner = await NativeAssetsBuildPlanner.fromPackageConfigUri( + packageConfigUri: packageLayout.packageConfigUri, + dartExecutable: Uri.file(Platform.resolvedExecutable), + logger: logger, + packageLayout: packageLayout, + fileSystem: _fileSystem, + ); + return planner; + }(); + Future<(List? plan, PackageGraph? dependencyGraph)> _makePlan({ required Hook hook, // TODO(dacoharkes): How to share these two? Make them extend each other? BuildResult? buildResult, }) async { - final packagesWithHook = await packageLayout.packagesWithAssets(hook); final List buildPlan; final PackageGraph? packageGraph; switch (hook) { case Hook.build: - final planner = await NativeAssetsBuildPlanner.fromPackageConfigUri( - packageConfigUri: packageLayout.packageConfigUri, - packagesWithNativeAssets: packagesWithHook, - dartExecutable: Uri.file(Platform.resolvedExecutable), - logger: logger, - ); - final plan = planner.plan(packageLayout.runPackageName); + final planner = await _planner; + final plan = await planner.makeBuildHookPlan(); return (plan, planner.packageGraph); case Hook.link: // Link hooks are not run in any particular order. @@ -755,6 +771,8 @@ ${compileResult.stdout} buildPlan = []; final skipped = []; final encodedAssetsForLinking = buildResult!.encodedAssetsForLinking; + final planner = await _planner; + final packagesWithHook = await planner.packagesWithHook(Hook.link); for (final package in packagesWithHook) { if (encodedAssetsForLinking[package.name]?.isNotEmpty ?? false) { buildPlan.add(package); @@ -802,6 +820,7 @@ ${compileResult.stdout} /// return path.replaceAll('\\', '\\\\').replaceAll(' ', '\\ '); /// } /// ``` +@internal List parseDepFileInputs(String contents) { final output = contents.substring(0, contents.indexOf(': ')); contents = contents.substring(output.length + ': '.length).trim(); @@ -844,6 +863,7 @@ Future> _readDepFile(File depFile) async { return dartSources.map(Uri.file).toList(); } +@internal Map filteredEnvironment(Set allowList) => { for (final entry in Platform.environment.entries) if (allowList.contains(entry.key.toUpperCase())) entry.key: entry.value, diff --git a/pkgs/native_assets_builder/lib/src/package_layout/package_layout.dart b/pkgs/native_assets_builder/lib/src/package_layout/package_layout.dart index 88f388b7dd..813b7e0c80 100644 --- a/pkgs/native_assets_builder/lib/src/package_layout/package_layout.dart +++ b/pkgs/native_assets_builder/lib/src/package_layout/package_layout.dart @@ -3,19 +3,25 @@ // BSD-style license that can be found in the LICENSE file. import 'package:file/file.dart'; -import 'package:native_assets_cli/native_assets_cli_internal.dart'; import 'package:package_config/package_config.dart'; +import '../../native_assets_builder.dart'; + /// Directory layout for dealing with native assets. /// -/// Build hooks for native assets will be run from the context of another root -/// package. +/// For the [NativeAssetsBuildRunner] to correctly run hooks, multiple pieces of +/// information are required: +/// * [packageConfig] to know the list of all packages that may contain hooks. +/// * [packageConfigUri] to be able to get a dependency graph with `pub` and to +/// know where to cache/share asset builds. +/// * [runPackageName] to know which package build hooks to invoke and ignore. +/// Only dependencies of the "run package" are built. /// -/// The directory layout follows pub's convention for caching: +/// The [NativeAssetsBuildRunner] builds assets in +/// `.dart_tool/native_assets_builder`. The directory layout follows pub's +/// convention for caching: /// https://dart.dev/tools/pub/package-layout#project-specific-caching-for-tools class PackageLayout { - final FileSystem _fileSystem; - /// Package config containing the information of where to foot the root [Uri]s /// of other packages. /// @@ -29,7 +35,6 @@ class PackageLayout { final String runPackageName; PackageLayout._( - this._fileSystem, this.packageConfig, this.packageConfigUri, this.runPackageName, @@ -44,7 +49,6 @@ class PackageLayout { assert(fileSystem.file(packageConfigUri).existsSync()); packageConfigUri = packageConfigUri.normalizePath(); return PackageLayout._( - fileSystem, packageConfig, packageConfigUri, runPackageName, @@ -62,7 +66,6 @@ class PackageLayout { assert(await fileSystem.file(packageConfigUri).exists()); final packageConfig = await loadPackageConfigUri(packageConfigUri!); return PackageLayout._( - fileSystem, packageConfig, packageConfigUri, runPackgeName, @@ -121,40 +124,4 @@ class PackageLayout { } return package.root; } - - /// All packages in [packageConfig] with native assets. - /// - /// Whether a package has native assets is defined by whether it contains - /// a `hook/build.dart` or `hook/link.dart`. - /// - /// For backwards compatibility, a toplevel `build.dart` is also supported. - // TODO(https://github.com/dart-lang/native/issues/823): Remove fallback when - // everyone has migrated. (Probably once we stop backwards compatibility of - // the protocol version pre 1.2.0 on some future version.) - Future> packagesWithAssets(Hook hook) async => switch (hook) { - Hook.build => _packagesWithBuildAssets ??= - await _packagesWithHook(hook), - Hook.link => _packagesWithLinkAssets ??= await _packagesWithHook(hook), - }; - - List? _packagesWithBuildAssets; - List? _packagesWithLinkAssets; - - Future> _packagesWithHook(Hook hook) async { - final result = []; - for (final package in packageConfig.packages) { - final packageRoot = package.root; - if (packageRoot.scheme == 'file') { - if (await _fileSystem - .file(packageRoot.resolve('hook/').resolve(hook.scriptName)) - .exists() || - await _fileSystem - .file(packageRoot.resolve(hook.scriptName)) - .exists()) { - result.add(package); - } - } - } - return result; - } } diff --git a/pkgs/native_assets_builder/pubspec.yaml b/pkgs/native_assets_builder/pubspec.yaml index cce634a1b5..4878fac635 100644 --- a/pkgs/native_assets_builder/pubspec.yaml +++ b/pkgs/native_assets_builder/pubspec.yaml @@ -15,6 +15,7 @@ dependencies: file: ^7.0.1 graphs: ^2.3.1 logging: ^1.2.0 + meta: ^1.16.0 # native_assets_cli: ^0.10.0 native_assets_cli: path: ../native_assets_cli/ diff --git a/pkgs/native_assets_builder/test/build_runner/build_planner_test.dart b/pkgs/native_assets_builder/test/build_runner/build_planner_test.dart index 5c2f47df32..01feb4ccb5 100644 --- a/pkgs/native_assets_builder/test/build_runner/build_planner_test.dart +++ b/pkgs/native_assets_builder/test/build_runner/build_planner_test.dart @@ -13,45 +13,6 @@ import '../helpers.dart'; import 'helpers.dart'; void main() async { - test('build dependency graph from pub', () async { - await inTempDir((tempUri) async { - await copyTestProjects(targetUri: tempUri); - final nativeAddUri = tempUri.resolve('native_add/'); - - // First, run `pub get`, we need pub to resolve our dependencies. - await runPubGet(workingDirectory: nativeAddUri, logger: logger); - - final result = await runProcess( - executable: Uri.file(Platform.resolvedExecutable), - arguments: [ - 'pub', - 'deps', - '--json', - ], - workingDirectory: nativeAddUri, - logger: logger, - ); - expect(result.exitCode, 0); - - final graph = PackageGraph.fromPubDepsJsonString(result.stdout); - - final packageLayout = await PackageLayout.fromWorkingDirectory( - const LocalFileSystem(), nativeAddUri, 'native_add'); - final packagesWithNativeAssets = - await packageLayout.packagesWithAssets(Hook.build); - - final planner = NativeAssetsBuildPlanner( - packageGraph: graph, - packagesWithNativeAssets: packagesWithNativeAssets, - dartExecutable: Uri.file(Platform.resolvedExecutable), - logger: logger, - ); - final buildPlan = planner.plan('native_add'); - expect(buildPlan!.length, 1); - expect(buildPlan.single.name, 'native_add'); - }); - }); - test('build dependency graph fromPackageRoot', () async { await inTempDir((tempUri) async { await copyTestProjects(targetUri: tempUri); @@ -62,18 +23,17 @@ void main() async { final packageLayout = await PackageLayout.fromWorkingDirectory( const LocalFileSystem(), nativeAddUri, 'native_add'); - final packagesWithNativeAssets = - await packageLayout.packagesWithAssets(Hook.build); final nativeAssetsBuildPlanner = await NativeAssetsBuildPlanner.fromPackageConfigUri( packageConfigUri: nativeAddUri.resolve( '.dart_tool/package_config.json', ), - packagesWithNativeAssets: packagesWithNativeAssets, dartExecutable: Uri.file(Platform.resolvedExecutable), logger: logger, + packageLayout: packageLayout, + fileSystem: const LocalFileSystem(), ); - final buildPlan = nativeAssetsBuildPlanner.plan('native_add'); + final buildPlan = await nativeAssetsBuildPlanner.makeBuildHookPlan(); expect(buildPlan!.length, 1); expect(buildPlan.single.name, 'native_add'); }); @@ -94,18 +54,17 @@ void main() async { nativeAddUri, runPackageName, ); - final packagesWithNativeAssets = - await packageLayout.packagesWithAssets(Hook.build); final nativeAssetsBuildPlanner = await NativeAssetsBuildPlanner.fromPackageConfigUri( packageConfigUri: nativeAddUri.resolve( '.dart_tool/package_config.json', ), - packagesWithNativeAssets: packagesWithNativeAssets, dartExecutable: Uri.file(Platform.resolvedExecutable), logger: logger, + packageLayout: packageLayout, + fileSystem: const LocalFileSystem(), ); - final buildPlan = nativeAssetsBuildPlanner.plan(runPackageName); + final buildPlan = await nativeAssetsBuildPlanner.makeBuildHookPlan(); expect(buildPlan!.length, 0); }); }); diff --git a/pkgs/native_assets_builder/test/build_runner/pub_workspace_test.dart b/pkgs/native_assets_builder/test/build_runner/pub_workspace_test.dart index 93afb088df..f26e79297b 100644 --- a/pkgs/native_assets_builder/test/build_runner/pub_workspace_test.dart +++ b/pkgs/native_assets_builder/test/build_runner/pub_workspace_test.dart @@ -4,6 +4,8 @@ import 'dart:io'; +import 'package:file/local.dart'; +import 'package:native_assets_builder/native_assets_builder.dart'; import 'package:test/test.dart'; import '../helpers.dart'; @@ -84,4 +86,34 @@ workspace: // Reuse hook results of other packages in the same workspace expect(logs.join('\n'), contains('Skipping build for native_add')); }); + + test('hasBuildHooks', () async { + const fileSystem = LocalFileSystem(); + final packageUri = tempUri.resolve('no_hook/'); + await makePubWorkspace([ + // Some package that has no hooks and has no dependencies with hooks. + 'no_hook', + // Some unrelated package with native assets. + 'dart_app', + 'native_add', + 'native_subtract', + ]); + for (final (runPackageName, hasBuildHooks) in [ + ('no_hook', false), + ('dart_app', true), + ]) { + final packageLayoutNoHook = await PackageLayout.fromWorkingDirectory( + fileSystem, + packageUri, + runPackageName, + ); + final builderNoHook = NativeAssetsBuildRunner( + logger: logger, + dartExecutable: dartExecutable, + fileSystem: fileSystem, + packageLayout: packageLayoutNoHook, + ); + expect(await builderNoHook.hasBuildHooks(), equals(hasBuildHooks)); + } + }); } diff --git a/pkgs/native_assets_builder/test_data/manifest.yaml b/pkgs/native_assets_builder/test_data/manifest.yaml index a429dab2bf..39854857e4 100644 --- a/pkgs/native_assets_builder/test_data/manifest.yaml +++ b/pkgs/native_assets_builder/test_data/manifest.yaml @@ -101,6 +101,8 @@ - native_subtract/src/native_subtract.h - no_asset_for_link/hook/link.dart - no_asset_for_link/pubspec.yaml +- no_hook/lib/no_hook.dart +- no_hook/pubspec.yaml - package_reading_metadata/hook/build.dart - package_reading_metadata/pubspec.yaml - package_with_metadata/hook/build.dart @@ -121,6 +123,12 @@ - simple_link/pubspec.yaml - some_dev_dep/bin/some_dev_dep.dart - some_dev_dep/pubspec.yaml +- system_library/hook/build.dart +- system_library/lib/memory_executable.dart +- system_library/lib/memory_process.dart +- system_library/lib/memory_system.dart +- system_library/pubspec.yaml +- system_library/test/memory_test.dart - transformer/data/data0.json - transformer/data/data1.json - transformer/data/data2.json @@ -157,9 +165,3 @@ - wrong_linker/pubspec.yaml - wrong_namespace_asset/hook/build.dart - wrong_namespace_asset/pubspec.yaml -- system_library/pubspec.yaml -- system_library/hook/build.dart -- system_library/lib/memory_executable.dart -- system_library/lib/memory_process.dart -- system_library/lib/memory_system.dart -- system_library/test/memory_test.dart diff --git a/pkgs/native_assets_builder/test_data/no_hook/lib/no_hook.dart b/pkgs/native_assets_builder/test_data/no_hook/lib/no_hook.dart new file mode 100644 index 0000000000..6698464881 --- /dev/null +++ b/pkgs/native_assets_builder/test_data/no_hook/lib/no_hook.dart @@ -0,0 +1,7 @@ +// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +import 'package:native_assets_cli/code_assets.dart'; + +String get useSomeNativeAssetsCli => CodeAsset.type; diff --git a/pkgs/native_assets_builder/test_data/no_hook/manifest.yaml b/pkgs/native_assets_builder/test_data/no_hook/manifest.yaml new file mode 100644 index 0000000000..b94e452b5f --- /dev/null +++ b/pkgs/native_assets_builder/test_data/no_hook/manifest.yaml @@ -0,0 +1,2 @@ +- lib/no_hook.dart +- pubspec.yaml diff --git a/pkgs/native_assets_builder/test_data/no_hook/pubspec.yaml b/pkgs/native_assets_builder/test_data/no_hook/pubspec.yaml new file mode 100644 index 0000000000..0dec30951e --- /dev/null +++ b/pkgs/native_assets_builder/test_data/no_hook/pubspec.yaml @@ -0,0 +1,24 @@ +name: no_hook +description: Has no hook, but has a dep on native_assets_cli. +version: 0.1.0 + +publish_to: none + +environment: + sdk: '>=3.6.0 <4.0.0' + +dependencies: + logging: ^1.1.1 + # native_assets_cli: ^0.10.0 + native_assets_cli: + path: ../../../native_assets_cli/ + # native_toolchain_c: ^0.7.0 + native_toolchain_c: + path: ../../../native_toolchain_c/ + +dev_dependencies: + ffigen: ^8.0.2 + lints: ^3.0.0 + some_dev_dep: + path: ../some_dev_dep/ + test: ^1.23.1