diff mbox

[v2] win32: Fix CRLF problem in make_device_config.sh

Message ID 1292690066-4937-1-git-send-email-weil@mail.berlios.de
State Accepted
Headers show

Commit Message

Stefan Weil Dec. 18, 2010, 4:34 p.m. UTC
QEMU source code with CRLF line endings
which is quite common on windows hosts
fails with current make_device_config.sh.

The awk script gets the name of the included
file with \r, so instead of pci.mak it will
search for pci.mak\r which of course does
not work.

Fix this by removing any \r.

v2:
    Avoid using sub() and \r with awk because they are unsupported
    on some platforms. Use tr to remove \r. This new solution
    improves portability and was suggested by Paolo Bonzini.

Signed-off-by: Stefan Weil <weil@mail.berlios.de>
---
 make_device_config.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Comments

Andreas Färber Dec. 18, 2010, 5:09 p.m. UTC | #1
Am 18.12.2010 um 17:34 schrieb Stefan Weil:

> QEMU source code with CRLF line endings
> which is quite common on windows hosts
> fails with current make_device_config.sh.
>
> The awk script gets the name of the included
> file with \r, so instead of pci.mak it will
> search for pci.mak\r which of course does
> not work.
>
> Fix this by removing any \r.
>
> v2:
>    Avoid using sub() and \r with awk because they are unsupported
>    on some platforms. Use tr to remove \r. This new solution
>    improves portability and was suggested by Paolo Bonzini.
>
> Signed-off-by: Stefan Weil <weil@mail.berlios.de>

Acked-by: Andreas Färber <andreas.faerber@web.de>

> ---
> make_device_config.sh |    2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/make_device_config.sh b/make_device_config.sh
> index 8abadfe..596fc5b 100644
> --- a/make_device_config.sh
> +++ b/make_device_config.sh
> @@ -18,7 +18,7 @@ process_includes () {
>
> f=$src
> while [ -n "$f" ] ; do
> -  f=`awk '/^include / {ORS=" " ; print "'$src_dir'/" $2}' $f`
> +  f=`tr -d '\r' < $f | awk '/^include / {ORS=" "; print  
> "'$src_dir'/" $2}'`
>   [ $? = 0 ] || exit 1
>   all_includes="$all_includes $f"
> done
> -- 
> 1.7.2.3
Blue Swirl Dec. 18, 2010, 6:59 p.m. UTC | #2
Thanks, applied.

On Sat, Dec 18, 2010 at 5:09 PM, Andreas Färber <andreas.faerber@web.de> wrote:
> Am 18.12.2010 um 17:34 schrieb Stefan Weil:
>
>> QEMU source code with CRLF line endings
>> which is quite common on windows hosts
>> fails with current make_device_config.sh.
>>
>> The awk script gets the name of the included
>> file with \r, so instead of pci.mak it will
>> search for pci.mak\r which of course does
>> not work.
>>
>> Fix this by removing any \r.
>>
>> v2:
>>   Avoid using sub() and \r with awk because they are unsupported
>>   on some platforms. Use tr to remove \r. This new solution
>>   improves portability and was suggested by Paolo Bonzini.
>>
>> Signed-off-by: Stefan Weil <weil@mail.berlios.de>
>
> Acked-by: Andreas Färber <andreas.faerber@web.de>
>
>> ---
>> make_device_config.sh |    2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/make_device_config.sh b/make_device_config.sh
>> index 8abadfe..596fc5b 100644
>> --- a/make_device_config.sh
>> +++ b/make_device_config.sh
>> @@ -18,7 +18,7 @@ process_includes () {
>>
>> f=$src
>> while [ -n "$f" ] ; do
>> -  f=`awk '/^include / {ORS=" " ; print "'$src_dir'/" $2}' $f`
>> +  f=`tr -d '\r' < $f | awk '/^include / {ORS=" "; print "'$src_dir'/"
>> $2}'`
>>  [ $? = 0 ] || exit 1
>>  all_includes="$all_includes $f"
>> done
>> --
>> 1.7.2.3
>
>
>
Stefan Weil Dec. 19, 2010, 3:42 p.m. UTC | #3
Am 18.12.2010 19:59, schrieb Blue Swirl:
> Thanks, applied.
>
> On Sat, Dec 18, 2010 at 5:09 PM, Andreas Färber 
> <andreas.faerber@web.de> wrote:
>> Am 18.12.2010 um 17:34 schrieb Stefan Weil:
>>
>>> QEMU source code with CRLF line endings
>>> which is quite common on windows hosts
>>> fails with current make_device_config.sh.
>>>
>>> The awk script gets the name of the included
>>> file with \r, so instead of pci.mak it will
>>> search for pci.mak\r which of course does
>>> not work.
>>>
>>> Fix this by removing any \r.
>>>
>>> v2:
>>>   Avoid using sub() and \r with awk because they are unsupported
>>>   on some platforms. Use tr to remove \r. This new solution
>>>   improves portability and was suggested by Paolo Bonzini.
>>>
>>> Signed-off-by: Stefan Weil <weil@mail.berlios.de>
>>
>> Acked-by: Andreas Färber <andreas.faerber@web.de>
>>> ---
>>> make_device_config.sh |    2 +-
>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/make_device_config.sh b/make_device_config.sh
>>> index 8abadfe..596fc5b 100644
>>> --- a/make_device_config.sh
>>> +++ b/make_device_config.sh
>>> @@ -18,7 +18,7 @@ process_includes () {
>>>
>>> f=$src
>>> while [ -n "$f" ] ; do
>>> -  f=`awk '/^include / {ORS=" " ; print "'$src_dir'/" $2}' $f`
>>> +  f=`tr -d '\r' < $f | awk '/^include / {ORS=" "; print "'$src_dir'/"
>>> $2}'`
>>>  [ $? = 0 ] || exit 1
>>>  all_includes="$all_includes $f"
>>> done
>>> --
>>> 1.7.2.3

The new code raises a new problem (sorry that I did not detect it earlier):

On hosts with /bin/sh != bash, make displays an error from 
make_device_config.sh:

$ touch default-configs/i386-softmmu.mak
$ make
   GEN   i386-softmmu/config-devices.mak
/qemu/make_device_config.sh: 24: cannot open 
/qemu/default-configs/pci.mak : No such file

make continues, so the error message is not fatal but a cosmetical issue.
It took me some time to find the reason for this error message although
it is quite simple:

The filename f which is calculated using awk ends with a blank character
caused by ORS=" ".

Obviously this blank does not matter for bash and other shells when
$f is used as a parameter. I/O redirection with bash works, too.
But dash (and perhaps other simple shells) work different. For dash,
< $f works like < "$f", so the blank is part of the filename,
and "pci.mak " of course does not exist. Is this a dash bug or a feature?

Using ORS="" solves the problem for me, but might raise new compatibility
problems (is an empty records separator always supported?).

Andreas, may I ask you for one more test?

$ echo -e 'include xy\r' | tr -d '\r' | awk '/^include / {ORS=""; print 
$2}' | od -c

It should return 0000002, not 0000003 like the previous test:

0000000   x   y
0000002

Regards,
Stefan
Andreas Färber Dec. 19, 2010, 4:06 p.m. UTC | #4
Am 19.12.2010 um 16:42 schrieb Stefan Weil:

> Am 18.12.2010 19:59, schrieb Blue Swirl:
>> Thanks, applied.
>>
>> On Sat, Dec 18, 2010 at 5:09 PM, Andreas Färber <andreas.faerber@web.de 
>> > wrote:
>>> Am 18.12.2010 um 17:34 schrieb Stefan Weil:
>>>
>>>> QEMU source code with CRLF line endings
>>>> which is quite common on windows hosts
>>>> fails with current make_device_config.sh.
>>>>
>>>> The awk script gets the name of the included
>>>> file with \r, so instead of pci.mak it will
>>>> search for pci.mak\r which of course does
>>>> not work.
>>>>
>>>> Fix this by removing any \r.
>>>>
>>>> v2:
>>>> Avoid using sub() and \r with awk because they are unsupported
>>>> on some platforms. Use tr to remove \r. This new solution
>>>> improves portability and was suggested by Paolo Bonzini.
>>>>
>>>> Signed-off-by: Stefan Weil <weil@mail.berlios.de>
>>>
>>> Acked-by: Andreas Färber <andreas.faerber@web.de>
>>>> ---
>>>> make_device_config.sh |    2 +-
>>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/make_device_config.sh b/make_device_config.sh
>>>> index 8abadfe..596fc5b 100644
>>>> --- a/make_device_config.sh
>>>> +++ b/make_device_config.sh
>>>> @@ -18,7 +18,7 @@ process_includes () {
>>>>
>>>> f=$src
>>>> while [ -n "$f" ] ; do
>>>> -  f=`awk '/^include / {ORS=" " ; print "'$src_dir'/" $2}' $f`
>>>> +  f=`tr -d '\r' < $f | awk '/^include / {ORS=" "; print  
>>>> "'$src_dir'/"
>>>> $2}'`
>>>> [ $? = 0 ] || exit 1
>>>> all_includes="$all_includes $f"
>>>> done
>>>> --
>>>> 1.7.2.3
>
> The new code raises a new problem (sorry that I did not detect it  
> earlier):
>
> On hosts with /bin/sh != bash, make displays an error from  
> make_device_config.sh:
>
> $ touch default-configs/i386-softmmu.mak
> $ make
> GEN   i386-softmmu/config-devices.mak
> /qemu/make_device_config.sh: 24: cannot open /qemu/default-configs/ 
> pci.mak : No such file
>
> make continues, so the error message is not fatal but a cosmetical  
> issue.

I'm pretty sure I saw that with some bash, too. Possibly on Haiku.  
Anyway...

> It took me some time to find the reason for this error message  
> although
> it is quite simple:
>
> The filename f which is calculated using awk ends with a blank  
> character
> caused by ORS=" ".
>
> Obviously this blank does not matter for bash and other shells when
> $f is used as a parameter. I/O redirection with bash works, too.
> But dash (and perhaps other simple shells) work different. For dash,
> < $f works like < "$f", so the blank is part of the filename,
> and "pci.mak " of course does not exist. Is this a dash bug or a  
> feature?
>
> Using ORS="" solves the problem for me, but might raise new  
> compatibility
> problems (is an empty records separator always supported?).
>
> Andreas, may I ask you for one more test?
>
> $ echo -e 'include xy\r' | tr -d '\r' | awk '/^include / {ORS="";  
> print $2}' | od -c
>
> It should return 0000002, not 0000003 like the previous test:
>
> 0000000   x   y
> 0000002

Tested-by: Andreas Färber <andreas.faerber@web.de>
Blue Swirl Dec. 19, 2010, 4:21 p.m. UTC | #5
On Sun, Dec 19, 2010 at 3:42 PM, Stefan Weil <weil@mail.berlios.de> wrote:
> Am 18.12.2010 19:59, schrieb Blue Swirl:
>>
>> Thanks, applied.
>>
>> On Sat, Dec 18, 2010 at 5:09 PM, Andreas Färber <andreas.faerber@web.de>
>> wrote:
>>>
>>> Am 18.12.2010 um 17:34 schrieb Stefan Weil:
>>>
>>>> QEMU source code with CRLF line endings
>>>> which is quite common on windows hosts
>>>> fails with current make_device_config.sh.
>>>>
>>>> The awk script gets the name of the included
>>>> file with \r, so instead of pci.mak it will
>>>> search for pci.mak\r which of course does
>>>> not work.
>>>>
>>>> Fix this by removing any \r.
>>>>
>>>> v2:
>>>>  Avoid using sub() and \r with awk because they are unsupported
>>>>  on some platforms. Use tr to remove \r. This new solution
>>>>  improves portability and was suggested by Paolo Bonzini.
>>>>
>>>> Signed-off-by: Stefan Weil <weil@mail.berlios.de>
>>>
>>> Acked-by: Andreas Färber <andreas.faerber@web.de>
>>>>
>>>> ---
>>>> make_device_config.sh |    2 +-
>>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/make_device_config.sh b/make_device_config.sh
>>>> index 8abadfe..596fc5b 100644
>>>> --- a/make_device_config.sh
>>>> +++ b/make_device_config.sh
>>>> @@ -18,7 +18,7 @@ process_includes () {
>>>>
>>>> f=$src
>>>> while [ -n "$f" ] ; do
>>>> -  f=`awk '/^include / {ORS=" " ; print "'$src_dir'/" $2}' $f`
>>>> +  f=`tr -d '\r' < $f | awk '/^include / {ORS=" "; print "'$src_dir'/"
>>>> $2}'`
>>>>  [ $? = 0 ] || exit 1
>>>>  all_includes="$all_includes $f"
>>>> done
>>>> --
>>>> 1.7.2.3
>
> The new code raises a new problem (sorry that I did not detect it earlier):
>
> On hosts with /bin/sh != bash, make displays an error from
> make_device_config.sh:
>
> $ touch default-configs/i386-softmmu.mak
> $ make
>  GEN   i386-softmmu/config-devices.mak
> /qemu/make_device_config.sh: 24: cannot open /qemu/default-configs/pci.mak :
> No such file
>
> make continues, so the error message is not fatal but a cosmetical issue.
> It took me some time to find the reason for this error message although
> it is quite simple:
>
> The filename f which is calculated using awk ends with a blank character
> caused by ORS=" ".
>
> Obviously this blank does not matter for bash and other shells when
> $f is used as a parameter. I/O redirection with bash works, too.
> But dash (and perhaps other simple shells) work different. For dash,
> < $f works like < "$f", so the blank is part of the filename,
> and "pci.mak " of course does not exist. Is this a dash bug or a feature?
>
> Using ORS="" solves the problem for me, but might raise new compatibility
> problems (is an empty records separator always supported?).
>
> Andreas, may I ask you for one more test?
>
> $ echo -e 'include xy\r' | tr -d '\r' | awk '/^include / {ORS=""; print $2}'
> | od -c
>
> It should return 0000002, not 0000003 like the previous test:
>
> 0000000   x   y
> 0000002

How about something like this instead:
(SRC_DIR=/src; echo -e 'include xy' | sed -n 's|^include
*\(.*\)|'$SRC_DIR/'\1|p')
/src/xy
Andreas Färber Dec. 19, 2010, 4:43 p.m. UTC | #6
Am 19.12.2010 um 17:21 schrieb Blue Swirl:

> On Sun, Dec 19, 2010 at 3:42 PM, Stefan Weil <weil@mail.berlios.de>  
> wrote:
>> Am 18.12.2010 19:59, schrieb Blue Swirl:
>>>
>>> Thanks, applied.
>>>
>>> On Sat, Dec 18, 2010 at 5:09 PM, Andreas Färber <andreas.faerber@web.de 
>>> >
>>> wrote:
>>>>
>>>> Am 18.12.2010 um 17:34 schrieb Stefan Weil:
>>>>
>>>>> QEMU source code with CRLF line endings
>>>>> which is quite common on windows hosts
>>>>> fails with current make_device_config.sh.
>>>>>
>>>>> The awk script gets the name of the included
>>>>> file with \r, so instead of pci.mak it will
>>>>> search for pci.mak\r which of course does
>>>>> not work.
>>>>>
>>>>> Fix this by removing any \r.
>>>>>
>>>>> v2:
>>>>>  Avoid using sub() and \r with awk because they are unsupported
>>>>>  on some platforms. Use tr to remove \r. This new solution
>>>>>  improves portability and was suggested by Paolo Bonzini.
>>>>>
>>>>> Signed-off-by: Stefan Weil <weil@mail.berlios.de>
>>>>
>>>> Acked-by: Andreas Färber <andreas.faerber@web.de>
>>>>>
>>>>> ---
>>>>> make_device_config.sh |    2 +-
>>>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>>>
>>>>> diff --git a/make_device_config.sh b/make_device_config.sh
>>>>> index 8abadfe..596fc5b 100644
>>>>> --- a/make_device_config.sh
>>>>> +++ b/make_device_config.sh
>>>>> @@ -18,7 +18,7 @@ process_includes () {
>>>>>
>>>>> f=$src
>>>>> while [ -n "$f" ] ; do
>>>>> -  f=`awk '/^include / {ORS=" " ; print "'$src_dir'/" $2}' $f`
>>>>> +  f=`tr -d '\r' < $f | awk '/^include / {ORS=" "; print  
>>>>> "'$src_dir'/"
>>>>> $2}'`
>>>>>  [ $? = 0 ] || exit 1
>>>>>  all_includes="$all_includes $f"
>>>>> done
>>>>> --
>>>>> 1.7.2.3
>>
>> The new code raises a new problem (sorry that I did not detect it  
>> earlier):
>>
>> On hosts with /bin/sh != bash, make displays an error from
>> make_device_config.sh:
>>
>> $ touch default-configs/i386-softmmu.mak
>> $ make
>>  GEN   i386-softmmu/config-devices.mak
>> /qemu/make_device_config.sh: 24: cannot open /qemu/default-configs/ 
>> pci.mak :
>> No such file
>>
>> make continues, so the error message is not fatal but a cosmetical  
>> issue.
>> It took me some time to find the reason for this error message  
>> although
>> it is quite simple:
>>
>> The filename f which is calculated using awk ends with a blank  
>> character
>> caused by ORS=" ".
>>
>> Obviously this blank does not matter for bash and other shells when
>> $f is used as a parameter. I/O redirection with bash works, too.
>> But dash (and perhaps other simple shells) work different. For dash,
>> < $f works like < "$f", so the blank is part of the filename,
>> and "pci.mak " of course does not exist. Is this a dash bug or a  
>> feature?
>>
>> Using ORS="" solves the problem for me, but might raise new  
>> compatibility
>> problems (is an empty records separator always supported?).
>>
>> Andreas, may I ask you for one more test?
>>
>> $ echo -e 'include xy\r' | tr -d '\r' | awk '/^include / {ORS="";  
>> print $2}'
>> | od -c
>>
>> It should return 0000002, not 0000003 like the previous test:
>>
>> 0000000   x   y
>> 0000002
>
> How about something like this instead:
> (SRC_DIR=/src; echo -e 'include xy' | sed -n 's|^include
> *\(.*\)|'$SRC_DIR/'\1|p')
> /src/xy

Produces the above result on Sol10. With 'include xy\r' ... | od -c  
leads to:

0000000   /   s   r   c   /   x   y   \r  \n
0000011

In bash.
Paolo Bonzini Dec. 20, 2010, 10:37 a.m. UTC | #7
On 12/19/2010 05:06 PM, Andreas Färber wrote:
>
> Andreas, may I ask you for one more test?
>
> $ echo -e 'include xy\r' | tr -d '\r' | awk '/^include / {ORS=""; print
> $2}' | od -c

I'd not bother with ORS and use '$1 == "include" { printf "%s", $2 }' 
instead.

Paolo
Andreas Färber Dec. 30, 2010, 9:53 p.m. UTC | #8
Am 19.12.2010 um 17:06 schrieb Andreas Färber:

> Am 19.12.2010 um 16:42 schrieb Stefan Weil:
>
>> Am 18.12.2010 19:59, schrieb Blue Swirl:
>>> Thanks, applied.
>>>
>>> On Sat, Dec 18, 2010 at 5:09 PM, Andreas Färber <andreas.faerber@web.de 
>>> > wrote:
>>>> Am 18.12.2010 um 17:34 schrieb Stefan Weil:
>>>>
>>>>> QEMU source code with CRLF line endings
>>>>> which is quite common on windows hosts
>>>>> fails with current make_device_config.sh.
>>>>>
>>>>> The awk script gets the name of the included
>>>>> file with \r, so instead of pci.mak it will
>>>>> search for pci.mak\r which of course does
>>>>> not work.
>>>>>
>>>>> Fix this by removing any \r.
>>>>>
>>>>> v2:
>>>>> Avoid using sub() and \r with awk because they are unsupported
>>>>> on some platforms. Use tr to remove \r. This new solution
>>>>> improves portability and was suggested by Paolo Bonzini.
>>>>>
>>>>> Signed-off-by: Stefan Weil <weil@mail.berlios.de>
>>>>
>>>> Acked-by: Andreas Färber <andreas.faerber@web.de>
>>>>> ---
>>>>> make_device_config.sh |    2 +-
>>>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>>>
>>>>> diff --git a/make_device_config.sh b/make_device_config.sh
>>>>> index 8abadfe..596fc5b 100644
>>>>> --- a/make_device_config.sh
>>>>> +++ b/make_device_config.sh
>>>>> @@ -18,7 +18,7 @@ process_includes () {
>>>>>
>>>>> f=$src
>>>>> while [ -n "$f" ] ; do
>>>>> -  f=`awk '/^include / {ORS=" " ; print "'$src_dir'/" $2}' $f`
>>>>> +  f=`tr -d '\r' < $f | awk '/^include / {ORS=" "; print  
>>>>> "'$src_dir'/"
>>>>> $2}'`
>>>>> [ $? = 0 ] || exit 1
>>>>> all_includes="$all_includes $f"
>>>>> done
>>>>> --
>>>>> 1.7.2.3
>>
>> The new code raises a new problem (sorry that I did not detect it  
>> earlier):
>>
>> On hosts with /bin/sh != bash, make displays an error from  
>> make_device_config.sh:
>>
>> $ touch default-configs/i386-softmmu.mak
>> $ make
>> GEN   i386-softmmu/config-devices.mak
>> /qemu/make_device_config.sh: 24: cannot open /qemu/default-configs/ 
>> pci.mak : No such file
>>
>> make continues, so the error message is not fatal but a cosmetical  
>> issue.
>
> I'm pretty sure I saw that with some bash, too. Possibly on Haiku.

I'm still seeing this issue, on Mac OS X with bash. And yes, on Haiku  
with bash, too.

Any update?

Thanks,
Andreas

> Anyway...
>
>> It took me some time to find the reason for this error message  
>> although
>> it is quite simple:
>>
>> The filename f which is calculated using awk ends with a blank  
>> character
>> caused by ORS=" ".
>>
>> Obviously this blank does not matter for bash and other shells when
>> $f is used as a parameter. I/O redirection with bash works, too.
>> But dash (and perhaps other simple shells) work different. For dash,
>> < $f works like < "$f", so the blank is part of the filename,
>> and "pci.mak " of course does not exist. Is this a dash bug or a  
>> feature?
>>
>> Using ORS="" solves the problem for me, but might raise new  
>> compatibility
>> problems (is an empty records separator always supported?).
>>
>> Andreas, may I ask you for one more test?
>>
>> $ echo -e 'include xy\r' | tr -d '\r' | awk '/^include / {ORS="";  
>> print $2}' | od -c
>>
>> It should return 0000002, not 0000003 like the previous test:
>>
>> 0000000   x   y
>> 0000002
>
> Tested-by: Andreas Färber <andreas.faerber@web.de>
diff mbox

Patch

diff --git a/make_device_config.sh b/make_device_config.sh
index 8abadfe..596fc5b 100644
--- a/make_device_config.sh
+++ b/make_device_config.sh
@@ -18,7 +18,7 @@  process_includes () {
 
 f=$src
 while [ -n "$f" ] ; do
-  f=`awk '/^include / {ORS=" " ; print "'$src_dir'/" $2}' $f`
+  f=`tr -d '\r' < $f | awk '/^include / {ORS=" "; print "'$src_dir'/" $2}'`
   [ $? = 0 ] || exit 1
   all_includes="$all_includes $f"
 done