-
Notifications
You must be signed in to change notification settings - Fork 577
feat(binding/dart): add pubspec info #5751
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
Conversation
Since we need to build our rust core first, I assume we need to generate artifacts in CI and then publish them? |
0d0111c
to
df99165
Compare
Did some research, and an issue. fzyzcjy/flutter_rust_bridge#2624 (comment) but it will need yet another tool called cargokit https://github.com/irondash/cargokit/blob/main/docs/precompiled_binaries.md So I guess just let the user manually put the prebuild .so to the directory will be simpler. in CI, build it, upload it, and let user put them into the folder. And if |
Hi, my expectations for OpenDAL Dart are that users can use it just like a native Dart package: name: my_app
dependencies:
opendal: ^0.1.0 They shouldn't need to install rustup or even be aware of rust. This is how our Python and Node.js bindings work. Can we implement the same approach for dart? |
pyo3 build wheels and upload them to pypi, user download them based on their py version and system. napi-rs upload all the binaries to different packages. so in theory dart will need to similar things. something like this: https://github.com/dart-archive/tflite_native/tree/master/lib/src/blobs |
Yep, that's the thing we have to figure out. Can we upload it to pub.dev directly? Or we have to store it in a git repo like golang? |
I find this https://pub.dev/packages/tflite_native/install so we can. One way I guess is to build all version of so/dll, then put them to repo in CI, pub it to pub.dev (is this possible?) So we don't need to put binary in repo. |
That will be really nice! |
it's possible lol. I tested it on another simpler repo. |
ok, it works. The final step is "pub it to pub.dev", which require the "opendal" account I guess. |
6886b06
to
16992cc
Compare
We need to give this more thought. For example, Linux x86_64 and Linux ARM will require different artifacts. We can implement them in the future, but we should choose the right naming pattern now.
We need to build |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other looks good to me, I think it's fine to start with a build only actions.
name: build-${{ matrix.os }} | ||
path: bindings/dart/rust/target/release/*opendal* | ||
|
||
combine-artifacts: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should build only for RC tags and publish only for formal tags.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how to modify it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need something like
opendal/.github/workflows/release_python.yml
Line 117 in b8e778c
if: ${{ startsWith(github.ref, 'refs/tags/') && contains(github.ref, '-') }} |
We can implement in next PR.
Idea: cargo build --target will put the binary to a sub folder, name follows https://doc.rust-lang.org/beta/rustc/platform-support.html and fzyzcjy/flutter_rust_bridge#2644 (comment) says we can input the dll path. so now each arch will have it's own path. |
16992cc
to
6687e56
Compare
from the output
now all lib at their unique position. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, looks great!
Which issue does this PR close?
part of #5722 .
Rationale for this change
prepare for the publish
What changes are included in this PR?
Are there any user-facing changes?
user may able to install it through https://pub.dev/
ref
Seems that user need to install rust and build the package themselves.