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

Omair Majid omajid at redhat.com
Tue Mar 1 10:38:33 PST 2011 

On 03/01/2011 08:36 AM, Dr Andrew John Hughes wrote:
> On 11:10 Mon 28 Feb     , Omair Majid wrote:
>> Hi,
>>
>> The attached patch replaces the binary launchers with simple shell scripts.
>>
>> It also (I hope) fixes some mistakes in $(DESTDIR) usage.
>>
>
> I'd rather that was a separate fix.
>

Sorry, I will do that.

>> Any thoughts or comments?
>>
>
> Inline below.  It's very confusing at present, and needs a ChangeLog and NEWS update.
>

Fixed.

> You do need to remove all the dead launcher code if this goes through, but please
> do that in a separate patch (no need to approve IMHO).
>

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)?

>> Cheers,
>> Omair
>
>> diff -r 02ef9ba4d8a2 Makefile.am
>> --- a/Makefile.am	Fri Feb 25 18:16:48 2011 -0500
>> +++ b/Makefile.am	Mon Feb 28 10:59:27 2011 -0500
>> @@ -18,8 +18,8 @@
>>   IT_JAVACFLAGS=$(IT_JAVAC_SETTINGS) -source $(IT_LANGUAGE_SOURCE_VERSION) -target $(IT_CLASS_TARGET_VERSION)
>>
>>   JRE='"$(SYSTEM_JDK_DIR)/jre"'
>> -LAUNCHER_BOOTCLASSPATH="-J-Xbootclasspath/a:$(DESTDIR)$(datadir)/$(PACKAGE_NAME)/netx.jar"
>> -PLUGIN_BOOTCLASSPATH='"-Xbootclasspath/a:$(DESTDIR)$(datadir)/$(PACKAGE_NAME)/netx.jar:$(DESTDIR)$(datadir)/$(PACKAGE_NAME)/plugin.jar"'
>> +LAUNCHER_BOOTCLASSPATH="-Xbootclasspath/a:$(datadir)/$(PACKAGE_NAME)/netx.jar"
>> +PLUGIN_BOOTCLASSPATH='"-Xbootclasspath/a:$(datadir)/$(PACKAGE_NAME)/netx.jar:$(datadir)/$(PACKAGE_NAME)/plugin.jar"'
>>
>
> Separate issue.
>

I have removed the DESTDIR changes from the patch; however we still need 
to remove the leading -J.

>>   # Fake update version to shut up the plugin detector hosted by Oracle.
>>   # If Oracle ever release a JDK update greater than 50, this needs to be increased.
>> @@ -65,28 +65,27 @@
>>   endif
>>   endif
>>
>> -# Launcher
>> -
>> -LAUNCHER_SRCDIR = $(abs_top_srcdir)/launcher
>> -LAUNCHER_OBJECTS = java.o java_md.o splashscreen_stubs.o jli_util.o parse_manifest.o version_comp.o wildcard.o
>> -NETX_LAUNCHER_OBJECTS = $(addprefix $(NETX_DIR)/launcher/,$(LAUNCHER_OBJECTS))
>> -CONTROLPANEL_LAUNCHER_OBJECTS = $(addprefix $(NETX_DIR)/launcher/controlpanel/,$(LAUNCHER_OBJECTS))
>> -LAUNCHER_FLAGS = -O2 -fno-strict-aliasing -fPIC -pthread -W -Wall -Wno-unused -Wno-parentheses -pipe -fno-omit-frame-pointer \
>> -	-g -D_LARGEFILE64_SOURCE -D_GNU_SOURCE -D_REENTRANT -DLAUNCHER_NAME='"java"' -I$(LAUNCHER_SRCDIR) \
>> -	-DJDK_MAJOR_VERSION='"1"' -DJDK_MINOR_VERSION='"6"' -DLIBARCHNAME='"$(JRE_ARCH_DIR)"' \
>> -	-DEXPAND_CLASSPATH_WILDCARDS
>> -LAUNCHER_LINK = -o $@ -pthread -Xlinker -O1 -Xlinker -z -Xlinker defs -L$(BOOT_DIR)/lib/$(INSTALL_ARCH_DIR) \
>> -	-Wl,-soname=lib.so -Wl,-z -Wl,origin -Wl,--allow-shlib-undefined $(X11_CFLAGS) $(X11_LIBS) -ldl -lz
>>   PLUGIN_VERSION = IcedTea-Web $(FULL_VERSION)
>>
>> -EXTRA_DIST = $(top_srcdir)/netx $(top_srcdir)/plugin javaws.png javaws.desktop.in extra launcher \
>> +EXTRA_DIST = $(top_srcdir)/netx $(top_srcdir)/plugin javaws.png javaws.desktop.in extra \
>>    itweb-settings.desktop.in
>>
>> +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].

>>   # Top-Level Targets
>>   # =================
>>
>> -all-local: stamps/netx-dist.stamp extra-lib/about.jar stamps/plugin.stamp $(NETX_DIR)/launcher/javaws \
>> - javaws.desktop stamps/docs.stamp $(NETX_DIR)/launcher/controlpanel/itweb-settings itweb-settings.desktop
>> +all-local: stamps/netx-dist.stamp extra-lib/about.jar stamps/plugin.stamp launcher/javaws \
>> + javaws.desktop stamps/docs.stamp launcher/itweb-settings itweb-settings.desktop
>>
>>   clean-local: clean-netx clean-plugin clean-liveconnect clean-extra clean-bootstrap-directory \
>>    clean-native-ecj clean-desktop-files clean-docs
>> @@ -104,9 +103,9 @@
>>   	${INSTALL_DATA} $(abs_top_builddir)/liveconnect/lib/classes.jar $(DESTDIR)$(datadir)/$(PACKAGE_NAME)/plugin.jar
>>   endif
>>   	${INSTALL_DATA} $(NETX_DIR)/lib/classes.jar $(DESTDIR)$(datadir)/$(PACKAGE_NAME)/netx.jar
>> -	${INSTALL_PROGRAM} $(NETX_DIR)/launcher/javaws $(DESTDIR)$(bindir)
>> +	${INSTALL_PROGRAM} launcher/javaws $(DESTDIR)$(bindir)
>>   	${INSTALL_DATA} extra-lib/about.jar $(DESTDIR)$(datadir)/$(PACKAGE_NAME)/about.jar
>> -	${INSTALL_PROGRAM} $(NETX_DIR)/launcher/controlpanel/itweb-settings $(DESTDIR)$(bindir)
>> +	${INSTALL_PROGRAM} launcher/itweb-settings $(DESTDIR)$(bindir)
>>
>>   install-data-local:
>>   	${mkinstalldirs} -d $(DESTDIR)$(mandir)/man1
>> @@ -306,23 +305,11 @@
>>   extra-lib/about.jar: stamps/extra-class-files.stamp
>>   	$(BOOT_DIR)/bin/jar cf $@ -C extra-lib net ;
>>
>> -$(NETX_DIR)/launcher/%.o: $(LAUNCHER_SRCDIR)/%.c
>> -	mkdir -p $(NETX_DIR)/launcher&&  \
>> -	$(CC) $(LAUNCHER_FLAGS) \
>> -	  -DJAVA_ARGS='{ $(LAUNCHER_BOOTCLASSPATH), "-J-ms8m", "-J-Djava.icedtea-web.bin=$(DESTDIR)$(bindir)/javaws", "net.sourceforge.jnlp.runtime.Boot",  }' \
>> -	  -DICEDTEA_WEB_JRE=$(JRE) -DPROGNAME='"javaws"' -c -o $@ $<
>> +launcher/javaws: launcher/javaws.in
>> +	$(edit_launcher_script)<  $<  >  $@
>>
>> -$(NETX_DIR)/launcher/controlpanel/%.o: $(LAUNCHER_SRCDIR)/%.c
>> -	mkdir -p $(NETX_DIR)/launcher/controlpanel&&  \
>> -	$(CC) $(LAUNCHER_FLAGS) \
>> -	-DJAVA_ARGS='{ $(LAUNCHER_BOOTCLASSPATH), "-J-ms8m", "-Dprogram.name=itweb-settings", "net.sourceforge.jnlp.controlpanel.CommandLine",  }' \
>> -	-DICEDTEA_WEB_JRE=$(JRE) -DPROGNAME='"itweb-settings"' -c -o $@ $<
>> -
>> -$(NETX_DIR)/launcher/javaws: $(NETX_LAUNCHER_OBJECTS)
>> -	$(CC) $(NETX_LAUNCHER_OBJECTS) $(LAUNCHER_LINK)
>> -
>> -$(NETX_DIR)/launcher/controlpanel/itweb-settings: $(CONTROLPANEL_LAUNCHER_OBJECTS)
>> -	$(CC) $(CONTROLPANEL_LAUNCHER_OBJECTS) $(LAUNCHER_LINK)
>> +launcher/itweb-settings: launcher/itweb-settings.in
>> +	$(edit_launcher_script)<  $<  >  $@
>>
>
> Again, I think this could be done by configure.
>

Actually, the parts that contain $(bindir) or $(datadir) wont be handled 
properly.

>>   javaws.desktop: javaws.desktop.in
>>   	sed "s#PATH_TO_JAVAWS#$(DESTDIR)$(bindir)/javaws#"<  $(srcdir)/javaws.desktop.in>  javaws.desktop
>> diff -r 02ef9ba4d8a2 acinclude.m4
>> --- a/acinclude.m4	Fri Feb 25 18:16:48 2011 -0500
>> +++ b/acinclude.m4	Mon Feb 28 10:59:27 2011 -0500
>> @@ -683,3 +683,5 @@
>>     AC_MSG_RESULT([${FULL_VERSION}])
>>     AC_SUBST([FULL_VERSION])
>>   ])
>> +
>> +
>
> Changes to this file need reverting.
>

Fixed.

>> 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)?

>>   AC_OUTPUT
>> diff -r 02ef9ba4d8a2 launcher/itweb-settings.in.in
>> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
>> +++ b/launcher/itweb-settings.in.in	Mon Feb 28 10:59:27 2011 -0500
>> @@ -0,0 +1,15 @@
>> +#!/bin/bash
>> +
>> +JAVA=@JAVA@
>> +LAUNCHER_BOOTCLASSPATH=@LAUNCHER_BOOTCLASSPATH@
>> +LAUNCHER_FLAGS=@LAUNCHER_FLAGS@
>> +CLASSNAME=@ITWEB_SETTINGS_MAIN_CLASS@
>> +BINARY_LOCATION=@ITWEB_SETTINGS_BIN_LOCATION@
>> +PROGRAM_NAME=@ITWEB_SETTINGS_PROGRAM_NAME@
>> +
>> +${JAVA} ${LAUNCHER_BOOTCLASSPATH} ${LAUNCHER_FLAGS} \
>> +  -Djava.icedtea-web.bin.name=${PROGRAM_NAME} \
>> +  -Djava.icedtea-web.bin.location=${BINARY_LOCATION} \
>> +  ${CLASSNAME} \
>> +  $@
>> +
>> diff -r 02ef9ba4d8a2 launcher/javaws.in.in
>> --- /dev/null	Thu Jan 01 00:00:00 1970 +0000
>> +++ b/launcher/javaws.in.in	Mon Feb 28 10:59:27 2011 -0500
>> @@ -0,0 +1,14 @@
>> +#!/bin/bash
>> +
>> +JAVA=@JAVA@
>> +LAUNCHER_BOOTCLASSPATH=@LAUNCHER_BOOTCLASSPATH@
>> +LAUNCHER_FLAGS=@LAUNCHER_FLAGS@
>> +CLASSNAME=@JAVAWS_MAIN_CLASS@
>> +BINARY_LOCATION=@JAVAWS_BIN_LOCATION@
>> +PROGRAM_NAME=@JAVAWS_PROGRAM_NAME@
>> +
>> +${JAVA} ${LAUNCHER_BOOTCLASSPATH} ${LAUNCHER_FLAGS} \
>> +  -Djava.icedtea-web.bin.name=${PROGRAM_NAME} \
>> +  -Djava.icedtea-web.bin.location=${BINARY_LOCATION} \
>> +  ${CLASSNAME} \
>> +  $@
>> 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.
>

I am not sure. I cant find references to reserved properties. On the 
other hand, I can certainly change it.

Cheers,
Omair

[1] 
http://www.gnu.org/software/hello/manual/autoconf/Installation-Directory-Variables.html#Installation-Directory-Variables
-------------- next part --------------
A non-text attachment was scrubbed...
Name: replace-launcher-with-shell-scripts-02.patch
Type: text/x-patch
Size: 10911 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20110301/c265155f/replace-launcher-with-shell-scripts-02.patch

More information about the distro-pkg-dev mailing list