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.

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

ダウンロードスクリプトの作成とドキュメントの修正 #249

Merged
merged 100 commits into from
Sep 11, 2022
Merged

ダウンロードスクリプトの作成とドキュメントの修正 #249

merged 100 commits into from
Sep 11, 2022

Conversation

qwerty2501
Copy link
Contributor

内容

voicevox_coreのダウンロードと追加ライブラリのダウンロードを行うスクリプトを作成します(shell script,power shell)
また現状READMEを修正している人がいないのでそれに伴いREADMEを修正する必要があるがこのさいRust化したことによってつじつまが合わなくなった部分や、不足しているもの(Pythonのexampleの説明はトップREADMEにあるのにC++のexamleの説明はリンクすらないこと)などを直していこうと思います。

関連 Issue

refs #213

その他

@Hiroshiba
Copy link
Member

ダウンロードスクリプトの用途はなんでしょう👀
ユーザー向けか、別リポジトリで配布予定の追加リソースで使う用か、その両方かを想定しています。

もし別リポジトリで配布するだけの用途なら、言語の数を極力少なくするためにpower shellをやめてbashで統一する、という手もあるかな〜とおもいました！

@qwerty2501
Copy link
Contributor Author

主目的としてはユーザー向けです。なのでpower shell,bash両方あったほうがいいかなと考えてます。
まあWindowsでもbashは使えないこともないですし、第一弾としてはbashのみというのもありかなと考えてます

別リポジトリで配布予定の追加リソースで使う用

これちょっとわからなかったんですが https://github.com/VOICEVOX/voicevox_additional_libraries で配布予定のものをダウンロードする処理ということでよかったですか？

@Hiroshiba
Copy link
Member

Hiroshiba commented Aug 17, 2022 •
edited
Loading

voicevox_additional_librariesで使う用のスクリプト、という意図でした！

voicevox_additional_librariesでは本家のCUDAからのダウンロード、こちらのPRではvoicevox_additional_librariesからのダウンロード、ということなら賛成です！

@qwerty2501
Copy link
Contributor Author

voicevox_additional_librariesでは本家のCUDAからのダウンロード、こちらのPRではvoicevox_additional_librariesからのダウンロード、ということなら賛成です！

はい、このPRでは voicevox_additional_libraries からとってくる処理を実装する予定です

@qwerty2501 qwerty2501 force-pushed the feature/download_script_and_fix_documents branch 2 times, most recently from af7cd9f to 990b88b Compare August 20, 2022 17:17
@qwerty2501
Copy link
Contributor Author

とりあえず仮でダウンロードスクリプトを作ってみましたがまだ 追加ライブラリが https://github.com/VOICEVOX/voicevox_additional_libraries でリリースされていないため追加ライブラリのダウンロードは実装できてません。

ドキュメントについてもダウンロードスクリプトを前提としたものに変更しました。
exampleもダウンロードスクリプトを前提としたものにしましたが、Windowsについては確認できないため変更してません。

尚ドキュメントにあるダウンロードコマンドはダウンロードスクリプトが正式にrelease(pre releaseではだめ)されないと動作しないものになっています

@Hiroshiba
Copy link
Member

Hiroshiba commented Aug 20, 2022 •
edited
Loading

ざっと眺めて、方針的に問題なさそうに思いました！
（スクリプト二種のメンテナンスはなかなか大変そうですね。。可能ならダウンロードし続けられるかテストがあると修正する力学が働きやすいかもと少し思いました）

@qwerty2501
Copy link
Contributor Author

そうですね。テストは必要そうです。
ただリリース時にこのテストが実行されてしまうとお金がかかりすぎてしまうので工夫する必要はあるかもです

@Hiroshiba
Copy link
Member

たしかに…！！
いまリリースは手動でcore zipをアップロードする運用なので、そこが自動化するまでは手動にせざるを得ないかもと思いました。
（秘匿リソースの一般公開化まではもうしばらく時間が必要そうです…🙇‍♂️）

@qwerty2501
ダウンロードスクリプトの作成とドキュメントの修正
8cb4662 
 ## 内容
 voicevox_coreのダウンロードと追加ライブラリのダウンロードを行うスクリプトを作成します(shell script,power shell)
 また現状READMEを修正している人がいないのでそれに伴いREADMEを修正する必要があるがこのさいRust化したことによってつじつまが合わなくなった部分や、不足しているもの(Pythonのexampleの説明はトップREADMEにあるのにC++のexamleの説明はリンクすらないこと)などを直していこうと思います。
@qwerty2501
WIP releasesからartifactをダウンロードして展開するスクリプトを作成した
715d8b5
@qwerty2501
renamed baseurl
78d84c0
@qwerty2501
open_jtalkダウンロード機能を実装した
b99df45
@qwerty2501
directory名変更する
57edf71
@qwerty2501
新しいバージョンを取得する関数を分けた
e2713e7
@qwerty2501
option指定方法を変更
9e195d1
@qwerty2501
変数名を短くした
aa90248
@qwerty2501
cudaはlinuxのみと記載した
5ff5464
@qwerty2501
format
5d00043
@qwerty2501
説明文を修正
628f238
@qwerty2501
script名変更
7ef7448
@qwerty2501
path変更した
26cd1e6
@qwerty2501
default値が有効になるようにした
7663a13
@qwerty2501
powershellダウンロードスクリプトを作った
05c2cc0
@qwerty2501
full optionをminにして通常はすべてダウンロードするようにした　
b607725
@qwerty2501
downloaderのドキュメントを作成した
64c0545
@qwerty2501
download関連のドキュメントを修正した
2f08cfa
@qwerty2501
pythonのexampleをdownloaderを使用した新しいものに修正した
b2f9a09
@qwerty2501
cmakeでのexampleをdownloaderを使用したものに更新
ae77e8e
@qwerty2501 qwerty2501 temporarily deployed to false September 10, 2022 15:03 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 10, 2022 15:03 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 10, 2022 15:03 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 10, 2022 15:03 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 10, 2022 15:04 Inactive
PickledChair
PickledChair approved these changes Sep 10, 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！（ #249 (comment) の問題については別 PR を出そうかなと思いました）

@qwerty2501
Copy link
Contributor Author

あ、取り込んじゃったんですけどこれ元のやつが正しかったですね

@PickledChair
Copy link
Member

PickledChair commented Sep 10, 2022 •
edited
Loading

すみません、先ほど指摘した修正点 #249 (comment) は私の勘違いでした……（Invoke-WebRequest のオプションと勘違いしていました）-Output で正しかったです。

@qwerty2501 qwerty2501 temporarily deployed to false September 10, 2022 15:54 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 10, 2022 15:54 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 10, 2022 15:54 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 10, 2022 15:54 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 10, 2022 15:55 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 10, 2022 15:55 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 10, 2022 15:55 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 10, 2022 15:55 Inactive
@qwerty2501
Copy link
Contributor Author

@PickledChair 戻しました

@qwerty2501
Copy link
Contributor Author

@Hiroshiba これmerge後にpre-release buildしたほうが良いかもです。ドキュメント通りにダウンロードできるのはlatestじゃないとだめですが、一応コマンド変えればダウンロードできるようになるので

@PickledChair
Copy link
Member

@PickledChair 戻しました

すみません、ありがとうございます。また、ついでに CMakeLists.txt の件についても対応していただきありがとうございます。

@Hiroshiba
Copy link
Member

@Hiroshiba これmerge後にpre-release buildしたほうが良いかもです。

なるほどです。久々にビルドしてみたいと思います。

Hiroshiba
Hiroshiba approved these changes Sep 11, 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！！

まだlatestが更新されてないから、READMEに書かれてる通りにダウンローダーを実行してもうまく配置されないという認識で合っていますか👀
もしそうならしばらくは迷子の方が現れそうなので、issue作ってpinnedにして案内しようかなとちょっと思いました。

@PickledChair
Copy link
Member

PickledChair commented Sep 11, 2022 •
edited
Loading

まだlatestが更新されてないから、READMEに書かれてる通りにダウンローダーを実行してもうまく配置されないという認識で合っていますか👀 もしそうならしばらくは迷子の方が現れそうなので、issue作ってpinnedにして案内しようかなとちょっと思いました。

そうですね、まだ README 通りに実行しても、Releases の latest にダウンロードスクリプトがないのでそもそもスクリプトを取ってこれない、という結果になると思います（pre-release にスクリプトがある場合でも）。確かに新しい latest のリリースがあるまでは案内があると親切でよさそうですね

@qwerty2501
Copy link
Contributor Author

まだlatestが更新されてないから、READMEに書かれてる通りにダウンローダーを実行してもうまく配置されないという認識で合っていますか

はい。そのとおりです。
issueつくってpinで止めるのは良さそうですね。
approveもらってますがこれmergeしても大丈夫ですか？

@Hiroshiba
Copy link
Member

マージOKです！　マージしちゃいます･･･！
かなり大変な作業だったと思います、ありがとうございました！！

@Hiroshiba Hiroshiba merged commit 89a0911 into VOICEVOX:main Sep 11, 2022
@Hiroshiba Hiroshiba mentioned this pull request Sep 11, 2022
Closed
@Hiroshiba
Copy link
Member

pinned issueを作りました！

@qryxip qryxip mentioned this pull request Oct 16, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PickledChair PickledChair PickledChair approved these changes

@Hiroshiba Hiroshiba Hiroshiba 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.
4 participants
@qwerty2501 @Hiroshiba @PickledChair @qryxip