Re: [Quilt-dev] [patch 6/7] Exit with an error when diffs retcode=2 (error) on patch

[Jean Delvare]

Hi Martin,

On Sat, 21 Dec 2013 21:27:58 +0100, address@hidden wrote:
> Description:
>  This is trigered e.g. when you try to add a binary file to a patch.
>  This is actually creepy to think that we were not checking the
>  retcode of patch :)

You mean diff, not patch, right?

> Forwarded: 2013-12-21
> Bug-Debian: http://bugs.debian.org/638313
> Author: Martin Quinson
> 
> ---
>  quilt/refresh.in          |    2 +-
>  quilt/scripts/patchfns.in |    6 ++++++
>  2 files changed, 7 insertions(+), 1 deletion(-)
>  
> pipestatus: 1  
> Index: b/quilt/scripts/patchfns.in
> ===================================================================
> --- a/quilt/scripts/patchfns.in
> +++ b/quilt/scripts/patchfns.in
> @@ -766,6 +766,12 @@
>               echo "$line"
>               cat
>       fi
> +
> +     # Test the return value of diff, and propagate the error retcode if any
> +     if [ ${PIPESTATUS[0]} == 2 ] ;

The semi-colon is not needed.

> +     then
> +             return 1
> +     fi
>  }
>  
>  cat_file()
> Index: b/quilt/refresh.in
> ===================================================================
> --- a/quilt/refresh.in
> +++ b/quilt/refresh.in
> @@ -231,7 +231,7 @@
>       fi
>       if ! diff_file "$file" "$old_file" "$new_file"
>       then
> -             printf $"Diff failed, aborting\n" >&2
> +             printf $"Diff failed on '$new_file', aborting. Is it a binary 
> file?\n" >&2

I don't think it is a good idea to suggest a reason when the problem
could be completely different. It adds confusion more than it helps.

If you want to be able to report this specific case then you should
grep for '^Binary files' in the output of diff, although this could be
fragile.

>               die 1
>       fi
>  

I also would like you to add a test case for this in the test suite.

Thanks,
-- 
Jean Delvare
Suse L3 Support