mirror of
https://github.com/git/git
synced 2024-08-24 18:26:02 +00:00
Merge branch 'jc/blame-ignore-fix'
"git blame --ignore-rev/--ignore-revs-file" failed to validate their input are valid revision, and failed to take into account that the user may want to give an annotated tag instead of a commit, which has been corrected. * jc/blame-ignore-fix: blame: validate and peel the object names on the ignore list t8013: minimum preparatory clean-up
This commit is contained in:
commit
230ff3e997
|
@ -27,6 +27,7 @@
|
|||
#include "object-store.h"
|
||||
#include "blame.h"
|
||||
#include "refs.h"
|
||||
#include "tag.h"
|
||||
|
||||
static char blame_usage[] = N_("git blame [<options>] [<rev-opts>] [<rev>] [--] <file>");
|
||||
|
||||
|
@ -803,6 +804,26 @@ static int is_a_rev(const char *name)
|
|||
return OBJ_NONE < oid_object_info(the_repository, &oid, NULL);
|
||||
}
|
||||
|
||||
static int peel_to_commit_oid(struct object_id *oid_ret, void *cbdata)
|
||||
{
|
||||
struct repository *r = ((struct blame_scoreboard *)cbdata)->repo;
|
||||
struct object_id oid;
|
||||
|
||||
oidcpy(&oid, oid_ret);
|
||||
while (1) {
|
||||
struct object *obj;
|
||||
int kind = oid_object_info(r, &oid, NULL);
|
||||
if (kind == OBJ_COMMIT) {
|
||||
oidcpy(oid_ret, &oid);
|
||||
return 0;
|
||||
}
|
||||
if (kind != OBJ_TAG)
|
||||
return -1;
|
||||
obj = deref_tag(r, parse_object(r, &oid), NULL, 0);
|
||||
oidcpy(&oid, &obj->oid);
|
||||
}
|
||||
}
|
||||
|
||||
static void build_ignorelist(struct blame_scoreboard *sb,
|
||||
struct string_list *ignore_revs_file_list,
|
||||
struct string_list *ignore_rev_list)
|
||||
|
@ -815,10 +836,12 @@ static void build_ignorelist(struct blame_scoreboard *sb,
|
|||
if (!strcmp(i->string, ""))
|
||||
oidset_clear(&sb->ignore_list);
|
||||
else
|
||||
oidset_parse_file(&sb->ignore_list, i->string);
|
||||
oidset_parse_file_carefully(&sb->ignore_list, i->string,
|
||||
peel_to_commit_oid, sb);
|
||||
}
|
||||
for_each_string_list_item(i, ignore_rev_list) {
|
||||
if (get_oid_committish(i->string, &oid))
|
||||
if (get_oid_committish(i->string, &oid) ||
|
||||
peel_to_commit_oid(&oid, sb))
|
||||
die(_("cannot find revision %s to ignore"), i->string);
|
||||
oidset_insert(&sb->ignore_list, &oid);
|
||||
}
|
||||
|
|
9
oidset.c
9
oidset.c
|
@ -42,6 +42,12 @@ int oidset_size(struct oidset *set)
|
|||
}
|
||||
|
||||
void oidset_parse_file(struct oidset *set, const char *path)
|
||||
{
|
||||
oidset_parse_file_carefully(set, path, NULL, NULL);
|
||||
}
|
||||
|
||||
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;
|
||||
|
@ -66,7 +72,8 @@ void oidset_parse_file(struct oidset *set, const char *path)
|
|||
if (!sb.len)
|
||||
continue;
|
||||
|
||||
if (parse_oid_hex(sb.buf, &oid, &p) || *p != '\0')
|
||||
if (parse_oid_hex(sb.buf, &oid, &p) || *p != '\0' ||
|
||||
(fn && fn(&oid, cbdata)))
|
||||
die("invalid object name: %s", sb.buf);
|
||||
oidset_insert(set, &oid);
|
||||
}
|
||||
|
|
9
oidset.h
9
oidset.h
|
@ -73,6 +73,15 @@ void oidset_clear(struct oidset *set);
|
|||
*/
|
||||
void oidset_parse_file(struct oidset *set, const char *path);
|
||||
|
||||
/*
|
||||
* Similar to the above, but with a callback which can (1) return non-zero to
|
||||
* signal displeasure with the object and (2) replace object ID with something
|
||||
* else (meant to be used to "peel").
|
||||
*/
|
||||
typedef int (*oidset_parse_tweak_fn)(struct object_id *, void *);
|
||||
void oidset_parse_file_carefully(struct oidset *set, const char *path,
|
||||
oidset_parse_tweak_fn fn, void *cbdata);
|
||||
|
||||
struct oidset_iter {
|
||||
kh_oid_set_t *set;
|
||||
khiter_t iter;
|
||||
|
|
|
@ -21,6 +21,7 @@ test_expect_success setup '
|
|||
test_tick &&
|
||||
git commit -m X &&
|
||||
git tag X &&
|
||||
git tag -a -m "X (annotated)" XT &&
|
||||
|
||||
git blame --line-porcelain file >blame_raw &&
|
||||
|
||||
|
@ -31,20 +32,36 @@ test_expect_success setup '
|
|||
grep -E "^[0-9a-f]+ [0-9]+ 2" blame_raw | sed -e "s/ .*//" >actual &&
|
||||
git rev-parse X >expect &&
|
||||
test_cmp expect actual
|
||||
'
|
||||
|
||||
# Ensure bogus --ignore-rev requests are caught
|
||||
test_expect_success 'validate --ignore-rev' '
|
||||
test_must_fail git blame --ignore-rev X^{tree} file
|
||||
'
|
||||
|
||||
# Ensure bogus --ignore-revs-file requests are caught
|
||||
test_expect_success 'validate --ignore-revs-file' '
|
||||
git rev-parse X^{tree} >ignore_x &&
|
||||
test_must_fail git blame --ignore-revs-file ignore_x file
|
||||
'
|
||||
|
||||
for I in X XT
|
||||
do
|
||||
# Ignore X (or XT), make sure A is blamed for line 1 and B for line 2.
|
||||
# Giving X (i.e. commit) and XT (i.e. annotated tag to commit) should
|
||||
# produce the same result.
|
||||
test_expect_success "ignore_rev_changing_lines ($I)" '
|
||||
git blame --line-porcelain --ignore-rev $I file >blame_raw &&
|
||||
|
||||
grep -E "^[0-9a-f]+ [0-9]+ 1" blame_raw | sed -e "s/ .*//" >actual &&
|
||||
git rev-parse A >expect &&
|
||||
test_cmp expect actual &&
|
||||
|
||||
grep -E "^[0-9a-f]+ [0-9]+ 2" blame_raw | sed -e "s/ .*//" >actual &&
|
||||
git rev-parse B >expect &&
|
||||
test_cmp expect actual
|
||||
'
|
||||
|
||||
# Ignore X, make sure A is blamed for line 1 and B for line 2.
|
||||
test_expect_success ignore_rev_changing_lines '
|
||||
git blame --line-porcelain --ignore-rev X file >blame_raw &&
|
||||
|
||||
grep -E "^[0-9a-f]+ [0-9]+ 1" blame_raw | sed -e "s/ .*//" >actual &&
|
||||
git rev-parse A >expect &&
|
||||
test_cmp expect actual &&
|
||||
|
||||
grep -E "^[0-9a-f]+ [0-9]+ 2" blame_raw | sed -e "s/ .*//" >actual &&
|
||||
git rev-parse B >expect &&
|
||||
test_cmp expect actual
|
||||
'
|
||||
done
|
||||
|
||||
# For ignored revs that have added 'unblamable' lines, attribute those to the
|
||||
# ignored commit.
|
||||
|
@ -67,7 +84,7 @@ test_expect_success ignore_rev_adding_unblamable_lines '
|
|||
|
||||
grep -E "^[0-9a-f]+ [0-9]+ 4" blame_raw | sed -e "s/ .*//" >actual &&
|
||||
test_cmp expect actual
|
||||
'
|
||||
'
|
||||
|
||||
# Ignore X and Y, both in separate files. Lines 1 == A, 2 == B.
|
||||
test_expect_success ignore_revs_from_files '
|
||||
|
@ -82,7 +99,7 @@ test_expect_success ignore_revs_from_files '
|
|||
grep -E "^[0-9a-f]+ [0-9]+ 2" blame_raw | sed -e "s/ .*//" >actual &&
|
||||
git rev-parse B >expect &&
|
||||
test_cmp expect actual
|
||||
'
|
||||
'
|
||||
|
||||
# Ignore X from the config option, Y from a file.
|
||||
test_expect_success ignore_revs_from_configs_and_files '
|
||||
|
@ -96,7 +113,7 @@ test_expect_success ignore_revs_from_configs_and_files '
|
|||
grep -E "^[0-9a-f]+ [0-9]+ 2" blame_raw | sed -e "s/ .*//" >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.
|
||||
|
@ -120,7 +137,7 @@ test_expect_success bad_files_and_revs '
|
|||
echo NOREV >ignore_norev &&
|
||||
test_must_fail git blame file --ignore-revs-file ignore_norev 2>err &&
|
||||
test_i18ngrep "invalid object name: NOREV" err
|
||||
'
|
||||
'
|
||||
|
||||
# For ignored revs that have added 'unblamable' lines, mark those lines with a
|
||||
# '*'
|
||||
|
@ -138,7 +155,7 @@ test_expect_success mark_unblamable_lines '
|
|||
|
||||
sed -n "4p" blame_raw | cut -c1 >actual &&
|
||||
test_cmp expect actual
|
||||
'
|
||||
'
|
||||
|
||||
# Commit Z will touch the first two lines. Y touched all four.
|
||||
# A--B--X--Y--Z
|
||||
|
@ -171,7 +188,7 @@ test_expect_success mark_ignored_lines '
|
|||
|
||||
sed -n "4p" blame_raw | cut -c1 >actual &&
|
||||
! test_cmp expect actual
|
||||
'
|
||||
'
|
||||
|
||||
# For ignored revs that added 'unblamable' lines and more recent commits changed
|
||||
# the blamable lines, mark the unblamable lines with a
|
||||
|
@ -190,7 +207,7 @@ test_expect_success mark_unblamable_lines_intermediate '
|
|||
|
||||
sed -n "4p" blame_raw | cut -c1 >actual &&
|
||||
test_cmp expect actual
|
||||
'
|
||||
'
|
||||
|
||||
# The heuristic called by guess_line_blames() tries to find the size of a
|
||||
# blame_entry 'e' in the parent's address space. Those calculations need to
|
||||
|
@ -227,7 +244,7 @@ test_expect_success ignored_chunk_negative_parent_size '
|
|||
git tag C &&
|
||||
|
||||
git blame file --ignore-rev B >blame_raw
|
||||
'
|
||||
'
|
||||
|
||||
# Resetting the repo and creating:
|
||||
#
|
||||
|
@ -269,6 +286,6 @@ test_expect_success ignore_merge '
|
|||
grep -E "^[0-9a-f]+ [0-9]+ 9" blame_raw | sed -e "s/ .*//" >actual &&
|
||||
git rev-parse C >expect &&
|
||||
test_cmp expect actual
|
||||
'
|
||||
'
|
||||
|
||||
test_done
|
||||
|
|
Loading…
Reference in a new issue