overcommit --run skips Git-quoted filenames with include filters

オープン
#908 コメント 0 件 リアクション 0 件 担当者 0 名 GitHub で見る

メンテナーはふだん 1 日以内に返信

まだ誰も着手していません。

評価

難易度
3/5
見積もり時間
半日
初心者へのやさしさ
62/100
issue の種類
バグ
明瞭さ
明確に書かれている
活発さ
活発
技術スタック
git, ruby
領域
devtools

調査の方向性

Start with GitRepo.all_files in lib/overcommit/git_repo.rb around lines 138-143, which splits both git ls-files and git ls-files --deleted output on newlines. Reproduce with the scratch-repo steps in the issue, using a filename such as café.rb under core.quotePath=true, then check whether the TrailingWhitespace hook reports a failure. Done means NUL-delimited parsing for both lists, with regression specs covering Unicode, quoted, control-character, nested, deleted, untracked, empty-index and broken-symlink paths.

索引モデルが issue の本文から書いたものです。

説明

Reproduced on current main 50f3e866bef75e3c3be7f92b47b1c45b6e7436ba (0.74.0), macOS 15.6.1, Ruby 4.0.7, Apple Git 2.50.1.

With Git's default core.quotePath=true, GitRepo.all_files returns the quoted/escaped output of git ls-files rather than actual paths. When a hook has an include pattern such as **/*.rb, those paths no longer match, and overcommit --run can report success without checking a tracked file.

Reproduction

Use a scratch repository, not an existing project:

mkdir overcommit-path-repro
cd overcommit-path-repro
git init
git config --local user.name 'Local Fixture'
git config --local user.email '[email protected]'
git config --local commit.gpgsign false
git config --local core.hooksPath /dev/null
git config --local core.quotePath true
git commit --allow-empty -m 'Initial fixture'

cat > .overcommit.yml <<'YAML'
PreCommit:
  ALL:
    enabled: false
  TrailingWhitespace:
    enabled: true
    on_warn: fail
    include:
      - '**/*.rb'
YAML

overcommit --sign
printf 'puts "fixture" \n' > normal.rb
git add -- normal.rb
overcommit --run
# TrailingWhitespace FAILED; exit 65.

git mv -- normal.rb 'café.rb'
overcommit --run
# All pre-commit hooks passed; exit 0. The content is unchanged.

git config --local core.quotePath false
overcommit --run
# TrailingWhitespace FAILED again; exit 65.

The same false-success result occurs for filenames containing double quotes, tabs, or newlines. core.quotePath=false is a control for the Unicode case, not a general workaround for those other characters.

Scope

GitRepo.all_files in lib/overcommit/git_repo.rb:138-143 splits both git ls-files and git ls-files --deleted on newlines. Using NUL-delimited output for both lists would avoid interpreting Git's C-style filename quoting while retaining the deleted-file filtering added in #905.

This behavior predates #905; I am not reporting it as a regression from that PR. The older #220/#222 discussion concerned a repository root containing spaces and list_files. I reproduced this with the repository root itself containing a space, and ordinary space-containing basenames still work.

The regression cases should include live Unicode/quoted/control-character paths, a nested Unicode path, unstaged deletions, an empty index, untracked files, and a tracked broken symlink. The broken symlink must remain visible to the BrokenSymlinks hook.

Would a small all_files parsing fix with these tests be welcome? I would leave other Git helpers, command execution, and index-error handling out of this change.

主要言語
Ruby
スター
4k
フォーク
288
平均マージ
2日 4時間
マージ済み PR(30日)
18

環境構築

はじめの一歩

  1. issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
  2. 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
  3. リポジトリをフォークし、ブランチを切って変更します。
  4. issue 番号を参照したプルリクエストを送ります。

sds/overcommit のほかの issue

sds/overcommit の issue をすべて見る

似ている issue

Ruby の issue をもっと見る

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。