[icedtea-web] RFC: replace binary launchers with shell scripts

[Omair Majid]

On 03/01/2011 08:38 PM, Dr Andrew John Hughes wrote:
> On 13:38 Tue 01 Mar     , Omair Majid wrote:
>>
>> Yeah, that was the plan. Removing all that code in this patch would have
>> created a large (and hard-to-understand) diff. I take it that you are
>> okay with the actual idea behind the patch (replacing launchers with
>> shell scripts)?
>>
>
> Yes, it will be good to get rid of that launcher code.
>

The updated attached patch should address all the issues you pointed out.

>>>>
>>>> +edit = sed \
>>>> +  -e 's|[@]bindir[@]|$(bindir)|g' \
>>>> +  -e 's|[@]libdir[@]|$(libdir)|g' \
>>>> +  -e 's|[@]prefix[@]|$(prefix)|g'
>>>> +
>>>> +edit_launcher_script = sed \
>>>> +  -e 's|[@]LAUNCHER_BOOTCLASSPATH[@]|$(LAUNCHER_BOOTCLASSPATH)|g' \
>>>> +  -e 's|[@]JAVAWS_BIN_LOCATION[@]|$(bindir)/javaws|g' \
>>>> +  -e 's|[@]ITWEB_SETTINGS_BIN_LOCATION[@]|$(bindir)/itweb-settings|g'
>>>> +
>>>> +
>>>
>>> Why are we doing this here when configure would do it?
>>>
>>
>> Configure will replace @bindir@ with ${prefix}/bin.
>> LAUNCHER_BOOTCLASSPATH also contains $(datadir) so that will also get
>> replaced with ${prefix}/share.
>>
>> The snippet of code is pretty much directly copied from the autoconf
>> manual [1].
>>
>
> Ok, same issue as before.  I'm surprised there is not a better solution.
>
>>>> diff -r 02ef9ba4d8a2 configure.ac
>>>> --- a/configure.ac	Fri Feb 25 18:16:48 2011 -0500
>>>> +++ b/configure.ac	Mon Feb 28 10:59:27 2011 -0500
>>>> @@ -79,4 +79,21 @@
>>>>    IT_CHECK_FOR_CLASS(SUN_APPLET_APPLETIMAGEREF, [sun.applet.AppletImageRef])
>>>>    IT_CHECK_FOR_APPLETVIEWERPANEL_HOLE
>>>>
>>>> +
>>>> +LAUNCHER_FLAGS=-Xms8m
>>>> +AC_SUBST([LAUNCHER_FLAGS])
>>>> +JAVAWS_MAIN_CLASS=net.sourceforge.jnlp.runtime.Boot
>>>> +AC_SUBST([JAVAWS_MAIN_CLASS])
>>>> +ITWEB_SETTINGS_MAIN_CLASS=net.sourceforge.jnlp.controlpanel.CommandLine
>>>> +AC_SUBST([ITWEB_SETTINGS_MAIN_CLASS])
>>>> +JAVAWS_PROGRAM_NAME=javaws
>>>> +AC_SUBST([JAVAWS_PROGRAM_NAME])
>>>> +ITWEB_SETTINGS_PROGRAM_NAME=itweb-settings
>>>> +AC_SUBST([ITWEB_SETTINGS_PROGRAM_NAME])
>>>> +
>>>> +AC_CONFIG_FILES([
>>>> +launcher/javaws.in
>>>> +launcher/itweb-settings.in
>>>> +])
>>>> +
>>>
>>> So you're doing it by autoconf also? What is going on here? Why are all these
>>> Makefile substitutions needed?
>>>
>>
>> There are really two class of variables that we want to substitute:
>> those that can be determined at configure-time (like version, main class
>> and so on) and those that can be determined at make-time (including
>> prefix). The idea that I am trying to implement is that autoconf handles
>> the first class of substitutions while the makefile handles the second
>> class of substitutions.
>>
>> Do you think it might be better if we just stick to one? That is, doing
>> all the substitutions at make time (or at configure time, which will
>> break make --prefix)?
>>
>
> I'd make them all make-time substitutions if configure is broken for some;
> it makes it clearer what's going on.
>
> We can't have --prefix broken.  That effectively breaks the whole install process.
>

It only breaks "make --prefix", not "configure --prefix", but yes, I am 
trying to avoid that.

>>>> diff -r 02ef9ba4d8a2 netx/net/sourceforge/jnlp/Launcher.java
>>>> --- a/netx/net/sourceforge/jnlp/Launcher.java	Fri Feb 25 18:16:48 2011 -0500
>>>> +++ b/netx/net/sourceforge/jnlp/Launcher.java	Mon Feb 28 10:59:27 2011 -0500
>>>> @@ -327,7 +327,7 @@
>>>>                List<String>   commands = new LinkedList<String>();
>>>>
>>>>                // this property is set by the javaws launcher to point to the javaws binary
>>>> -            String pathToWebstartBinary = System.getProperty("java.icedtea-web.bin");
>>>> +            String pathToWebstartBinary = System.getProperty("java.icedtea-web.bin.location");
>>>
>>> What's this for?
>>>
>>
>> Some parts of icedtea-web need to print out their own name, or need to
>> execute themselves. These properties
>> (java.icedtea-web.bin.{name,location}) allow icedtea-web to do that.
>>
>>> Should you be using java. as the prefix? I think it's reserved.
>>>
>
> Maybe just icedtea-web.{name,location}?  I don't see the need for java. anyway.
>

I have changed it to icedtea-web.bin.{name,location} now.

Okay to commit?

Thanks,
Omair
-------------- next part --------------
A non-text attachment was scrubbed...
Name: replace-launcher-with-shell-scripts-03.patch
Type: text/x-patch
Size: 9892 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20110309/6fd25770/replace-launcher-with-shell-scripts-03.patch