Upgrade to uniffi @0.24

[dignifiedquire]

Started work on this.

Feature Work

• updates to get the basics to compile
• update and get tests to pass
  • arithmetic_test.go
  • callbacks_test.go
  • chronological_test.go
  • coverall_test.go
  • custom_types_test.go
  • destroy_test.go
  • errors_test.go
  • ext_types_test.go
  • fixture_callbacks_test.go
  • foreign_executor_test.go
  • futures_test.go
  • geometry_test.go
  • large_enum_test.go compile only
  • objects_test.go
  • proc_macro_test.go
  • rondpoint_test.go
  • simple_fns_test.go
  • simple_iface_test.go
  • sprites_test.go
  • todolist_test.go
  • trait_methods_test.go
  • type_limits_test.go
• add support for new features
  • generate bindings with --library flag instead of UDL
  • bytes type
  • async functions
  • callbacks need to be updated to new FFI API
  • implement FFI function checksum checking at runtime (previously the job was left to the linker by prefixing FFI functions with a checksum)

Cleanup

• Look into moving the .h file back into the generated go file not possible, due to the need of the .c file
• Can we avoid the double nesting of package names?
• General cleanup of the generated code
• look into using https://pkg.go.dev/runtime/[email protected]#Handle for callback handling (can be done in another PR): moved to Investigate usage of cgo.Handle for callback handling #18
• run tests with race detector
• update submodule after test(fixtures): remove potential duplicate in proc-macro fixture mozilla/uniffi-rs#1763 is merged

Ref #12

[arg0d]

Preliminary list of features for 0.24 from https://github.com/mozilla/uniffi-rs/blob/main/CHANGELOG.md:

• generate bindings with --library flag instead of UDL
• bytes type
• async functions
• callbacks need to be updated to new FFI API
• implement FFI function checksum checking at runtime (previously the job was left to the linker by prefixing FFI functions with a checksum)

[dignifiedquire]

I am back working on this, will update the issue description to document my progress as I go along.

[gogo2464]

great thank you!!!

[gogo2464]

I saw a commit name "test compile". Is it ready? Could I already test on my own code please?

[dignifiedquire]

I saw a commit name "test compile". Is it ready? Could I already test on my own code please?

no it is not yet ready, sorry

[arg0d]

I see, failing tests are not acceptable 😅 Even so, the generators must target tagged versions to maintain cohesion. We have faced this problem before in 0.23. We ended up simply creating a uniffi-rs fork. Maintaining a fork is not ideal, but gets the job done quite well. The fork works quite well for these small changes. The fork also contains docstring support, which is the only more significant change. The fork maintains compatibility with upstream versions. There is already 0.24 branch in our fork, I could add your patch there.

Alternatively we could ask upstream guys to cherry-pick the patch into upstream 0.24 release.

[dignifiedquire]

I would suggest we ask how long out a new tagged version is, and if it is too long we use the fork option for now

[arg0d]

I raised the question in uniffi chat room. There aren't plans for a release, but they aren't against the idea. No clear timeline yet.

[dignifiedquire]

Okay, @arg0d if you could just prepare your fork, I am happy to update the branch to that.

[arg0d]

Updated.

Also made library mode code reusable for external generators, library_mode::generate_external_bindings. I saw that your changes include merging uniffi.toml config with the override config, but that is inconsistent with upstream generators, and I'm thinking if there is a better approach to configure go_mod for all packages. Merging configs is convenient for go_mod, but at the same time doesn't make much sense for all other values, i.e. package_name, c_module_filename, custom_types. You could try sending a PR to upstream for this change, and see what they think.

[arg0d]

I just discovered the hard way that merging with override config is very useful for configuring upstream test crates 😅

I also see upstream Swift generator has an option omit_argument_labels that would also benefit from overriding the config for all crates (instead of having to configure the same option for all crates).

[arg0d]

Created mozilla/uniffi-rs#1788

[dignifiedquire]

nd I'm thinking if there is a better approach to configure go_mod for all packages.

Very curious about your ideas here, because I need to find a way to pass this into the config of the fixture dependencies.

[arg0d]

I can't really think of any better solution for go-mod than merging configs. For custom_types, I have simply updated the upstream test to include C# and Go configurations. NordSecurity/uniffi-rs@1e21763

• One way would be to pass --go-mod value to the generator CLI, but this is cumbersome to do each time, since go-mod value is not expected to change that often, especially not between daily CLI invocations.

• A step up from above would be to create a separate file for "global" configuration values, separate from default uniffi.toml, and pass the path to this config via CLI. This is also cumbersome, because then there would be several config file formats, and that would be confusing to the users. Also, go-mod value should still be applicable to both crate and override configs, i.e. if I'm working with a single crate I should be able to change this value, and also I should be able to change it for multiple crates (merging). So, there would be 2 config files, each of which share some of the values, but also don't support all of the values.

I'm going add the config merge logic to NordSecurity fork, and try to convince Mozilla guys to accept the change.

[arg0d]

Seems like 0.25 is released, but I don't see a tag in Github repo yet for some reason 🤔

[dignifiedquire]

https://github.com/mozilla/uniffi-rs/commits/v0.25.0 tags are there now

[arg0d]

Do you want to jump straight into 0.25, or finish this as 0.24? I'm thinking to finish this as 0.24, and work on 0.25 as a separate PR. 0.25 is very new, it might have issues. I think both options will require some changes. 0.24 doesn't have trait methods, and 0.25 has reworked async functions.

[kegsay]

+1 to finishing and landing 0.24, as it's a huge improvement over mainline for me at least.

[dignifiedquire]

The issue with finishing on 0.24 that I see is, that I would to have to revert a bunch of changes, that are already adapted that happened for 0.25. The remaining changes for 0.25 should be much easier to finish I would expect.

[arg0d]

Is this necessary? Before I implemented the default value simply using var.

[dignifiedquire]

I had issues with this, I don't remember which types, but some of the newer ones didn't end up producing the correct defaults. It might have been the external ones

[arg0d]

Where did you observe these issues? In tests, or in your own project? I ran the tests and everything passed.

diff --git a/bindgen/templates/AsyncTypesTemplate.go b/bindgen/templates/AsyncTypesTemplate.go
index 0359737..2bafaa4 100644
--- a/bindgen/templates/AsyncTypesTemplate.go
+++ b/bindgen/templates/AsyncTypesTemplate.go
@@ -28,9 +28,10 @@ func {{ result_type|future_callback }}(
        if err != nil {
                {%- match result_type.return_type -%}
                {%- when Some with (return_type) -%}
+               var _uniffiDefaultValue {{ return_type|type_name }}
                // {{ format!("{:?}", result_type.return_type) }}
                done <- {{ result_type|future_chan_type }} {
-                       val: {{ result_type.return_type|default_type }},
+                       val: _uniffiDefaultValue,
                        err: err,
                }
                {%- else %}

[dignifiedquire]

In tests, but it might be other changes that made this obsolete

[arg0d]

I would use var if there are no problems with it, it seems like it would also fix #21.

[dignifiedquire]

fixed

[arg0d]

If you decided to use 0.25, please use v0.25.0-1 tag from our fork.

[dignifiedquire]

I will try and get back to this towards the end of the week

[arg0d]

FYI: I'm going on vacation for a week, be back on Dec 08.

[dignifiedquire]

enjoy, will hopefully have some more code for you to review when you come back

[arg0d]

Oops, I meant until Nov 8, will be back next week :D

[dignifiedquire]

Dez 8 would be an awesome extended holiday 🤣

[dignifiedquire]

Started work on upgrading to 0.25 here #26

[arg0d]

Closed in favor of #26.