blame: validate and peel the object names on the ignore list
The command reads list of object names to place on the ignore list either from the command line or from a file, but they are not checked with their object type (those read from the file are not even checked for object existence). Extend the oidset_parse_file() API and allow it to take a callback that can be used to die (e.g. when an inappropriate input is read) or modify the object name read (e.g. when a tag pointing at a commit is read, and the caller wants a commit object name), and use it in the code that handles ignore list. Signed-off-by: Junio C Hamano <gitster@pobox.com>
Junio C Hamano committed
Sep 24, 2020 at 21:55 UTC
610e2b924020fe2d6a55e7ca6651f309b85c2d1d
4 files changed
+69
-13
builtin/blame.c
+25
-2
@@ -27,6 +27,7 @@
27
#include "object-store.h"
28
#include "blame.h"
29
#include "refs.h"
30
+#include "tag.h"
31
32
static char blame_usage[] = N_("git blame [<options>] [<rev-opts>] [<rev>] [--] <file>");
33
@@ -803,6 +804,26 @@ static int is_a_rev(const char *name)
804
return OBJ_NONE < oid_object_info(the_repository, &oid, NULL);
805
}
806
807
+static int peel_to_commit_oid(struct object_id *oid_ret, void *cbdata)
808
+{
809
+ struct repository *r = ((struct blame_scoreboard *)cbdata)->repo;
810
+ struct object_id oid;
811
+
812
+ oidcpy(&oid, oid_ret);
813
+ while (1) {
814
+ struct object *obj;
815
+ int kind = oid_object_info(r, &oid, NULL);
816
+ if (kind == OBJ_COMMIT) {
817
+ oidcpy(oid_ret, &oid);
818
+ return 0;
819
+ }
820
+ if (kind != OBJ_TAG)
821
+ return -1;
822
+ obj = deref_tag(r, parse_object(r, &oid), NULL, 0);
823
+ oidcpy(&oid, &obj->oid);
824
+ }
825
+}
826
+
827
static void build_ignorelist(struct blame_scoreboard *sb,
828
struct string_list *ignore_revs_file_list,
829
struct string_list *ignore_rev_list)
@@ -815,10 +836,12 @@ static void build_ignorelist(struct blame_scoreboard *sb,
836
if (!strcmp(i->string, ""))
837
oidset_clear(&sb->ignore_list);
838
else
818
- oidset_parse_file(&sb->ignore_list, i->string);
839
+ oidset_parse_file_carefully(&sb->ignore_list, i->string,
840
+ peel_to_commit_oid, sb);
841
}
842
for_each_string_list_item(i, ignore_rev_list) {
821
- if (get_oid_committish(i->string, &oid))
843
+ if (get_oid_committish(i->string, &oid) ||
844
+ peel_to_commit_oid(&oid, sb))
845
die(_("cannot find revision %s to ignore"), i->string);
846
oidset_insert(&sb->ignore_list, &oid);
847
}
oidset.c
+8
-1
@@ -42,6 +42,12 @@ int oidset_size(struct oidset *set)
42
}
43
44
void oidset_parse_file(struct oidset *set, const char *path)
45
+{
46
+ oidset_parse_file_carefully(set, path, NULL, NULL);
47
+}
48
+
49
+void oidset_parse_file_carefully(struct oidset *set, const char *path,
50
+ oidset_parse_tweak_fn fn, void *cbdata)
51
{
52
FILE *fp;
53
struct strbuf sb = STRBUF_INIT;
@@ -66,7 +72,8 @@ void oidset_parse_file(struct oidset *set, const char *path)
72
if (!sb.len)
73
continue;
74
69
- if (parse_oid_hex(sb.buf, &oid, &p) || *p != '\0')
75
+ if (parse_oid_hex(sb.buf, &oid, &p) || *p != '\0' ||
76
+ (fn && fn(&oid, cbdata)))
77
die("invalid object name: %s", sb.buf);
78
oidset_insert(set, &oid);
79
}
oidset.h
+9
@@ -73,6 +73,15 @@ void oidset_clear(struct oidset *set);
73
*/
74
void oidset_parse_file(struct oidset *set, const char *path);
75
76
+/*
77
+ * Similar to the above, but with a callback which can (1) return non-zero to
78
+ * signal displeasure with the object and (2) replace object ID with something
79
+ * else (meant to be used to "peel").
80
+ */
81
+typedef int (*oidset_parse_tweak_fn)(struct object_id *, void *);
82
+void oidset_parse_file_carefully(struct oidset *set, const char *path,
83
+ oidset_parse_tweak_fn fn, void *cbdata);
84
+
85
struct oidset_iter {
86
kh_oid_set_t *set;
87
khiter_t iter;
t/t8013-blame-ignore-revs.sh
+27
-10
@@ -21,6 +21,7 @@ test_expect_success setup '
21
test_tick &&
22
git commit -m X &&
23
git tag X &&
24
+ git tag -a -m "X (annotated)" XT &&
25
26
git blame --line-porcelain file >blame_raw &&
27
@@ -33,19 +34,35 @@ test_expect_success setup '
34
test_cmp expect actual
35
'
36
36
-# Ignore X, make sure A is blamed for line 1 and B for line 2.
37
-test_expect_success ignore_rev_changing_lines '
38
- git blame --line-porcelain --ignore-rev X file >blame_raw &&
39
-
40
- grep -E "^[0-9a-f]+ [0-9]+ 1" blame_raw | sed -e "s/ .*//" >actual &&
41
- git rev-parse A >expect &&
42
- test_cmp expect actual &&
37
+# Ensure bogus --ignore-rev requests are caught
38
+test_expect_success 'validate --ignore-rev' '
39
+ test_must_fail git blame --ignore-rev X^{tree} file
40
+'
41
44
- grep -E "^[0-9a-f]+ [0-9]+ 2" blame_raw | sed -e "s/ .*//" >actual &&
45
- git rev-parse B >expect &&
46
- test_cmp expect actual
42
+# Ensure bogus --ignore-revs-file requests are caught
43
+test_expect_success 'validate --ignore-revs-file' '
44
+ git rev-parse X^{tree} >ignore_x &&
45
+ test_must_fail git blame --ignore-revs-file ignore_x file
46
'
47
48
+for I in X XT
49
+do
50
+ # Ignore X (or XT), make sure A is blamed for line 1 and B for line 2.
51
+ # Giving X (i.e. commit) and XT (i.e. annotated tag to commit) should
52
+ # produce the same result.
53
+ test_expect_success "ignore_rev_changing_lines ($I)" '
54
+ git blame --line-porcelain --ignore-rev $I file >blame_raw &&
55
+
56
+ grep -E "^[0-9a-f]+ [0-9]+ 1" blame_raw | sed -e "s/ .*//" >actual &&
57
+ git rev-parse A >expect &&
58
+ test_cmp expect actual &&
59
+
60
+ grep -E "^[0-9a-f]+ [0-9]+ 2" blame_raw | sed -e "s/ .*//" >actual &&
61
+ git rev-parse B >expect &&
62
+ test_cmp expect actual
63
+ '
64
+done
65
+
66
# For ignored revs that have added 'unblamable' lines, attribute those to the
67
# ignored commit.
68
# A--B--X--Y