Request for review (S): 7099454 /bin/sh does not support syntax used in the src/os/posix/launcher/launcher.script shell script

Dmitry Samersoff Dmitry.Samersoff at oracle.com
Tue Oct 11 04:19:58 PDT 2011


Bengt,

I'm against of committing broken fix especially if it's two lines 
changes. So it's better to add quotes.


I'm not sure it's in the scope of your fix, but :

I guess adding "" everywhere around ${MYDIR} is enough to make it 
working correctly with spaces in pathnames.

Also

102     JDK=${ALT_JAVA_HOME%%/jre};

doesn't work with plain solaris.sh
it have to be

JDK=`echo ${ALT_JAVA_HOME} | sed -e "s:/jre::g"`


-Dmitry



On 2011-10-11 14:59, Bengt Rutisson wrote:
>
> Dmitry,
>
> Thanks for your comments.
>
> On 2011-10-11 11:45, Dmitry Samersoff wrote:
>> Bent,
>>
>> ll 72-73 should be:
>>
>> REL_MYDIR=`dirname "$0"`
>> MYDIR=`cd "${REL_MYDIR}" && pwd`
>>
>> Otherwise it will not work if a name of the current directory contains
>> spaces.


>
> Unfortunately this is not enough to make it work with space in path
> names. I ran into more issues when I tried it out. On the other hand the
> script before my changes also does not work with spaces in path names.
>
> I suggest that I leave my change as it is and if you think it is
> necessary we can file a separate CR to handle the space in path issue.
> Does that sound ok?
>
> Thanks,
> Bengt
>
>>
>>
>> -Dmitry
>>
>>
>> On 2011-10-11 13:06, Bengt Rutisson wrote:
>>>
>>> Hi all,
>>>
>>> Sending this review request to both Runtime and GC. I think the change
>>> is Runtime responsibility, but I would like to submit it to hotspot-gc
>>> since it is getting in the way of testing that I am doing at the moment.
>>>
>>> The problem is that the hotspot script that the build process creates
>>> has been updated to use /bin/sh instead of /bin/bash. This is more
>>> correct since bash is optional. But the /bin/sh shell on Solaris does
>>> not accept some of the syntax in the script.
>>>
>>> In particular the $() notation is not supported by Solaris /bin/sh. The
>>> script also contains the bash specific "source" command.
>>>
>>> The fix is to change this to use `` and . respectively.
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~brutisso/7099454/webrev.01/
>>>
>>> CR:
>>> 7099454 /bin/sh does not support syntax used in the
>>> src/os/posix/launcher/launcher.script shell script
>>> http://monaco.us.oracle.com/detail.jsf?cr=7099454
>>>
>>> Testing
>>> Tested the new script on Linux (Ubuntu) and Solaris. Ran JPRT.
>>
>>
>


-- 
Dmitry Samersoff
Java Hotspot development team, SPB04
* There will come soft rains ...


More information about the hotspot-gc-dev mailing list