[rfc][icedtea-web] get the custom jre in shell-launchers too

[Adam Domurad]

On 03/20/2013 06:54 AM, Jiri Vanek wrote:
> On 02/22/2013 05:21 PM, Adam Domurad wrote:
>> On 02/22/2013 09:43 AM, Jiri Vanek wrote:
>>> And finally bash launchers can check for the custom jre value too....
>>>
>>>
>>> J.
>>>
>>> ps: part of.. you know...
>>> pps: does this really replace all the c code!??!?! /me started to 
>>> like bash
>>
>> It would be nice if C++ had a regex library. (added in C++11, but I 
>> doubt we'll see icedtea-web move
>> to C++11.)
>>
>> Actually during my internship I started to quite like hacking stuff 
>> with bash. Great & modular
>> 'standard library' :-)
>>
>>> diff -r ad2e15f15a75 launcher/itweb-settings.in
>>> --- a/launcher/itweb-settings.in    Thu Feb 21 15:26:38 2013 -0500
>>> +++ b/launcher/itweb-settings.in    Fri Feb 22 15:41:10 2013 +0100
>>> @@ -7,6 +7,16 @@
>>>  BINARY_LOCATION=@ITWEB_SETTINGS_BIN_LOCATION@
>>>  PROGRAM_NAME=itweb-settings
>>>
>>> +a="^deployment.jre.dir *= *"
>>
>> 'a' could probably get a better name here, ie 'JRE_PROPERTY' or 
>> some-such.
>>
>>> +CUSTOM_JRE=`grep "$a" ~/.icedtea/deployment.properties 2>/dev/null 
>>> | sed "s/$a//g"`
>>> +if [ "x$CUSTOM_JRE" = "x" ] ; then
>>> +  CUSTOM_JRE=`grep "$a" /etc/.java/.deploy/deployment.properties 
>>> 2>/dev/null |  sed "s/$a//g"`
>>> +fi;
>>> +if [ "x$CUSTOM_JRE" != "x" ] ; then
>>> +  JAVA=$CUSTOM_JRE/bin/java
>>> +  CP=$CUSTOM_JRE/lib/rt.jar
>>
>> AFAIK this CP= is not needed in itweb-settings.in.
>>
>>> +fi;
>>> +
>>>  ${JAVA} ${LAUNCHER_BOOTCLASSPATH} ${LAUNCHER_FLAGS} \
>>>    -Dicedtea-web.bin.name=${PROGRAM_NAME} \
>>>    -Dicedtea-web.bin.location=${BINARY_LOCATION} \
>>> diff -r ad2e15f15a75 launcher/javaws.in
>>> --- a/launcher/javaws.in    Thu Feb 21 15:26:38 2013 -0500
>>> +++ b/launcher/javaws.in    Fri Feb 22 15:41:10 2013 +0100
>>> @@ -9,6 +9,16 @@
>>>  PROGRAM_NAME=javaws
>>>  CP=@JRE@/lib/rt.jar
>>>
>>> +a="^deployment.jre.dir *= *"
>>> +CUSTOM_JRE=`grep "$a" ~/.icedtea/deployment.properties 2>/dev/null 
>>> | sed "s/$a//g"`
>>> +if [ "x$CUSTOM_JRE" = "x" ] ; then
>>> +  CUSTOM_JRE=`grep "$a" /etc/.java/.deploy/deployment.properties 
>>> 2>/dev/null |  sed "s/$a//g"`
>>> +fi;
>>> +if [ "x$CUSTOM_JRE" != "x" ] ; then
>>
>> This is funny. Are you trying to test for a non-empty string ?
>> [ "$CUSTOM_JRE" != "" ] works fine. ( [ $CUSTOM_JRE != "x" ] is the 
>> one that does not )
>> Since we have a default, it may also be worth checking [ -e 
>> "$CUSTOM_JRE" ] (file exists) here (and
>> above).
>>
>>> +  JAVA=$CUSTOM_JRE/bin/java
>>> +  CP=$CUSTOM_JRE/lib/rt.jar
>>> +fi;
>>> +
>>>  JAVA_ARGS=( )
>>>  ARGS=( )
>>>  COMMAND=()
>>
>> Overall this is quite OK, if we do end up including the JRE path in 
>> the deployment properties file.
>> Maybe though it will make life simpler if it's in its own file, eg 
>> '.icedtea/jre-path'?
>>
>> You have a very valid point about the fact that we should avoid 
>> spawning a Java instance just to get
>> the JVM arguments, however.
>> We might still want to parse the properties file for this, or we 
>> could place it in the file with the
>> JRE path.
>> Considering that we need to extract this property from bash, C & 
>> Java, it might make sense to use
>> the simplest format possible though. It would be very bad if a small 
>> mistake in how the property
>> file is treated in Bash or C causes ITW not to be able to run.
>>
>> Anyway properties file is not an overly scary format, but still worth 
>> thinking about.
>>
>> Happy hacking,
>> -Adam
>
>
> So I think this can go inside. Oook?
>
> J.

Overall it looks fine, but...

> diff -r e6c51c4b4d20 -r a9920eea7c17 launcher/itweb-settings.in
> --- a/launcher/itweb-settings.in    Wed Mar 13 14:51:20 2013 +0100
> +++ b/launcher/itweb-settings.in    Mon Mar 18 11:30:39 2013 +0100
> @@ -7,6 +7,15 @@
>  BINARY_LOCATION=@ITWEB_SETTINGS_BIN_LOCATION@
>  PROGRAM_NAME=itweb-settings
>
> +CUSTOM_JRE_REGEX="^deployment.jre.dir *= *"
> +CUSTOM_JRE=`grep "$CUSTOM_JRE_REGEX" ~/.icedtea/deployment.properties 
> 2>/dev/null |  sed "s/$CUSTOM_JRE_REGEX//g"`
> +if [ "x$CUSTOM_JRE" = "x" ] ; then
> +  CUSTOM_JRE=`grep "$CUSTOM_JRE_REGEX" 
> /etc/.java/.deploy/deployment.properties 2>/dev/null |  sed 
> "s/$CUSTOM_JRE_REGEX//g"`
> +fi;
> +if [ "x$CUSTOM_JRE" != "x" ] ; then
> +  JAVA=$CUSTOM_JRE/bin/java
> +fi;
> +
>  ${JAVA} ${LAUNCHER_BOOTCLASSPATH} ${LAUNCHER_FLAGS} \
>    -Dicedtea-web.bin.name=${PROGRAM_NAME} \
>    -Dicedtea-web.bin.location=${BINARY_LOCATION} \
> diff -r e6c51c4b4d20 -r a9920eea7c17 launcher/javaws.in
> --- a/launcher/javaws.in    Wed Mar 13 14:51:20 2013 +0100
> +++ b/launcher/javaws.in    Mon Mar 18 11:30:39 2013 +0100
> @@ -9,6 +9,16 @@
>  PROGRAM_NAME=javaws
>  CP=@JRE@/lib/rt.jar
>
> +CUSTOM_JRE_REGEX="^deployment.jre.dir *= *"
> +CUSTOM_JRE=`grep "$CUSTOM_JRE_REGEX" ~/.icedtea/deployment.properties 
> 2>/dev/null |  sed "s/$CUSTOM_JRE_REGEX//g"`
> +if [ "x$CUSTOM_JRE" = "x" ] ; then

Please just use [ "$CUSTOM_JRE" = "" ] instead of this mysterious thing.

> +  CUSTOM_JRE=`grep "$CUSTOM_JRE_REGEX" 
> /etc/.java/.deploy/deployment.properties 2>/dev/null |  sed 
> "s/$CUSTOM_JRE_REGEX//g"`
> +fi;
> +if [ "x$CUSTOM_JRE" != "x" ] ; then


Please just use [ "$CUSTOM_JRE" != "" ] instead of this mysterious thing.

> +  JAVA=$CUSTOM_JRE/bin/java
> +  CP=$CUSTOM_JRE/lib/rt.jar
> +fi;
> +
>  JAVA_ARGS=( )
>  ARGS=( )
>  COMMAND=()

-Adam