Re: [PATCH v2 2/2] blame: ignore revs in HEAD:.git-blame-ignore-revs
flat view
From: Junio C Hamano <hidden>
Date: 2026-10-09 20:17:12
"Ravi Mistry via GitGitGadget" [off-list ref] writes:
From: Ravi Mistry <redacted>
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 add the
HEAD:.git-blame-ignore-revs blob, if it exists, as the initial element
in the list of ignore-revs files in both bare and non-bare
repositories. Reading the committed blob from HEAD rather than the
working tree ensures that local runs match hosting platforms even when
an untracked .git-blame-ignore-revs file is present or a tracked one
has uncommitted local changes.
To ensure consistent precedence and override semantics:
- The default HEAD:.git-blame-ignore-revs entry is added before reading
configuration and CLI options, preserving user and repository config
overrides.
- In git_blame_config(), blame.ignoreRevsFile entries are appended via
string_list_append() rather than inserted in sorted order via
string_list_insert() so that configuration entries preserve their
order relative to the initial default entry.
- The HEAD:.git-blame-ignore-revs tree entry is resolved quietly via
get_oid_with_context(). Its mode is checked with S_ISREG() before
reading the object so that non-regular tree entries (such as a
committed symbolic link whose blob stores a target path rather than
revision IDs, a subdirectory, or a gitlink) are skipped instead of
being read and rejected as malformed object names. The blob is parsed
in memory via a new oidset_parse_buffer_carefully() helper in
oidset.c that shares line parsing with oidset_parse_file_carefully().
- In build_ignorelist(), ignore-revs entries are processed 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 blob without
attempting to read or parse it, allowing users to bypass a malformed
default blob.
Update documentation in blame-options.adoc and config/blame.adoc, and
add comprehensive test coverage in t8013 for the default blob lookup,
subdirectory invocations, bare repositories, uncommitted and untracked
working-tree files, CLI and config overrides, committed symlink
entries, and comments and whitespace handling.
Based-on-patch-by: Abhijeetsingh Meena [off-list ref]
Helped-by: Kristoffer Haugsbakk [off-list ref]
Helped-by: Phillip Wood [off-list ref]
Helped-by: Eric Sunshine [off-list ref]These Helped-by: drew my attention as none of these folks commented on v1 of this series. I do see they have helped the original series [ref], but it is not clear how much their inputs have survivied to this version. They are all CC'ed so they can give their Acked-by: or Reviewed-by: on this round if they want. [...]
quoted hunk ↗ jump to hunk
diff --git a/builtin/blame.c b/builtin/blame.c index 6741a7b9df..a730ee87ea 100644 --- a/builtin/blame.c +++ b/builtin/blame.c@@ -769,7 +769,7 @@ static int git_blame_config(const char *var, const char *value, if (ret) return ret; if (str) - string_list_insert(&ignore_revs_file_list, str); + string_list_append(&ignore_revs_file_list, str); free(str); return 0; }
Good, and the log message is clear why we make this change.
quoted hunk ↗ jump to hunk
@@ -946,17 +946,52 @@ static int peel_to_commit_oid(struct object_id *oid_ret, void *cbdata) } } +static void parse_default_ignore_revs_blob(struct blame_scoreboard *sb, + const char *name) +{ + struct object_context oc; + struct object_id oid; + enum object_type type; + size_t size; + char *buf; + + if (get_oid_with_context(the_repository, name, GET_OID_QUIETLY, + &oid, &oc)) + goto out; + if (!S_ISREG(oc.mode)) + goto out; + + buf = odb_read_object(the_repository->objects, &oid, &type, &size); + if (!buf) + goto out; + if (type == OBJ_BLOB) + oidset_parse_buffer_carefully(&sb->ignore_list, buf, size, + the_repository->hash_algo, + peel_to_commit_oid, sb); + free(buf); + +out: + object_context_release(&oc); +}
OK.
quoted hunk ↗ jump to hunk
static void build_ignorelist(struct blame_scoreboard *sb, struct string_list *ignore_revs_file_list, struct string_list *ignore_rev_list) { struct string_list_item *i; struct object_id oid; + 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; + }
OK, we make two passes, and during the first pass, we find where the last "empty" entry that signals "forget everything you have seen" is.
quoted hunk ↗ jump to hunk
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); + for (idx = start_idx; idx < ignore_revs_file_list->nr; idx++) {
And we scan starting from there. Very clean.
quoted hunk ↗ jump to hunk
+ i = &ignore_revs_file_list->items[idx]; + if (i->util) + parse_default_ignore_revs_blob(sb, i->string); else oidset_parse_file_carefully(&sb->ignore_list, i->string, the_repository->hash_algo,@@ -1036,6 +1071,8 @@ int cmd_blame(int argc, const char *const *opt_usage = cmd_is_annotate ? annotate_opt_usage : blame_opt_usage; setup_default_color_by_age(); + string_list_append(&ignore_revs_file_list, + "HEAD:.git-blame-ignore-revs")->util = &sb;
Cute. This takes advantage of the fact that everybody else just appends to the string_list without populating the .util member.
quoted hunk ↗ jump to hunk
diff --git a/oidset.c b/oidset.c index 90d39204d3..8469d03b9b 100644 --- a/oidset.c +++ b/oidset.c@@ -70,44 +70,76 @@ void oidset_parse_file(struct oidset *set, const char *path, oidset_parse_file_carefully(set, path, algop, NULL, NULL); } +static void parse_oidset_line(struct oidset *set, struct strbuf *sb, + const struct git_hash_algo *algop, + oidset_parse_tweak_fn fn, void *cbdata) +{ + const char *p; + const char *name; + struct object_id oid; + + if (memchr(sb->buf, '\0', sb->len)) + die("invalid object name: %s", sb->buf); + + /* + * Allow trailing comments, leading whitespace + * (including before commits), and empty or whitespace + * only lines. + */ + name = strchr(sb->buf, '#'); + if (name) + strbuf_setlen(sb, name - sb->buf); + strbuf_trim(sb); + if (!sb->len) + return; + + if (parse_oid_hex_algop(sb->buf, &oid, &p, algop) || *p != '\0') + die("invalid object name: %s", sb->buf); + if (fn && fn(&oid, cbdata)) + return; + oidset_insert(set, &oid); +} + void oidset_parse_file_carefully(struct oidset *set, const char *path, const struct git_hash_algo *algop, oidset_parse_tweak_fn fn, void *cbdata) { FILE *fp; struct strbuf sb = STRBUF_INIT; - struct object_id oid; fp = fopen(path, "r"); if (!fp) die("could not open object name list: %s", path); - while (!strbuf_getline(&sb, fp)) { - const char *p; - const char *name; - - if (memchr(sb.buf, '\0', sb.len)) - die("invalid object name: %s", sb.buf); - - /* - * Allow trailing comments, leading whitespace - * (including before commits), and empty or whitespace - * only lines. - */ - name = strchr(sb.buf, '#'); - if (name) - strbuf_setlen(&sb, name - sb.buf); - strbuf_trim(&sb); - if (!sb.len) - continue; - - if (parse_oid_hex_algop(sb.buf, &oid, &p, algop) || *p != '\0') - die("invalid object name: %s", sb.buf); - if (fn && fn(&oid, cbdata)) - continue; - oidset_insert(set, &oid); - } + while (!strbuf_getline(&sb, fp)) + parse_oidset_line(set, &sb, algop, fn, cbdata); if (ferror(fp)) die_errno("Could not read '%s'", path); fclose(fp); strbuf_release(&sb); }
Shouldn't the above refactoring have been part of the previous step instead? Thanks.