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

PyInstaller: Dockerfileからビルド関連のコードを削除 #484

Merged

Conversation

y-chan
Copy link
Member

@y-chan y-chan commented Oct 7, 2022

内容

CI上で使わなくなった部分を一旦削除し、PyInstallerブランチをmainにマージできる状態にします。
Dockerfileの扱いについては引き続き #482 で議論します。

関連 Issue

その他

私の手元でDokcer Daemon周りが壊れてしまって実行できないので、一旦Draftにしておきます。

@y-chan y-chan changed the title Dockerfileからビルド関連のコードを削除 PyInstaller: Dockerfileからビルド関連のコードを削除 Oct 7, 2022
@github-actions
Copy link

github-actions bot commented Oct 7, 2022

Coverage Result

Resultを開く
Name Stmts Miss Cover
voicevox_engine/init.py 1 0 coverage-100%
voicevox_engine/acoustic_feature_extractor.py 75 0 coverage-100%
voicevox_engine/dev/synthesis_engine/init.py 2 0 coverage-100%
voicevox_engine/dev/synthesis_engine/mock.py 36 2 coverage-94%
voicevox_engine/full_context_label.py 162 3 coverage-98%
voicevox_engine/kana_parser.py 86 1 coverage-99%
voicevox_engine/model.py 154 7 coverage-95%
voicevox_engine/mora_list.py 4 0 coverage-100%
voicevox_engine/part_of_speech_data.py 5 0 coverage-100%
voicevox_engine/preset/Preset.py 12 0 coverage-100%
voicevox_engine/preset/PresetLoader.py 34 1 coverage-97%
voicevox_engine/preset/init.py 3 0 coverage-100%
voicevox_engine/synthesis_engine/init.py 5 0 coverage-100%
voicevox_engine/synthesis_engine/core_wrapper.py 199 159 coverage-20%
voicevox_engine/synthesis_engine/make_synthesis_engines.py 48 39 coverage-19%
voicevox_engine/synthesis_engine/synthesis_engine.py 133 12 coverage-91%
voicevox_engine/synthesis_engine/synthesis_engine_base.py 66 9 coverage-86%
voicevox_engine/user_dict.py 131 10 coverage-92%
voicevox_engine/utility/init.py 3 0 coverage-100%
voicevox_engine/utility/connect_base64_waves.py 35 3 coverage-91%
voicevox_engine/utility/engine_root.py 9 2 coverage-78%
TOTAL 1203 248 coverage-79%

@Hiroshiba
Copy link
Member

PRありがとうございます!!

私の手元でDokcer Daemon周りが壊れてしまって実行できないので、

ちょっと待機などが面倒かもですが、y-chanさんのリポジトリ内でreleaseを作ってactions上でデバッグしちゃえるかもと思いました!
Draft開けお待ちしています・・・!

@Hiroshiba Hiroshiba closed this Oct 7, 2022
@Hiroshiba Hiroshiba reopened this Oct 7, 2022
@Hiroshiba
Copy link
Member

@y-chan さん、こちらの調子いかがでしょう👀 (催促すみません 🙇‍♂️ )

一部の環境でDirectMLが動かない問題を解決するために、pyinstaller化されていると嬉しいなという感じなのでちょっとお伺いしてみた次第です!

@y-chan y-chan marked this pull request as ready for review November 3, 2022 05:54
@y-chan
Copy link
Member Author

y-chan commented Nov 3, 2022

大変遅くなりました!
手元でビルドできることが確認できたのでReady for reviewにしました!

一点だけ、この状態では問題があって、ユーザーフォルダに一時ファイルが残るみたいなので、それを解消する必要があるかもです。
(PyInstallerを利用しているSHAREVOXの例です https://twitter.com/sabonerune/status/1569326970777006080?s=20&t=517afIrH3_wxm2_KD400iQ )

@Hiroshiba
Copy link
Member

Ready for reviewありがとうございます!!

--onefileを指定しているとたしかにTEMPになにか生まれたりするのですが、指定してないから原因はonefile関連じゃない・・・かも・・・?)

@y-chan
Copy link
Member Author

y-chan commented Nov 3, 2022

あ、説明が足りてなかったです...
onefileオプションを適用した設定のspecファイルを使っていたので、それをonefileを使わないバージョンに書き換えてあげる必要があるって感じです...!
#503 の内容を適用するだけで済むはずです...!

@Hiroshiba
Copy link
Member

あっなるほど!すみません勘違いで指摘しちゃってました! 早速のPRありがとうございます!!

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.

Dockerfileを書いてくださったのは @aoirint さんなので、もしよければレビュー頂けるととても心強いです・・・!

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!!

ライセンスを作成する部分・VOICEVOX RESOURCEを使う部分、onnxruntimeを持ってくる部分などがActions側と同じで2箇所に分散している形なので、いずれ1つに統合したいですね・・・!
action側でビルドしたのをdockerに持ってくる形を提案していましたが、どっちかというとビルド用シェルスクリプトを用意してaction内とdockerfile内両方から参照するのが良いのかなとちょっと思いました!

Copy link
Member

@aoirint aoirint left a comment

Choose a reason for hiding this comment

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

LGTM!!

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.

3 participants