Patchwork [Bug,636315,NEW] configure and build errors on Solaris 10 due to /bin/sh usage

login
register
mail settings
Submitter Blue Swirl
Date Sept. 12, 2010, 5:47 p.m.
Message ID <AANLkTimyhrj9NG0vPTJv9oJyUN+UBdyos6wBDR9Mcss_@mail.gmail.com>
Download mbox | patch
Permalink /patch/64553/
State New
Headers show

Comments

Blue Swirl - Sept. 12, 2010, 5:47 p.m.
On Sun, Sep 12, 2010 at 5:35 PM, Andreas Färber <andreas.faerber@web.de> wrote:
> Am 12.09.2010 um 19:22 schrieb Blue Swirl:
>
>> On Sun, Sep 12, 2010 at 11:26 AM, Andreas Färber
>> <636315@bugs.launchpad.net> wrote:
>>>
>>> Error: invalid trace backend
>>> Please choose a supported trace backend.
>>
>> What is the output of "sh ./tracetool --nop --check-backend"?
>
> ./tracetool: syntax error at line 51: `$' unexpected

Does this patch fix the problem?
Andreas Färber - Sept. 12, 2010, 5:58 p.m.
Am 12.09.2010 um 19:47 schrieb Blue Swirl:

> On Sun, Sep 12, 2010 at 5:35 PM, Andreas Färber <andreas.faerber@web.de 
> > wrote:
>> Am 12.09.2010 um 19:22 schrieb Blue Swirl:
>>
>>> What is the output of "sh ./tracetool --nop --check-backend"?
>>
>> ./tracetool: syntax error at line 51: `$' unexpected
>
> Does this patch fix the problem?
>
> diff --git a/tracetool b/tracetool
> index 534cc70..c7582bf 100755
> --- a/tracetool
> +++ b/tracetool
> @@ -48,7 +48,8 @@ get_argnames()
> {
>     local nfields field name
>     nfields=0
> -    for field in $(get_args "$1"); do
> +    args=get_args "$1"
> +    for field in "$args"; do

This part yes. (I took the liberty of adding args to the local line  
above)

>         nfields=$((nfields + 1))

Next error is here:
./tracetool: syntax error at line 53: `nfields=$' unexpected
Blue Swirl - Sept. 12, 2010, 9:05 p.m.
On Sun, Sep 12, 2010 at 5:58 PM, Andreas Färber <andreas.faerber@web.de> wrote:
> Am 12.09.2010 um 19:47 schrieb Blue Swirl:
>
>> On Sun, Sep 12, 2010 at 5:35 PM, Andreas Färber <andreas.faerber@web.de>
>> wrote:
>>>
>>> Am 12.09.2010 um 19:22 schrieb Blue Swirl:
>>>
>>>> What is the output of "sh ./tracetool --nop --check-backend"?
>>>
>>> ./tracetool: syntax error at line 51: `$' unexpected
>>
>> Does this patch fix the problem?
>>
>> diff --git a/tracetool b/tracetool
>> index 534cc70..c7582bf 100755
>> --- a/tracetool
>> +++ b/tracetool
>> @@ -48,7 +48,8 @@ get_argnames()
>> {
>>    local nfields field name
>>    nfields=0
>> -    for field in $(get_args "$1"); do
>> +    args=get_args "$1"
>> +    for field in "$args"; do
>
> This part yes. (I took the liberty of adding args to the local line above)
>
>>        nfields=$((nfields + 1))
>
> Next error is here:
> ./tracetool: syntax error at line 53: `nfields=$' unexpected

That looks like fully standards compliant, so Solaris' /bin/sh is not.
Can you try what happens with /usr/xpg4/bin/sh?
Andreas Färber - Sept. 12, 2010, 10:02 p.m.
Am 12.09.2010 um 23:05 schrieb Blue Swirl:

> On Sun, Sep 12, 2010 at 5:58 PM, Andreas Färber <andreas.faerber@web.de 
> > wrote:
>> Am 12.09.2010 um 19:47 schrieb Blue Swirl:
>>>        nfields=$((nfields + 1))
>>
>> ./tracetool: syntax error at line 53: `nfields=$' unexpected
>
> That looks like fully standards compliant, so Solaris' /bin/sh is not.
> Can you try what happens with /usr/xpg4/bin/sh?

Works fine! Must've done something wrong when testing that earlier  
today.

configure, create_config and tracetool with your fix are silent when / 
usr/xpg4/bin is in the $PATH.
If you commit it, we can close this ticket. Thanks for your help, Blue.

Build still fails, in qemu-nbd.c due to err.h, but that's unrelated  
to /bin/sh.
After disabling the tools in configure, sparc-softmmu builds fine again.
Michael Tokarev - Sept. 13, 2010, 8:49 a.m.
13.09.2010 01:05, Blue Swirl wrote:
> On Sun, Sep 12, 2010 at 5:58 PM, Andreas Färber <andreas.faerber@web.de> wrote:
>> Am 12.09.2010 um 19:47 schrieb Blue Swirl:
>>
>>> On Sun, Sep 12, 2010 at 5:35 PM, Andreas Färber <andreas.faerber@web.de>
>>> wrote:
>>>>
>>>> Am 12.09.2010 um 19:22 schrieb Blue Swirl:
>>>>
>>>>> What is the output of "sh ./tracetool --nop --check-backend"?
>>>>
>>>> ./tracetool: syntax error at line 51: `$' unexpected
>>>
>>> Does this patch fix the problem?
>>>
>>> diff --git a/tracetool b/tracetool
>>> index 534cc70..c7582bf 100755
>>> --- a/tracetool
>>> +++ b/tracetool
>>> @@ -48,7 +48,8 @@ get_argnames()
>>> {
>>>    local nfields field name
>>>    nfields=0
>>> -    for field in $(get_args "$1"); do
>>> +    args=get_args "$1"
>>> +    for field in "$args"; do
>>
>> This part yes. (I took the liberty of adding args to the local line above)

Um.  Are you sure it works as expected?  I'm not at all shure.
There are 2 errors in the above patch:

 +    args=get_args "$1"

After this line, variable $args will contain one word: "get_args".
Shell will try to execute a command or call a shell function which
name is stored in $1, if it is assigned.  If it is not, at least
bash will complain that it can't execute command "".

The proper way is to add backticks:

 +    args=`get_args "$1"`

In the second line:

 +    for field in "$args"; do

the double quotes ensure that all words in $args are
processed as single word, all at once.  So the for loop
will be executed exactly one time, no matter how many
arguments are given (even if there's none).

So the right solution is to drop double quotes.

JFYI.

/mjt
Andreas Färber - Sept. 13, 2010, 9:34 p.m.
Am 13.09.2010 um 10:49 schrieb Michael Tokarev:

> 13.09.2010 01:05, Blue Swirl wrote:
>> On Sun, Sep 12, 2010 at 5:58 PM, Andreas Färber <andreas.faerber@web.de 
>> > wrote:
>>> Am 12.09.2010 um 19:47 schrieb Blue Swirl:
>>>
>>>> diff --git a/tracetool b/tracetool
>>>> index 534cc70..c7582bf 100755
>>>> --- a/tracetool
>>>> +++ b/tracetool
>>>> @@ -48,7 +48,8 @@ get_argnames()
>>>> {
>>>>   local nfields field name
>>>>   nfields=0
>>>> -    for field in $(get_args "$1"); do
>>>> +    args=get_args "$1"
>>>> +    for field in "$args"; do
>>>
>>> This part yes. (I took the liberty of adding args to the local  
>>> line above)
>
> Um.  Are you sure it works as expected?

No, I'm not sure. It's Dark Magic to me and happened to unbreak  
configure; I'll try out your suggestions the next days.

Thanks,
Andreas

>  I'm not at all shure.
> There are 2 errors in the above patch:
>
> +    args=get_args "$1"
>
> After this line, variable $args will contain one word: "get_args".
> Shell will try to execute a command or call a shell function which
> name is stored in $1, if it is assigned.  If it is not, at least
> bash will complain that it can't execute command "".
>
> The proper way is to add backticks:
>
> +    args=`get_args "$1"`
>
> In the second line:
>
> +    for field in "$args"; do
>
> the double quotes ensure that all words in $args are
> processed as single word, all at once.  So the for loop
> will be executed exactly one time, no matter how many
> arguments are given (even if there's none).
>
> So the right solution is to drop double quotes.
>
> JFYI.
>
> /mjt
>
blueswirl - Sept. 14, 2010, 4:34 p.m.
On Sun, Sep 12, 2010 at 10:02 PM, Andreas Färber <andreas.faerber@web.de> wrote:
> Am 12.09.2010 um 23:05 schrieb Blue Swirl:
>
>> On Sun, Sep 12, 2010 at 5:58 PM, Andreas Färber <andreas.faerber@web.de>
>> wrote:
>>>
>>> Am 12.09.2010 um 19:47 schrieb Blue Swirl:
>>>>
>>>>       nfields=$((nfields + 1))
>>>
>>> ./tracetool: syntax error at line 53: `nfields=$' unexpected
>>
>> That looks like fully standards compliant, so Solaris' /bin/sh is not.
>> Can you try what happens with /usr/xpg4/bin/sh?
>
> Works fine! Must've done something wrong when testing that earlier today.
>
> configure, create_config and tracetool with your fix are silent when
> /usr/xpg4/bin is in the $PATH.
> If you commit it, we can close this ticket. Thanks for your help, Blue.

Does /usr/xpg4/bin/sh work without the patch?
Blue Swirl - Sept. 14, 2010, 4:41 p.m.
On Mon, Sep 13, 2010 at 8:49 AM, Michael Tokarev <mjt@tls.msk.ru> wrote:
> 13.09.2010 01:05, Blue Swirl wrote:
>> On Sun, Sep 12, 2010 at 5:58 PM, Andreas Färber <andreas.faerber@web.de> wrote:
>>> Am 12.09.2010 um 19:47 schrieb Blue Swirl:
>>>
>>>> On Sun, Sep 12, 2010 at 5:35 PM, Andreas Färber <andreas.faerber@web.de>
>>>> wrote:
>>>>>
>>>>> Am 12.09.2010 um 19:22 schrieb Blue Swirl:
>>>>>
>>>>>> What is the output of "sh ./tracetool --nop --check-backend"?
>>>>>
>>>>> ./tracetool: syntax error at line 51: `$' unexpected
>>>>
>>>> Does this patch fix the problem?
>>>>
>>>> diff --git a/tracetool b/tracetool
>>>> index 534cc70..c7582bf 100755
>>>> --- a/tracetool
>>>> +++ b/tracetool
>>>> @@ -48,7 +48,8 @@ get_argnames()
>>>> {
>>>>    local nfields field name
>>>>    nfields=0
>>>> -    for field in $(get_args "$1"); do
>>>> +    args=get_args "$1"
>>>> +    for field in "$args"; do
>>>
>>> This part yes. (I took the liberty of adding args to the local line above)
>
> Um.  Are you sure it works as expected?  I'm not at all shure.
> There are 2 errors in the above patch:
>
>  +    args=get_args "$1"
>
> After this line, variable $args will contain one word: "get_args".
> Shell will try to execute a command or call a shell function which
> name is stored in $1, if it is assigned.  If it is not, at least
> bash will complain that it can't execute command "".
>
> The proper way is to add backticks:
>
>  +    args=`get_args "$1"`
>
> In the second line:
>
>  +    for field in "$args"; do
>
> the double quotes ensure that all words in $args are
> processed as single word, all at once.  So the for loop
> will be executed exactly one time, no matter how many
> arguments are given (even if there's none).
>
> So the right solution is to drop double quotes.

Do you see any bug with the original?

If the problem is in fact that Solaris' /bin/sh is not standards
compliant, we shouldn't fix the script but instead make sure that the
shell used to run tracetool is the compliant one.
Michael Tokarev - Sept. 14, 2010, 6:18 p.m.
14.09.2010 20:41, blueswirl wrote:
> On Mon, Sep 13, 2010 at 8:49 AM, Michael Tokarev <mjt@tls.msk.ru> wrote:
[]
>>>>> diff --git a/tracetool b/tracetool
>>>>> index 534cc70..c7582bf 100755
>>>>> --- a/tracetool
>>>>> +++ b/tracetool
>>>>> @@ -48,7 +48,8 @@ get_argnames()
>>>>> {
>>>>>    local nfields field name
>>>>>    nfields=0
>>>>> -    for field in $(get_args "$1"); do
>>>>> +    args=get_args "$1"
>>>>> +    for field in "$args"; do
>>>>
>>>> This part yes. (I took the liberty of adding args to the local line above)
>>
>> Um.  Are you sure it works as expected?  I'm not at all shure.
>> There are 2 errors in the above patch:
[]
> Do you see any bug with the original?

The thing is - I've no idea what's the talk about.
There's no mentions of tracetool in 0.13.0-rc1 tarball.

> If the problem is in fact that Solaris' /bin/sh is not standards
> compliant, we shouldn't fix the script but instead make sure that the
> shell used to run tracetool is the compliant one.

Solaris's /bin/sh isn't exactly non-standard-compliant,
it's very limited.  In particular, $() construct weren't
in the standard for quite some time, and I'm not sure
it is now.

The usual idiom is to use backticks `` for this.  Only
in case of nested subshells or using case..esac (with
closing brases for individual cases) standard backticks
does not work.

/mjt
Andreas Färber - Sept. 14, 2010, 8:37 p.m.
Am 14.09.2010 um 18:34 schrieb Blue Swirl:

> On Sun, Sep 12, 2010 at 10:02 PM, Andreas Färber <andreas.faerber@web.de 
> > wrote:
>> Am 12.09.2010 um 23:05 schrieb Blue Swirl:
>>
>>> On Sun, Sep 12, 2010 at 5:58 PM, Andreas Färber <andreas.faerber@web.de 
>>> >
>>> wrote:
>>>>
>>>> Am 12.09.2010 um 19:47 schrieb Blue Swirl:
>>>>>
>>>>>       nfields=$((nfields + 1))
>>>>
>>>> ./tracetool: syntax error at line 53: `nfields=$' unexpected
>>>
>>> That looks like fully standards compliant, so Solaris' /bin/sh is  
>>> not.
>>> Can you try what happens with /usr/xpg4/bin/sh?
>>
>> Works fine! Must've done something wrong when testing that earlier  
>> today.
>>
>> configure, create_config and tracetool with your fix are silent when
>> /usr/xpg4/bin is in the $PATH.
>> If you commit it, we can close this ticket. Thanks for your help,  
>> Blue.
>
> Does /usr/xpg4/bin/sh work without the patch?

No.
Blue Swirl - Sept. 14, 2010, 8:53 p.m.
On Tue, Sep 14, 2010 at 8:37 PM, Andreas Färber <andreas.faerber@web.de> wrote:
> Am 14.09.2010 um 18:34 schrieb Blue Swirl:
>
>> On Sun, Sep 12, 2010 at 10:02 PM, Andreas Färber <andreas.faerber@web.de>
>> wrote:
>>>
>>> Am 12.09.2010 um 23:05 schrieb Blue Swirl:
>>>
>>>> On Sun, Sep 12, 2010 at 5:58 PM, Andreas Färber <andreas.faerber@web.de>
>>>> wrote:
>>>>>
>>>>> Am 12.09.2010 um 19:47 schrieb Blue Swirl:
>>>>>>
>>>>>>      nfields=$((nfields + 1))
>>>>>
>>>>> ./tracetool: syntax error at line 53: `nfields=$' unexpected
>>>>
>>>> That looks like fully standards compliant, so Solaris' /bin/sh is not.
>>>> Can you try what happens with /usr/xpg4/bin/sh?
>>>
>>> Works fine! Must've done something wrong when testing that earlier today.
>>>
>>> configure, create_config and tracetool with your fix are silent when
>>> /usr/xpg4/bin is in the $PATH.
>>> If you commit it, we can close this ticket. Thanks for your help, Blue.
>>
>> Does /usr/xpg4/bin/sh work without the patch?
>
> No.

How about with the attached patch? If yes, does it work even with /bin/sh?
Andreas Färber - Sept. 17, 2010, 9:14 p.m.
Am 14.09.2010 um 22:53 schrieb Blue Swirl:

> How about with the attached patch? If yes, does it work even with / 
> bin/sh?

LC_ALL=C /usr/xpg4/bin/sh ./tracetool --nop --check-backend

works fine,

LC_ALL=C sh ./tracetool --nop --check-backend
./tracetool: bad substitution

Patch

diff --git a/tracetool b/tracetool
index 534cc70..c7582bf 100755
--- a/tracetool
+++ b/tracetool
@@ -48,7 +48,8 @@  get_argnames()
 {
     local nfields field name
     nfields=0
-    for field in $(get_args "$1"); do
+    args=get_args "$1"
+    for field in "$args"; do
         nfields=$((nfields + 1))

         # Drop pointer star