Skip to content

ci: check-shebangs-and-filemodesワークフローを追加#1080

Merged
qryxip merged 3 commits intoVOICEVOX:mainfrom
qryxip:pr/ci-add-check-shebangs-and-filemodes-workflow
May 4, 2025
Merged

ci: check-shebangs-and-filemodesワークフローを追加#1080
qryxip merged 3 commits intoVOICEVOX:mainfrom
qryxip:pr/ci-add-check-shebangs-and-filemodes-workflow

Conversation

@qryxip
Copy link
Copy Markdown
Member

@qryxip qryxip commented May 4, 2025

内容

#1077 の続き。

このリポジトリ内のすべてのファイルに対し、以下のチェックを義務付ける。

if has_shebang && ! is_executable; then
  # shellcheck disable=SC2016
  echo 'A shebanged file must be executable (note: if you are using Windows, please change the filemode with `git update-index --chmod+x`)'
  exit 1
fi

if is_shellscript && ! is_executable; then
  # shellcheck disable=SC2016
  echo 'A shell script file must be executable (note: if you are using Windows, please change the filemode with `git update-index --chmod+x`)'
  exit 1
fi

if is_executable && ! has_shebang; then
  echo 'An executable blob must have a shebang'
  exit 1
fi

関連 Issue

その他

@qryxip qryxip requested a review from Hiroshiba May 4, 2025 06:23
@Hiroshiba Hiroshiba requested a review from Copilot May 4, 2025 14:52
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds a GitHub Actions workflow to enforce consistency between shebang usage and file executability across the repository.

  • Introduces a new workflow to check file permissions for files with shebangs and shell scripts.
  • Checks that shebanged files are executable, shell scripts are executable, and executables have a shebang.

first_line=$(head -n 1 "$filename")
shebang_line=$(grep '^#!/' <<< "$first_line" || true)

has-shebang() {
Copy link

Copilot AI May 4, 2025

Choose a reason for hiding this comment

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

Function names in bash should not contain hyphens as they can cause syntax errors; please rename 'has-shebang' to 'has_shebang'.

Suggested change
has-shebang() {
has_shebang() {

Copilot uses AI. Check for mistakes.
[ -n "$shebang_line" ]
}

is-shellscript() {
Copy link

Copilot AI May 4, 2025

Choose a reason for hiding this comment

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

Function names in bash should not contain hyphens; please rename 'is-shellscript' to 'is_shellscript'.

Suggested change
is-shellscript() {
is_shellscript() {

Copilot uses AI. Check for mistakes.
Comment on lines +33 to +51
is-executable() {
[ "$filemode" = 100755 ]
}

echo -n "$filename ($filemode): "

if has-shebang && ! is-executable; then
# shellcheck disable=SC2016
echo 'A shebanged file must be executable (note: if you are using Windows, please change the filemode with `git update-index --chmod+x`)'
exit 1
fi

if is-shellscript && ! is-executable; then
# shellcheck disable=SC2016
echo 'A shell script file must be executable (note: if you are using Windows, please change the filemode with `git update-index --chmod+x`)'
exit 1
fi

if is-executable && ! has-shebang; then
Copy link

Copilot AI May 4, 2025

Choose a reason for hiding this comment

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

Function names in bash should not contain hyphens; please rename 'is-executable' to 'is_executable'.

Suggested change
is-executable() {
[ "$filemode" = 100755 ]
}
echo -n "$filename ($filemode): "
if has-shebang && ! is-executable; then
# shellcheck disable=SC2016
echo 'A shebanged file must be executable (note: if you are using Windows, please change the filemode with `git update-index --chmod+x`)'
exit 1
fi
if is-shellscript && ! is-executable; then
# shellcheck disable=SC2016
echo 'A shell script file must be executable (note: if you are using Windows, please change the filemode with `git update-index --chmod+x`)'
exit 1
fi
if is-executable && ! has-shebang; then
is_executable() {
[ "$filemode" = 100755 ]
}
echo -n "$filename ($filemode): "
if has-shebang && ! is_executable; then
# shellcheck disable=SC2016
echo 'A shebanged file must be executable (note: if you are using Windows, please change the filemode with `git update-index --chmod+x`)'
exit 1
fi
if is-shellscript && ! is_executable; then
# shellcheck disable=SC2016
echo 'A shell script file must be executable (note: if you are using Windows, please change the filemode with `git update-index --chmod+x`)'
exit 1
fi
if is_executable && ! has-shebang; then

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
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!!

関数をケバブケースかスネークケースにするかですが、個人的には統一されていればどちらでもいいと思います!

たぶんbashに慣れていない人は混乱すると思うので、倒すとしたらスネークケースの方が良さそうではある。
ちなみにPOSIX標準?だと-は関数名に認められてない・・・ってChatGPT君が言ってました!!
https://chatgpt.com/share/6817815e-e204-8008-a0fa-bfa424397c2f

Comment on lines +14 to +57
- name: Check shebangs and filemodes
run: |
blobs=$(git ls-files -s | grep '^100')

while read -r blob; do
filemode=$(awk '{ print $1 }' <<< "$blob")
filename=$(awk '{ print $4 }' <<< "$blob")

first_line=$(head -n 1 "$filename")
shebang_line=$(grep '^#!/' <<< "$first_line" || true)

has-shebang() {
[ -n "$shebang_line" ]
}

is-shellscript() {
[[ "$filename" =~ \.(ba)?sh$ ]]
}

is-executable() {
[ "$filemode" = 100755 ]
}

echo -n "$filename ($filemode): "

if has-shebang && ! is-executable; then
# shellcheck disable=SC2016
echo 'A shebanged file must be executable (note: if you are using Windows, please change the filemode with `git update-index --chmod+x`)'
exit 1
fi

if is-shellscript && ! is-executable; then
# shellcheck disable=SC2016
echo 'A shell script file must be executable (note: if you are using Windows, please change the filemode with `git update-index --chmod+x`)'
exit 1
fi

if is-executable && ! has-shebang; then
echo 'An executable blob must have a shebang'
exit 1
fi

echo OK
done <<< "$blobs"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

(ただのコメントです)

build_utilディレクトリにシェルスクリプト置いて、それをここから叩くようにしても良いかもですね!
ネストが減って読みやすい・ちゃんとシンタックスハイライトがつく・ローカルからも実行デバッグできる利点がありそう。
その代わりファイル数が増えちゃいそう。

たまに.github/workflowsディレクトリ内にbash置いて叩いてるプロジェクトもちらほら見かけます。
そういうのもありかも(と思いつつちゃんと考えれてませんが)。

@qryxip
Copy link
Copy Markdown
Member Author

qryxip commented May 4, 2025

6d888e5 (#1080): GNUとかGitとかでもsnake_caseっぽかったので、snake_caseにしました。

@qryxip qryxip merged commit f4724c0 into VOICEVOX:main May 4, 2025
28 checks passed
@qryxip qryxip deleted the pr/ci-add-check-shebangs-and-filemodes-workflow branch May 4, 2025 15:30
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