Re: [bug-gnu-libiconv] Re: [Mingw-msys] sed on msys error

[Bruno Haible]

Hi Sunil,

Thank you for the report.

Hi Keith,

Thank you for the forward of the problem. I've fixed it through the
appended patch.

Greg Chicares wrote:
> > So I would guess the problem is that this 'sed':
> > 
> > > $ sed --version
> > > GNU sed version 3.02
> >
> > is just too old.

Correct. When I had tested the sed expressions, I had been using some
4.x version on Linux.

> In fact, the original expression, 
> appearing in the sed command above, is invalid, and while some sed 
> implementations may forgive it, MSYS sed correctly rejects it.
> 
> Note the placement of the right hand brace, in the above expression; 
> it is immediately preceded by the `q' action verb, but POSIX is very 
> explicit here:--

Whether the used sed expressions are POSIX compliant or not, is not
relevant. This script is only meant to be executed on mingw. So the only
thing that matters is whether it executes correctly on mingw. Unfortunately
I had done some last-minute modifications on them without testing them on the
old sed-3.02.

> IMHO, this is a classical example of clever misuse of an inappropriate 
> tool, leading to a fragile implementation

Agreed, and thank you for sending a partial fix (your patch-1).

> IMO, to eliminate this complex sed expression altogether, in favour 
> of the much simpler, and therefore more robust, implementation 
> provided in my recommended alternative libiconv-1.12.patch-2.

But this one fails to return a number for the subminor:

version=2.17.3-pre3
IFS=.; set $version; echo ${1-0}
IFS=.; set $version; echo ${2-0}
IFS=.; set $version; echo ${3-0}
=>
2
17
3-pre3

... leading to syntax errors during the 'windres' invocation.

Trying to parse strings via bash builtins? This is a classical example for
an inappropriate tool, leading to an implementation that is broken by design.

Thanks for having made me aware of the bug.


2008-06-30  Bruno Haible  <address@hidden>

        Fix sed expressions to work with the old sed-3.02 on MSYS.
        * windows/windres-options (sed_extract_major, sed_extract_minor,
        sed_extract_subminor): Put a semicolon before the closing brace. Use
        an i\ command instead of an a\ command.
        Reported by Sunil Negi <address@hidden>
        in <http://article.gmane.org/gmane.comp.gnu.mingw.msys/4410>
        via Keith Marshall <address@hidden>.

diff -u -r1.1 windres-options
--- windows/windres-options     27 May 2007 20:22:06 -0000      1.1
+++ windows/windres-options     30 Jun 2008 22:57:12 -0000
@@ -14,18 +14,18 @@
 fi
 version="$1" # something like 2.0 or 2.17 or 2.17.3 or 2.17.3-pre3
 
-sed_extract_major='/^[0-9]/{s/^\([0-9]*\).*/\1/p;q}
-a\
+sed_extract_major='/^[0-9]/{s/^\([0-9]*\).*/\1/p;q;}
+i\
 0
 q
 '
-sed_extract_minor='/^[0-9][0-9]*[.][0-9]/{s/^[0-9]*[.]\([0-9]*\).*/\1/p;q}
-a\
+sed_extract_minor='/^[0-9][0-9]*[.][0-9]/{s/^[0-9]*[.]\([0-9]*\).*/\1/p;q;}
+i\
 0
 q
 '
-sed_extract_subminor='/^[0-9][0-9]*[.][0-9][0-9]*[.][0-9]/{s/^[0-9]*[.][0-9]*[.]\([0-9]*\).*/\1/p;q}
-a\
+sed_extract_subminor='/^[0-9][0-9]*[.][0-9][0-9]*[.][0-9]/{s/^[0-9]*[.][0-9]*[.]\([0-9]*\).*/\1/p;q;}
+i\
 0
 q
 '