[infra] Use Pub Workspaces #1884

Levi-Lesches · 2025-01-12T23:04:51Z

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 · 2025-01-12T23:07:14Z

@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 · 2025-01-13T01:42:54Z

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

Levi-Lesches · 2025-01-13T02:02:30Z

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.

liamappelbe · 2025-01-14T03:33:04Z

Unrelated, just wanted to flag the following test:
// pkgs\ffigen\test\collision_tests\decl_decl_collision_test.dart

/// Conflicts with ffi library prefix, name of prefix is changed.
Struct(name: 'ffi'),
This seems like it should result in a line like
import "dart:ffi" as _ffi;
but it doesn't -- both the struct and the import prefix are called ffi, and this causes analysis issues. I'd imagine this can be an issue in production code, but I'm also seeing the _expected file clearly shows it expects the generated code to use as ffi and have all those errors. Is the test incorrect?

Looking at the blame for that file, this test has been broken for over a year. I'll need to fix it later. Filed a bug: #1889

liamappelbe

ffigen and objective_c changes LGTM, but there are some CI failures (all infra stuff)

dcharkes · 2025-01-14T07:24:52Z

Every pub get command needs to be flutter instead of dart?

It should be enough to download Flutter instead of Dart, then running the dart executable from there.

How do you want to handle all the logic around the dev channel? That doesn't exist in Flutter. I can use beta, but would that really solve the issue? Or would these tests need to work on Dart before the Flutter team picks that SDK version for their next beta? For now, I switched the CI to Flutter, so let's see if it goes green this time.

Does the GitHub action support using the master channel? It's all about quick feedback.

The native_assets_builder/ tests that copy the test projects around now start failing because resolution requires a workspace. We should be able to remove the resolution: workspace entry in pkgs/native_assets_builder/test/helpers.dart copyTestProjects.

Levi-Lesches · 2025-01-14T23:54:14Z

Does the GitHub action support using the master channel? It's all about quick feedback.

Done, switched to Flutter's main channel

The native_assets_builder/ tests that copy the test projects around now start failing because resolution requires a workspace. We should be able to remove the resolution: workspace entry in pkgs/native_assets_builder/test/helpers.dart copyTestProjects.

Done. I had to add dependency_overrides for native_assets_cli and native_toolchain_c in all packages, except for native_add_version_skew. All the infra issues in native_assets_builder tests are gone, but I'm just getting this error with VisualStudioResolver in the skew test. Could be an artifact of my system, let's see if it happens on CI.

Error output

Null check operator used on a null value
#0      VisualStudioResolver.resolve (package:native_toolchain_c/src/native_toolchain/msvc.dart:283:65)
<asynchronous suspension>
#1      RelativeToolResolver.resolve (package:native_toolchain_c/src/tool/tool_resolver.dart:259:32)
<asynchronous suspension>
#2      PathVersionResolver.resolve (package:native_toolchain_c/src/tool/tool_resolver.dart:151:27)
<asynchronous suspension>
#3      RelativeToolResolver.resolve (package:native_toolchain_c/src/tool/tool_resolver.dart:259:32)
<asynchronous suspension>
#4      CliVersionResolver.resolve (package:native_toolchain_c/src/tool/tool_resolver.dart:91:27)
<asynchronous suspension>
#5      CompilerResolver._tryLoadToolFromNativeToolchain (package:native_toolchain_c/src/cbuilder/compiler_resolver.dart:117:23)
<asynchronous suspension>
#6      CompilerResolver.resolveCompiler (package:native_toolchain_c/src/cbuilder/compiler_resolver.dart:45:18)
<asynchronous suspension>
#7      RunCBuilder.compiler (package:native_toolchain_c/src/cbuilder/run_cbuilder.dart:84:44)
<asynchronous suspension>
#8      RunCBuilder.run (package:native_toolchain_c/src/cbuilder/run_cbuilder.dart:115:50)
<asynchronous suspension>
#9      CBuilder.run (package:native_toolchain_c/src/cbuilder/cbuilder.dart:162:7)
<asynchronous suspension>
#10     main.<anonymous closure> (file:///c:/users/levi/appdata/local/temp/7d1c4bb/native_add_version_skew/hook/build.dart:19:5)
<asynchronous suspension>
#11     build (package:native_assets_cli/src/api/build.dart:101:3)
<asynchronous suspension>
#12     main (file:///c:/users/levi/appdata/local/temp/7d1c4bb/native_add_version_skew/hook/build.dart:10:3)
<asynchronous suspension>

  stdout:
  FINER: 2025-01-14 18:17:16.642465: No compiler set in BuildConfig.cCompiler.cc.
FINER: 2025-01-14 18:17:16.656951: Looking for Visual Studio Locator on PATH.
INFO: 2025-01-14 18:17:16.663228: Running `where vswhere.exe`.
SEVERE: 2025-01-14 18:17:16.892507: INFO: Could not find files for the given pattern(s).
FINE: 2025-01-14 18:17:16.909991: Did not find  Visual Studio Locator on PATH.
FINER: 2025-01-14 18:17:16.912369: Looking for Visual Studio Locator in [C:/Program Files \(x86\)/Microsoft Visual Studio/Installer/vswhere.exe, C:/Program Files/Microsoft Visual Studio/Installer/vswhere.exe].
FINE: 2025-01-14 18:17:17.047220: Found [ToolInstance(Visual Studio Locator, null, file:///C:/Program%20Files%20(x86)/Microsoft%20Visual%20Studio/Installer/vswhere.exe)].
FINER: 2025-01-14 18:17:17.048224: Looking up version with --version for ToolInstance(Visual Studio Locator, null, file:///C:/Program%20Files%20(x86)/Microsoft%20Visual%20Studio/Installer/vswhere.exe).
INFO: 2025-01-14 18:17:17.050225: Running `C:\Program Files (x86)\Microsoft Visual Studio\Installer\vswhere.exe`.
FINE: 2025-01-14 18:17:17.148590: Visual Studio Locator version 3.1.7+f39851e70f [query version 3.7.2174.19405]
FINE: 2025-01-14 18:17:17.148590: Copyright (C) Microsoft Corporation. All rights reserved.
FINE: 2025-01-14 18:17:17.148590:
FINE: 2025-01-14 18:17:17.171589: Found version for ToolInstance(Visual Studio Locator, 3.1.7+f39851e70f, file:///C:/Program%20Files%20(x86)/Microsoft%20Visual%20Studio/Installer/vswhere.exe).
INFO: 2025-01-14 18:17:17.172591: Running `C:\Program Files (x86)\Microsoft Visual Studio\Installer\vswhere.exe`.
FINE: 2025-01-14 18:17:17.261727: Visual Studio Locator version 3.1.7+f39851e70f [query version 3.7.2174.19405]
FINE: 2025-01-14 18:17:17.261727: Copyright (C) Microsoft Corporation. All rights reserved.
FINE: 2025-01-14 18:17:17.261727:

Found why the health checks aren't finishing:

https://github.com/bmw-tech/dart_apitool/blob/e3b27c7f764ae34a00dd9f18bc2d68c146a5739a/lib/src/cli/commands/command_mixin.dart#L111-L116

https://github.com/bmw-tech/dart_apitool/blob/e3b27c7f764ae34a00dd9f18bc2d68c146a5739a/lib/src/cli/commands/command_mixin.dart#L294-L295

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.

I merged main and fixed the other CI errors, so let's try it again. 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!

dcharkes · 2025-01-15T19:19:10Z

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 · 2025-01-15T20:52:55Z

(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 · 2025-01-15T21:10:10Z

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 · 2025-01-15T21:33:20Z

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 · 2025-01-15T22:47:46Z

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.1-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 · 2025-01-16T10:42:27Z

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 · 2025-01-16T22:12:26Z

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 · 2025-01-17T08:16:30Z

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 · 2025-01-17T08:18:03Z

Filed dart-lang/ecosystem#338

Edit: merged.

HosseinYousefi · 2025-01-17T16:47:49Z

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 · 2025-01-17T19:45:23Z

@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 · 2025-01-17T19:56:38Z

@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!

Levi-Lesches added 3 commits January 12, 2025 17:59

Added ffi to workspace

e2d528f

Added ffigen to workspace

935a52c

Added jni to workspace (FAILED)

4abff69

Levi-Lesches marked this pull request as draft January 12, 2025 23:04

github-actions bot added package:ffigen package:jni package:ffi labels Jan 12, 2025

Levi-Lesches changed the title ~~Use Pub Workspaces~~ [infra] Use Pub Workspaces Jan 12, 2025

Levi-Lesches added 4 commits January 12, 2025 19:22

Fixed jni issues

58df689

Fixed issues in ffigen

6d1fb6b

Added jnigen to workspace

2998552

Added native_assets_builder to the workspace

fefc30f

github-actions bot added package:native_assets_builder package:jnigen labels Jan 13, 2025

Fixed issues with native_assets_builder/test_data/*

349c05c

github-actions bot added package:native_assets_cli package:native_toolchain_c labels Jan 13, 2025

Levi-Lesches added 5 commits January 12, 2025 20:20

Fix lints

aad35e2

Fixed issues in native_assets_cli

d512ee2

Added objective_c to the workspace

c6069a0

Added swift2objc ot the workspace

7394804

Added swiftgen to the workspace

c1ec515

Bumped publish CI to stable channel (from dev)

eaf70d0

github-actions bot added type-infra A repository infrastructure change or enhancement package:objective_c package:swiftgen package:swift2objc labels Jan 13, 2025

Updated CI to use Flutter and dev -> beta channel

815671e

liamappelbe reviewed Jan 14, 2025

View reviewed changes

Levi-Lesches added 2 commits January 14, 2025 17:43

CI: Replaced beta -> main channel

ca1602f

Reworked native_assets_builder test helpers to remove workspace

33640fa

Levi-Lesches mentioned this pull request Jan 14, 2025

Support Dart workspaces bmw-tech/dart_apitool#201

Closed

Fixed more CI checks

58f28cd

Levi-Lesches added 4 commits January 14, 2025 18:59

Removed extra pub gets from CI

f389a65

Added native_assets_cli/example/* to workspace

bc7ec72

Added ffigen/example/* to the workspace

29c6eea

Merge branch 'main' into workspaces

fbe5cd4

Levi-Lesches added 2 commits January 15, 2025 15:45

Merged main branch

b3f91b2

Bumped versions and added Dart 3.6 to changelogs

e7e5adb

dcharkes mentioned this pull request Jan 15, 2025

[native_assets_builder] Support pub workspaces #1905

Open

4 tasks

Levi-Lesches added 2 commits January 15, 2025 16:36

Fix some more CIs

c4fe35c

Fix even more CI

a2f412a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[infra] Use Pub Workspaces #1884

[infra] Use Pub Workspaces #1884

Levi-Lesches commented Jan 12, 2025 •

edited

Loading

Levi-Lesches commented Jan 12, 2025 •

edited

Loading

Levi-Lesches commented Jan 13, 2025 •

edited

Loading

Levi-Lesches commented Jan 13, 2025

liamappelbe commented Jan 14, 2025

liamappelbe left a comment

dcharkes commented Jan 14, 2025

Levi-Lesches commented Jan 14, 2025 •

edited

Loading

dcharkes commented Jan 15, 2025

Levi-Lesches commented Jan 15, 2025 •

edited

Loading

Levi-Lesches commented Jan 15, 2025

Levi-Lesches commented Jan 15, 2025

github-actions bot commented Jan 15, 2025

dcharkes commented Jan 16, 2025

Levi-Lesches commented Jan 16, 2025 •

edited

Loading

mosuem commented Jan 17, 2025

mosuem commented Jan 17, 2025 •

edited

Loading

HosseinYousefi commented Jan 17, 2025

Levi-Lesches commented Jan 17, 2025

HosseinYousefi commented Jan 17, 2025

[infra] Use Pub Workspaces #1884

Are you sure you want to change the base?

[infra] Use Pub Workspaces #1884

Conversation

Levi-Lesches commented Jan 12, 2025 • edited Loading

Levi-Lesches commented Jan 12, 2025 • edited Loading

Levi-Lesches commented Jan 13, 2025 • edited Loading

Levi-Lesches commented Jan 13, 2025

liamappelbe commented Jan 14, 2025

liamappelbe left a comment

Choose a reason for hiding this comment

dcharkes commented Jan 14, 2025

Levi-Lesches commented Jan 14, 2025 • edited Loading

dcharkes commented Jan 15, 2025

Levi-Lesches commented Jan 15, 2025 • edited Loading

Levi-Lesches commented Jan 15, 2025

Levi-Lesches commented Jan 15, 2025

github-actions bot commented Jan 15, 2025

Package publishing

dcharkes commented Jan 16, 2025

Levi-Lesches commented Jan 16, 2025 • edited Loading

mosuem commented Jan 17, 2025

mosuem commented Jan 17, 2025 • edited Loading

HosseinYousefi commented Jan 17, 2025

Levi-Lesches commented Jan 17, 2025

HosseinYousefi commented Jan 17, 2025

Levi-Lesches commented Jan 12, 2025 •

edited

Loading

Levi-Lesches commented Jan 12, 2025 •

edited

Loading

Levi-Lesches commented Jan 13, 2025 •

edited

Loading

Levi-Lesches commented Jan 14, 2025 •

edited

Loading

Levi-Lesches commented Jan 15, 2025 •

edited

Loading

Levi-Lesches commented Jan 16, 2025 •

edited

Loading

mosuem commented Jan 17, 2025 •

edited

Loading