Re: [Quilt-dev] [PATCH] Fix handling of patch files with ':' in their name.

[Jean Delvare]

Hi Benjamin,

Le lundi 03 décembre 2012 à 16:25 -0500, Benjamin Poirier a écrit :
> On 2012/12/03 15:06, Jean Delvare wrote: 
> > It would be great to avoid yet another call to an external tool and a
> > pipe, these are bad for performance.
> > 
> > So I'd suggest the following fix instead, using bash's powerful
> > parameter expansion:
> > 
> > --- quilt.orig/quilt/scripts/patchfns.in
> > +++ quilt/quilt/scripts/patchfns.in
> > @@ -651,7 +651,7 @@ files_in_patch()
> >     then
> >             find "$path" -type f \
> >                            -a ! -path "$path/.timestamp" |
> > -           sed -e "s:$path/::"
> > +           sed -e "s:${path//:/\\:}/::"
> >     fi
> >  }
> >  
> > 
> > Probably not the more readable piece of code you've seen ;) but it
> > should work. Benjamin, Satoru, is this OK with you?
> 
> No, that does not work with, ex. "strange\:name.patch". If we want to
> avoid find -printf, then I suggest the patch that follows.

Good catch. We're almost there with this new proposal of yours...

> Admitedly, strange\:name does not work anyways ATM. Quilt's handling of
> \ in names needs some work as well...
> 
>       $ quilt import strange\\\:name
>       Importing patch strange\:name (stored as patches/strange\:name)
>       $ quilt push
>       Applying patch patches/strange:name
>       Patch patches/strange:name does not exist; applied empty patch
> 
>       Now at patch patches/strange:name

Yes, quilt has a long history of issues with "unusual" characters in
file, directory or patch names, for example:
https://savannah.nongnu.org/bugs/index.php?25579
https://savannah.nongnu.org/bugs/index.php?23682
https://savannah.nongnu.org/bugs/index.php?19477

We fix them as they are reported but it seems to be a never-ending
battle. I have created a test case for this and am adding tests to it
each time a specific issue is fixed, to prevent fixed ones from creeping
back in.

> Back to the original problem:
> 
> Subject: [PATCH] Fix handling of patch files with ':' in their name.
> 
> avoids errors like this:
> $ quilt refresh
> sed: -e expression #1, char 21: unknown option to `s'
> Nothing in patch patches/strange:name
> ---
>  quilt/scripts/patchfns.in |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/quilt/scripts/patchfns.in b/quilt/scripts/patchfns.in
> index d0c426e..08b7469 100644
> --- a/quilt/scripts/patchfns.in
> +++ b/quilt/scripts/patchfns.in
> @@ -651,7 +651,7 @@ files_in_patch()
>       then
>               find "$path" -type f \
>                              -a ! -path "$path/.timestamp" |
> -             sed -e "s:$path/::"
> +             sed -e "s/$(quote_bre "$path")\///"
>       fi
>  }
>  

The sed part is always working now, but the -path part isn't... -path
treats its argument as a pattern, i.e. it will try to expand '?', '*'
and [] (at least) in the pattern. Not an issue in your specific case,
':' isn't affected, but let's fix it as well while we're here:

--- quilt.orig/quilt/scripts/patchfns.in
+++ quilt/quilt/scripts/patchfns.in
@@ -88,6 +88,12 @@ quote_re()
        echo "$1" | sed -e 's:\([][?{(|)}^$/.+*\\]\):\\\1:g'
 }
 
+# Quote a string for use in a pattern.
+quote_pat()
+{
+       echo "$1" | sed -e 's:\([][*?\\]\):\\\1:g'
+}
+
 patch_file_name()
 {
        echo "$QUILT_PATCHES/$1"
@@ -650,8 +656,8 @@ files_in_patch()
        if [ -d "$path" ]
        then
                find "$path" -type f \
-                              -a ! -path "$path/.timestamp" |
-               sed -e "s:$path/::"
+                              -a ! -path "$(quote_pat $path)/.timestamp" |
+               sed -e "s/$(quote_bre "$path")\///"
        fi
 }
 
--- quilt.orig/test/space-in-filenames.test
+++ quilt/test/space-in-filenames.test
@@ -91,8 +91,27 @@ $ quilt files
 > foo
 > foo bar
 
+# Test with uncommon characters in patch names too
+$ quilt rename "patch_with:strange[name]"
+> Patch patches/test.diff renamed to patches/patch_with:strange[name]
+
+$ quilt top
+> patches/patch_with:strange[name]
+
+$ quilt diff -p ab -P "patch_with:strange[name]"
+> Index: b/foo bar
+> ===================================================================
+> --- a/foo bar
+> +++ b/foo bar
+> @@ -1 +1 @@
+> -foo
+> +bar
+
+$ quilt refresh -p ab
+> Refreshed patch patches/patch_with:strange[name]
+
 $quilt remove "foo bar"
-> File foo bar removed from patch patches/test.diff
+> File foo bar removed from patch patches/patch_with:strange[name]
 
 $ quilt files
 > foo


-- 
Jean Delvare
Suse L3