Skip to content

[native_assets_cli] Unify Metadata with Assets #2164

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Apr 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .github/workflows/native.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,12 @@ jobs:
- run: dart pub get -C test_data/native_dynamic_linking/
if: ${{ matrix.package == 'native_assets_builder' }}

- run: dart pub get -C test_data/reusable_dynamic_library/
if: ${{ matrix.package == 'native_assets_builder' }}

- run: dart pub get -C test_data/reuse_dynamic_library/
if: ${{ matrix.package == 'native_assets_builder' }}

- run: dart pub get -C test_data/no_hook/
if: ${{ matrix.package == 'native_assets_builder' }}

Expand Down
5 changes: 3 additions & 2 deletions pkgs/native_assets_builder/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
## 0.13.1-wip
## 0.14.0-wip

- Nothing yet.
- Bump `package:native_assets_cli` to 0.14.0.
- Route assets from build hook to build hook with `ToBuild` `Routing`.

## 0.13.0

Expand Down
42 changes: 36 additions & 6 deletions pkgs/native_assets_builder/lib/src/build_runner/build_runner.dart
Original file line number Diff line number Diff line change
Expand Up @@ -91,22 +91,33 @@ class NativeAssetsBuildRunner {
if (buildPlan == null) return null;

var hookResult = HookResult();

/// Key is packageName.
final globalMetadata = <String, Metadata>{};

/// Key is packageName.
final globalAssetsForBuild = <String, List<EncodedAsset>>{};
for (final package in buildPlan) {
final metadata = <String, Metadata>{};
_metadataForPackage(
packageGraph: packageGraph!,
final metadata =
_metadataForPackage(
packageGraph: packageGraph!,
packageName: package.name,
targetMetadata: globalMetadata,
) ??
{};
final assetsForBuild = _assetsForBuildForPackage(
packageGraph: packageGraph,
packageName: package.name,
targetMetadata: globalMetadata,
)?.forEach((key, value) => metadata[key] = value);
globalAssetsForBuild: globalAssetsForBuild,
);

final inputBuilder = BuildInputBuilder();

for (final e in extensions) {
e.setupBuildInput(inputBuilder);
}
inputBuilder.config.setupBuild(linkingEnabled: linkingEnabled);
inputBuilder.setupBuildInput(metadata: metadata);
inputBuilder.setupBuildInput(metadata: metadata, assets: assetsForBuild);

final (buildDirUri, outDirUri, outDirSharedUri) = await _setupDirectories(
Hook.build,
Expand Down Expand Up @@ -152,6 +163,8 @@ class NativeAssetsBuildRunner {
final (hookOutput, hookDeps) = result;
hookResult = hookResult.copyAdd(hookOutput, hookDeps);
globalMetadata[package.name] = (hookOutput as BuildOutput).metadata;
globalAssetsForBuild[package.name] =
hookOutput.assets.encodedAssetsForBuild;
}

// We only perform application wide validation in the final result of
Expand Down Expand Up @@ -732,6 +745,23 @@ ${compileResult.stdout}
};
}

/// Returns only the assets output as assetForBuild by the packages that are
/// the direct dependencies of [packageName].
Map<String, List<EncodedAsset>>? _assetsForBuildForPackage({
required PackageGraph packageGraph,
required String packageName,
Map<String, List<EncodedAsset>>? globalAssetsForBuild,
}) {
if (globalAssetsForBuild == null) {
return null;
}
final dependencies = packageGraph.neighborsOf(packageName).toSet();
return {
for (final entry in globalAssetsForBuild.entries)
if (dependencies.contains(entry.key)) entry.key: entry.value,
};
}

Future<ValidationErrors> _validate(
HookInput input,
HookOutput output,
Expand Down
2 changes: 1 addition & 1 deletion pkgs/native_assets_builder/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
name: native_assets_builder
description: >-
This package is the backend that invokes build hooks.
version: 0.13.1-wip
version: 0.14.0-wip
repository: https://github.com/dart-lang/native/tree/main/pkgs/native_assets_builder

publish_to: none
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Copyright (c) 2024, 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.

@OnPlatform({'mac-os': Timeout.factor(2), 'windows': Timeout.factor(10)})
library;

import 'package:test/test.dart';

import '../build_runner/helpers.dart';
import '../helpers.dart';

void main() async {
const name = 'reuse_dynamic_library';

test(
'$name build',
() => inTempDir((tempUri) async {
await copyTestProjects(targetUri: tempUri);
final packageUri = tempUri.resolve('$name/');

await runPubGet(workingDirectory: packageUri, logger: logger);

final logMessages = <String>[];
final result =
(await build(
packageUri,
logger,
dartExecutable,
capturedLogs: logMessages,
buildAssetTypes: [BuildAssetType.code],
))!;

expect(result.encodedAssets.length, 2);
}),
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ void main(List<String> arguments) async {
input: input,
output: output,
logger: logger,
linkInPackage: 'add_asset_link',
routing: const [ToLinkHook('add_asset_link')],
);
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,10 @@ void main(List<String> args) async {

output.assets.data.add(
DataAsset(package: packageName, name: name, file: dataAsset.uri),
linkInPackage: input.config.linkingEnabled ? packageName : null,
routing:
input.config.linkingEnabled
? ToLinkHook(packageName)
: const ToAppBundle(),
);
// TODO(https://github.com/dart-lang/native/issues/1208): Report
// dependency on asset.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,10 @@ void main(List<String> args) async {
final forLinking = name.contains('2') || name.contains('3');
output.assets.data.add(
DataAsset(package: packageName, name: name, file: dataAsset.uri),
linkInPackage:
forLinking && input.config.linkingEnabled ? 'complex_link' : null,
routing:
forLinking && input.config.linkingEnabled
? const ToLinkHook('complex_link')
: const ToAppBundle(),
);
// TODO(https://github.com/dart-lang/native/issues/1208): Report
// dependency on asset.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,30 +14,22 @@ void main(List<String> arguments) async {
..onRecord.listen((record) {
print('${record.level.name}: ${record.time}: ${record.message}');
});
final linkInPackage =
input.config.linkingEnabled ? input.packageName : null;
final routing =
input.config.linkingEnabled
? [ToLinkHook(input.packageName)]
: [const ToAppBundle()];
await CBuilder.library(
name: 'add',
assetName: 'dylib_add',
sources: ['src/native_add.c'],
linkModePreference: LinkModePreference.dynamic,
).run(
input: input,
output: output,
logger: logger,
linkInPackage: linkInPackage,
);
).run(input: input, output: output, logger: logger, routing: routing);

await CBuilder.library(
name: 'multiply',
assetName: 'dylib_multiply',
sources: ['src/native_multiply.c'],
linkModePreference: LinkModePreference.dynamic,
).run(
input: input,
output: output,
logger: logger,
linkInPackage: linkInPackage,
);
).run(input: input, output: output, logger: logger, routing: routing);
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,10 @@ void main(List<String> arguments) async {
file: input.packageRoot.resolve('assets/data.json'),
package: input.packageName,
),
linkInPackage:
input.config.linkingEnabled ? 'fail_on_os_sdk_version_linker' : null,
routing:
input.config.linkingEnabled
? const ToLinkHook('fail_on_os_sdk_version_linker')
: const ToAppBundle(),
);
});
}
15 changes: 15 additions & 0 deletions pkgs/native_assets_builder/test_data/manifest.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,21 @@
- relative_path/assets/test_asset.txt
- relative_path/hook/build.dart
- relative_path/pubspec.yaml
- reusable_dynamic_library/ffigen.yaml
- reusable_dynamic_library/hook/build.dart
- reusable_dynamic_library/lib/add.dart
- reusable_dynamic_library/lib/hook.dart
- reusable_dynamic_library/pubspec.yaml
- reusable_dynamic_library/src/add.c
- reusable_dynamic_library/src/add.h
- reusable_dynamic_library/test/add_test.dart
- reuse_dynamic_library/ffigen.yaml
- reuse_dynamic_library/hook/build.dart
- reuse_dynamic_library/lib/my_add.dart
- reuse_dynamic_library/pubspec.yaml
- reuse_dynamic_library/src/my_add.c
- reuse_dynamic_library/src/my_add.h
- reuse_dynamic_library/test/add_test.dart
- simple_data_asset/assets/test_asset.txt
- simple_data_asset/bin/deep_modify_data_asset.dart.debug
- simple_data_asset/bin/modify_data_asset.dart.debug
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,10 @@ void main(List<String> arguments) async {
output.assets.code.add(
tempBuildOutput.assets.code.single,
// Send dylib to linking if linking is enabled.
linkInPackage: input.config.linkingEnabled ? packageName : null,
routing:
Copy link
Member

Choose a reason for hiding this comment

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

Just an idea: We could avoid the input.config.linkingEnabled bool calls by instead having something like an input.config.routing, which can be a LinkingEnabledRouter which has a toLinker method, or a LinkingNotEnabledRouter which only has bundleInApp or toBuilder`.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I like having a input.config.routing. 👍 That makes the API symmetric again.

I'm not sure that having to take the value from the input would make a very nice API. Especially for ToBuild or BundleInApp where you don't need to check the config. Let me give it a try.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I gave this a spin, but make the call sites even more verbose.

      routing: switch (input.config.routing) {
        final LinkingEnabledRouter r => r.toLinkHook(packageName),
        LinkingDisabledRouter() => const ToAppBundle(),
      },

Then the following is more readable:

      routing: switch (input.config.routing) {
        LinkingEnabledRouter() => ToLinkHook(packageName),
        LinkingDisabledRouter() => const ToAppBundle(),
      },

Current solution (this doesn't show the routing)

      routing: switch (input.config.linkingEnabled) {
        true => ToLinkHook(packageName),
        false => const ToAppBundle(),
      },

With routing added:

      routing: switch (input.config.routing.linkHooksEnabled) {
        true => ToLinkHook(packageName),
        false => const ToAppBundle(),
      },

True false is not very pretty. Let's try a when

      routing: switch (input.config.routing) {
        final routing when routing.linkHooksEnabled => ToLinkHook(packageName),
        _ => const ToAppBundle(),
      },

Also not that great.

Then the following is more readable:

      routing: switch (input.config.routing) {
        LinkHooksEnabled() => ToLinkHook(packageName),
        LinkHooksDisabled() => const ToAppBundle(),
      },

But it only works if that's the only option we ever want to add to the routing. It's only the Dart API, so we could do it, it's easy to refactor.

(P.S. If we want to go the route with the method, we should probably do the same with LinkModePreference and LinkMode.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Let's try this in a more holistic way for everywhere where the input-output constraint could be made more explicit.

#2175

input.config.linkingEnabled
? ToLinkHook(packageName)
: const ToAppBundle(),
);
output.addDependencies(tempBuildOutput.dependencies);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,21 @@
// 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.

// ignore_for_file: deprecated_member_use

import 'package:native_assets_cli/native_assets_cli.dart';

void main(List<String> args) async {
await build(args, (input, _) async {
final someValue = input.metadatum('package_with_metadata', 'some_key');
final someValue = input.metadata['package_with_metadata']['some_key'];
assert(someValue != null);
final someInt = input.metadatum('package_with_metadata', 'some_int');
final someInt = input.metadata['package_with_metadata']['some_int'];
assert(someInt != null);
print({'some_int': someInt, 'some_key': someValue});

// ignore: deprecated_member_use
final someValueOld = input.metadatum('package_with_metadata', 'some_key');
assert(someValueOld != null);
// ignore: deprecated_member_use
final someIntOld = input.metadatum('package_with_metadata', 'some_int');
assert(someIntOld != null);
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import 'package:native_assets_cli/native_assets_cli.dart';

void main(List<String> arguments) async {
await build(arguments, (input, output) async {
output.metadata.addAll({'some_key': 'some_value', 'some_int': 3});
// ignore: deprecated_member_use
output.addMetadata({'some_key': 'some_value', 'some_int': 3});
});
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
An example of a package with a dynamic library that can be linked against in
a dependent package.

## Usage

Run tests with `dart --enable-experiment=native-assets test`.

Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# Run with `flutter pub run ffigen --config ffigen.yaml`.
name: AddBindings
description: |
Bindings for `src/add.h`.

Regenerate bindings with `flutter pub run ffigen --config ffigen.yaml`.
output: 'lib/add.dart'
headers:
entry-points:
- 'src/add.h'
include-directives:
- 'src/add.h'
preamble: |
// 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.
comments:
style: any
length: full
ffi-native:
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// Copyright (c) 2024, 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:logging/logging.dart';
import 'package:native_assets_cli/native_assets_cli.dart';
import 'package:native_toolchain_c/native_toolchain_c.dart';

void main(List<String> args) async {
await build(args, (input, output) async {
final logger =
Logger('')
..level = Level.ALL
..onRecord.listen((record) => print(record.message));

final builder = CBuilder.library(
name: 'add',
assetName: 'add.dart',
sources: ['src/add.c'],
buildMode: BuildMode.debug,
);

await builder.run(
input: input,
output: output,
logger: logger,
routing: const [
// Bundle the dylib in the app, someone might use it.
ToAppBundle(),
// Enable other packages to link to the dylib.
ToBuildHooks(),
],
);

// Enable other packages to find the headers.
output.metadata['include'] = input.packageRoot.resolve('src/').toFilePath();
});
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
// 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.

// AUTO GENERATED FILE, DO NOT EDIT.
//
// Generated by `package:ffigen`.
// ignore_for_file: type=lint
import 'dart:ffi' as ffi;

@ffi.Native<ffi.Int32 Function(ffi.Int32, ffi.Int32)>(symbol: 'add')
external int add(int a, int b);
Loading