-
Notifications
You must be signed in to change notification settings - Fork 40
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] Dependencies should be per asset #1208
Comments
We should probably merge the addition of dependencies and assets into a single add function. Merging in
|
Nesting dependencies under assets will not fully work. A build hook can also compile an executable that is then stored in the metadata, which is then used other build hooks. For example (along the lines of flutter/flutter#144259).
If we want to support hot-reload for those compiled shaders, the question is whether the However, if we're cold starting, most likely we should check if any of the dependencies for the So we need to also store dependencies that are not tied to an asset, but tied to the hook itself. We could specify that these dependencies are ignored when hot-reloading/hot-restarting. But maybe there are use cases where something from the meta-data should actually be rebuilt in a hot-reload? (If we want to go really wild, we could rethink whether storing an executable in meta data in an unstructured way is the right way to go. Then we could possibly track the dependencies per file that we possible store in metadata. But now we're building a full fledged build system. 😆 ) Thoughts @mkustermann @mosuem @HosseinYousefi? |
Another situation in which this will not fully work is when writing hooks that report all files in an void main(List<String> args) async {
await build(
args,
(config, output) async {
final assetDirectory =
Directory.fromUri(config.packageRoot.resolve('assets/'));
// If assets are added, rerun hook. <------------------------
output.addDependency(assetDirectory.uri);
await for (final dataAsset in assetDirectory.list()) {
if (dataAsset is! File) {
continue;
}
final name = dataAsset.uri.pathSegments.last;
output.addAsset(
DataAsset(
package: config.packageName,
name: name,
file: dataAsset.uri,
),
dependencies: [
dataAsset.uri,
],
);
}
},
);
} So, we'll likely need to keep the "hook dependencies" in addition to asset dependencies. Maybe the "hook dependencies" need to be reported per asset type, because the hot restart hook would only want to build some asset types (#1207). |
While working on #1208, I released that most likely you'd want to report all files from the `assets/` directory as `DataAsset`s in a hook. (This also means that there's implicitly a `dependency` on the directory itself.) This PR also cleans up the remaining dart files listed in `dependencies`.
Circling back here after discussing #1368 with @mosuem. One aspect of the design we don't really like is having both We need some kind of global dependencies for knowing that a hook needs to be re-run if a file gets added in a directory: The current PR looks like: void main(List<String> args) async {
await build(args, (config, output) async {
final packageName = config.packageName;
final assetDirectory =
Directory.fromUri(config.packageRoot.resolve('assets/'));
// If assets are added, rerun hook.
output.addAssetTypeDependency(DataAsset.type, assetDirectory.uri);
await for (final dataAsset in assetDirectory.list()) {
if (dataAsset is! File) {
continue;
}
// The file path relative to the package root, with forward slashes.
final name = dataAsset.uri
.toFilePath(windows: false)
.substring(config.packageRoot.toFilePath(windows: false).length);
output.addAsset(
DataAsset(
package: packageName,
name: name,
file: dataAsset.uri,
),
linkInPackage: config.linkingEnabled ? packageName : null,
dependencies: [dataAsset.uri],
);
}
});
} Some alternatives:
I think we're leaning towards 2.b. Friendly ping for @HosseinYousefi and @mkustermann. |
From discussion with @HosseinYousefi:
|
From more discussion with @HosseinYousefi: If we report dependencies per asset, we expect the build system to be smart and do something per asset as well (#1375 option 2). But it's not clear that we would benefit from doing that unless we make a fine-grained build graph that tracks every asset and its dependencies individually with a closure to rerun that specific part of the build. And that would lead us to basically making a full build system, which we're trying to avoid. @HosseinYousefi suggested we take a completely different direction: #1207 (comment). |
As discussed in #1207 (comment), we don't want assets per dependency. |
Currently, the
dependencies
is a list of dependencies for all assets together. (This was modeled after the Rust build.rs.) But in the context of hot-reload/hot-restart we might want to only rebuild some assets (#1207). So then we'd need to know the dependencies per asset.Some open questions:
assetId
optional. E.g. an assetId would only be needed for things referenced from Dart code. And not for JARs for example. But we'll likely need some way to uniquely identify an asset.Version skew:
The text was updated successfully, but these errors were encountered: