Re: [PATCH v6 0/6] blame: add the ability to ignore commits

[Barret Rhoden <brho@google.com>]

Hi Michael -

I cobbled something together that passes my old git tests and all but 
one of your tests, though an assertion fails when I use it for real. 
See below.

On 4/22/19 6:26 PM, michael@platin.gs wrote:
[snip]
> +static int *get_similarity(int *similarities, int max_search_distance_a,
> +			   int local_line_a, int local_line_b,
> +			   int closest_local_line_a) {
> +	assert(abs(local_line_a - closest_local_line_a) <= max_search_distance_a);

This assert fails.  In my example,
	local_line_a = 1
	closest_local_line_a = 12
	max_search_distance_a = 10

Backtrace tells me it was called here:

> +static void fuzzy_find_matching_lines_recurse(

[snip]

> +	for (i = invalidate_min; i < invalidate_max; ++i) {
> +		closest_local_line_a = get_closest_local_line(
> +			start_a, i,
> +			closest_line_calc_offset1,
> +			closest_line_calc_offset2,
> +			closest_line_calc_numerator,
> +			closest_line_calc_denominator);
> +		*get_similarity(similarities, max_search_distance_a,
> +				most_certain_line_a - start_a, i,
> +				closest_local_line_a) = -1;
> +	}

So it looks like that '12' came from get_closest_local_line(),  The args 
to that call were:

	start_a 258, i 3, off1 0, off2 258 num 83, denom 23

The equation reduces to 12 + off2 - start_a = 12

I don't know what those values mean, but it looks like A's values affect 
off2 and start_a, but those are only used at the very end of the 
calculation.  Does 'A' affect off1, numer, or denom?

Any thoughts?  What all is going on here?

If you want to play with it, it's at

	git@github.com:brho/git.git (master)

(Beware the push -f / forced update.  And I figured the repo would be 
preferable to spamming with unrelated patches).

If you want an example commit the assert fails on, it's this repo:
	
	git@github.com:brho/akaros.git
	master branch
	ignore-rev-file=.git-blame-ignore-revs
	file user/vmm/virtio_mmio.c

On an unrelated note:

 > The significant change here is that when a line is matched, its 
      > fingerprint is subtracted from the matched parent line's 
fingerprint. > This prevents two lines matching the same part of a 
parent line.

Does this limit us so that our second pass (the fallback when fuzzy 
failed) won't be able to match to a parent line that was already matched?


The test that is failing is your Expand Lines test.  The 'final' diff was:

--- a/1
+++ b/1
@@ -1,5 +1,7 @@
  aaa
-bbb
+bbbx
+bbbx
  ccc
-ddd
+dddx
+dddx
  eee

Which should be two diff chunks and two calls to guess_line_blames().

And the 'actual' was:

1
2
Final  (not 2)
3
4
Final  (not 2)
5

I didn't dig much, but your older stuff (when merged with mine) also 
didn't pass that test.  Maybe something with the offset/parent_slno stuff.

Thanks,

Barret