Skip to content

[Rust]release用のbuildとdeployのCI設定を作った #191

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jul 19, 2022
Merged

[Rust]release用のbuildとdeployのCI設定を作った #191

merged 2 commits into from
Jul 19, 2022

Conversation

qwerty2501
Copy link
Contributor

内容

T/O

関連 Issue

refs #128

その他

deployは未実装
workflow_runでtest workflowが実行されてから実行される設定にしてるが、この設定はworkflow_runが設定されてるworkflowがdefault branchにマージしないと動作しないらしく現段階だとどうさせることができないそうです

qwerty2501
qwerty2501 commented Jul 17, 2022
View reviewed changes
.github/workflows/build_and_deploy.yml
ORT_USE_CUDA: ${{ matrix.use_gpu }}
- name: deploy
if: startsWith( github.ref, 'refs/tags/' )
run: echo "TODO:deploy処理を実装する"
Copy link
Contributor Author

@qwerty2501 qwerty2501 Jul 17, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

普段のブランチでもrelease buildが通ることは確認したいのでrelease buildまでは実行されるようにし、tagが切られてるときだけdeploy処理が呼ばれるようにした(deploy自体はまだ未実装)

qwerty2501
qwerty2501 commented Jul 17, 2022
View reviewed changes
@qwerty2501 qwerty2501 changed the title release用のbuildとdeployのCI設定を作った [Rust]release用のbuildとdeployのCI設定を作った Jul 17, 2022
@qwerty2501 qwerty2501 force-pushed the feature/build_and_deploy_ci branch 2 times, most recently from b6bd619 to c8f61b4 Compare July 17, 2022 20:06
Hiroshiba
Hiroshiba reviewed Jul 17, 2022
View reviewed changes
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.

ありがとうございます！！

元からあるbulid.ymlの機能を足す必要があるので、元のbuild.ymlを変更する形で作るとあとで結合が楽になりそうかもと感じました。
が、一旦とりあえずビルドできる形を目指して、あとで結合する形でも大丈夫だと思います。
（github actions周りなら僕も参戦できるので）

そういえば、あとあとDirectML版も必要になりそうです。
こちらもまた後で全然良さそう。

@qwerty2501
Copy link
Contributor Author

qwerty2501 commented Jul 18, 2022 •
edited
Loading

@Hiroshiba

元からあるbulid.ymlの機能を足す必要があるので、元のbuild.ymlを変更する形で作るとあとで結合が楽になりそうかもと感じました。

その方法とも迷ったんですが、後で必要な部分を持ってくる方針のほうが楽かと思って今回の対応になってます

そういえば、あとあとDirectML版も必要になりそうです。

これについてはそもそもonnxruntime-rsが対応してないようなのでちょっと時間かかりそうです
まあこれを対応するころにはMSがonnxruntime-rsをhostingしてる可能性がありますが

@qwerty2501
release用のbuildとdeployのCI設定を作った
38fb9d1 
deployは未実装
windows x86はエラーがでるので未対応
@qwerty2501 qwerty2501 force-pushed the feature/build_and_deploy_ci branch from c38e1cd to 38fb9d1 Compare July 18, 2022 14:59
@qwerty2501 qwerty2501 force-pushed the feature/build_and_deploy_ci branch from 1623d99 to a2f117f Compare July 18, 2022 19:48
Hiroshiba
Hiroshiba approved these changes Jul 19, 2022
View reviewed changes
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！！

あ、featureってなんでしょう👀
みんなdefaultならmatrixに無い方が見通しいいのかなと感じた次第です

@PickledChair
Copy link
Member

@Hiroshiba

あ、featureってなんでしょう👀
みんなdefaultならmatrixに無い方が見通しいいのかなと感じた次第です

feature はコンパイル時に特定の機能を有効・無効にするための条件を指定できる Rust の機能です。voicevox_core クレート内では directml という feature を定義していて、例えば以下の箇所で使われています：

voicevox_core/crates/voicevox_core/src/status.rs

Lines 10 to 14 in f763655

cfg_if! {
if #[cfg(not(feature="directml"))]{
use onnxruntime::CudaProviderOptions;
}
}

voicevox_core/crates/voicevox_core/src/status.rs

Lines 179 to 190 in f763655

cfg_if! {
if #[cfg(feature = "directml")]{
session_builder
.with_disable_mem_pattern()?
.with_execution_mode(onnxruntime::ExecutionMode::ORT_SEQUENTIAL)?
} else {
let options = CudaProviderOptions::default();
session_builder
.with_disable_mem_pattern()?
.with_append_execution_provider_cuda(options)?
}
}

今の段階で全部 default なのは、 #191 (comment) で触れられていたように onnxruntime-rs がまだ DirectML に対応していないからだと思います。対応した場合はビルド時に directml feature を指定する必要があるので、今すでに feature の指定があっても良いのかな、と感じました！

PickledChair
PickledChair approved these changes Jul 19, 2022
View reviewed changes
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 mentioned this pull request Jul 19, 2022
Closed
43 tasks
@qwerty2501
Copy link
Contributor Author

featureについてはこのとおりです
#191 (comment)

@Hiroshiba
Copy link
Member

featureに関して理解しました！！ありがとうございます！

@Hiroshiba Hiroshiba merged commit da44655 into VOICEVOX:rust Jul 19, 2022
qwerty2501 added a commit to qwerty2501/voicevox_core that referenced this pull request Jul 23, 2022
@qwerty2501
[Rust]release用のbuildとdeployのCI設定を作った (VOICEVOX#191)
9914b41 
* release用のbuildとdeployのCI設定を作った

deployは未実装
windows x86はエラーがでるので未対応

* update version open_jtalk
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Hiroshiba Hiroshiba Hiroshiba approved these changes

@PickledChair PickledChair PickledChair approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@qwerty2501 @PickledChair @Hiroshiba