From 22a100d00d0a4db6e672a8bb22d7ff2dcd497c3e Mon Sep 17 00:00:00 2001 From: Ravi Mistry Date: Thu, 10 Sep 2026 20:26:11 +0000 Subject: [PATCH] blame: default to ignoring revisions in .git-blame-ignore-revs git-blame(1) can ignore a list of commits specified via --ignore-revs-file or the blame.ignoreRevsFile configuration option. This is useful for skipping uninteresting revisions such as tree-wide formatting changes, large-scale refactors, and code modernizations that would otherwise obscure genuine historical authorship. When revision-ignoring was introduced in commit ae3f36dea1 ("blame: add blame.ignoreRevsFile config option", 2019-10-18), it intentionally avoided adopting a default ignore file. At the time, the capability was new and unproven, so avoiding unrequested filesystem I/O or unexpected attribution shifts took priority over a project-wide default. Requiring explicit opt-in per clone was therefore the prudent design. Since then, maintaining a .git-blame-ignore-revs file in the repository root has become the de facto standard across the Git ecosystem, adopted by major hosting platforms (GitHub, GitLab, Gerrit) and prominent open source projects (such as Chromium and LLVM). As a consequence, developers frequently encounter a jarring mismatch: web interfaces seamlessly ignore formatting commits, but local git-blame(1) and git-annotate(1) runs do not, unless each user manually configures blame.ignoreRevsFile for every local checkout. Teach git-blame(1) and git-annotate(1) to automatically check for a regular .git-blame-ignore-revs file at the root of the working tree when operating in a non-bare repository. To ensure consistent precedence, security, and override semantics: - Loading the default file occurs before reading configuration and CLI options, preserving user and repository config overrides. - Path resolution is anchored to repo_get_work_tree() and verified via lstat() to ensure it is a regular file. Symbolic links, directories, FIFOs, and sockets are safely skipped, preventing local information disclosure and denial-of-service hangs. - In build_ignorelist(), ignore-rev files are parsed starting after the last empty string entry. This ensures setting blame.ignoreRevsFile to "" or passing --ignore-revs-file "" or --no-ignore-revs-file cleanly discards the default file without attempting to open or parse it, allowing users to bypass corrupted default files. - Duplicate parsing is prevented by tracking seen files in a strset. Update documentation in blame-options.adoc and config/blame.adoc, and add comprehensive test coverage in t8013 for the default file lookup, subdirectory invocations, CLI and config overrides, symlink rejection, comments and whitespace handling, and bare repositories. Based-on-patch-by: Abhijeetsingh Meena Helped-by: Kristoffer Haugsbakk Helped-by: Phillip Wood Helped-by: Eric Sunshine Signed-off-by: Ravi Mistry --- Documentation/blame-options.adoc | 6 +- Documentation/config/blame.adoc | 9 +- builtin/blame.c | 42 +++++++-- t/t8013-blame-ignore-revs.sh | 151 +++++++++++++++++++++++++++++++ 4 files changed, 196 insertions(+), 12 deletions(-) diff --git a/Documentation/blame-options.adoc b/Documentation/blame-options.adoc index 1ae1222b6b5ffa..17f5734d6120dc 100644 --- a/Documentation/blame-options.adoc +++ b/Documentation/blame-options.adoc @@ -133,8 +133,10 @@ take effect. Ignore revisions listed in __, which must be in the same format as an `fsck.skipList`. This option may be repeated, and these files will be processed after any files specified with the `blame.ignoreRevsFile` config - option. An empty file name, `""`, will clear the list of revs from - previously processed files. + option or the default `.git-blame-ignore-revs` file. An empty file name, + `""`, will clear the list of revs from previously processed files. + `--no-ignore-revs-file` will clear all previously specified ignore revs + files, including the default `.git-blame-ignore-revs` file. `--color-lines`:: Color line annotations in the default format differently if they come from diff --git a/Documentation/config/blame.adoc b/Documentation/config/blame.adoc index 4d047c17908cd6..6f9be627e15bfa 100644 --- a/Documentation/config/blame.adoc +++ b/Documentation/config/blame.adoc @@ -23,9 +23,12 @@ blame.showRoot:: blame.ignoreRevsFile:: Ignore revisions listed in the file, one unabbreviated object name per line, in linkgit:git-blame[1]. Whitespace and comments beginning with - `#` are ignored. This option may be repeated multiple times. Empty - file names will reset the list of ignored revisions. This option will - be handled before the command line option `--ignore-revs-file`. + `#` are ignored. If `.git-blame-ignore-revs` exists at the root of the + working tree in a non-bare repository, it is used by default. This option + may be repeated multiple times; files specified here are processed after + the default file. An empty file name will reset the list of ignored + revisions from previously processed files and disable the default file. + This option is handled before the command-line option `--ignore-revs-file`. blame.markUnblamableLines:: Mark lines that were changed by an ignored revision that we could not diff --git a/builtin/blame.c b/builtin/blame.c index 48d5251c6df700..0935d864ade197 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -15,9 +15,11 @@ #include "hex.h" #include "commit.h" #include "diff.h" +#include "path.h" #include "revision.h" #include "quote.h" #include "string-list.h" +#include "strmap.h" #include "mailmap.h" #include "parse-options.h" #include "prio-queue.h" @@ -768,8 +770,12 @@ static int git_blame_config(const char *var, const char *value, ret = git_config_pathname(&str, var, value); if (ret) return ret; - if (str) - string_list_insert(&ignore_revs_file_list, str); + if (str) { + if (!*str) + string_list_clear(&ignore_revs_file_list, 0); + else + string_list_append(&ignore_revs_file_list, str); + } free(str); return 0; } @@ -936,16 +942,24 @@ static void build_ignorelist(struct blame_scoreboard *sb, { struct string_list_item *i; struct object_id oid; + struct strset seen_files = STRSET_INIT; + size_t start_idx = 0, idx; + + for (idx = 0; idx < ignore_revs_file_list->nr; idx++) { + if (!*ignore_revs_file_list->items[idx].string) + start_idx = idx + 1; + } oidset_init(&sb->ignore_list, 0); - for_each_string_list_item(i, ignore_revs_file_list) { - if (!strcmp(i->string, "")) - oidset_clear(&sb->ignore_list); - else - oidset_parse_file_carefully(&sb->ignore_list, i->string, + for (idx = start_idx; idx < ignore_revs_file_list->nr; idx++) { + const char *path = ignore_revs_file_list->items[idx].string; + + if (strset_add(&seen_files, path)) + oidset_parse_file_carefully(&sb->ignore_list, path, the_repository->hash_algo, peel_to_commit_oid, sb); } + strset_clear(&seen_files); for_each_string_list_item(i, ignore_rev_list) { if (repo_get_oid_committish(the_repository, i->string, &oid) || peel_to_commit_oid(&oid, sb)) @@ -1020,6 +1034,20 @@ int cmd_blame(int argc, const char *const *opt_usage = cmd_is_annotate ? annotate_opt_usage : blame_opt_usage; setup_default_color_by_age(); + { + const char *work_tree = repo_get_work_tree(the_repository); + + if (work_tree) { + char *default_file = mkpathdup("%s/%s", work_tree, + ".git-blame-ignore-revs"); + struct stat st; + + if (!lstat(default_file, &st) && S_ISREG(st.st_mode) && + !access(default_file, R_OK)) + string_list_append(&ignore_revs_file_list, default_file); + free(default_file); + } + } repo_config(the_repository, git_blame_config, &output_option); repo_init_revisions(the_repository, &revs, NULL); revs.date_mode = blame_date_mode; diff --git a/t/t8013-blame-ignore-revs.sh b/t/t8013-blame-ignore-revs.sh index cace00ae8d6ae7..a43654a7bab402 100755 --- a/t/t8013-blame-ignore-revs.sh +++ b/t/t8013-blame-ignore-revs.sh @@ -327,4 +327,155 @@ test_expect_success ignore_merge ' test_cmp expect actual ' +# Tests for default .git-blame-ignore-revs file +test_expect_success 'setup default .git-blame-ignore-revs' ' + git checkout -b default-file-branch && + test_write_lines line1 line2 >def-file && + git add def-file && + test_tick && + git commit -m "default base" && + git tag DEF_A && + + test_write_lines line1-modified line2-modified >def-file && + git add def-file && + test_tick && + git commit -m "default mod" && + git tag DEF_B && + + git rev-parse DEF_B >.git-blame-ignore-revs +' + +test_expect_success 'default .git-blame-ignore-revs is used by default' ' + git blame --line-porcelain def-file >blame_raw && + sed -ne "/^[0-9a-f][0-9a-f]* [0-9][0-9]* 1/s/ .*//p" blame_raw >actual && + git rev-parse DEF_A >expect && + test_cmp expect actual && + sed -ne "/^[0-9a-f][0-9a-f]* [0-9][0-9]* 2/s/ .*//p" blame_raw >actual && + test_cmp expect actual +' + +test_expect_success 'default .git-blame-ignore-revs respected by git annotate' ' + git rev-parse --short DEF_A >expect_sha && + git annotate def-file >actual && + test_grep "^$(cat expect_sha)" actual +' + +test_expect_success 'default .git-blame-ignore-revs works from subdirectory' ' + mkdir -p sub && + ( + cd sub && + git blame --line-porcelain ../def-file >blame_raw && + sed -ne "/^[0-9a-f][0-9a-f]* [0-9][0-9]* 1/s/ .*//p" blame_raw >actual && + git rev-parse DEF_A >expect && + test_cmp expect actual + ) +' + +test_expect_success 'disable default .git-blame-ignore-revs with --no-ignore-revs-file' ' + git blame --line-porcelain --no-ignore-revs-file def-file >blame_raw && + sed -ne "/^[0-9a-f][0-9a-f]* [0-9][0-9]* 1/s/ .*//p" blame_raw >actual && + git rev-parse DEF_B >expect && + test_cmp expect actual && + sed -ne "/^[0-9a-f][0-9a-f]* [0-9][0-9]* 2/s/ .*//p" blame_raw >actual && + test_cmp expect actual +' + +test_expect_success 'disable default .git-blame-ignore-revs with --ignore-revs-file ""' ' + git blame --line-porcelain --ignore-revs-file "" def-file >blame_raw && + sed -ne "/^[0-9a-f][0-9a-f]* [0-9][0-9]* 1/s/ .*//p" blame_raw >actual && + git rev-parse DEF_B >expect && + test_cmp expect actual && + sed -ne "/^[0-9a-f][0-9a-f]* [0-9][0-9]* 2/s/ .*//p" blame_raw >actual && + test_cmp expect actual +' + +test_expect_success 'disable default .git-blame-ignore-revs with blame.ignoreRevsFile=""' ' + test_config blame.ignoreRevsFile "" && + git blame --line-porcelain def-file >blame_raw && + sed -ne "/^[0-9a-f][0-9a-f]* [0-9][0-9]* 1/s/ .*//p" blame_raw >actual && + git rev-parse DEF_B >expect && + test_cmp expect actual && + sed -ne "/^[0-9a-f][0-9a-f]* [0-9][0-9]* 2/s/ .*//p" blame_raw >actual && + test_cmp expect actual +' + +test_expect_success 'default .git-blame-ignore-revs handles comments and whitespace' ' + test_when_finished "git rev-parse DEF_B >.git-blame-ignore-revs" && + { + echo "# Leading comment" && + echo "" && + echo " $(git rev-parse DEF_B) " && + echo "# Trailing comment" + } >.git-blame-ignore-revs && + git blame --line-porcelain def-file >blame_raw && + sed -ne "/^[0-9a-f][0-9a-f]* [0-9][0-9]* 1/s/ .*//p" blame_raw >actual && + git rev-parse DEF_A >expect && + test_cmp expect actual +' + +test_expect_success 'empty default .git-blame-ignore-revs is harmless' ' + test_when_finished "git rev-parse DEF_B >.git-blame-ignore-revs" && + : >.git-blame-ignore-revs && + git blame def-file +' + +test_expect_success SYMLINKS 'symlink .git-blame-ignore-revs is ignored' ' + test_when_finished "rm -f target_file .git-blame-ignore-revs && git rev-parse DEF_B >.git-blame-ignore-revs" && + git rev-parse DEF_B >target_file && + ln -sf target_file .git-blame-ignore-revs && + git blame --line-porcelain def-file >blame_raw && + sed -ne "/^[0-9a-f][0-9a-f]* [0-9][0-9]* 1/s/ .*//p" blame_raw >actual && + git rev-parse DEF_B >expect && + test_cmp expect actual +' + +test_expect_success 'malformed default .git-blame-ignore-revs fails but can be bypassed' ' + test_when_finished "git rev-parse DEF_B >.git-blame-ignore-revs" && + echo "invalid-oid-value" >.git-blame-ignore-revs && + test_must_fail git blame def-file && + git blame --no-ignore-revs-file def-file && + git blame --ignore-revs-file "" def-file +' + +test_expect_success 'default .git-blame-ignore-revs deduplicated when also set in config' ' + test_config blame.ignoreRevsFile .git-blame-ignore-revs && + git blame --line-porcelain def-file >blame_raw && + sed -ne "/^[0-9a-f][0-9a-f]* [0-9][0-9]* 1/s/ .*//p" blame_raw >actual && + git rev-parse DEF_A >expect && + test_cmp expect actual +' + +test_expect_success 'default .git-blame-ignore-revs combined with config blame.ignoreRevsFile' ' + test_write_lines line1-modified line2-c >def-file && + git add def-file && + test_tick && + git commit -m C && + git tag DEF_C && + git rev-parse DEF_C >custom_ignore && + test_config blame.ignoreRevsFile custom_ignore && + git blame --line-porcelain def-file >blame_raw && + sed -ne "/^[0-9a-f][0-9a-f]* [0-9][0-9]* 1/s/ .*//p" blame_raw >actual && + git rev-parse DEF_A >expect && + test_cmp expect actual && + sed -ne "/^[0-9a-f][0-9a-f]* [0-9][0-9]* 2/s/ .*//p" blame_raw >actual && + git rev-parse DEF_A >expect && + test_cmp expect actual +' + +test_expect_success 'default .git-blame-ignore-revs ignored in bare repo' ' + git clone --bare . bare.git && + git -C bare.git blame --line-porcelain def-file >blame_raw && + sed -ne "/^[0-9a-f][0-9a-f]* [0-9][0-9]* 2/s/ .*//p" blame_raw >actual && + git rev-parse DEF_C >expect && + test_cmp expect actual +' + +test_expect_success 'blame works when .git-blame-ignore-revs does not exist' ' + rm -f .git-blame-ignore-revs && + git blame --line-porcelain def-file >blame_raw && + sed -ne "/^[0-9a-f][0-9a-f]* [0-9][0-9]* 1/s/ .*//p" blame_raw >actual && + git rev-parse DEF_B >expect && + test_cmp expect actual +' + test_done