Re: [Quilt-dev] [PATCH] patchfns: Simplify find_patch_file

[Jean Delvare]

Hi all,

Le Tuesday 11 February 2014 à 16:23 +0100, Jean Delvare a écrit :
> echo $(function) is equivalent to calling function directly.
> ---
> As suggested by Andreas in a similar case.
> 
>  quilt/scripts/patchfns.in |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- quilt.orig/quilt/scripts/patchfns.in
> +++ quilt/quilt/scripts/patchfns.in
> @@ -639,7 +639,7 @@ find_patch_file()
>               return 1
>       fi
>  
> -     echo $(patch_file_name "$patch")
> +     patch_file_name "$patch"
>  }
>  
>  file_in_patch()
> 

As I was double-checking that the patch was correct, I realized that I
do not really understand what this piece of code is good for.

find_patch_file is only used by "quilt import". It is called on patch
file names passed on the command line. The first part of the function
clearly looks for "external" patch files (with absolute or relative
paths.)

The second part looks for the patch in the series file. There are 3
possibilities:
* The patch is not in series, then the import fails with:
Patch foo.patch does not exist
* The patch is in series, applied, then the import fails with:
Patch foo.patch is applied
* The patch is in series, not applied, then the import fails with:
Patch foo.patch already exists in series.

In all cases, the import fails (as I expected). So I am wondering, what
is this piece of code supposedly good for? Can we get rid of it with the
following?

--- a/quilt/scripts/patchfns.in
+++ b/quilt/scripts/patchfns.in
@@ -630,16 +630,8 @@ find_patch_file()
                fi
        fi
 
-       local patch
-
-       patch=$(find_patch_in_series "$name" 2>/dev/null)
-       if [ -z "$patch" ]
-       then
-               printf $"Patch %s does not exist\n" "$name" >&2
-               return 1
-       fi
-
-       patch_file_name "$patch"
+       printf $"Patch %s does not exist\n" "$name" >&2
+       return 1
 }
 
 file_in_patch()


FWIW the test suite runs just fine with this change. Andreas, am I
missing something? "quilt blame" points to commit 3a6c8c1e:

    - import command: catch several error conditions. Patches in
      QUILT_PATCHES can now e imported. A minor fix in the output.

Importing patches from $QUILT_PATCHES works just fine as far as I can
tell, with or without the code above. All you have to do is provide the
path to the patch file (e.g. patches/foo.patch from the root of the
working tree.)

Thanks,
-- 
Jean Delvare
Suse L3 Support