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

version取得APIを作った #260

Merged
merged 14 commits into from
Sep 8, 2022
Merged

Conversation

qwerty2501
Copy link
Contributor

@qwerty2501 qwerty2501 commented Aug 29, 2022

内容

バージョン判別の需要がありそうだったのでこの際作ることにした
release build時にCargo.tomlにバージョン情報を埋め込むことで対応する

関連 Issue

VOICEVOX/voicevox_engine#454

その他

python APIがmergeされてから本格的にreviewしたほうがよさそう

@qwerty2501 qwerty2501 force-pushed the feature/version_api branch from d85c369 to 3abd816 Compare August 29, 2022 17:13
qryxip and others added 3 commits September 3, 2022 10:41
* Export more items in `voicevox_core`

* Add `voicevox_core_python_api`

* Add pyd files to .gitignore

* Install Python 3.8

* Build only `voicevox_core_c_api` in `build-unix-cpp-example`

* Add `build-python-api` job

* Deny non-Unicode path for `open_jtalk_dict_dir`

* Remove an unused dependency

* Use pyo3-log again

* Add `VoicevoxCore.is_gpu_mode` as a `@property`

* Add example/pyo3/.gitignore

* Add `help`s

* Add example/pyo3/README.md

* Insert an empty line

* Use `@pydantic.dataclasses.dataclass` for `SupportedDevices`

* Load `./*.dll`

* Modify run.py

* Do not allow `None` for `acceleration_mode`

* Reorder methods

* Move pyproject.toml

* Move requirements.txt

* Release whl

* Leave the `package.version` of voicevox_core_python_api as is

* Add crates/voicevox_core_python_api/README.md

* Minor refactor

* Improve "環境構築"

* Fix a typo

Co-authored-by: Nanashi. <[email protected]>

* Fix a link

* Add `voicevox_core_python_api/directml` feature

* Update crates/voicevox_core_python_api/README.md

* Add a comment

* Remove /requirements.txt

* `pass` → `...`

* Type the return type of `VoicevoxCore.decode`

* Minor refactor

Co-authored-by: Nanashi. <[email protected]>
@qwerty2501 qwerty2501 marked this pull request as ready for review September 3, 2022 01:42
@qwerty2501
Copy link
Contributor Author

reviewできるようになりました

@qwerty2501
Copy link
Contributor Author

versionを0.0.0にしました

@qwerty2501 qwerty2501 temporarily deployed to false September 4, 2022 07:56 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 4, 2022 07:56 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 4, 2022 07:57 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 4, 2022 07:57 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 4, 2022 07:58 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 4, 2022 07:59 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 4, 2022 07:59 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 4, 2022 07:59 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 4, 2022 08:08 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 4, 2022 08:09 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 4, 2022 08:11 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 4, 2022 08:12 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 4, 2022 08:13 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 4, 2022 08:13 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 4, 2022 08:13 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 4, 2022 08:14 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 4, 2022 08:14 Inactive
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!!

Comment on lines 92 to 95
run: |
set-cargo-version ./crates/voicevox_core/Cargo.toml ${{ env.VERSION }}
set-cargo-version ./crates/voicevox_core_c_api/Cargo.toml ${{ env.VERSION }}
set-cargo-version ./crates/voicevox_core_python_api/Cargo.toml ${{ env.VERSION }}
Copy link
Member

Choose a reason for hiding this comment

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

将来増えたときに抜け漏れしそうだなとちょっとだけ思いました。
voiccevox_*/Cargo.tomlとかできるならこっちでも良いかも。

Copy link
Member

@sevenc-nanashi sevenc-nanashi Sep 7, 2022

Choose a reason for hiding this comment

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

for cargo_toml in $( echo ./crate/voicevox_core_*/Cargo.toml ); do
  set-cargo-version $cargo_toml ${{ env.VERSION }}
done

で出来なくはなさそう。

Copy link
Member

@qryxip qryxip Sep 7, 2022

Choose a reason for hiding this comment

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

Suggested change
run: |
set-cargo-version ./crates/voicevox_core/Cargo.toml ${{ env.VERSION }}
set-cargo-version ./crates/voicevox_core_c_api/Cargo.toml ${{ env.VERSION }}
set-cargo-version ./crates/voicevox_core_python_api/Cargo.toml ${{ env.VERSION }}
shell: bash
run: |
cargo metadata --format-version 1 |
jq -r '.workspace_members as $members | .packages[] | select(.id as $id | $members | any(. == $id)) | .manifest_path' |
xargs -d '\n' -i{} -n 1 set-cargo-version {} "$VERSION"

ちゃんとやるんだったらこんな感じかな、とは思います。
(edit) 改行を入れました

Copy link
Member

Choose a reason for hiding this comment

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

./crate/voicevox_core*/Cargo.tomlでBash的に駄目な文字は出ないでしょうからnanashiさんのやつでもいいとは思いますが。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

あまり複雑過ぎてもメンテできなくなりそうなので、nanashiさんの案をもとにつくりました

@qwerty2501 qwerty2501 temporarily deployed to false September 7, 2022 14:19 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 7, 2022 14:19 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 7, 2022 14:19 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 7, 2022 14:19 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 7, 2022 14:19 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 7, 2022 14:19 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 7, 2022 14:19 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 7, 2022 14:19 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 7, 2022 17:40 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 7, 2022 17:40 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 7, 2022 17:40 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 7, 2022 17:40 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 7, 2022 17:40 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 7, 2022 17:41 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 7, 2022 17:41 Inactive
@qwerty2501 qwerty2501 temporarily deployed to false September 7, 2022 17:42 Inactive
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!

@Hiroshiba Hiroshiba merged commit 266a50c into VOICEVOX:main Sep 8, 2022
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