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

Null Safe Support #13

Open
richard457 opened this issue Mar 14, 2021 · 18 comments
Open

Null Safe Support #13

richard457 opened this issue Mar 14, 2021 · 18 comments

Comments

@richard457
Copy link
Contributor

  1. Flutter v2 is out with sound null safety
  2. It would be nice to have this in the package.
@Rudiksz
Copy link
Owner

Rudiksz commented Mar 14, 2021

It was in the works, but with the new "stable" release of Dart they introduced some breaking changes in dart:ffi, Changes that somehow I had no idea are coming even though I develop and test on the master branch.

It's in the works.

@richard457
Copy link
Contributor Author

@Rudiksz looked at the codebase, but I think in order for me to contribute, it's good if we can have a talk so we can have some common understanding.

@Rudiksz
Copy link
Owner

Rudiksz commented Apr 12, 2021

I have almost completed the migration and I hope I'll be able to publish the new version this week.

@richard457
Copy link
Contributor Author

@Rudiksz I really like this package and I am doing extensive experiments with it, will be happy to write documentation and some writing about it and also to contribute moving forward. Looking to #null safety

@Rudiksz
Copy link
Owner

Rudiksz commented Apr 23, 2021

You can find a prerelease on the ffi1.0 branch: https://github.com/Rudiksz/couchbase_lite_dart/tree/ffi1.0

Pub release:
https://pub.dev/packages/couchbase_lite_dart/versions/0.5.0-nullsafety.2

A few things to note:

  • All tests are green on Windows and MacOS, but I did not test it yet with a Flutter app (todo).
  • Added support for MacOS.
  • Changed the location where the binaries should be added to projects
  • Binaries now are available on the Releases page of this repo.
  • This version is not backwards compatible and I'm not planning to support Dart below 2.12 and Flutter 1.x anymore
  • Note: Building Android and iOS binaries and testing them is still work in progress. Help is much appreciated.

No API changes were made so it should work with existing code, as long as the code is null-safe.
Added empty constructors the Database, Document, Query and Replicator classes to create 'empty' classes - this needs some extra testing.

@richard457
Copy link
Contributor Author

@Rudiksz Thanks for the good work, doing testing with the existing app now.

@richard457
Copy link
Contributor Author

richard457 commented Apr 30, 2021

Tested the new API here is my finding

  1. The API did not change ( :) happy)
  2. When building a windows app the build does not copy the dynlib inbuilt app release so I have to manually copy the folder into the release folder
  3. The android does not work (No binaries known)

@Rudiksz
Copy link
Owner

Rudiksz commented Apr 30, 2021

  1. The API did not change ( :) happy)

Yes. but with a big caveat. While it's nullsafe for Dart, it is definitely not null-safe yet, and there's a few extra internal checks I need to implement to make the C memory management truly nullsafe. At the moment you should consider every class (Database, Document, Query, Replicator) as nullable and use their isEmpty method before performing operations on them - unless you know for sure you initialised them. isEmpty basically checks that the underlying C pointer is not a nullpointer. There are still some cases that would cause a hard crash on the C side. It's definitely not production ready yet.

I'm still deciding how to handle these checks on the Dart side.
Take the following Query class for example.

final query = Query.empty();
final string = query.explain();

This will crash the app with a nullptr exception. Of course I will add a guard in the method, but the question is: should it throw a Dart excpetion, or just fail silently and return an empty string? I'm leaning to the second option, but I'm open to other opinions.

  1. When building a windows app the build does not copy the dynlib inbuilt app release so I have to manually copy the folder into the release folder

This was the case also before. When Flutter builds a windws app it only copies the flutter dll at the moment. Maybe there's a gradle setting or something to include custom artifacts in the release folder, but I haven't looked yet.

Flutter's build process does not seem to support bundling dynamic libraries from external packages. All the examples they have is how to link them if you are building an app. Linking in a package and then requiring the package does not automatically link them to you app. That's why you have to manually copy the libraries in your app's folder when you are developing.

What is new I guess, is that the windows dll is no longer bundled with the package, and you need to manually copy it to your app. Just like the android and macos files. I felt there's no reason to keep it separate and it makes the pub package itself much smaller and releases that don't affect the C library (I do have some custom hacks) much leaner to upgrade to.

  1. The android does not work (No binaries known)

I uploaded them to the release page. Did some initial testing only for now.

@richard457
Copy link
Contributor Author

did another deep testing now on the release app, the app works just fine on the same machine where the development is, but then run the same app to another machine it does not work, even when copied the folder "dynlib" in the same folder. I can confirm that the issue is how couchbase_lite_dart is loading .dll files. @Rudiksz

@richard457
Copy link
Contributor Author

richard457 commented May 6, 2021

  1. The API did not change ( :) happy)

Yes. but with a big caveat. While it's nullsafe for Dart, it is definitely not null-safe yet, and there's a few extra internal checks I need to implement to make the C memory management truly nullsafe. At the moment you should consider every class (Database, Document, Query, Replicator) as nullable and use their isEmpty method before performing operations on them - unless you know for sure you initialised them. isEmpty basically checks that the underlying C pointer is not a nullpointer. There are still some cases that would cause a hard crash on the C side. It's definitely not production ready yet.

I'm still deciding how to handle these checks on the Dart side.
Take the following Query class for example.

final query = Query.empty();
final string = query.explain();

This will crash the app with a nullptr exception. Of course I will add a guard in the method, but the question is: should it throw a Dart excpetion, or just fail silently and return an empty string? I'm leaning to the second option, but I'm open to other opinions.

  1. When building a windows app the build does not copy the dynlib inbuilt app release so I have to manually copy the folder into the release folder

This was the case also before. When Flutter builds a windws app it only copies the flutter dll at the moment. Maybe there's a gradle setting or something to include custom artifacts in the release folder, but I haven't looked yet.

Flutter's build process does not seem to support bundling dynamic libraries from external packages. All the examples they have is how to link them if you are building an app. Linking in a package and then requiring the package does not automatically link them to you app. That's why you have to manually copy the libraries in your app's folder when you are developing.

What is new I guess, is that the windows dll is no longer bundled with the package, and you need to manually copy it to your app. Just like the android and macos files. I felt there's no reason to keep it separate and it makes the pub package itself much smaller and releases that don't affect the C library (I do have some custom hacks) much leaner to upgrade to.

  1. The android does not work (No binaries known)

I uploaded them to the release page. Did some initial testing only for now.

This will crash the app with a nullptr exception. Of course I will add a guard in the method, but the question is: should it throw a Dart excpetion, or just fail silently and return an empty string? I'm leaning to the second option, but I'm open to other opinions. I think for this case failing silently is not bad I like non-blocking error and throwing dart error I guess will break the entire execution

@richard457
Copy link
Contributor Author

bundling dynamic libraries from external packages

Was looking to this https://pub.dev/packages/external_asset_bundle as a way we can load external library but would like to see other options, still Researching.

@richard457
Copy link
Contributor Author

bundling dynamic libraries from external packages

flutter/flutter#81978

@Rudiksz
Copy link
Owner

Rudiksz commented May 6, 2021

Copying the dll file should work, but probably I have to fix in the library, because I changed that part.

@richard457
Copy link
Contributor Author

@Rudiksz any update?

@Rudiksz
Copy link
Owner

Rudiksz commented May 21, 2021

Try the new 0.5.0-nullsafety.3 version.

Please note that I changed the location of the libraries (again), from dynlib to vendor/cblite. This is relative to your project root. I'm not aware of any accepted standard in Dart and Flutter to where to put third party packages, and vendor being the standard in some other languages I like it more than dynlib. I will keep support for vendor even if a different location emerges in the dart community.

Edit: for windows, when distributing exe files, the dll can be either next to the exe file (in the same folder), or in the vendor/cblite subfolder relative to the exe.

@richard457
Copy link
Contributor Author

richard457 commented May 22, 2021

should the .dll files from v.0.5.0-null safety.2 be used? or there are other compiled files for this 0.5.0-nullsafety.3 versions.?

@richard457
Copy link
Contributor Author

I agree with you, the vendor seems familiar with the developer community

@Rudiksz
Copy link
Owner

Rudiksz commented May 22, 2021

should the .dll files from v.0.5.0-null safety.2 be used? or there are other compiled files for this 0.5.0-nullsafety.3 versions.?

The same same ones are good. There's no other change in this release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants