workspace 1.64

[Roms1383]

Rust 1.64 is out and that could be the opportunity to make use of the workspace features ?

Checklist

• [ ] An issue to be fixed by this PR is listed above.
• [ ] New tests are added to ensure new features are working. End-to-end tests are usually in the ./frb_example/pure_dart example, more specifically, rust/src/api.rs and dart/lib/main.dart.
• [x] The code generator is run and the code is formatted (e.g. via just refresh_all).
• [ ] If this PR adds/changes features, documentations (in the ./book folder) are updated.
• [x] CI is passing.

[Roms1383]

spoiler alert : locally test-pure-web fails with a conversion error like :

00:01 +30 ~2 -3: dart call handleOptionBoxArguments [E]

```sh Error: expected a bigint argument pkg/flutter_rust_bridge_example.js 268:39 _assertBigInt pkg/flutter_rust_bridge_example.js 1011:5 __exports.new_box_autoadd_i64_0 main.web.dart.js 12080:23 LegacyJavaScriptObject.new_box_autoadd_i64_0$1 main.web.dart.js 11495:43 Object.new_box_autoadd_i64_0$1$x main.web.dart.js 24900:16 FlutterRustBridgeExampleSingleBlockTestPlatform.api2wire_exotic_optionals$1 main.web.dart.js 24605:53 FlutterRustBridgeExampleSingleBlockTestImpl_handleOptionBoxArguments_closure.call$1 main.web.dart.js 23285:20 FlutterRustBridgeExampleSingleBlockTestPlatform.executeNormal$1$1 main.web.dart.js 25023:48 main.web.dart.js 3302:15 _wrapJsFunctionForAsync_closure.$protected main.web.dart.js 14563:12 _wrapJsFunctionForAsync_closure.call$2 ```

00:11 +61 ~2 -7: chrono feature Duration [E]

```sh Error: expected a bigint argument pkg/flutter_rust_bridge_example.js 268:39 _assertBigInt pkg/flutter_rust_bridge_example.js 881:5 __exports.wire_duration main.web.dart.js 12044:23 LegacyJavaScriptObject.wire_duration$2 main.web.dart.js 11546:43 Object.wire_duration$2$x main.web.dart.js 24814:16 FlutterRustBridgeExampleSingleBlockTestImpl_duration_closure.call$1 main.web.dart.js 23285:20 FlutterRustBridgeExampleSingleBlockTestPlatform.executeNormal$1$1 main.web.dart.js 25023:48 main.web.dart.js 3302:15 _wrapJsFunctionForAsync_closure.$protected main.web.dart.js 14563:12 _wrapJsFunctionForAsync_closure.call$2 main.web.dart.js 14551:32 _awaitOnObject_closure.call$1 ```

[Roms1383]

It's not on master I already checked, and it's surely because of my changes here but any idea where BigInt conversion breaks ? @Desdaemon

spoiler alert : locally test-pure-web fails with a conversion error like :

00:01 +30 ~2 -3: dart call handleOptionBoxArguments [E] 00:11 +61 ~2 -7: chrono feature Duration [E]

[Desdaemon]

It's not on master I already checked, and it's surely because of my changes here but any idea where BigInt conversion breaks ? @Desdaemon

spoiler alert : locally test-pure-web fails with a conversion error like : 00:01 +30 ~2 -3: dart call handleOptionBoxArguments [E] 00:11 +61 ~2 -7: chrono feature Duration [E]

Not sure what's happening here, looks similar to #703 ~though js-sys is not updated here yet.~

[Roms1383]

That wouldn't fix the BigInt assertion failure, but I noticed that the CI runs in 1.63, cargo workspace new features are fresh from 1.64.

See in the workflow run :

 The package requires the Cargo feature called `workspace-inheritance`, but that feature is not stabilized in this version of Cargo (1.63.0 (fd9c4297c 2022-07-01)).

[Roms1383]

CI fails for this :

strategy:
      matrix:
        cargo_toolchain_version:
          - stable
          - 1.55.0 # ❌ < 1.64

The package requires the Cargo feature called `workspace-inheritance`, but that feature is not stabilized in this version of Cargo (1.63.0 (fd9c4297c 2022-07-01)).

Can I remove it @fzyzcjy ?

[fzyzcjy]

Looks yes

[Roms1383]

@fzyzcjy really weird:

[Roms1383]

Yeah I know where the problem comes from. So CI uses actions/cache@v3 and none of the cache keys have changed, as far as the CI sees, since the Cargo.lock hasn't changed for example.

[Roms1383]

[Roms1383]

I'm gonna get back at it this afternoon but honestly I'm starting to turn around in circles lol

[fzyzcjy]

Take your time!

Maybe firstly make it work locally?

[Roms1383]

Thing is, it already works locally 😅. But to be honest I always run test-pure-web because when I run test-pure locally I get the same error as #715

[Roms1383]

Ok so we're left with 2 errors that I'm not familiar with, and also give a bit obscure answers when searched online:

1. flutter with iOS (any device) obviously the error is with Cargo Lipo

   [INFO  cargo_lipo::lipo] Creating universal library for flutter_rust_bridge_example
   cp: target/universal/debug/libflutter_rust_bridge_example.a: No such file or directory

2. flutter with desktop (Windows / Linux) obviously the error is with CMake and Corrosion

   [        ] CMake Error at 
   D:/a/flutter_rust_bridge/flutter_rust_bridge/frb_example/with_flutter/build/windows/_deps/corrosion-src/cmake/Corrosion.cmake:326 (add_library):
   [        ]   add_library cannot create target "flutter_rust_bridge_example" because
   [        ]   another target with the same name already exists.  The existing target is
   [        ]   an interface library created in source directory
   [        ]   "D:/a/flutter_rust_bridge/flutter_rust_bridge/frb_example/with_flutter/windows".
   [        ]   See documentation for policy CMP0002 for more details.

   Any hint ?

[fzyzcjy]

cp: target/universal/debug/libflutter_rust_bridge_example.a: No such file or directory

Maybe debug to see which files are generated? And enable more debug logs

flutter with desktop (Windows / Linux)

/cc @Desdaemon who made that part :)

[Roms1383]

@fzyzcjy I'm currently trying to change cp because locally cargo lipo build into root target, but not sure.

[Roms1383]

@fzyzcjy I'm currently trying to change cp because locally cargo lipo build into root target, but not sure.

Ok so apparently it fixes iOS jobs (failing iPhone 12 Pro Max Simulator (15.2) is a false-positive, see its output).

[Roms1383]

any idea on how to fix Corrosion related issue @Desdaemon ?

[Roms1383]

oh I think I know :

error from Flutter (Windows) integration test / flutter_windows_test :

[        ] CMake Error at D:/a/flutter_rust_bridge/flutter_rust_bridge/frb_example/with_flutter/build/windows/_deps/corrosion-src/cmake/Corrosion.cmake:326 (add_library):
[        ]   add_library cannot create target "flutter_rust_bridge_example" because
[        ]   another target with the same name already exists.  The existing target is
[        ]   an interface library created in source directory
[        ]   "D:/a/flutter_rust_bridge/flutter_rust_bridge/frb_example/with_flutter/windows".
[        ]   See documentation for policy CMP0002 for more details.

and looking at frb_example/with_flutter/windows/rust.cmake :

set(CRATE_NAME "flutter_rust_bridge_example")

not sure with which artifact it conflicts because I've never used CMake 😅

[Desdaemon]

Based on the tests in the corrosion repo, you may have to change the import path to the workspace manifest (!) and specify the member crate to import.

[Roms1383]

Honestly I have no idea how it works, so I'm guessing something like : corrosion_import_crate(MANIFEST_PATH ${CMAKE_CURRENT_SOURCE_DIR}/Cargo.toml CRATES flutter_rust_bridge_example) in top declaration of CMakeLists.txt. I'm gonna give it a try.

[Roms1383]

That's weird that subosito/flutter-action@v2 fails so often at retrieving 3.3.1-x64, don't you think so ?

[Roms1383]

Unknown CMake command "corrosion_import_crate".

Well ... 😅

[Roms1383]

sorry for all the CI runs, it seems that glob(s) are ignored in workspace's exclude (as in 64a13407cab47b49dc8bc7b6c943ef001d08be1d and c9dc31bdbd9c0fa1c1d5173b9a2a62c2f57988cd), or I wrote these wrong. also I couldn't find the list of generated crates by corrosion.

[Roms1383]

also I couldn't find the list of generated crates by corrosion.

found it

[Roms1383]

[Roms1383]

so ok the issues were :

1. @Desdaemon was right, basically an additional setting needed to be added to corrosion_import_crate, but I was confused at first and the files that needed to be updated were all rust.cmake, not CMakeLists.txt.
2. in CI tests for Linux and Windows, corrosion is checkout from github and contains itself a lot of cargo projects. For some reason that I can't tell, [workspace]'s exclude property doesn't take into account glob(s), or I made a typo, but honestly usually corrosion or corrosion/**/* or corrosion/* should have been fine. Once found out, I just added all corrosion projects path manually, so beware that if in the future corrosion create some new internal project, subsequent Linux / Windows CI runs will fail for the same reason.

[Roms1383]

if this CI run goes fine, we're probably ready for a review @fzyzcjy. the PR has became a little bit of a mess with all these oddities, so 2 options regarding the reusable workflows.

1. I leave it the way it is, and we can still refactor the rest later
2. I revert reusable workflows completely, keeping ci.yml as close as it was before (plus the fixes, of course)

[Roms1383]

Is there any reason not to use needs directive in Github workflows ? I mean you could save a lot of CI time. For example, run Mergeable + Codacy Static Code Analysis + CI / Dart Linter + CI / Rust linter + CI / Build and run Rust + CI / Valgrind. Then once they're all successful, run all CI / Run codegen ... and all CI / ... integration tests.

[fzyzcjy]

Good job! Looks like the CI system has been modified a lot :)

the PR has became a little bit of a mess with all these oddities, so 2 options regarding the reusable workflows.

The large direction of using reuseable workflow looks reasonable IMHO

[fzyzcjy]

I am still busy working on https://docs.google.com/document/d/1FuNcBvAPghUyjeqQCOYxSt6lGDAQ1YxsNlOvrUx0Gko/edit# so may not have time to review the PR now. Maybe @Desdaemon can provide some reviews :)

[Roms1383]

Good job! Looks like the CI system has been modified a lot :)

Actually mainly a couple missing scaffold actions (e.g. like Install Rust toolchain or some missing toolchain components here and there), forcing override and 1.64.0 for Rust toolchain all over the place, and especially disabling cache (then re-enabling it) to force the CI to really use 1.64.0 everywhere. Looking back I think I was a bit too prompt to implement it. A couple of weeks from now it would have been the chosen stable by default, cache would have been naturally updated seamlessly.

[Roms1383]

I noticed the manifest .release files haven't been updated in a while, I'm about to commit them. @Desdaemon we should surely double-check that post-release scripts are ok, to avoid CI failure once merged (I'm gonna do it, but it doesn't hurt if we are both checking).

[Roms1383]

LGTM 😉 only point left, + your review(s) 🙂 🙏 🚀

[fzyzcjy]

Good job!

[fzyzcjy]

I will release a new version when having some time :) Still quite busy working on the open sourced https://github.com/fzyzcjy/flutter_smooth