-
Notifications
You must be signed in to change notification settings - Fork 128
[Rust]load_metas,metasを実装した #140
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
[Rust]load_metas,metasを実装した #140
Conversation
46513b7
to
463edcf
Compare
手元の環境(windows 10)で |
463edcf
to
eba2324
Compare
とりあえずCIでテスト結果が正確に反映されるようにしたのですがWindows server 2022だと失敗してますね。あとUbuntu18でも何故か失敗してます |
5c9819c
to
f4d691a
Compare
f4d691a
to
52ec149
Compare
Windowsの方はやはりdll読み込みで失敗してるようです |
@Hiroshiba 参考までに正しいバイナリファイルのURLってどれ使いましたか? |
@qwerty2501 |
Ubuntu 18.04 と 20.04 の場合で、rust-cache が同じキャッシュを使用してしまっているからと推測しました。 https://github.com/VOICEVOX/voicevox_core/actions/runs/2402391185 では両者ともキーとして以下が使用されていました。
この結果、18.04 の方で対応できない glibc のバージョンの設定があったか何かでこのエラーが出たのではないか、と考えています。念の為、自分のリポジトリでも CI を回してみました https://github.com/PickledChair/voicevox_core/actions/runs/2404701848 。初回はキャッシュがないので 18.04 でもビルドに成功していますが、「他のジョブで同名のキャッシュを作っているからキャッシュが保存できない」という警告が出ていました。 対応策としては、 https://github.com/Swatinem/rust-cache#inputs によるとキャッシュのキーに追加の文字列を付け加えられそうなので、それで解決できそうです(OSのバージョンなどを付け加えるなど)。ちょっと試してみて、うまくいったら PR を出そうと思います(あるいは、この PR 内で解決しますか?)。 |
cargo testがCI上でちゃんと実行されてないのもこのPRでやっちゃったんですが分けたほうが良かったですかね |
ちょっと考えてたのですが、「rust移行できそうかをなるはやで検証する」のを目的に動くのが良いのかなと思いました! |
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.
LGTM!
include_str!良いですね。バイナリも簡単に埋め込みできそう。
Ubuntu18.04の方は直しました |
0ab910c
to
49fec5b
Compare
@Hiroshiba いくつか調査と更新を加えました。 まずonnxruntime-rsのビルドに clang及びllvm が必要になりました。これは元のonnxruntime-rsだとC headerから生成したrustコードをgit repository上で管理していたのですが、それだとheaderが新しいバージョンになった際に各OSごとのコード生成を行わなければならないからです。その際に実際にそのOSで生成する必要があるためメンテナンスが困難(特にMac)になるため、ビルド時に各自で自動的にrustコードを生成するようにしてそれを参照するようにしました。clangとllvmはその際に必要になります。 次にテスト実行時にdllが読み込まれない点についてですが、 どうやら ここ でその設定が行われているようです。 rustc-link-searchに加えられたディレクトリはWindows環境だと PATH環境変数に含まれるため、dllが読み込まれるようです
そのためdllが読み込めてないのは何かがおかしいのでコピーするのはちょっと待ったほうが良さそうです clangとllvmについてですが恐らくopenjtalkをRustでwrapする際にも必要かと思われるのでそこまで痛手にはならないはずです 追記: |
clangとllvmが必要な件、一旦了解です!共有ありがとうございます。 windowsに関しても調査ありがとうございます。 テストは全部通らないかもですが、いったんマージできればと思っています。 |
まとめると、
ということでしょうか。 |
issue記載しておきました。 |
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.
キャッシュのキーに関して一点質問させていただきました。それ以外は大丈夫そうです!
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.
LGTM!
* load_metasを実装した * Statusのnew関数テストを追加した * load_metasのテストを追加した * なぜか.が入っていたため削除 * assertでresultの内容を表示するようにした * output err string * エラーを詳細に表示するように修正 * cache keyを設定した * onnxruntime versionup * Windows2019向けのlibclang install * onnxruntime version up * キャッシュ制御を環境変数で行うことをやめた
内容
Status.load_metasとそれに関連する dllで exportする用のC API関数であるmetas関数の実装を行った
関連 Issue
refs #128
その他
initialize関数内部でstatus.load_metasを呼び出すようにした
#139 が mergeされてからレビューすること