Duy Nguyen <pclouds@xxxxxxxxx> writes:
> On Tue, Jan 28, 2014 at 02:51:45PM -0800, Junio C Hamano wrote:
>> >> This replaces 'diff: turn off skip_stat_unmatch on "diff --cached"'
>> >> The previous patch obviously leaves skip_stat_unmatch on in "diff
>> >> <rev> <rev>" and maybe other cases.
>> >
>> > Oops, I lost track. Sorry.
>>
>> Together with {1,2}/3 applied on maint-1.8.4, this sems to break
>> t3417 (there may be others, but I didn't have time to check).
>
> My bad. I thought I covered all cases in my last patch (and didn't
> retest it!). It turns out I should have set skip_stat_unmatch in
> builtin_diff_b_f() too. This on top of 3/3 passes the tests
Thanks, will squash it in.
This however shows that the existing test *KNEW* that it was enough
to check just a few cases (especially, there is no reason to make
sure that blob vs file-in-working-tree case behaves sanely), because
the auto-refresh would kick in for all codepaths. Now you are
making that assumption invalid, shouldn't the patch also split the
tests to cover individual cases?
> -- 8< --
> diff --git a/builtin/diff.c b/builtin/diff.c
> index 88542d9..8ab5e3d 100644
> --- a/builtin/diff.c
> +++ b/builtin/diff.c
> @@ -89,6 +89,7 @@ static int builtin_diff_b_f(struct rev_info *revs,
> if (blob[0].mode == S_IFINVALID)
> blob[0].mode = canon_mode(st.st_mode);
>
> + revs->diffopt.skip_stat_unmatch = !!diff_auto_refresh_index;
> stuff_change(&revs->diffopt,
> blob[0].mode, canon_mode(st.st_mode),
> blob[0].sha1, null_sha1,
> -- 8< --
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html