Merge branch 'jk/blame-fixes'

"git blame --porcelain" misidentified the "previous" <commit, path>
pair (aka "source") when contents came from two or more files.

* jk/blame-fixes:
  blame: output porcelain "previous" header for each file
  blame: handle --no-abbrev
  blame: fix alignment with --abbrev=40
This commit is contained in:
Junio C Hamano 2017-01-18 15:12:13 -08:00
commit 256d3dabbd
3 changed files with 166 additions and 10 deletions

View file

@ -1700,13 +1700,23 @@ static void get_commit_info(struct commit *commit,
}
/*
* Write out any suspect information which depends on the path. This must be
* handled separately from emit_one_suspect_detail(), because a given commit
* may have changes in multiple paths. So this needs to appear each time
* we mention a new group.
*
* To allow LF and other nonportable characters in pathnames,
* they are c-style quoted as needed.
*/
static void write_filename_info(const char *path)
static void write_filename_info(struct origin *suspect)
{
if (suspect->previous) {
struct origin *prev = suspect->previous;
printf("previous %s ", oid_to_hex(&prev->commit->object.oid));
write_name_quoted(prev->path, stdout, '\n');
}
printf("filename ");
write_name_quoted(path, stdout, '\n');
write_name_quoted(suspect->path, stdout, '\n');
}
/*
@ -1735,11 +1745,6 @@ static int emit_one_suspect_detail(struct origin *suspect, int repeat)
printf("summary %s\n", ci.summary.buf);
if (suspect->commit->object.flags & UNINTERESTING)
printf("boundary\n");
if (suspect->previous) {
struct origin *prev = suspect->previous;
printf("previous %s ", oid_to_hex(&prev->commit->object.oid));
write_name_quoted(prev->path, stdout, '\n');
}
commit_info_destroy(&ci);
@ -1760,7 +1765,7 @@ static void found_guilty_entry(struct blame_entry *ent,
oid_to_hex(&suspect->commit->object.oid),
ent->s_lno + 1, ent->lno + 1, ent->num_lines);
emit_one_suspect_detail(suspect, 0);
write_filename_info(suspect->path);
write_filename_info(suspect);
maybe_flush_or_die(stdout, "stdout");
}
pi->blamed_lines += ent->num_lines;
@ -1884,7 +1889,7 @@ static void emit_porcelain_details(struct origin *suspect, int repeat)
{
if (emit_one_suspect_detail(suspect, repeat) ||
(suspect->commit->object.flags & MORE_THAN_ONE_PATH))
write_filename_info(suspect->path);
write_filename_info(suspect);
}
static void emit_porcelain(struct scoreboard *sb, struct blame_entry *ent,
@ -2655,9 +2660,11 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
} else if (show_progress < 0)
show_progress = isatty(2);
if (0 < abbrev)
if (0 < abbrev && abbrev < GIT_SHA1_HEXSZ)
/* one more abbrev length is needed for the boundary commit */
abbrev++;
else if (!abbrev)
abbrev = GIT_SHA1_HEXSZ;
if (revs_file && read_ancestry(revs_file))
die_errno("reading graft file '%s' failed", revs_file);

View file

@ -86,4 +86,36 @@ test_expect_success 'blame with showEmail config true' '
test_cmp expected_n result
'
test_expect_success 'set up abbrev tests' '
test_commit abbrev &&
sha1=$(git rev-parse --verify HEAD) &&
check_abbrev () {
expect=$1; shift
echo $sha1 | cut -c 1-$expect >expect &&
git blame "$@" abbrev.t >actual &&
perl -lne "/[0-9a-f]+/ and print \$&" <actual >actual.sha &&
test_cmp expect actual.sha
}
'
test_expect_success 'blame --abbrev=<n> works' '
# non-boundary commits get +1 for alignment
check_abbrev 31 --abbrev=30 HEAD &&
check_abbrev 30 --abbrev=30 ^HEAD
'
test_expect_success 'blame -l aligns regular and boundary commits' '
check_abbrev 40 -l HEAD &&
check_abbrev 39 -l ^HEAD
'
test_expect_success 'blame --abbrev=40 behaves like -l' '
check_abbrev 40 --abbrev=40 HEAD &&
check_abbrev 39 --abbrev=40 ^HEAD
'
test_expect_success '--no-abbrev works like --abbrev=40' '
check_abbrev 40 --no-abbrev
'
test_done

117
t/t8011-blame-split-file.sh Executable file
View file

@ -0,0 +1,117 @@
#!/bin/sh
test_description='
The general idea is that we have a single file whose lines come from
multiple other files, and those individual files were modified in the same
commits. That means that we will see the same commit in multiple contexts,
and each one should be attributed to the correct file.
Note that we need to use "blame -C" to find the commit for all lines. We will
not bother testing that the non-C case fails to find it. That is how blame
behaves now, but it is not a property we want to make sure is retained.
'
. ./test-lib.sh
# help avoid typing and reading long strings of similar lines
# in the tests below
generate_expect () {
while read nr data
do
i=0
while test $i -lt $nr
do
echo $data
i=$((i + 1))
done
done
}
test_expect_success 'setup split file case' '
# use lines long enough to trigger content detection
test_seq 1000 1010 >one &&
test_seq 2000 2010 >two &&
git add one two &&
test_commit base &&
sed "6s/^/modified /" <one >one.tmp &&
mv one.tmp one &&
sed "6s/^/modified /" <two >two.tmp &&
mv two.tmp two &&
git add -u &&
test_commit modified &&
cat one two >combined &&
git add combined &&
git rm one two &&
test_commit combined
'
test_expect_success 'setup simulated porcelain' '
# This just reads porcelain-ish output and tries
# to output the value of a given field for each line (either by
# reading the field that accompanies this line, or referencing
# the information found last time the commit was mentioned).
cat >read-porcelain.pl <<-\EOF
my $field = shift;
while (<>) {
if (/^[0-9a-f]{40} /) {
flush();
$hash = $&;
} elsif (/^$field (.*)/) {
$cache{$hash} = $1;
}
}
flush();
sub flush {
return unless defined $hash;
if (defined $cache{$hash}) {
print "$cache{$hash}\n";
} else {
print "NONE\n";
}
}
EOF
'
for output in porcelain line-porcelain
do
test_expect_success "generate --$output output" '
git blame --root -C --$output combined >output
'
test_expect_success "$output output finds correct commits" '
generate_expect >expect <<-\EOF &&
5 base
1 modified
10 base
1 modified
5 base
EOF
perl read-porcelain.pl summary <output >actual &&
test_cmp expect actual
'
test_expect_success "$output output shows correct filenames" '
generate_expect >expect <<-\EOF &&
11 one
11 two
EOF
perl read-porcelain.pl filename <output >actual &&
test_cmp expect actual
'
test_expect_success "$output output shows correct previous pointer" '
generate_expect >expect <<-EOF &&
5 NONE
1 $(git rev-parse modified^) one
10 NONE
1 $(git rev-parse modified^) two
5 NONE
EOF
perl read-porcelain.pl previous <output >actual &&
test_cmp expect actual
'
done
test_done