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

[Jean Delvare]

Le dimanche 02 décembre 2012 à 12:01 +0900, Satoru Takeuchi a écrit :
> Hi Benjamin, Bert,
> 
> At Sat, 1 Dec 2012 15:06:58 +0100,
> Bert Wesarg wrote:
> > 
> > On Fri, Nov 30, 2012 at 10:44 PM, Benjamin Poirier <address@hidden> wrote:
> > > 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 |    4 ++--
> > >  1 files changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/quilt/scripts/patchfns.in b/quilt/scripts/patchfns.in
> > > index d0c426e..2c884e0 100644
> > > --- a/quilt/scripts/patchfns.in
> > > +++ b/quilt/scripts/patchfns.in
> > > @@ -650,8 +650,8 @@ files_in_patch()
> > >         if [ -d "$path" ]
> > >         then
> > >                 find "$path" -type f \
> > > -                              -a ! -path "$path/.timestamp" |
> > > -               sed -e "s:$path/::"
> > > +                              -a ! -path "$path/.timestamp" \
> > > +                              -printf "%P\n"
> > 
> > I can't remember the policy in quilt, but I can't find evidence, that
> > -printf is in the POSIX standard yet.
> 
> I also don't know the policy. The following patch would also work,
> but it's ugly and I don't know how this fix help us since I don't
> use ":" for file name.
> 
> -             sed -e "s:$path/::"
> +             sed -e "s:"$(echo "$path" | sed -e 's/:/\\:/')"/::"

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?

I'll also add a test for this in the test suite. We already have such
tests for file names but not for patch names.

Thanks,
-- 
Jean Delvare
Suse L3