Re: [PATCH] diff-lib.c: Fix diff-files --diff-filter --quiet exit code

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Jakob Pfender <jpfender@xxxxxxxxxxxxx> writes:

> Given the following status:
>
> 	$ git status -s
> 	 M bar
> 	UU foo
>
> There is an unexpected difference in the return code of git diff-files
> when used with --quiet as opposed to --exit-code:
>
> 	$ git diff-files --diff-filter=U --quiet
> 	$ echo $?
> 	0
>
> 	$ git diff-files --diff-filter=U --exit-code
> 	<usual output, lots of 0's and U<TAB>foo>
> 	$ echo $?
> 	1
>
> Notice the different return codes. Now, according to the documentation,
> --quiet implies --return-code. However, the return code of the former is
> clearly not correct if --return-code was supposed to be on.
>
> This patch removes a useless bit of code that caused the return codes to
> differ.

How did you determine it is "useless bit"?

The code notices that the caller, by specifying --quiet, does not want any
details of the changes and instead wants to know if there is a change or
not.  And it breaks out of the loop because it already found what it
wanted to know, namely, there is a change.

When you have a post-process filter (like -w or -S), the path we found to
be different here may be uninteresting and there may be no output (hence
we should exit with status 0).  So it is true that the optimization you
are removing needs to be disabled in _some_ situations, and the current
code doesn't, and it needs fixing.

But is it a justifiable fix to disable the optimization for even the
normal cases?

> Signed-off-by: Jakob Pfender <jpfender@xxxxxxxxxxxxx>
> ---
>  diff-lib.c |    4 ----
>  1 files changed, 0 insertions(+), 4 deletions(-)
>
> diff --git a/diff-lib.c b/diff-lib.c
> index 392ce2b..a7aa42b 100644
> --- a/diff-lib.c
> +++ b/diff-lib.c
> @@ -102,10 +102,6 @@ int run_diff_files(struct rev_info *revs,
> unsigned int option)
>  		int changed;
>  		unsigned dirty_submodule = 0;
>
> -		if (DIFF_OPT_TST(&revs->diffopt, QUICK) &&
> -			DIFF_OPT_TST(&revs->diffopt, HAS_CHANGES))
> -			break;
> -
>  		if (!ce_path_match(ce, revs->prune_data))
>  			continue;
--
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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]