[infra] Use Pub Workspaces

[Levi-Lesches]

Closes #1223

• Update all packages to require Dart 3.6 or higher
• Use Pub workspaces for all packages

TODO:

• Finish remaining packages
• Verify that native_assets_builder/test_data/* still works with dart pub get
• Verify that native_assets_cli/example/* still works with dart pub get
• Verify that dart analyze passes at the workspace level`
• Bump CI to use Dart 3.6 for all packages (stable/beta, migrated dev -> stable)
• Consolidate analysis_options.yaml into workspace
• Verify tests still run (waiting for CI)
• Update changelogs and pubspec versions (Is this needed?)

Leaving these packages unmigrated:

• jnigen/android_test_runner
• native_assets_builder/test_data/native_add_version_skew

Blocked by:

• [native_assets_builder] Support pub workspaces #1905
• package:firehose needs to use package:dart_apitool: ^0.20.2

---

• I’ve reviewed the contributor guide and applied the relevant portions to this PR.

Contribution guidelines:

• See our contributor guide for general expectations for PRs.
• Larger or significant changes should be discussed in an issue before creating a PR.
• Contributions to our repos should follow the Dart style guide and use dart format.
• Most changes should add an entry to the changelog and may need to rev the pubspec package version.
• Changes to packages require corresponding tests.

Note that many Dart repos have a weekly cadence for reviewing PRs - please allow for some latency before initial review feedback.

[Levi-Lesches]

@dcharkes The jni package contains the following:

  ## Pin ffigen version because we are depending on internal APIs.
  ffigen: 8.0.2

But the current ffigen version is 16.1.0-wip, and that's causing conflicts. In general, because it's pinning a specific version, even if I update it to 16.0.0, it will fail on 16.0.1. Does that mean jni should be excluded from the workspace, or should ffigen start exposing those APIs and keep them stable?

---

Edit: I bumped the dependency in jni and used the workspace version, and updated renamed members, and it passes analysis now

[Levi-Lesches]

Possibly silly question, but does this require a pubspec bump and changelog addition?

[Levi-Lesches]

I tried running jni/tools/generate_ffi_bindings.dart because I thought I broke it. Here's the output:

$ dart tool\generate_ffi_bindings.dart              
INFO: Generating C wrappers
Unknown type ffi.UnsignedInt for return type

I tracked that down to these lines in jni/tool/wrapper_generator/generate_c_extensions.dart:

String jValueGetterOf(String returnType) {
  const getters = {
    'jboolean': 'z',
    'jbyte': 'b',
    'jshort': 's',
    'jchar': 'c',
    'jint': 'i',
    'jsize': 'i', // jsize is an alias to jint
    'jfloat': 'f',
    'jlong': 'j',
    'jdouble': 'd',
    'jobject': 'l',
    'jweak': 'l',
    'jarray': 'l',
    'jstring': 'l',
    'jthrowable': 'l',
  };
  if (!getters.containsKey(returnType)) {
    stderr.writeln('Unknown type $returnType for return type');
    exit(1);
  }
  return getters[returnType]!;
}

Seems it's expecting some Java-y value and is getting a C-ish value instead. Any idea why this is happening? And should I be fixing it in this PR? I'm concerned it happened due to changing the version of ffigen from 8.0.2 to 16.0.0.

[dcharkes]

Thanks a ton @Levi-Lesches! 🚀

This is coming from package:dart_apitool. They're expecting each package to have its own .dart_tool directory, but that isn't true in the case of mono-repos. This is the case as of today, version 0.20.1. Thankfully, there's already a PR in the works at bmw-tech/dart_apitool#202, with all checks green just 4 hours ago.

@mosuem Do we need a bump in the ecosystem repo to use the newest version of api_tool?

Also, my VS Code only takes up 500MB of RAM now, when analyzing the entire repo. I don't know how much it used to be, but I remember my computer getting really slow before!

🔥 😄

[Levi-Lesches]

(They just released version 0.20.2 with the fix, and firehose picked it up automatically)

pubspec bump: I don't believe so, users on an older SDK will not have pub resolve to newer versions of the packages.

changelog: yes, bumping minimum SDK constraint should be in the changelog. (Also, soon I'd like to bump things to 3.7 when it's out, because I want the new formatter.)

Kinda confused here, since some of these packages are not -wip at the moment, so not bumping would mean modifying the changelog for versions that are already out there. So I just bumped the patch version (since no features were added) and added Dart 3.6 to the changelog under that new version.

[Levi-Lesches]

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);
  }

[Levi-Lesches]

Also, it seems that ffigen somehow depends on native_assets_builder, although I can't see how:

• dart test in packge:ffi works fine
• dart test in package:ffigen fails because of cyclic_package_1 and cyclic_package_2

[github-actions]

Package publishing

Package	Version	Status	Publish tag (post-merge)
package:ffi	2.1.4	ready to publish	ffi-v2.1.4
package:jni	0.13.1	ready to publish	jni-v0.13.1
package:ffigen	17.0.0-wip	WIP (no publish necessary)
package:jnigen	0.13.2-wip	WIP (no publish necessary)
package:objective_c	5.0.0-wip	WIP (no publish necessary)
package:swift2objc	0.0.1-wip	WIP (no publish necessary)
package:swiftgen	0.0.1-wip	WIP (no publish necessary)

Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation.

[dcharkes]

• dart test in package:ffigen fails because of cyclic_package_1 and cyclic_package_2

I've dove into this. It's because we don't properly pass the runPackageName everywhere. In package:dartdev we do, so dart run in package:ffi we only consider transitive dependencies of package:ffi. If we do flutter run in package:ffi we have the package:flutter_tools implementation, which does not pass the runPackageName. So then it starts trying to build native assets for all packages in the package graph, instead of only the packages which are transitive dependencies of package:ffi.

The same issue exists for the check if the native assets experiment should be enabled or not. It currently also looks at all packages in the package graph.

Before pub workspaces, if we didn't pass runPackageName it was always the root package. And without pub workspaces, that is identical to the package resolution graph. But now with pub workspaces, this is no longer true.

I'm going to fix this behavior in package:native_assets_builder and then roll it into dartdev and flutter tools. When the fix is in flutter master branch, we can disable the CI for flutter stable, and things should become more green.

[Levi-Lesches]

Sounds good. And I found this in the firehose checks:

Running `/opt/hostedtoolcache/dart/3.7.0-309.0.dev/x64/bin/dart pub global activate -sgit https://github.com/bmw-tech/dart_apitool.git --git-ref 123049d3fa3c1459a5129b2b61d852a388a8511e` in /home/runner/work/native/native/current_repo

So there probably needs to be an update there to use the latest dart_apitool, since it's not using semver directly. Seems this PR is gonna have to wait for this and #1905 then. Feel free to ping me when those are resolved and glad I can help in the meantime!

[mosuem]

Re: firehose, dart_apitool version is set here https://github.com/dart-lang/ecosystem/blob/94b4cea02f2948432a1c9218c573cc03d5ef144c/pkgs/firehose/lib/src/health/health.dart#L20C22-L20C62. I will file a PR to update it!

[mosuem]

Filed dart-lang/ecosystem#338

Edit: merged.

[HosseinYousefi]

Package:jni used an older version of ffigen as a dev dependency because it relied on its internals. dart run tool/generate_ffi_bindings.dart in jni now doesn't work.

[Levi-Lesches]

@HosseinYousefi I fixed as much as I could, see the diff here. The issue now is that I get

$ dart tool\generate_ffi_bindings.dart
INFO: Generating C wrappers
Unknown type ffi.UnsignedInt for return type

And I described some more details in #1884 (comment)

[HosseinYousefi]

@HosseinYousefi I fixed as much as I could, see the diff here. The issue now is that I get

$ dart tool\generate_ffi_bindings.dart

INFO: Generating C wrappers

Unknown type ffi.UnsignedInt for return type

And I described some more details in #1884 (comment)

I will try to fix this part of your PR on Monday. Thanks!

[HosseinYousefi]

Added a commit that fixes package:jni issues.

[Levi-Lesches]

Fixed merge conflicts with the new native_assets changes -- if that's working, can we run CI again?

[dcharkes]

@Levi-Lesches The Dart version containing the fixes is https://github.com/dart-lang/sdk/releases/tag/3.8.0-28.0.dev. The next dart dev release (Tuesday next week), should contain the fixes. The Flutter master channel contains the fixes for flutter but the dart in Flutter master is still 3.8.0-24.0.dev.

Once Dart in Flutter rolls to 28.0.dev you should be able to rebase this PR (assuming all the GitHub actions will be using flutter master, and not Dart dev).

(Sorry for the merge mess, I had to release a new version of every package, so all pubspecs now have merge conflicts. 😄 )

[Levi-Lesches]

Sounds good!

Sorry for the merge mess

At first I was confused how it got this bad, but then I realized I've never seen a workflow where in order to merge a PR, you must first merge parts of it, flag it for release, wait for the release to come out, then let CI run and hit merge 😂.

I'll try to remember about this in a week, but if I don't, feel free to ping me!

[dcharkes]

At first I was confused how it got this bad, but then I realized I've never seen a workflow where in order to merge a PR, you must first merge parts of it, flag it for release, wait for the release to come out, then let CI run and hit merge 😂.

Yeah, we're building Dart with Dart. (Where Dart and Dart are different versions of Dart.) 🤓

[dcharkes]

I'll try to remember about this in a week, but if I don't, feel free to ping me!

Friendly ping @Levi-Lesches!

It would be good split up this PR in separate PRs in which the last one introduces the pub workspace. This would (1) enable us to review separate aspects separately, and (2) land those separate aspects separately (avoiding any future merge issues with these changes 😄 ).

• A PR that bumps deps and addresses lints for package:ffigen + package:objective_c to be reviewed by @liamappelbe
• A PR that bumps deps and addresses lints for package:swift2objc to be reviewed by @liamappelbe
• A PR that bumps deps and addresses lints for package:jni + package:jnigen to be reviewed by @HosseinYousefi
• A PR that bumps deps and addresses lints for package:ffi to be reviewed by me.
• A PR that bumps deps and addresses lints for package:native_ to be reviewed by me.
• A PR that does some of the github workflow cleanups, to be reviewed by me.

Then the actual flipping to a pub workspace (with the remaining github workflow) changes will possibly need some more iteration:

• FFIgen, package:ffi, JNIgen, JNI all need to run against Dart/Flutter stable
• package:native_assets_cli might want a dart dev release as a lower bound ([native_assets_cli][native_assets_builder] Supporting version skew between SDK and build.dart #93 (comment)) which would force the whole workspace to have the dev release for that package I believe. --> I need to think a bit about what the right solution is there.

Landing all the prerequisite changes first, will make it easier for me to experiment with what's the right solution. (I would really like one workspace, so that we have one set of lints, one set of dependencies etc!)