-
Notifications
You must be signed in to change notification settings - Fork 119
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
Add: x86_64-linux-androidを追加 #473
Add: x86_64-linux-androidを追加 #473
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aarch64と同様にbuild and deploy workflow
に加えないとビルドできないと思います。
おっと。追加しました |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
UNDERSCORED_TRIPLE=$(sed 's/-/_/g' <<< "${{ matrix.target }}") | ||
echo "AR_$UNDERSCORED_TRIPLE=llvm-ar" >> "$GITHUB_ENV" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actionlintがエラーを吐いてますねぇ。
$(sed 's/-/_/g' <<< "${{ matrix.target }}")
という記法が怒られてそうです。
(たしかに見慣れない記法だなとは思いました)
actionlintでshellcheckを省けそうでした。
https://github.com/rhysd/actionlint/blob/94c4f0119fdc7f776c7a472a981121cafd17a3cd/docs/usage.md?plain=1#L36-L42
actionlintはこちらにあります。
voicevox_core/.github/workflows/test.yml
Lines 41 to 42 in 997847f
bash <(curl https://raw.githubusercontent.com/rhysd/actionlint/main/scripts/download-actionlint.bash) | |
./actionlint -color |
@qryxip actionlint、開発者側でlintする方法が示されていないので、どうしようか迷っています。
あったほうが絶対いいのですが、どうしようかなーと。
通らなくてもエラーにしないようにする(warnにする)か、READMEで案内するかとか・・・?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actionlintはエディタに統合するのが面倒ですし、確かにCIで落ちて気付くというのが大半になりそうです。
思ったのですが、#466 (comment)で言ってたreviewdogをactionlintに使うというのはどうでしょうか? reviewdog/action-actionlintというのがありますし、これをそのまま使えばよさそう。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reviewdogを使うの良いと思います!
あとついでにwarnにしてあげれば良いかも。
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1点以外はLGTMでした!
CI通りましたー。 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
shell: bash | ||
run: | | ||
echo "$ANDROID_NDK/toolchains/llvm/prebuilt/linux-x86_64/bin" >> "$GITHUB_PATH" | ||
echo "AR_aarch64_linux_android=llvm-ar" >> "$GITHUB_ENV" | ||
TRIPLE="${{ matrix.target }}" | ||
echo "AR_${TRIPLE//-/-}=llvm-ar" >> "$GITHUB_ENV" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
眺めていてこの部分が気になった(多分誤植?)のですが、CI が通っているのでちょっと不思議に思っています……(私の勘違いで、元の方が正しいということでしたらすみません)
echo "AR_${TRIPLE//-/-}=llvm-ar" >> "$GITHUB_ENV" | |
echo "AR_${TRIPLE//-/_}=llvm-ar" >> "$GITHUB_ENV" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
あ、本当だ... なんで通るんでしょうか?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
x86_64-android-29(略)でいい感じになってる説?
…もしかしてこのActionなくても動く?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ログを見る限り、AR-x64_86_linux(略)
とAR-x64_86-linux(略)
を両方チェックしてそうです。のでsedいらないと思います
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!!
マージします! @sevenc-nanashi このアーキテクチャの製品版coreはビルドしたほうが良いでしょうか 👀 |
あー、お願いしますー。 |
内容
タイトル通り。
関連 Issue
その他