From 9a9c9d39bd0cf71c5e5e54b8c2bbba717f2641d5 Mon Sep 17 00:00:00 2001 From: ShortArrow Date: Wed, 14 Oct 2026 07:40:23 +0900 Subject: [PATCH 1/2] fix(diff): decide binary diffs by the diff attribute, as git diff does (ADR 0033) Commit diffs and untracked files judged binary by a NUL byte alone, so files .gitattributes marks -diff or binary were counted and diffed as text. --- CHANGELOG.md | 7 +++ docs/ROADMAP.jp.md | 4 +- docs/ROADMAP.md | 8 +-- docs/adr/0033-diff-attribute.jp.md | 26 +++++++++ docs/adr/0033-diff-attribute.md | 53 +++++++++++++++++ docs/adr/README.jp.md | 1 + docs/adr/README.md | 1 + src/git/detail.rs | 37 +++++++----- src/git/diff_attributes.rs | 72 +++++++++++++++++++++++ src/git/mod.rs | 1 + src/git/uncommitted.rs | 54 ++++++++++------- tests/git_adapter.rs | 93 ++++++++++++++++++++++++++++++ 12 files changed, 315 insertions(+), 42 deletions(-) create mode 100644 docs/adr/0033-diff-attribute.jp.md create mode 100644 docs/adr/0033-diff-attribute.md create mode 100644 src/git/diff_attributes.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 83c72f0..d5d5f84 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Commit diffs and untracked files follow the `diff` attribute of + `.gitattributes`, as `git diff` does: `-diff` and `binary` show as binary + with no line counts, and `diff` shows as text + ([ADR 0033](https://github.com/ShortArrow/gitreant/blob/main/docs/adr/0033-diff-attribute.md)). + ## [0.2.3] - 2026-10-09 ### Fixed diff --git a/docs/ROADMAP.jp.md b/docs/ROADMAP.jp.md index f543dda..ba436de 100644 --- a/docs/ROADMAP.jp.md +++ b/docs/ROADMAP.jp.md @@ -60,9 +60,9 @@ - **入れ子の worktree をドロワーのフィルタで見つけられるようにする** — 現状はトップレベル行だけが対象 - **切断されたネットワークドライブ上の worktree で一覧が待たされない** — 存在確認をタイムアウト付きにするか、一覧から外して遅延させる -## 差分表示（ADR 0029 の残課題） +## 差分表示（ADR 0029 / 0033 の残課題） -- **`.gitattributes` の `binary` / `-diff` を尊重する** — git はこうしたファイルをバイナリとして扱う（numstat では `- -`）が、gitreant はテキストとして行数を数え、差分を表示している（差分ペインは 0.2.0 からこの状態） +- **ファイル一覧でバイナリを区別する** — 一覧はバイナリのファイルを `+0 −0` と表示し、行数の変わらないテキストと見分けがつかない。git の numstat は `- -` と出す - **ファイル末尾の改行の有無の変化を見せる** — 末尾の改行を消したり足したりすると、git の「\ No newline at end of file」の印が落ちるため、同じ内容の `-x` と `+x` が並ぶだけになる。さらに作業ツリー側ではこのファイルに「改行コードのみ」のラベルが付き、コミット側では付かない（`--ignore-cr-at-eol` はこの差を無視し、blob の比較は無視しない）。両者をそろえる - **行末の種類を正確に呼ぶ** — 行末の単独の CR（旧 Mac の改行など、LF を伴わない CR）を現状は CRLF と呼んでいる。また `a\r\r\n` のような入力では、コピーしたテキストに CR が残ることがある - **向きが混在する変換にも向きを示す** — 一部の行が LF → CRLF、別の行が CRLF → LF に変わったファイルには、ラベルと印は付くが差分ヘッダに向きが出ない diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 1fbabf6..fbe5fce 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -81,11 +81,11 @@ keyed off the remote URL's host. list** — check existence with a timeout, or leave it out of the list and resolve it later -## Diff display (follow-ups to ADR 0029) +## Diff display (follow-ups to ADR 0029 / 0033) -- **Respect `.gitattributes` `binary` / `-diff`** — git shows such files - as binary (`- -` in numstat); gitreant counts and diffs them as text (the - diff pane has done so since 0.2.0) +- **Tell binary files apart in the file list** — the list shows a binary + file as `+0 −0`, the same as text whose line count did not change; git's + numstat prints `- -` - **Show the end-of-file newline change** — removing or adding the final newline shows identical `-x` / `+x` lines, because git's "\ No newline at end of file" marker is dropped. The working-tree side diff --git a/docs/adr/0033-diff-attribute.jp.md b/docs/adr/0033-diff-attribute.jp.md new file mode 100644 index 0000000..798f84f --- /dev/null +++ b/docs/adr/0033-diff-attribute.jp.md @@ -0,0 +1,26 @@ +# ADR 0033: バイナリかどうかは git diff と同じく diff 属性で決める + +## Status + +Accepted (2026-10-14) + +## Context + +git は、`.gitattributes` の `diff` 属性でファイルの差分の扱いを決める。`-diff` と、それを含む `binary` マクロはバイナリ、`diff` はテキスト、`diff=` は `diff..binary` の設定に従う。属性が無いときにだけ、先頭 8000 バイトに NUL があるかで判断する（[ADR 0006](0006-file-diff-on-demand.jp.md)）。 + +0.2.3 までの gitreant は、コミットの差分と未追跡ファイルを NUL の有無だけで判断していた。そのため、git がバイナリとして扱うファイル（numstat で `- -`）でも行数を数え、テキストの差分を表示していた。未コミットの変更のうち追跡しているファイルは git CLI を通すので、すでに git と同じ扱いだった。 + +`git diff ` は、比べるコミットの中の `.gitattributes` ではなく、作業ツリーの `.gitattributes` を読む。作業ツリーに無いディレクトリについては index を読む。gix の差分パイプライン（`diff_resource_cache_for_tree_diff`）は HEAD の index から属性を読むので、作業ツリーで書き換えた `.gitattributes` が効かない。また、このパイプラインは ADR 0029 の行数の数え方と合わないので、0.2.1 で使うのをやめている。 + +## Decision + +- **差分がバイナリかどうかは、まず `diff` 属性で決め、属性が何も言わないときだけ NUL の有無で決める。** `-diff` と `binary` はバイナリ、`diff` はテキスト、`diff=` は `diff..binary` に従い、その設定が無ければ NUL の有無で決める。 +- **属性は `git diff` と同じ場所から読む。** 作業ツリーの `.gitattributes`、作業ツリーに無ければ index、それに `$GIT_DIR/info/attributes` と `core.attributesFile` を使う。比べるコミットの中の `.gitattributes` は読まない。bare リポジトリでは index だけを読む。 +- **判断は `src/git/diff_attributes.rs` の 1 か所にまとめる。** コミットの行数と差分、未追跡ファイルの行数と差分は、どれもここを通す。 + +## Consequences + +- 行数と差分の表示が、ファイル単位で `git diff` と一致する。作業ツリーで `.gitattributes` を書き換えると、過去のコミットの表示も変わる。これも git と同じ振る舞いである。 +- `diff..textconv` は実行しない。textconv を設定したドライバのファイルも、`diff..binary` と NUL の有無で判断する。 +- ファイル一覧はバイナリを `+0 −0` と表示する。git の `- -` のような区別は、まだ無い。 +- 属性を読めないとき（index が壊れているなど）は、NUL の有無だけで判断する。 diff --git a/docs/adr/0033-diff-attribute.md b/docs/adr/0033-diff-attribute.md new file mode 100644 index 0000000..e251ab6 --- /dev/null +++ b/docs/adr/0033-diff-attribute.md @@ -0,0 +1,53 @@ +# ADR 0033: Decide binary diffs by the diff attribute, as git diff does + +## Status + +Accepted (2026-10-14) + +## Context + +git decides how a file diffs by the `diff` attribute in `.gitattributes`. +`-diff`, and the `binary` macro that includes it, mean binary; `diff` +means text; `diff=` follows the `diff..binary` setting. +Only when no attribute applies does git look for a NUL in the first 8000 +bytes ([ADR 0006](0006-file-diff-on-demand.md)). + +Up to 0.2.3, gitreant judged commit diffs and untracked files by the NUL +check alone. It counted lines and showed a text diff even for files git +treats as binary (`- -` in numstat). Tracked files among the uncommitted +changes go through the git CLI, so they already matched git. + +`git diff ` reads the `.gitattributes` of the working +tree, not the ones inside the commits it compares. For a directory missing +from the working tree, it reads the index. gix's diff pipeline +(`diff_resource_cache_for_tree_diff`) reads attributes from the HEAD index, +so an edited `.gitattributes` in the working tree has no effect there. That +pipeline also does not fit the line counting of ADR 0029, and 0.2.1 +stopped using it. + +## Decision + +- **Whether a diff is binary is decided by the `diff` attribute first, and + by the NUL check only when no attribute applies.** `-diff` and `binary` + are binary, `diff` is text, and `diff=` follows + `diff..binary`, falling back to the NUL check when that is unset. +- **Attributes are read from the same places as `git diff`:** the working + tree's `.gitattributes`, the index where the working tree has none, and + `$GIT_DIR/info/attributes` and `core.attributesFile`. The + `.gitattributes` inside the compared commits is not read. A bare + repository reads the index only. +- **The decision lives in one place, `src/git/diff_attributes.rs`.** Commit + line counts and diffs, and untracked file line counts and diffs, all go + through it. + +## Consequences + +- Line counts and diffs match `git diff` file by file. Editing + `.gitattributes` in the working tree changes how past commits show, just + as it does in git. +- `diff..textconv` is not run. A file whose driver sets textconv is + judged by `diff..binary` and the NUL check. +- The file list shows a binary file as `+0 −0`; there is no counterpart of + git's `- -` yet. +- When the attributes cannot be read (a corrupt index, for instance), the + NUL check decides alone. diff --git a/docs/adr/README.jp.md b/docs/adr/README.jp.md index ce5ff70..ba59546 100644 --- a/docs/adr/README.jp.md +++ b/docs/adr/README.jp.md @@ -36,3 +36,4 @@ gitreant の設計判断の記録。各 ADR は決定時点で凍結され、変 | [0030](0030-release-safeguards.jp.md) | リリースはタグを検査し、CHANGELOGを正とし、公開は承認を経る | Accepted | | [0031](0031-wording-only-changes.jp.md) | 片方の言語だけの言い回しの修正はコミットの印で日英同時更新の対象から外す | Accepted | | [0032](0032-static-crt-on-windows.jp.md) | Windows 版は C ランタイムを静的リンクする | Accepted | +| [0033](0033-diff-attribute.jp.md) | バイナリかどうかは git diff と同じく diff 属性で決める | Accepted | diff --git a/docs/adr/README.md b/docs/adr/README.md index 58275bb..30d17f0 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -40,3 +40,4 @@ are canonical; the files below are their English translations. | [0030](0030-release-safeguards.md) | Check the tag, take notes from the changelog, and approve publishing | Accepted | | [0031](0031-wording-only-changes.md) | Exempt a one-language rewording from the bilingual check by a commit mark | Accepted | | [0032](0032-static-crt-on-windows.md) | Link the C runtime statically on Windows | Accepted | +| [0033](0033-diff-attribute.md) | Decide binary diffs by the diff attribute, as git diff does | Accepted | diff --git a/src/git/detail.rs b/src/git/detail.rs index 45dfb14..6dab300 100644 --- a/src/git/detail.rs +++ b/src/git/detail.rs @@ -2,6 +2,8 @@ use std::path::Path; +use super::diff_attributes::DiffAttributes; + /// One changed file in a commit, with line counts against the first parent. #[derive(Debug, Clone, PartialEq)] pub struct FileChange { @@ -87,7 +89,8 @@ pub struct FileDiff { pub path: String, /// "A" (added), "M" (modified) or "D" (deleted). pub status: String, - /// True when either side looks binary; `text` is empty then. + /// The file diffs as binary (its `diff` attribute, else a NUL byte on + /// either side); `text` is empty then. pub binary: bool, /// Unified diff hunks ("@@ ..." headers with -/+/context lines). pub text: String, @@ -99,7 +102,8 @@ pub fn read_file_diff(path: &Path, id: &str, file: &str) -> Result Result, String> let commit = find_commit(&repo, id)?; let tree = commit.tree().map_err(|e| format!("tree: {e}"))?; let parent_tree = first_parent_tree(&repo, &commit)?; + let mut attributes = DiffAttributes::of(&repo); changed_files(&repo, &commit)? .into_iter() .map(|file| { - diff_blobs(&repo, &parent_tree, &tree, &file.path)? + diff_blobs(&mut attributes, &repo, &parent_tree, &tree, &file.path)? .ok_or_else(|| format!("{} vanished while diffing", file.path)) }) .collect() @@ -132,6 +137,7 @@ fn find_commit<'repo>( /// Diff the blobs at `file` between two trees; `None` when neither side has it. fn diff_blobs( + attributes: &mut DiffAttributes, repo: &gix::Repository, parent_tree: &gix::Tree<'_>, tree: &gix::Tree<'_>, @@ -147,7 +153,7 @@ fn diff_blobs( }; let old = old.unwrap_or_default(); let new = new.unwrap_or_default(); - let binary = looks_binary(&old) || looks_binary(&new); + let binary = attributes.binary(file, &old, &new); Ok(Some(FileDiff { path: file.to_string(), status: status.to_string(), @@ -199,11 +205,6 @@ fn blob_at( Ok(Some(blob.data.clone())) } -/// Shared with the uncommitted-changes reader: same rule for both sides. -pub(super) fn looks_binary(data: &[u8]) -> bool { - data.iter().take(8000).any(|&b| b == 0) -} - /// The unified hunks between two blobs, in the `git diff -u` text form. pub(super) fn unified(old: &[u8], new: &[u8]) -> String { use gix::diff::blob::{ @@ -264,13 +265,14 @@ pub(super) fn unified(old: &[u8], new: &[u8]) -> String { fn changed_files(repo: &gix::Repository, commit: &gix::Commit) -> Result, String> { let tree = commit.tree().map_err(|e| format!("tree: {e}"))?; let parent_tree = first_parent_tree(repo, commit)?; + let mut attributes = DiffAttributes::of(repo); let mut files = Vec::new(); parent_tree .changes() .map_err(|e| format!("diff: {e}"))? .for_each_to_obtain_tree(&tree, |change| { - if let Some(file) = file_change(repo, change) { + if let Some(file) = file_change(&mut attributes, repo, change) { files.push(file); } Ok::<_, std::convert::Infallible>(std::ops::ControlFlow::Continue(())) @@ -283,6 +285,7 @@ fn changed_files(repo: &gix::Repository, commit: &gix::Commit) -> Result, ) -> Option { @@ -306,9 +309,14 @@ fn file_change( ("R", blob(source_id.detach()), blob(id.detach())) } }; - let (additions, deletions) = line_counts(&old, &new); + let path = change.location().to_string(); + let (additions, deletions) = if attributes.binary(&path, &old, &new) { + (0, 0) + } else { + line_counts(&old, &new) + }; Some(FileChange { - path: change.location().to_string(), + path, status: status.to_string(), additions, deletions, @@ -316,13 +324,10 @@ fn file_change( }) } -/// Added and removed lines between two blobs; none for binary content. +/// Added and removed lines between two text blobs. fn line_counts(old: &[u8], new: &[u8]) -> (usize, usize) { use gix::diff::blob::{Algorithm, Diff, InternedInput}; - if looks_binary(old) || looks_binary(new) { - return (0, 0); - } let old = String::from_utf8_lossy(old); let new = String::from_utf8_lossy(new); let input = InternedInput::new(old.as_ref(), new.as_ref()); diff --git a/src/git/diff_attributes.rs b/src/git/diff_attributes.rs new file mode 100644 index 0000000..fcab790 --- /dev/null +++ b/src/git/diff_attributes.rs @@ -0,0 +1,72 @@ +//! Whether a file diffs as binary, decided the way `git diff` decides it: +//! the `diff` attribute first (gitattributes(5)), then a NUL byte in the +//! content (ADR 0033). + +use gix::attrs::StateRef; +use gix::worktree::stack::state::attributes::Source; + +/// The `diff` attribute of the paths in one repository. Attributes come from +/// the working tree's `.gitattributes`, then the index, then +/// `$GIT_DIR/info/attributes` and `core.attributesFile` — never from the +/// commits being diffed, as with `git diff`. +pub(super) struct DiffAttributes<'repo> { + repo: &'repo gix::Repository, + /// `None` when the attributes cannot be read; every path is then judged + /// by its content alone. + stack: Option>, + outcome: gix::attrs::search::Outcome, +} + +impl<'repo> DiffAttributes<'repo> { + pub(super) fn of(repo: &'repo gix::Repository) -> Self { + let source = if repo.workdir().is_some() { + Source::WorktreeThenIdMapping + } else { + Source::IdMapping + }; + let stack = repo + .index_or_empty() + .ok() + .and_then(|index| repo.attributes_only(&index, source).ok()); + let outcome = stack + .as_ref() + .map(|stack| stack.selected_attribute_matches(["diff"])) + .unwrap_or_default(); + Self { + repo, + stack, + outcome, + } + } + + /// `path` diffs as binary between the contents `old` and `new`. + pub(super) fn binary(&mut self, path: &str, old: &[u8], new: &[u8]) -> bool { + self.declared_binary(path) + .unwrap_or_else(|| looks_binary(old) || looks_binary(new)) + } + + /// What the `diff` attribute says: `-diff` (and the `binary` macro) is + /// binary, `diff` is text, and `diff=` is what + /// `diff..binary` says. `None` leaves it to the content. + fn declared_binary(&mut self, path: &str) -> Option { + self.stack + .as_mut()? + .at_entry(path, None) + .ok()? + .matching_attributes(&mut self.outcome); + match self.outcome.iter_selected().next()?.assignment.state { + StateRef::Unset => Some(true), + StateRef::Set => Some(false), + StateRef::Value(driver) => self + .repo + .config_snapshot() + .boolean(format!("diff.{}.binary", driver.as_bstr()).as_str()), + StateRef::Unspecified => None, + } + } +} + +/// A NUL byte within the first 8000 bytes, the same rule git uses (ADR 0006). +fn looks_binary(data: &[u8]) -> bool { + data.iter().take(8000).any(|&b| b == 0) +} diff --git a/src/git/mod.rs b/src/git/mod.rs index 9903def..4aad238 100644 --- a/src/git/mod.rs +++ b/src/git/mod.rs @@ -4,6 +4,7 @@ mod avatar; mod branch; mod detail; +mod diff_attributes; mod fetch; mod prs; mod repo; diff --git a/src/git/uncommitted.rs b/src/git/uncommitted.rs index ef5f68f..d639c5c 100644 --- a/src/git/uncommitted.rs +++ b/src/git/uncommitted.rs @@ -7,13 +7,15 @@ //! what the user's own `git status` / `git diff` show, and the user's //! `status.showUntrackedFiles` setting decides how untracked files are //! listed. An untracked file has nothing to diff against, so it is read -//! straight from disk, up to a size cap. +//! straight from disk, up to a size cap, and its `diff` attribute is looked +//! up the way the commit diffs do (ADR 0033). use std::collections::HashMap; use std::path::Path; use std::process::Command; -use super::detail::{looks_binary, unified, CommitDetail, FileChange, FileDiff}; +use super::detail::{unified, CommitDetail, FileChange, FileDiff}; +use super::diff_attributes::DiffAttributes; use super::fetch::hide_console; /// The id the changes row carries in place of a commit hash. Never a valid @@ -60,7 +62,12 @@ pub fn uncommitted_paths(path: &Path) -> Vec { /// HEAD as the only parent, and the changed files with line counts. Fails /// when there is no HEAD to compare against. pub fn read_uncommitted(path: &Path) -> Result { - let head = head_id(path)?; + let repo = open(path)?; + let head = repo + .head_id() + .map(|id| id.detach().to_string()) + .map_err(|e| format!("no HEAD to compare against: {e}"))?; + let mut attributes = DiffAttributes::of(&repo); let counts = parse_numstat(&output( path, &["diff", "HEAD", "--numstat", "-z", "--no-renames"], @@ -82,7 +89,13 @@ pub fn read_uncommitted(path: &Path) -> Result { read_untracked(&path.join(&entry.path), UNTRACKED_READ_LIMIT) .ok() .flatten() - .map(|data| new_file_counts(&data)) + .map(|data| { + if attributes.binary(&entry.path, &[], &data) { + (0, 0) + } else { + new_file_counts(&data) + } + }) .unwrap_or((0, 0)) } else { counts.get(&entry.path).copied().unwrap_or((0, 0)) @@ -119,7 +132,8 @@ pub fn read_uncommitted_diff(path: &Path, file: &str) -> Result Result Result, String> { + let repo = open(path)?; + let mut attributes = DiffAttributes::of(&repo); Ok(uncommitted_paths(path) .iter() - .filter_map(|entry| diff_entry(path, entry).ok()) + .filter_map(|entry| diff_entry(&mut attributes, path, entry).ok()) .collect()) } -fn diff_entry(path: &Path, entry: &UncommittedPath) -> Result { +fn diff_entry( + attributes: &mut DiffAttributes, + path: &Path, + entry: &UncommittedPath, +) -> Result { if entry.is_directory() { return Ok(FileDiff { path: entry.path.clone(), @@ -144,7 +164,9 @@ fn diff_entry(path: &Path, entry: &UncommittedPath) -> Result } if entry.untracked { let data = read_untracked(&path.join(&entry.path), UNTRACKED_READ_LIMIT)?; - let binary = data.as_deref().is_none_or(looks_binary); + let binary = data + .as_deref() + .is_none_or(|data| attributes.binary(&entry.path, &[], data)); return Ok(FileDiff { path: entry.path.clone(), status: entry.status.clone(), @@ -178,11 +200,8 @@ fn diff_entry(path: &Path, entry: &UncommittedPath) -> Result }) } -fn head_id(path: &Path) -> Result { - let repo = gix::discover(path).map_err(|e| format!("open {path:?}: {e}"))?; - repo.head_id() - .map(|id| id.detach().to_string()) - .map_err(|e| format!("no HEAD to compare against: {e}")) +fn open(path: &Path) -> Result { + gix::discover(path).map_err(|e| format!("open {path:?}: {e}")) } /// An untracked file's bytes, as git would see them: a symbolic link is @@ -285,13 +304,9 @@ fn parse_diff(out: &str) -> (bool, String) { (false, text) } -/// An untracked file is all additions: one per line, none for binaries. +/// An untracked text file is all additions, one per line. fn new_file_counts(data: &[u8]) -> (usize, usize) { - if looks_binary(data) { - (0, 0) - } else { - (String::from_utf8_lossy(data).lines().count(), 0) - } + (String::from_utf8_lossy(data).lines().count(), 0) } #[cfg(test)] @@ -369,7 +384,6 @@ mod tests { #[test] fn counts_a_new_file_as_pure_additions() { assert_eq!(new_file_counts(b"one\ntwo\n"), (2, 0)); - assert_eq!(new_file_counts(b"\0binary"), (0, 0)); assert_eq!(new_file_counts(b""), (0, 0)); } } diff --git a/tests/git_adapter.rs b/tests/git_adapter.rs index 184e80c..6a28a50 100644 --- a/tests/git_adapter.rs +++ b/tests/git_adapter.rs @@ -268,6 +268,99 @@ fn file_diff_covers_deletions_and_binary_files() { assert_eq!(binary.text, "", "binary diffs carry no text"); } +/// A repository whose `.gitattributes` decides how each file diffs: +/// `-diff`, the `binary` macro, a driver configured as binary and a +/// `.gitattributes` in a subdirectory make text binary, `diff` makes NUL +/// bytes text, and the rest is detected. +fn repo_with_diff_attributes(dir: &Path) { + git(dir, &["init", "-q", "-b", "main"]); + git(dir, &["config", "commit.gpgsign", "false"]); + git(dir, &["config", "diff.pdf.binary", "true"]); + std::fs::write( + dir.join(".gitattributes"), + "*.dat -diff\n*.lock binary\n*.pdf diff=pdf\n*.raw diff\n", + ) + .unwrap(); + std::fs::create_dir(dir.join("sub")).unwrap(); + std::fs::write(dir.join("sub/.gitattributes"), "*.txt -diff\n").unwrap(); + for name in ["a.dat", "b.lock", "c.pdf", "sub/e.txt", "plain.txt"] { + std::fs::write(dir.join(name), "one\n").unwrap(); + } + std::fs::write(dir.join("d.raw"), b"one\0\n").unwrap(); + git(dir, &["add", "."]); + commit(dir, "root", 1000); + for name in ["a.dat", "b.lock", "c.pdf", "sub/e.txt", "plain.txt"] { + std::fs::write(dir.join(name), "one\ntwo\n").unwrap(); + } + std::fs::write(dir.join("d.raw"), b"one\0\ntwo\n").unwrap(); + git(dir, &["add", "."]); + commit(dir, "change every file", 1001); +} + +#[test] +fn commit_diffs_follow_the_diff_attribute() { + let tmp = tempfile::tempdir().unwrap(); + let dir = tmp.path(); + repo_with_diff_attributes(dir); + let head = read_repo(dir).expect("read repo").commits[0].id.clone(); + + let counts: HashMap = read_commit(dir, &head) + .expect("read commit") + .files + .into_iter() + .map(|f| (f.path, (f.additions, f.deletions))) + .collect(); + let binary: HashMap = read_commit_diff(dir, &head) + .expect("commit diff") + .into_iter() + .map(|d| (d.path, d.binary)) + .collect(); + + for name in ["a.dat", "b.lock", "c.pdf", "sub/e.txt"] { + assert_eq!(counts[name], (0, 0), "{name} counts no lines"); + assert!(binary[name], "{name} diffs as binary"); + assert!(read_file_diff(dir, &head, name).unwrap().binary, "{name}"); + } + for name in ["plain.txt", "d.raw"] { + assert_eq!(counts[name], (1, 0), "{name} counts its line"); + assert!(!binary[name], "{name} diffs as text"); + } +} + +#[test] +fn commit_diffs_take_attributes_from_the_working_tree() { + let tmp = tempfile::tempdir().unwrap(); + let dir = tmp.path(); + repo_with_diff_attributes(dir); + let head = read_repo(dir).expect("read repo").commits[0].id.clone(); + std::fs::write(dir.join(".gitattributes"), "plain.txt -diff\n").unwrap(); + + let diff = read_file_diff(dir, &head, "plain.txt").expect("diff"); + assert!(diff.binary, "the uncommitted .gitattributes applies"); + let dat = read_file_diff(dir, &head, "a.dat").expect("diff"); + assert!(!dat.binary, "the committed .gitattributes no longer applies"); +} + +#[test] +fn an_untracked_file_follows_the_diff_attribute() { + let tmp = tempfile::tempdir().unwrap(); + let dir = tmp.path(); + repo_with_diff_attributes(dir); + git(dir, &["config", "status.showUntrackedFiles", "all"]); + std::fs::write(dir.join("new.dat"), "one\ntwo\n").unwrap(); + std::fs::write(dir.join("new.raw"), b"one\0\n").unwrap(); + + let files = read_uncommitted(dir).expect("uncommitted").files; + let counts = |name: &str| { + let f = files.iter().find(|f| f.path == name).expect(name); + (f.additions, f.deletions) + }; + assert_eq!(counts("new.dat"), (0, 0)); + assert_eq!(counts("new.raw"), (1, 0)); + assert!(read_uncommitted_diff(dir, "new.dat").unwrap().binary); + assert!(!read_uncommitted_diff(dir, "new.raw").unwrap().binary); +} + #[test] fn reads_the_whole_commit_diff_at_once() { let tmp = tempfile::tempdir().unwrap(); From 6fceb69cf04411569a743c38aecfea003617dba5 Mon Sep 17 00:00:00 2001 From: ShortArrow Date: Wed, 14 Oct 2026 07:50:21 +0900 Subject: [PATCH 2/2] fix(diff): read the diff attribute without the index, as git diff does git diff between commits ignores the index's .gitattributes; loading it also cost about 300 ms per request in a 700k-entry repository. --- docs/adr/0033-diff-attribute.jp.md | 15 +++++---- docs/adr/0033-diff-attribute.md | 50 ++++++++++++++++++------------ src/git/diff_attributes.rs | 18 ++++++----- tests/git_adapter.rs | 4 +++ 4 files changed, 53 insertions(+), 34 deletions(-) diff --git a/docs/adr/0033-diff-attribute.jp.md b/docs/adr/0033-diff-attribute.jp.md index 798f84f..4322154 100644 --- a/docs/adr/0033-diff-attribute.jp.md +++ b/docs/adr/0033-diff-attribute.jp.md @@ -6,21 +6,24 @@ Accepted (2026-10-14) ## Context -git は、`.gitattributes` の `diff` 属性でファイルの差分の扱いを決める。`-diff` と、それを含む `binary` マクロはバイナリ、`diff` はテキスト、`diff=` は `diff..binary` の設定に従う。属性が無いときにだけ、先頭 8000 バイトに NUL があるかで判断する（[ADR 0006](0006-file-diff-on-demand.jp.md)）。 +git は、`.gitattributes` の `diff` 属性でファイルの差分の扱いを決める。`-diff` と、それを含む `binary` マクロはバイナリ、`diff` はテキスト、`diff=` は `diff..binary` の設定に従う。属性が何も言わないときにだけ、先頭 8000 バイトに NUL があるかで判断する（[ADR 0006](0006-file-diff-on-demand.jp.md)）。 -0.2.3 までの gitreant は、コミットの差分と未追跡ファイルを NUL の有無だけで判断していた。そのため、git がバイナリとして扱うファイル（numstat で `- -`）でも行数を数え、テキストの差分を表示していた。未コミットの変更のうち追跡しているファイルは git CLI を通すので、すでに git と同じ扱いだった。 +`git diff ` が属性を読むのは、`$GIT_DIR/info/attributes`、作業ツリーの `.gitattributes`、グローバルとシステムの属性ファイル（`core.attributesFile` など）である。比べるコミットの中の `.gitattributes` も、index も読まない。そのため、作業ツリーから消したディレクトリの `.gitattributes` は、index に残っていても効かない。bare リポジトリでは、`info/attributes` とグローバル・システムのファイルだけになる。 -`git diff ` は、比べるコミットの中の `.gitattributes` ではなく、作業ツリーの `.gitattributes` を読む。作業ツリーに無いディレクトリについては index を読む。gix の差分パイプライン（`diff_resource_cache_for_tree_diff`）は HEAD の index から属性を読むので、作業ツリーで書き換えた `.gitattributes` が効かない。また、このパイプラインは ADR 0029 の行数の数え方と合わないので、0.2.1 で使うのをやめている。 +0.2.0 のコミットの行数は gix の差分パイプライン（`diff_resource_cache_for_tree_diff`）を通していたので、`diff` 属性に従っていた。ただし属性は HEAD の index から読むので、作業ツリーで書き換えた `.gitattributes` は効かなかった。また、差分の本文は NUL の有無だけで判断していた。このパイプラインは ADR 0029 の行数の数え方と合わないので、0.2.1 で使うのをやめた。それ以降の 0.2.1〜0.2.3 は、コミットの行数も差分も、未追跡ファイルも、NUL の有無だけで判断していた。そのため、git がバイナリとして扱うファイル（numstat で `- -`）でも行数を数え、テキストの差分を表示していた。未コミットの変更のうち追跡しているファイルは git CLI を通すので、ずっと git と同じ扱いだった。 ## Decision - **差分がバイナリかどうかは、まず `diff` 属性で決め、属性が何も言わないときだけ NUL の有無で決める。** `-diff` と `binary` はバイナリ、`diff` はテキスト、`diff=` は `diff..binary` に従い、その設定が無ければ NUL の有無で決める。 -- **属性は `git diff` と同じ場所から読む。** 作業ツリーの `.gitattributes`、作業ツリーに無ければ index、それに `$GIT_DIR/info/attributes` と `core.attributesFile` を使う。比べるコミットの中の `.gitattributes` は読まない。bare リポジトリでは index だけを読む。 +- **属性は `git diff ` と同じ場所から読む。** index と、比べるコミットの中の `.gitattributes` は読まない。gix には空の index を渡す。 - **判断は `src/git/diff_attributes.rs` の 1 か所にまとめる。** コミットの行数と差分、未追跡ファイルの行数と差分は、どれもここを通す。 ## Consequences -- 行数と差分の表示が、ファイル単位で `git diff` と一致する。作業ツリーで `.gitattributes` を書き換えると、過去のコミットの表示も変わる。これも git と同じ振る舞いである。 +- 行数と差分の表示は、下に挙げる場合を除いて `git diff` と一致する。作業ツリーで `.gitattributes` を書き換えると、過去のコミットの表示も変わる。これも git と同じ振る舞いである。 +- index を読み込まないので、大きなリポジトリでもリクエストごとの負担が増えない。 - `diff..textconv` は実行しない。textconv を設定したドライバのファイルも、`diff..binary` と NUL の有無で判断する。 +- 未追跡ファイルのうち 4 MiB を超えるものは読まないので、`diff` 属性があってもバイナリとして扱う。 +- リネームしたファイルは、新しいパスの属性だけで判断する。git は、元のパスと新しいパスのどちらかがバイナリならバイナリとして扱う。 - ファイル一覧はバイナリを `+0 −0` と表示する。git の `- -` のような区別は、まだ無い。 -- 属性を読めないとき（index が壊れているなど）は、NUL の有無だけで判断する。 +- 属性を読めないときは、NUL の有無だけで判断する。 diff --git a/docs/adr/0033-diff-attribute.md b/docs/adr/0033-diff-attribute.md index e251ab6..447578d 100644 --- a/docs/adr/0033-diff-attribute.md +++ b/docs/adr/0033-diff-attribute.md @@ -12,18 +12,24 @@ means text; `diff=` follows the `diff..binary` setting. Only when no attribute applies does git look for a NUL in the first 8000 bytes ([ADR 0006](0006-file-diff-on-demand.md)). -Up to 0.2.3, gitreant judged commit diffs and untracked files by the NUL -check alone. It counted lines and showed a text diff even for files git -treats as binary (`- -` in numstat). Tracked files among the uncommitted -changes go through the git CLI, so they already matched git. - -`git diff ` reads the `.gitattributes` of the working -tree, not the ones inside the commits it compares. For a directory missing -from the working tree, it reads the index. gix's diff pipeline -(`diff_resource_cache_for_tree_diff`) reads attributes from the HEAD index, -so an edited `.gitattributes` in the working tree has no effect there. That -pipeline also does not fit the line counting of ADR 0029, and 0.2.1 -stopped using it. +`git diff ` reads attributes from `$GIT_DIR/info/attributes`, +the `.gitattributes` files in the working tree, and the global and system +attribute files (`core.attributesFile` and the like). It reads neither the +`.gitattributes` inside the compared commits nor the index, so a +`.gitattributes` in a directory removed from the working tree has no effect, +even while the index still holds it. A bare repository has only +`info/attributes` and the global and system files. + +0.2.0 counted commit lines through gix's diff pipeline +(`diff_resource_cache_for_tree_diff`), so its counts followed the `diff` +attribute. That pipeline reads attributes from the HEAD index, so an edited +`.gitattributes` in the working tree had no effect, and the diff text was +judged by the NUL check alone. The pipeline does not fit the line counting +of ADR 0029, and 0.2.1 stopped using it. From 0.2.1 to 0.2.3, commit counts +and diffs and untracked files were judged by the NUL check alone. gitreant +counted lines and showed a text diff even for files git treats as binary +(`- -` in numstat). Tracked files among the uncommitted changes go through +the git CLI, so they have always matched git. ## Decision @@ -31,23 +37,27 @@ stopped using it. by the NUL check only when no attribute applies.** `-diff` and `binary` are binary, `diff` is text, and `diff=` follows `diff..binary`, falling back to the NUL check when that is unset. -- **Attributes are read from the same places as `git diff`:** the working - tree's `.gitattributes`, the index where the working tree has none, and - `$GIT_DIR/info/attributes` and `core.attributesFile`. The - `.gitattributes` inside the compared commits is not read. A bare - repository reads the index only. +- **Attributes are read from the same places as + `git diff `.** Neither the index nor the + `.gitattributes` inside the compared commits is read; gix is handed an + empty index. - **The decision lives in one place, `src/git/diff_attributes.rs`.** Commit line counts and diffs, and untracked file line counts and diffs, all go through it. ## Consequences -- Line counts and diffs match `git diff` file by file. Editing +- Line counts and diffs match `git diff` except in the cases below. Editing `.gitattributes` in the working tree changes how past commits show, just as it does in git. +- The index is never loaded, so a large repository adds no cost per + request. - `diff..textconv` is not run. A file whose driver sets textconv is judged by `diff..binary` and the NUL check. +- An untracked file over 4 MiB is not read, so it is binary even with the + `diff` attribute. +- A renamed file is judged by the attributes of its new path only. git + treats it as binary when either the old or the new path is. - The file list shows a binary file as `+0 −0`; there is no counterpart of git's `- -` yet. -- When the attributes cannot be read (a corrupt index, for instance), the - NUL check decides alone. +- When the attributes cannot be read, the NUL check decides alone. diff --git a/src/git/diff_attributes.rs b/src/git/diff_attributes.rs index fcab790..a9c5e0b 100644 --- a/src/git/diff_attributes.rs +++ b/src/git/diff_attributes.rs @@ -5,10 +5,11 @@ use gix::attrs::StateRef; use gix::worktree::stack::state::attributes::Source; -/// The `diff` attribute of the paths in one repository. Attributes come from -/// the working tree's `.gitattributes`, then the index, then -/// `$GIT_DIR/info/attributes` and `core.attributesFile` — never from the -/// commits being diffed, as with `git diff`. +/// The `diff` attribute of the paths in one repository, from the places +/// `git diff ` reads: `$GIT_DIR/info/attributes`, the +/// `.gitattributes` files in the working tree, and the global and system +/// attribute files. Neither the index nor the commits being diffed are read, +/// and a bare repository has only the first and last. pub(super) struct DiffAttributes<'repo> { repo: &'repo gix::Repository, /// `None` when the attributes cannot be read; every path is then judged @@ -18,16 +19,17 @@ pub(super) struct DiffAttributes<'repo> { } impl<'repo> DiffAttributes<'repo> { + /// Hands gix an empty index, so it has no `.gitattributes` to map from + /// the index; this also spares loading an index, which takes hundreds of + /// milliseconds in a large repository. pub(super) fn of(repo: &'repo gix::Repository) -> Self { + let no_index = gix::index::State::new(repo.object_hash()); let source = if repo.workdir().is_some() { Source::WorktreeThenIdMapping } else { Source::IdMapping }; - let stack = repo - .index_or_empty() - .ok() - .and_then(|index| repo.attributes_only(&index, source).ok()); + let stack = repo.attributes_only(&no_index, source).ok(); let outcome = stack .as_ref() .map(|stack| stack.selected_attribute_matches(["diff"])) diff --git a/tests/git_adapter.rs b/tests/git_adapter.rs index 6a28a50..7a4fa59 100644 --- a/tests/git_adapter.rs +++ b/tests/git_adapter.rs @@ -339,6 +339,10 @@ fn commit_diffs_take_attributes_from_the_working_tree() { assert!(diff.binary, "the uncommitted .gitattributes applies"); let dat = read_file_diff(dir, &head, "a.dat").expect("diff"); assert!(!dat.binary, "the committed .gitattributes no longer applies"); + + std::fs::remove_dir_all(dir.join("sub")).unwrap(); + let e = read_file_diff(dir, &head, "sub/e.txt").expect("diff"); + assert!(!e.binary, "a .gitattributes left only in the index is not read"); } #[test]