Re: [PATCH 1/2] diff: report copies and renames as changes in run_diff_cmd()
Hi René
On 08/09/2024 08:05, René Scharfe wrote:
Show 16 quoted lines
> The diff machinery has two ways to detect changes to set the exit code:
> Just comparing hashes and comparing blob contents. The latter is needed
> if certain changes have to be ignored, e.g. with --ignore-space-change
> or --ignore-matching-lines. It's enabled by the diff_options flag
> diff_from_contents.
>
> The slower mode has never considered copies and renames to be changes,
> which is inconsistent with the quicker one. Fix it. Even if we ignore
> the file contents (because it's empty or contains only ignored lines),
> there's still the meta data change of adding or changing a filename, so
> we need to report it in the exit code.
>
> d7b97b7185 (diff: let external diffs report that changes are
> uninteresting, 2024-06-09) set diff_from_contents if external diff
> programs are allowed. This is the default e.g. for git diff, and so
> that change exposed the inconsistency much more widely.
Thanks for fixing this - both patches looks good to me.
Best Wishes
Phillip
Show 49 quoted lines
> Reported-by: Jorge Luis Martinez Gomez <jol@jol.dev>
> Signed-off-by: René Scharfe <l.s.r@web.de>
> ---
> diff.c | 3 +++
> t/t4017-diff-retval.sh | 16 ++++++++++++++++
> 2 files changed, 19 insertions(+)
>
> diff --git a/diff.c b/diff.c
> index 4035a9374d..1d2057d4cb 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -4587,6 +4587,9 @@ static void run_diff_cmd(const struct external_diff *pgm,
> builtin_diff(name, other ? other : name,
> one, two, xfrm_msg, must_show_header,
> o, complete_rewrite);
> + if (p->status == DIFF_STATUS_COPIED ||
> + p->status == DIFF_STATUS_RENAMED)
> + o->found_changes = 1;
> } else {
> fprintf(o->file, "* Unmerged path %s\n", name);
> o->found_changes = 1;
> diff --git a/t/t4017-diff-retval.sh b/t/t4017-diff-retval.sh
> index f439f469bd..9a4f578614 100755
> --- a/t/t4017-diff-retval.sh
> +++ b/t/t4017-diff-retval.sh
> @@ -143,4 +143,20 @@ test_expect_success 'option errors are not confused by --exit-code' '
> grep '^usage:' err
> '
>
> +for option in --exit-code --quiet
> +do
> + test_expect_success "git diff $option returns 1 for copied file" "
> + git reset --hard &&
> + cp a copy &&
> + git add copy &&
> + test_expect_code 1 git diff $option --cached --find-copies-harder
> + "
> +
> + test_expect_success "git diff $option returns 1 for renamed file" "
> + git reset --hard &&
> + git mv a renamed &&
> + test_expect_code 1 git diff $option --cached
> + "
> +done
> +
> test_done
> --
> 2.46.0
>