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_builder] Support pub workspaces #1905

Open
4 of 6 tasks
dcharkes opened this issue Jan 15, 2025 · 3 comments · Fixed by #1921
Open
4 of 6 tasks

[native_assets_builder] Support pub workspaces #1905

dcharkes opened this issue Jan 15, 2025 · 3 comments · Fixed by #1921
Assignees

Comments

@dcharkes
Copy link
Collaborator

dcharkes commented Jan 15, 2025

The native assets builder should take into account pub workspaces:

  • .dart_tool/package_config.json is to possibly be in a ancestor directory
  • runPackageName argument should be required for build/link hooks, so only native assets that are a dependency are built (instead of all packages in the package resolution of the whole workspace).
  • runPackageName argument should be required for "if there are packages with native assets", the native assets experiment only has to be enabled if a dependency has a hook.
  • Native assets should be shared for the whole workspace. (The package resolution is identical, so the native assets should be identical.) I need to check if no information leaks from runPackageName into the BuildInput.

And this needs to be rolled into


Yet another CI issue: Testing native_assets_ci/example/build/native_dynamic_linking.dart fails:

PathNotFoundException: Cannot open file, path = 'D:\a\native\native\pkgs\native_assets_cli\example\build\native_add_app\.dart_tool\package_config.json' (OS Error: The system cannot find the file specified.

That's because the native_assets_builder package itself doesn't understand workspaces:

  static Future<PackageLayout> fromRootPackageRoot(
    FileSystem fileSystem,
    Uri rootPackageRoot,
  ) async {
    rootPackageRoot = rootPackageRoot.normalizePath();
    final packageConfigUri =
        rootPackageRoot.resolve('.dart_tool/package_config.json');  // <--
    assert(await fileSystem.file(packageConfigUri).exists());
    final packageConfig = await loadPackageConfigUri(packageConfigUri);
    return PackageLayout._(
        fileSystem, rootPackageRoot, packageConfig, packageConfigUri);
  }

Originally posted by @Levi-Lesches in #1884 (comment)

@dcharkes dcharkes added this to the Native Assets v1.0 milestone Jan 15, 2025
@dcharkes dcharkes self-assigned this Jan 15, 2025
@Levi-Lesches
Copy link
Contributor

Levi-Lesches commented Jan 16, 2025

For

  • It does not expect .dart_tool/package_config.json to possibly be in a ancestor directory

Could this be changed to use package_config.findPackageConfigConfig? I tried it and it seemed to work at any level in a workspace

@dcharkes
Copy link
Collaborator Author

dcharkes commented Jan 17, 2025

Actually, we'd want to know where the package config is, because we want to build native assets next to it.

  • root
    • pubspec.yaml (workspace)
    • .dart_tool
      • native_assets_builder <-- we want to build native assets here so that we can share the builds between my_package and my_package
    • pkgs
      • my_package
        • pubspec.yaml
        • .dart_tool
      • my_package2

auto-submit bot pushed a commit that referenced this issue Jan 21, 2025
…1919)

All Dart and Flutter commands have a single package config and runPackageName.

Part of a larger refactoring that will make the `NativeAssetsBuildRunner` take more arguments in the constructor such as the package config, and reduces the number of arguments to the `build` and `link` methods.

This will enable:

* Caching `BuildPlanner` logic (only one `runPackageName`)
* Changing `packagesWithAssets` to take `runPackageName` into account (and possibly moving it to the native assets builder instead). #1905
@auto-submit auto-submit bot closed this as completed in 4282a79 Jan 22, 2025
@dcharkes
Copy link
Collaborator Author

We'll still need to roll this into Dart and Flutter. Keeping this open until that's done.

@dcharkes dcharkes reopened this Jan 22, 2025
@dcharkes dcharkes moved this from Done to In Progress in Native Assets Jan 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Progress
Development

Successfully merging a pull request may close this issue.

2 participants