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

Add: x86_64-linux-android用のバインディングを追加 #21

Merged
merged 11 commits into from
Apr 23, 2023

Conversation

sevenc-nanashi
Copy link
Member

内容

タイトル通り。

関連 Issue

(なし)

スクリーンショット・動画など

(なし)

その他

(なし)

@sevenc-nanashi sevenc-nanashi marked this pull request as draft April 20, 2023 07:27
@sevenc-nanashi sevenc-nanashi marked this pull request as ready for review April 20, 2023 07:30
Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

この変更だけで良いんでしたっけ。。。
gen_bindとかの変更も必要…? (自信がない…)

参考になるかもプルリク↓

@PickledChair
Copy link
Member

PickledChair commented Apr 20, 2023

そうですね、 #17 と同様に .github/actions/auto_gen_bind_pr/action.yaml.github/workflows/gen_bind.yaml, .github/workflows/general.yml に target が x86_64-linux-android の場合の記述を書く必要があると思います。onnxruntime-sys/build.rsx86_64 の場合の記述もすでに入っているように見えたので大丈夫そう……? 若干足りなかったっぽいですね。記述の追加ありがとうございます。

@sevenc-nanashi
Copy link
Member Author

Workflow見逃してました。

Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ほぼLGTMです!!

.github/workflows/general.yml内に、ビルド後にファイルが有るかどうかのテスト?みたいなのがありました。
あとで実はなかったということにならないので、今書き足しとくと手戻りを避けられるかもです。
こちらが参考になりそうかも?↓
https://github.com/VOICEVOX/onnxruntime-rs/pull/17/files#diff-6226c87b3659341d3ff8baccd674e8bb1410faec979f28276d615854a1ada83d
(まあなくても良いかもです・・・。)

@sevenc-nanashi
Copy link
Member Author

うーん、いじってないのに落ちてますね

@qryxip
Copy link
Member

qryxip commented Apr 22, 2023

よく見てませんが、今のmaster上でre-runしたら落ちるやつではこれ...?

@Hiroshiba
Copy link
Member

よくわかってないですがとりあえずmasterをre-runさせてみました!

@qryxip
Copy link
Member

qryxip commented Apr 22, 2023

落ちましたね... 😇
https://github.com/VOICEVOX/onnxruntime-rs/actions/runs/4704867196

よくわかりませんが、とりあえず今 @sevenc-nanashi さんの手元にあるCargo.lockは保管しておいた方がいいかも。

@Hiroshiba
Copy link
Member

あれ、これなにも変えてないのになんで落ちてるんですかね・・・?
と思ったらCargo lockもrustのバージョンも管理されてないんですね。。。なるほどぉ。

@qryxip
Copy link
Member

qryxip commented Apr 22, 2023

「ライブラリ」クレートですからね…
(追記) いやvoicevox_coreもライブラリではあるんですが…

今名無し。さんの手元にあるCargo.lockは多分2日前のものだと思うので、それと最新のものとのdiffを見れば何が原因なのかを比較的容易に割り出せるかもしれません。

@Hiroshiba
Copy link
Member

なるほど!!! 

@qryxip
Copy link
Member

qryxip commented Apr 23, 2023

手元でcargo updateしてcargo testしましたが、再現しませんね...

Actions上では一貫してこれ(↓)で落ちているのですが、tracing系の更新は結構前のはず。バグったログメッセージが流れようとしている?

thread 'テスト名' panicked at 'called `Result::unwrap()` on an `Err` value: Syntax(
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
regex parse error:
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 1: (?x)
 2:                 ^(?P<global_level>(?i:trace|debug|info|warn|error|off|[0-5]))$ |
                                          ^
 3:                  #                 ^^^.
 4:                  #                     `note: we match log level names case-insensitively
 5:                 ^
 6:                 (?: # target name or span name
 7:                     (?P<target>[\w:-]+)|(?P<span>\[[^\]]*\])
 8:                 ){1,2}
 9:                 (?: # level or nothing
10:                     =(?P<level>(?i:trace|debug|info|warn|error|off|[0-5]))?
11:                      #          ^^^.
12:                      #              `note: we match log level names case-insensitively
13:                 )?
error: test failed, to rerun pass `-p onnxruntime --lib`
14:                 $
15:                 
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
error: Unicode-aware case insensitivity matching is not available (make sure the unicode-case feature is enabled)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
)', /Users/runner/.cargo/registry/src/github.com-1ecc6299db9ec823/tracing-subscriber-0.2.25/src/filter/env/directive.rs:167:14

Actionsのキャッシュが変な腐りかたをしている?とも思いましたが、キャッシュを行っていないcoverageも落ちてます。

@sevenc-nanashi
Copy link
Member Author

お、通りました

Copy link
Member

@qryxip qryxip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@PickledChair PickledChair left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@PickledChair PickledChair merged commit ebb9dcb into VOICEVOX:master Apr 23, 2023
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

Successfully merging this pull request may close these issues.

4 participants