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

ShellCheckとactionlintを導入する #525

Closed
Hiroshiba opened this issue Dec 8, 2022 · 2 comments · Fixed by #1124
Closed

ShellCheckとactionlintを導入する #525

Hiroshiba opened this issue Dec 8, 2022 · 2 comments · Fixed by #1124
Assignees

Comments

@Hiroshiba
Copy link
Member

内容

Github Actionsとシェルスクリプトは結構難しいので、linterを導入して間違いを予め防止できると嬉しそうです。
そこでShellCheckactionlintの導入を検討しています。

Pros 良くなる点

間違いが減る。

Cons 悪くなる点

最初に出てくるエラーを解消していく必要がある。

実現方法

coreにはすでに導入済みで、 @qryxip さんのこちらのプルリクエストがとても参考になると思います。

その他

導入したあとのエラー解消が難しかったらぜひコメントください。

@Hiroshiba Hiroshiba added 機能向上 初心者歓迎タスク 初心者にも優しい簡単めなタスク 優先度:低 (運用中止) labels Dec 8, 2022
@tarepan tarepan mentioned this issue Feb 4, 2024
@tarepan
Copy link
Contributor

tarepan commented Feb 27, 2024

実行結果:

shellcheck

root ➜ /workspaces/voicevox_engine (master) $ git ls-files | grep -E '\.(ba)?sh' | xargs shellcheck

In build_util/create_venv_and_generate_licenses.bash line 14:
    source $VENV_PATH/Scripts/activate
           ^-------------------------^ SC1091 (info): Not following: ./Scripts/activate was not specified as input (see shellcheck -x).


In build_util/create_venv_and_generate_licenses.bash line 16:
    source $VENV_PATH/bin/activate
           ^---------------------^ SC1091 (info): Not following: ./bin/activate was not specified as input (see shellcheck -x).


In build_util/process_voicevox_resource.bash line 25:
for f in $(ls $DOWNLOAD_RESOURCE_PATH/engine/engine_manifest_assets/* | grep -v update_infos.json); do
           ^-- SC2010 (warning): Don't use ls | grep. Use a glob or a for loop with a condition to allow non-alphanumeric filenames.
              ^---------------------^ SC2086 (info): Double quote to prevent globbing and word splitting.

Did you mean: 
for f in $(ls "$DOWNLOAD_RESOURCE_PATH"/engine/engine_manifest_assets/* | grep -v update_infos.json); do

actionlint

vscode ➜ /workspaces/voicevox_engine (master) $ ./actionlint
.github/workflows/build.yml:150:9: shellcheck reported issue in this script: SC1003:info:3:72: Want to escape a single quote? echo 'This is how it'\''s done' [shellcheck]
    |
150 |         run: |
    |         ^~~~
.github/workflows/build.yml:150:9: shellcheck reported issue in this script: SC2193:warning:6:24: The arguments to this comparison can never be equal. Make sure your syntax is correct [shellcheck]
    |
150 |         run: |
    |         ^~~~
.github/workflows/build.yml:202:9: shellcheck reported issue in this script: SC2193:warning:3:24: The arguments to this comparison can never be equal. Make sure your syntax is correct [shellcheck]
    |
202 |         run: |
    |         ^~~~
.github/workflows/build.yml:284:9: shellcheck reported issue in this script: SC2034:warning:10:1: i appears unused. Verify use (or export if used externally) [shellcheck]
    |
284 |         run: |
    |         ^~~~
.github/workflows/build.yml:357:9: shellcheck reported issue in this script: SC2193:warning:4:28: The arguments to this comparison can never be equal. Make sure your syntax is correct [shellcheck]
    |
357 |         run: |
    |         ^~~~
.github/workflows/build.yml:421:9: shellcheck reported issue in this script: SC2193:warning:2:24: The arguments to this comparison can never be equal. Make sure your syntax is correct [shellcheck]
    |
421 |         run: |
    |         ^~~~
.github/workflows/build.yml:441:9: shellcheck reported issue in this script: SC2193:warning:8:24: The arguments to this comparison can never be equal. Make sure your syntax is correct [shellcheck]
    |
441 |         run: |
    |         ^~~~
.github/workflows/build.yml:441:9: shellcheck reported issue in this script: SC2193:warning:11:26: The arguments to this comparison can never be equal. Make sure your syntax is correct [shellcheck]
    |
441 |         run: |
    |         ^~~~
.github/workflows/build.yml:467:9: shellcheck reported issue in this script: SC2193:warning:30:28: The arguments to this comparison can never be equal. Make sure your syntax is correct [shellcheck]
    |
467 |         run: |
    |         ^~~~
.github/workflows/build.yml:507:9: shellcheck reported issue in this script: SC2016:info:6:22: Expressions don't expand in single quotes, use double quotes for that [shellcheck]
    |
507 |         run: |
    |         ^~~~
.github/workflows/build.yml:586:9: shellcheck reported issue in this script: SC2045:error:6:13: Iterating over ls output is fragile. Use globs [shellcheck]
    |
586 |         run: |
    |         ^~~~
.github/workflows/build.yml:586:9: shellcheck reported issue in this script: SC2012:info:12:9: Use find instead of ls to better handle non-alphanumeric filenames [shellcheck]
    |
586 |         run: |
    |         ^~~~
.github/workflows/release-test.yml:70:9: shellcheck reported issue in this script: SC2002:style:3:5: Useless cat. Consider 'cmd < file | ..' or 'cmd file | ..' instead [shellcheck]
   |
70 |         run: |
   |         ^~~~

追記: 問題解消した模様
追追記: ShellCheck 連携 ON にすると上記エラー発生

@Hiroshiba
Copy link
Member Author

なかなか危なっかしいコードになってそうですね・・・!

シェルスクリプトに詳しい方とかいらっしゃったらお力お借りしたいかもです。
ということで、もしご興味あれば・・・!! @PickledChair @aoirint

@tarepan tarepan added 状態:実装者募集 実装者を募集している状態 and removed 優先度:低 (運用中止) labels Mar 5, 2024
@tarepan tarepan added 状態:実装 実装をおこなっている状態 and removed 状態:実装者募集 実装者を募集している状態 labels Mar 18, 2024
@tarepan tarepan self-assigned this Mar 18, 2024
@tarepan tarepan removed the 初心者歓迎タスク 初心者にも優しい簡単めなタスク label Mar 18, 2024
@tarepan tarepan removed the 状態:実装 実装をおこなっている状態 label May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants