Skip to content
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

[native_assets_cli] Unify metadata and assets #1251

Open
dcharkes opened this issue Jul 2, 2024 · 4 comments
Open

[native_assets_cli] Unify metadata and assets #1251

dcharkes opened this issue Jul 2, 2024 · 4 comments
Labels
P2 A bug or feature request we're likely to work on package:native_assets_cli

Comments

@dcharkes
Copy link
Collaborator

dcharkes commented Jul 2, 2024

We currently have 3 types of output of a build hook:

  • assets (for bundling)
  • assets (for link hooks)
  • metadata (for other build hooks)

The most common use cases for metadata are:

  • Lazily downloading a native toolchain and communicating the path to that toolchain. (For example the Android NDK (Installing NDK automatically #976) or the Rust compiler.)
  • Building a toolchain (for example your custom svg-to-tesselated or 3dmodel-to-shader)

Maybe there are other use cases but, we haven't come up with them yet.

If there's a use case for structured data, it might be fine to just put that structured data in a json file and make it a data asset.

Unifying these 3 types of assets would:

  1. Simplify the API. (Currently we have addAsset(...) for assets for bundling and assetAsset(linkIn: 'some_package', ...) for sending it to a link hook, and addMetadata(...) for sending info to another build hook.)
  2. Make dependency tracking per "asset" work properly for executables send to other build hooks ([native_assets_cli] Dependencies should be per asset #1208).

Then we need to come up with some name for these 3 types of "buildables".

  • Probably we should only the things used for bundling "asset"s?

Thanks @mosuem @mkustermann @liamappelbe @HosseinYousefi for the input! And please leave any other thoughts and suggestions.

@dcharkes dcharkes added P1 A high priority bug; for example, a single project is unusable or has many test failures package:native_assets_cli labels Jul 2, 2024
@dcharkes
Copy link
Collaborator Author

dcharkes commented Jul 8, 2024

@bdero Just a heads up, we might break the metadata soon (or temporarily remove it until we find an API that we want to add to do this).

As a workaround, you could try to build the compiler in my_compiler/hook/build.dart and do dart run my_compiler:main to run my_compiler/bin/main.dart which would do the compilation inside my_package_using_compiler/hook/build.dart.

@dcharkes
Copy link
Collaborator Author

I thought I had posted an API sketch here earlier, but apparently I didn't.

My current way of thinking is:

  void addAsset(
    Asset asset, {
    Iterable<Uri>? dependencies,
    bool forBuildHooks, // if true, not bundled, but available to build hooks that have this build hook as direct dependency
    String? linkInPackage, // if set, not bundled directly but sent to specific link hook
  });

This way we can properly track dependencies per asset no matter where these assets are wired to:

auto-submit bot pushed a commit that referenced this issue Jul 15, 2024
We might not address this issue fully before stable.

* #1251

But we're quite sure to remove the current metadata API, so deprecate it.
@mosuem
Copy link
Member

mosuem commented Jul 16, 2024

The API is starting to look a bit overloaded with arguments. I wonder if there is a way to simplify this. Maybe use asset types instead? This would make asset creation a bit more nested.

class MetadataAsset {
  final Asset asset;
}

class LinkableAsset {
  final String targetPackage;
  final Asset asset;
}

@lrhn, as you like to simplify APIs :)

@dcharkes
Copy link
Collaborator Author

The API is starting to look a bit overloaded with arguments. I wonder if there is a way to simplify this. Maybe use asset types instead? This would make asset creation a bit more nested.

class MetadataAsset {
  final Asset asset;
}

class LinkableAsset {
  final String targetPackage;
  final Asset asset;
}

@lrhn, as you like to simplify APIs :)

Or maybe we should have a "Routing" (name tbd) param instead. That would also give us a place to document the behavior.

addAsset(
  Asset asset, {
  Routing routing = const Routing.bundleInApp,
});

final class Routing {
  /// This asset will be bundled in the app by the Dart or Flutter SDK.
  static const Routing bundleInApp;

  /// This asset is available in build hooks of which the packages have a
  /// direct dependency on the package with this build hook.
  static const Routing forBuildHooks;

  /// The asset will not be bundled during the build step, but sent as input to
  /// the link hook of the specified package, where it can be further processed
  /// and possibly bundled.
  factory Routing.linkInPackage(String packageName)
}

@dcharkes dcharkes added P2 A bug or feature request we're likely to work on and removed P1 A high priority bug; for example, a single project is unusable or has many test failures labels Jul 30, 2024
@dcharkes dcharkes added this to the Native Assets v1.0 milestone Aug 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 A bug or feature request we're likely to work on package:native_assets_cli
Projects
None yet
Development

No branches or pull requests

2 participants