builtin/blame: ignore nonexistent ignore files

It's currently a problem to put blame.ignoreRevsFile in a global
gitconfig, for example, to use the GitHub (and other) supported filename
of .git-blame-ignore-revs by default if present in a repo, since the
current implementation exits the process if it fails to open the file.

If we change this to ignore nonexistent files when specified in the
config option (but NOT the command line, since the command line surely
means to actually assert the files exist), this remains API compatible
with old usages, while behaving properly when you want to ignore missing
ones.

Because of this idea of putting .git-blame-ignore-revs in there, it
doesn't seem to necessarily merit a warning, but maybe there should be
one to be slightly less confusing if you make a typo?

Signed-off-by: Jade Lovelace <[email protected]>

Author: lf-

Documentation/config/blame.adoc

@@ -26,6 +26,9 @@ blame.ignoreRevsFile::
 	`#` 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`.
++
+If a file in this list does not exist, it is disregarded and the next files in
+the list are processed as normal.
 
 blame.markUnblamableLines::
 	Mark lines that were changed by an ignored revision that we could not

builtin/blame.c

@@ -68,6 +68,9 @@ static int no_whole_file_rename;
 static int show_progress;
 static char repeated_meta_color[COLOR_MAXLEN];
 static int coloring_mode;
+/* Ignore-revs files specified in the config file. Processed separately to be
+ * able to ignore failing to open them. */
+static struct string_list ignore_revs_config_file_list = STRING_LIST_INIT_DUP;
 static struct string_list ignore_revs_file_list = STRING_LIST_INIT_DUP;
 static int mark_unblamable_lines;
 static int mark_ignored_lines;
@@ -731,7 +734,7 @@ static int git_blame_config(const char *var, const char *value,
 		ret = git_config_pathname(&str, var, value);
 		if (ret)
 			return ret;
-		string_list_insert(&ignore_revs_file_list, str);
+		string_list_insert(&ignore_revs_config_file_list, str);
 		free(str);
 		return 0;
 	}
@@ -848,13 +851,33 @@ static int peel_to_commit_oid(struct object_id *oid_ret, void *cbdata)
 }
 
 static void build_ignorelist(struct blame_scoreboard *sb,
+			     /* Ignore-revs files specified via the config,
+			      * which are allowed to fail to open. */
+			     struct string_list *ignore_revs_config_file_list,
 			     struct string_list *ignore_revs_file_list,
 			     struct string_list *ignore_rev_list)
 {
 	struct string_list_item *i;
 	struct object_id oid;
 
 	oidset_init(&sb->ignore_list, 0);
+	for_each_string_list_item(i, ignore_revs_config_file_list) {
+		if (!strcmp(i->string, ""))
+			oidset_clear(&sb->ignore_list);
+		else {
+			FILE *fp = fopen(i->string, "r");
+			if (!fp) {
+				if (errno == ENOENT) continue;
+				die("could not open blame.ignoreRevsFile file at %s", i->string);
+			}
+
+			oidset_parse_filep_carefully(&sb->ignore_list, fp, i->string,
+						    the_repository->hash_algo,
+						    peel_to_commit_oid, sb);
+
+			fclose(fp);
+		}
+	}
 	for_each_string_list_item(i, ignore_revs_file_list) {
 		if (!strcmp(i->string, ""))
 			oidset_clear(&sb->ignore_list);
@@ -1119,7 +1142,7 @@ int cmd_blame(int argc,
 	sb.reverse = reverse;
 	sb.repo = the_repository;
 	sb.path = path;
-	build_ignorelist(&sb, &ignore_revs_file_list, &ignore_rev_list);
+	build_ignorelist(&sb, &ignore_revs_config_file_list, &ignore_revs_file_list, &ignore_rev_list);
 	string_list_clear(&ignore_revs_file_list, 0);
 	string_list_clear(&ignore_rev_list, 0);
 	setup_scoreboard(&sb, &o);

oidset.c

@@ -59,12 +59,24 @@ void oidset_parse_file_carefully(struct oidset *set, const char *path,
 				 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);
+
+	oidset_parse_filep_carefully(set, fp, path, algop, fn, cbdata);
+
+	fclose(fp);
+}
+
+void oidset_parse_filep_carefully(struct oidset *set, FILE *fp,
+				 const char *path,
+				 const struct git_hash_algo *algop,
+				 oidset_parse_tweak_fn fn, void *cbdata)
+{
+	struct strbuf sb = STRBUF_INIT;
+	struct object_id oid;
+
 	while (!strbuf_getline(&sb, fp)) {
 		const char *p;
 		const char *name;
@@ -89,6 +101,5 @@ void oidset_parse_file_carefully(struct oidset *set, const char *path,
 	}
 	if (ferror(fp))
 		die_errno("Could not read '%s'", path);
-	fclose(fp);
 	strbuf_release(&sb);
 }

oidset.h

@@ -2,6 +2,7 @@
 #define OIDSET_H
 
 #include "khash.h"
+#include <stdio.h>
 
 /**
  * This API is similar to oid-array, in that it maintains a set of object ids
@@ -93,6 +94,15 @@ void oidset_parse_file_carefully(struct oidset *set, const char *path,
 				 const struct git_hash_algo *algop,
 				 oidset_parse_tweak_fn fn, void *cbdata);
 
+/*
+ * Same as oidset_parse_file_carefully, but is given a pre-opened file handle.
+ * The given path is only used for diagnostics.
+ */
+void oidset_parse_filep_carefully(struct oidset *set, FILE *fp,
+				 const char *path,
+				 const struct git_hash_algo *algop,
+				 oidset_parse_tweak_fn fn, void *cbdata);
+
 struct oidset_iter {
 	kh_oid_set_t *set;
 	khiter_t iter;

t/t8013-blame-ignore-revs.sh

@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/nix/store/cvlbhhrvzfkjl2hrrzhq3vr5gzan1r60-bash-interactive-5.2p37/bin/sh
 
 test_description='ignore revisions when blaming'
 
@@ -116,6 +116,20 @@ test_expect_success ignore_revs_from_configs_and_files '
 	test_cmp expect actual
 '
 
+# Ignore-revs from the config tolerates the file not existing
+test_expect_success ignore_revs_config_file_nonexistent '
+	git config --add blame.ignoreRevsFile nonexistent_ignore_file &&
+	git blame --line-porcelain file --ignore-revs-file ignore_y >blame_raw &&
+
+	sed -ne "/^[0-9a-f][0-9a-f]* [0-9][0-9]* 1/s/ .*//p" blame_raw >actual &&
+	git rev-parse 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 B >expect &&
+	test_cmp expect actual
+'
+
 # Override blame.ignoreRevsFile (ignore_x) with an empty string.  X should be
 # blamed now for lines 1 and 2, since we are no longer ignoring X.
 test_expect_success override_ignore_revs_file '