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

Java API:ビルドWorkflowを追加 #621

Merged
merged 40 commits into from
Oct 9, 2023

Conversation

sevenc-nanashi
Copy link
Member

内容

Java APIをビルドするWorkflowを追加します。
publishToMavenLocalした後に~/.m2/repository/をzip圧縮しています。
https://github.com/sevenc-nanashi/vv_core_registry/blob/c874a9569934d8442160353b6a2422fc4870cd5b/inject.ts#L166 こんな感じのスクリプトを書けば配信できます。

関連 Issue

その他

サンプルビルド: https://github.com/sevenc-nanashi/voicevox_core/releases/tag/0.15.0-preview.java-workflow.3

- name: Set up Java
uses: actions/setup-java@v2
with:
java-version: "17"
Copy link
Member Author

Choose a reason for hiding this comment

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

Android Gradle Pluginは17以上じゃないと動かなかったので。

- name: set cargo version
run: |
cargo set-version "$VERSION" --exclude voicevox_core_python_api --exclude download --exclude xtask
- name: "Download artifact (windows-x64-cpu)"
Copy link
Member Author

Choose a reason for hiding this comment

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

かなり混沌としてるので何とかしたい(何とかできなさそうな感じはする)

Copy link
Member

Choose a reason for hiding this comment

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

github cli使ってダウンロードするところからgradle叩くところまでをシェルスクリプトにして実行、とかですかねぇ 😇
(本当はこっちのほうがローカルでも動かせるので良い)

Copy link
Member Author

Choose a reason for hiding this comment

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

github cliは実行中のArtifactを落とせないっぽみがありました(記憶違いかも)

cp -v "artifact/$artifact_name"/* "crates/voicevox_core_java_api/lib/src/main/resources/jniLibs/${target}/"
done

cp ${{ steps.setup-ndk.outputs.ndk-path }}/toolchains/llvm/prebuilt/linux-x86_64/sysroot/usr/lib/aarch64-linux-android/libc++_shared.so crates/voicevox_core_java_api/lib/src/main/resources/jniLibs/arm64-v8a/
Copy link
Member Author

Choose a reason for hiding this comment

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

stdc++_shared.soがないとロード時にクラッシュするので。

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.

正直なところ、ちょっと需要に対してメンテコード大きすぎそうだな〜と感じました。。
2〜30行くらいなら良いのですが、250行は・・・。

とりあえず需要がわかっている(自分たちで使いたい)android版だけ作っといて、それ以外は実際にニーズが現れてから考えるのでも遅くないかも・・・?
(あるいは+windowsのdirectml版だけとか)

Comment on lines 297 to 298
ls -l ./target/wheels
echo "whl=$(find ./target/wheels -type f | head -1)" >> "$GITHUB_OUTPUT"
Copy link
Member

Choose a reason for hiding this comment

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

これなんで必要になった感じでしょう 👀

Copy link
Member Author

Choose a reason for hiding this comment

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

分からないですね…なぜか落ちるようになりました

Copy link
Member

Choose a reason for hiding this comment

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

もしかしたらキャッシュが原因かなと思いました。やっぱキャッシュむずい!!!!!!!!!!!!!

Copy link
Member Author

Choose a reason for hiding this comment

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

あー!その可能性はありますね
上でrm -rf ./target/wheels/*を走らせれば直りはするでしょうが、まぁ怖くなってきました

Comment on lines 260 to 269
- name: cache target
uses: actions/cache@v3
if: github.event.inputs.is_production != 'true'
with:
path: |
~/.cargo
target
key: ${{ matrix.artifact_name }}-rust-${{ hashFiles('**/Cargo.lock') }}
restore-keys: |
${{ matrix.artifact_name }}-rust-
Copy link
Member

@Hiroshiba Hiroshiba Sep 20, 2023

Choose a reason for hiding this comment

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

結構早くなる感じでしょうか?
(キャッシュ、すぐバグにつながる印象があるのでちょっと避けたいなという思いがあります 😇 )

Copy link
Member Author

Choose a reason for hiding this comment

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

5分くらいですね。
ビルドにスピードが求められるのはWorkflowテストくらいなので製品版ビルドでは無効化しています

Copy link
Member

Choose a reason for hiding this comment

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

なるほどです、確かに5分短縮は嬉しいのですが・・・。
キャッシュのキーってこのリポジトリ全てで共通なグローバル変数で、かつ未来方向と過去方向にもちゃんと一意になっているかを意識し続けないといけないため、相当気を使うんですよね・・・。
これ以降のコードがキャッシュファイルがあることを想定して書く必要もあり、かつそうなっていない気がするので、できれば避けたい気持ちがあります。

Copy link
Member

Choose a reason for hiding this comment

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

https://github.com/Swatinem/rust-cacheではなくactions/cacheを使っている理由はあったりしますでしょうか?

(キーとしてはCargo.lock以外にもrust-toolchainだったりあるかなと思った次第で、rust-cacheはそれも含んでくれます)

Copy link
Member Author

Choose a reason for hiding this comment

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

ないですね、そっちにしてみました。(まぁキャッシュ自体を消す可能性もありますが)

@sevenc-nanashi
Copy link
Member Author

とりあえず需要がわかっている(自分たちで使いたい)android版だけ作っといて、それ以外は実際にニーズが現れてから考えるのでも遅くないかも・・・?

Android版だけにしました。

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.

一点だけ!

.github/workflows/build_and_deploy.yml Outdated Show resolved Hide resolved
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

@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/build_and_deploy.yml Outdated Show resolved Hide resolved
.github/workflows/build_and_deploy.yml Outdated Show resolved Hide resolved
@sevenc-nanashi
Copy link
Member Author

レビューを反映しました。

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

すみません見落とし1箇所だけコメントしました!
意図的でしたらそのままマージしていただければ。

.github/workflows/build_and_deploy.yml Show resolved Hide resolved
.github/workflows/build_and_deploy.yml Outdated Show resolved Hide resolved
@sevenc-nanashi
Copy link
Member Author

マージします。

@sevenc-nanashi sevenc-nanashi merged commit ae8d247 into VOICEVOX:main Oct 9, 2023
30 checks passed
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