[icedtea-web] RFC: do not install netx.jar and plugin.jar into jdk dir

Omair Majid omajid at redhat.com
Thu Feb 17 15:03:05 PST 2011


On 02/17/2011 05:00 PM, Dr Andrew John Hughes wrote:
> On 16:39 Thu 17 Feb     , Omair Majid wrote:
>> Hi,
>>
>> The attached patch isolates icedtea-web into its own directories. With
>> this patch applied, icedtea-web does not need to be installed into a
>> jdk-style dir.
>>
>
> Finally!
>

:)

>> I have removed the pluginappletviewer binary. I had originally posted it
>> a separate patch, but since using pluginappletviewer would make this
>> patch more complex, I have removed it here.
>>
>
> Please do this in a separate patch prior to this.  The two issues shouldn't
> be confused.
>

I posted the patch already: 
http://mail.openjdk.java.net/pipermail/distro-pkg-dev/2011-February/012010.html. 
If you can approve it, I will remove pluginappletviewer before applying 
this patch.

>> Currently, the jdk it uses to run is determined at build-time. In the
>> future, this will probably be changed so that icedtea-web will be able
>> to use any installed jdk.
>>
>
> Yeah I pondered this when reading the patch.  What is wrong with just calling
> 'java'?
>

Well, we do rely on certain API's that may not be present on all JDKs. 
The configure checks are build time; so for now, the JRE to use is also 
determined at build time.

It's also in the interest of minimizing changes in this one patch. I am 
quite sure most of the launcher code can be replaced by simple shell 
scripts (or a simple call to java even). But that's for another patch.

> I seem to remember there were some issues with libjava not being found if 1.0
> was installed in the wrong location and this was due to a relative LD_LIBRARY_PATH
> extension.  I don't see this fixed.  Do the new binaries work with just this
> patch?
>

Hm.. no clue about the LD_LIBRARY_PATH issues. The binaries/plugin 
produced after building icedtea-web with this patch applied (I used 
./configure --prefix=/home/omajid/code/icedtea-web-build/ 
--with-jdk-home=/home/omajid/code/icedtea6/openjdk.build/j2sdk-image/ 
--disable-docs) work perfectly fine for me.

I did encounter an issue with libjava.so not being found. The method 
GetIcedTeaWebJREPath fixes that. It uses the predefined path at build 
time to locate the JRE rather than weird things like looking in the 
directory above.

>> Any thoughts or comments on the patch?
>>
>
> More inline.
>
>
>> diff -r 2289777074ae Makefile.am
>> --- a/Makefile.am	Tue Feb 15 17:03:09 2011 -0500
>> +++ b/Makefile.am	Thu Feb 17 16:33:53 2011 -0500
>> @@ -17,6 +17,10 @@
>>   IT_CLASS_TARGET_VERSION=6
>>   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"'
>> +
>>   # 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.
>>   JDK_UPDATE_VERSION=50
>> @@ -39,8 +43,7 @@
>>   PLUGIN_DIR=$(abs_top_builddir)/plugin/icedteanp
>>   PLUGIN_SRCDIR=$(abs_top_srcdir)/plugin/icedteanp
>>   LIVECONNECT_SRCS = $(PLUGIN_SRCDIR)/java
>> -ICEDTEAPLUGIN_TARGET = $(PLUGIN_DIR)/IcedTeaPlugin.so stamps/liveconnect-dist.stamp \
>> - $(PLUGIN_DIR)/launcher/pluginappletviewer
>> +ICEDTEAPLUGIN_TARGET = $(PLUGIN_DIR)/IcedTeaPlugin.so stamps/liveconnect-dist.stamp
>>   PLUGIN_PKGS = sun.applet netscape.security netscape.javascript
>>   endif
>>
>> @@ -66,7 +69,6 @@
>>
>>   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
>> -PLUGIN_LAUNCHER_OBJECTS = $(addprefix $(PLUGIN_DIR)/launcher/,$(LAUNCHER_OBJECTS))
>>   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 \
>> @@ -96,37 +98,20 @@
>>    clean-bootstrap-directory clean-native-ecj clean-desktop-files clean-netx-docs clean-docs clean-plugin-docs
>>
>>   install-exec-local:
>> -	${mkinstalldirs} $(DESTDIR)$(bindir) $(DESTDIR)$(prefix)/jre/lib/$(INSTALL_ARCH_DIR)
>> +	${mkinstalldirs} $(DESTDIR)$(bindir) $(DESTDIR)$(datadir)/$(PACKAGE_NAME)/ $(DESTDIR)$(libdir)
>>   if ENABLE_PLUGIN
>> -	${INSTALL_PROGRAM} $(PLUGIN_DIR)/IcedTeaPlugin.so $(DESTDIR)$(prefix)/jre/lib/$(INSTALL_ARCH_DIR)/
>> -	${INSTALL_PROGRAM} $(PLUGIN_DIR)/launcher/pluginappletviewer $(DESTDIR)$(bindir)
>> -	${INSTALL_DATA} $(abs_top_builddir)/liveconnect/lib/classes.jar $(DESTDIR)$(prefix)/jre/lib/plugin.jar
>> +	${INSTALL_PROGRAM} $(PLUGIN_DIR)/IcedTeaPlugin.so $(DESTDIR)$(libdir)
>> +	${INSTALL_DATA} $(abs_top_builddir)/liveconnect/lib/classes.jar $(DESTDIR)$(datadir)/$(PACKAGE_NAME)/plugin.jar
>>   endif
>> -	${INSTALL_DATA} $(NETX_DIR)/lib/classes.jar $(DESTDIR)$(prefix)/jre/lib/netx.jar
>> +	${INSTALL_DATA} $(NETX_DIR)/lib/classes.jar $(DESTDIR)$(datadir)/$(PACKAGE_NAME)/netx.jar
>>   	${INSTALL_PROGRAM} $(NETX_DIR)/launcher/javaws $(DESTDIR)$(bindir)
>> -	if [ -d $(DESTDIR)$(prefix)/jre/bin ] ; then \
>> -	  if [ -L $(DESTDIR)$(prefix)/jre/bin/javaws ] ; then \
>> -	    rm -f $(DESTDIR)$(prefix)/jre/bin/javaws ; \
>> -	  fi ; \
>> -	  if [ ! -e $(prefix)/jre/bin/javaws ] ; then \
>> -	    ln -s $(DESTDIR)$(bindir)/javaws $(DESTDIR)$(prefix)/jre/bin ; \
>> -	  fi ; \
>> -	fi
>> -	${INSTALL_DATA} extra-lib/about.jar $(DESTDIR)$(prefix)/jre/lib
>> +	${INSTALL_DATA} extra-lib/about.jar $(DESTDIR)$(datadir)/$(PACKAGE_NAME)/about.jar
>>   	${INSTALL_PROGRAM} $(NETX_DIR)/launcher/controlpanel/itweb-settings $(DESTDIR)$(bindir)
>> -	if [ -d $(DESTDIR)$(prefix)/jre/bin ] ; then \
>> -	  if [ -L $(DESTDIR)$(prefix)/jre/bin/itweb-settings ] ; then \
>> -	    rm -f $(DESTDIR)$(prefix)/jre/bin/itweb-settings ; \
>> -	  fi ; \
>> -	  if [ ! -e $(prefix)/jre/bin/itweb-settings ] ; then \
>> -	    ln -s $(DESTDIR)$(bindir)/itweb-settings $(DESTDIR)$(prefix)/jre/bin ; \
>> -	  fi ; \
>> -	fi
>>
>>   install-data-local:
>>   	${mkinstalldirs} -d $(DESTDIR)$(prefix)/man/man1
>>   	${INSTALL_DATA} $(NETX_SRCDIR)/javaws.1 $(DESTDIR)$(prefix)/man/man1
>> -	${INSTALL_DATA} $(NETX_RESOURCE_DIR)/about.jnlp $(DESTDIR)$(prefix)/jre/lib
>> +	${INSTALL_DATA} $(NETX_RESOURCE_DIR)/about.jnlp $(DESTDIR)$(datarootdir)/$(PACKAGE_NAME)
>>   if ENABLE_DOCS
>>   	${mkinstalldirs} $(DESTDIR)$(htmldir)
>>   	(cd ${abs_top_builddir}/docs/netx; \
>> @@ -144,22 +129,14 @@
>>   endif
>>
>>   uninstall-local:
>> -	rm -f $(DESTDIR)$(prefix)/jre/lib/$(INSTALL_ARCH_DIR)/IcedTeaPlugin.so
>> -	rm -f $(DESTDIR)$(prefix)/jre/lib/plugin.jar
>> -	rm -f $(DESTDIR)$(prefix)/jre/lib/netx.jar
>> -	rm -f $(DESTDIR)$(prefix)/jre/lib/about.jnlp
>> -	rm -f $(DESTDIR)$(prefix)/jre/lib/about.jar
>> +	rm -f $(DESTDIR)$(libdir)/IcedTeaPlugin.so
>> +	rm -f $(DESTDIR)$(datadir)/$(PACKAGE_NAME)/plugin.jar
>> +	rm -f $(DESTDIR)$(datadir)/$(PAKCAGE_NAME)/netx.jar
>> +	rm -f $(DESTDIR)$(datadir)/$(PACKAGE_NAME)/about.jnlp
>> +	rm -f $(DESTDIR)$(datadir)/$(PACKAGE_NAME)/about.jar
>>   	rm -f $(DESTDIR)$(prefix)/man/man1/javaws.1
>> -	rm -f $(DESTDIR)$(bindir)/pluginappletviewer
>>   	rm -f $(DESTDIR)$(bindir)/javaws
>> -	if [ -L $(DESTDIR)$(prefix)/jre/bin/javaws ] ; then \
>> -	  rm -f $(DESTDIR)$(prefix)/jre/bin/javaws ; \
>> -	fi
>> -	rm -f $(DESTDIR)$(prefix)/jre/bin/javaws
>>   	rm -f $(DESTDIR)$(bindir)/itweb-settings
>> -	if [ -L $(DESTDIR)$(prefix)/jre/bin/itweb-settings ] ; then \
>> -	  rm -f $(DESTDIR)$(prefix)/jre/bin/itweb-settings ; \
>> -	fi
>>   	rm -rf $(DESTDIR)$(htmldir)
>>
>>   # Plugin
>> @@ -187,6 +164,8 @@
>>   	  -DPLUGIN_VERSION="\"$(PLUGIN_VERSION)\"" \
>>   	  -DPACKAGE_URL="\"$(PACKAGE_URL)\"" \
>>   	  -DMOZILLA_VERSION_COLLAPSED="$(MOZILLA_VERSION_COLLAPSED)" \
>> +	  -DICEDTEA_WEB_JRE=$(JRE) \
>> +	  -DPLUGIN_BOOTCLASSPATH=$(PLUGIN_BOOTCLASSPATH) \
>>   	  $(GLIB_CFLAGS) \
>>   	  $(GTK_CFLAGS) \
>>   	  $(MOZILLA_CFLAGS) \
>> @@ -201,16 +180,8 @@
>>   	  $(MOZILLA_LIBS)\
>>   	  -shared -o $@
>>
>> -$(PLUGIN_DIR)/launcher/%.o: $(LAUNCHER_SRCDIR)/%.c
>> -	mkdir -p $(PLUGIN_DIR)/launcher&&  \
>> -	$(CC) $(LAUNCHER_FLAGS) -DJAVA_ARGS='{ "sun.applet.PluginMain" }' -DPROGNAME='"pluginappletviewer"'  -c -o $@ $<
>> -
>> -$(PLUGIN_DIR)/launcher/pluginappletviewer: $(PLUGIN_LAUNCHER_OBJECTS)
>> -	$(CC) $(PLUGIN_LAUNCHER_OBJECTS) $(LAUNCHER_LINK)
>> -
>>   clean-IcedTeaPlugin:
>>   	rm -f $(PLUGIN_DIR)/launcher/*.o
>> -	rm -f $(PLUGIN_DIR)/launcher/pluginappletviewer
>>   	if [ -e $(PLUGIN_DIR)/launcher ]; then \
>>   		rmdir $(PLUGIN_DIR)/launcher ; \
>>   	fi
>> @@ -344,14 +315,14 @@
>>   $(NETX_DIR)/launcher/%.o: $(LAUNCHER_SRCDIR)/%.c
>>   	mkdir -p $(NETX_DIR)/launcher&&  \
>>   	$(CC) $(LAUNCHER_FLAGS) \
>> -	  -DJAVA_ARGS='{ "-J-ms8m", "-J-Djava.icedtea-web.bin=$(DESTDIR)$(bindir)/javaws", "net.sourceforge.jnlp.runtime.Boot",  }' \
>> -	  -DPROGNAME='"javaws"' -c -o $@ $<
>> +	  -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 $@ $<
>>
>>   $(NETX_DIR)/launcher/controlpanel/%.o: $(LAUNCHER_SRCDIR)/%.c
>>   	mkdir -p $(NETX_DIR)/launcher/controlpanel&&  \
>>   	$(CC) $(LAUNCHER_FLAGS) \
>> -	-DJAVA_ARGS='{ "-J-ms8m", "-Dprogram.name=itweb-settings", "net.sourceforge.jnlp.controlpanel.CommandLine",  }' \
>> -	-DPROGNAME='"itweb-settings"' -c -o $@ $<
>> +	-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)
>> diff -r 2289777074ae launcher/java_md.c
>> --- a/launcher/java_md.c	Tue Feb 15 17:03:09 2011 -0500
>> +++ b/launcher/java_md.c	Thu Feb 17 16:33:53 2011 -0500
>> @@ -158,6 +158,7 @@
>>   static jboolean GetJVMPath(const char *jrepath, const char *jvmtype,
>>                              char *jvmpath, jint jvmpathsize, char * arch);
>>   static jboolean GetJREPath(char *path, jint pathsize, char * arch, jboolean speculative);
>> +static jboolean GetIcedTeaWebJREPath(char *path, jint pathsize, char * arch, jboolean speculative);
>>
>>   const char *
>>   GetArch()
>> @@ -280,7 +281,9 @@
>>            jvmpath does not exist */
>>         if (wanted == running) {
>>           /* Find out where the JRE is that we will be using. */
>> -        if (!GetJREPath(jrepath, so_jrepath, arch, JNI_FALSE) ) {
>> +
>> +        //if (!GetJREPath(jrepath, so_jrepath, arch, JNI_FALSE) ) {
>> +        if (!GetIcedTeaWebJREPath(jrepath, so_jrepath, arch, JNI_FALSE) ) {
>
> Please just remove the line.
>

Will do.

>>             fprintf(stderr, "Error: could not find Java 2 Runtime Environment.\n");
>>             exit(2);
>>           }
>> @@ -606,6 +609,33 @@
>>   }
>>
>>   /*
>> + * Find path to the JRE based on the the compile flag ICEDTEA_WEB_JRE
>> + */
>> +static jboolean
>> +GetIcedTeaWebJREPath(char* path, jint pathsize, char* arch, jboolean speculative)
>> +{
>> +    char libjava[MAXPATHLEN];
>> +    snprintf(libjava, MAXPATHLEN, ICEDTEA_WEB_JRE "/lib/%s/" JAVA_DLL, arch);
>> +
>> +    if (_launcher_debug) {
>> +      printf(ICEDTEA_WEB_JRE "/lib/%s/" JAVA_DLL "\n", arch);
>> +      printf("libjava is %s\n", libjava);
>> +    }
>> +
>
> Why is libjava being checked?  Is this needed? I'm not sure it's standard and may
> be specified to Oracle JDKs.
>

I wouldnt be surprised. The launcher code is fairly specific to OpenJDK. 
Most of the code in this method is based on GetJREPath (which is the 
method just below this one). The only significant change is where to 
look for libjava - this method looks for it based on a compile time 
constant; GetJREPath looks for it in the same directory as the binary.

>> +    if (access(libjava, F_OK) == 0) {
>> +        strncpy(path, ICEDTEA_WEB_JRE, pathsize);
>> +        goto found;
>> +    }
>> +
>> +    return JNI_FALSE;
>> +
>> + found:
>> +    if (_launcher_debug)
>> +      printf("JRE path is %s\n", path);
>> +    return JNI_TRUE;
>> +}
>> +
>> +/*
>>    * Find path to JRE based on .exe's location or registry settings.
>>    */
>>   static jboolean
>> diff -r 2289777074ae plugin/icedteanp/IcedTeaNPPlugin.cc
>> --- a/plugin/icedteanp/IcedTeaNPPlugin.cc	Tue Feb 15 17:03:09 2011 -0500
>> +++ b/plugin/icedteanp/IcedTeaNPPlugin.cc	Thu Feb 17 16:33:53 2011 -0500
>> @@ -1547,23 +1547,25 @@
>>
>>     if (plugin_debug)
>>     {
>> -      command_line = (gchar**) malloc(sizeof(gchar*)*8);
>> +      command_line = (gchar**) malloc(sizeof(gchar*)*9);
>>         command_line[0] = g_strdup(appletviewer_executable);
>> -      command_line[1] = g_strdup("-Xdebug");
>> -      command_line[2] = g_strdup("-Xnoagent");
>> -      command_line[3] = g_strdup("-Xrunjdwp:transport=dt_socket,address=8787,server=y,suspend=n");
>> -      command_line[4] = g_strdup("sun.applet.PluginMain");
>> -      command_line[5] = g_strdup(out_pipe_name);
>> -      command_line[6] = g_strdup(in_pipe_name);
>> -      command_line[7] = NULL;
>> +      command_line[1] = g_strdup(PLUGIN_BOOTCLASSPATH);
>> +      command_line[2] = g_strdup("-Xdebug");
>> +      command_line[3] = g_strdup("-Xnoagent");
>> +      command_line[4] = g_strdup("-Xrunjdwp:transport=dt_socket,address=8787,server=y,suspend=n");
>> +      command_line[5] = g_strdup("sun.applet.PluginMain");
>> +      command_line[6] = g_strdup(out_pipe_name);
>> +      command_line[7] = g_strdup(in_pipe_name);
>> +      command_line[8] = NULL;
>>      } else
>>      {
>> -       command_line = (gchar**) malloc(sizeof(gchar*)*5);
>> +       command_line = (gchar**) malloc(sizeof(gchar*)*6);
>>          command_line[0] = g_strdup(appletviewer_executable);
>> -       command_line[1] = g_strdup("sun.applet.PluginMain");
>> -       command_line[2] = g_strdup(out_pipe_name);
>> -       command_line[3] = g_strdup(in_pipe_name);
>> -       command_line[4] = NULL;
>> +       command_line[1] = g_strdup(PLUGIN_BOOTCLASSPATH);
>> +       command_line[2] = g_strdup("sun.applet.PluginMain");
>> +       command_line[3] = g_strdup(out_pipe_name);
>> +       command_line[4] = g_strdup(in_pipe_name);
>> +       command_line[5] = NULL;
>>      }
>>
>>     environment = plugin_filter_environment();
>> @@ -1590,17 +1592,21 @@
>>     command_line[0] = NULL;
>>     g_free (command_line[1]);
>>     command_line[1] = NULL;
>> +  g_free (command_line[2]);
>> +  command_line[2] = NULL;
>> +  g_free (command_line[3]);
>> +  command_line[3] = NULL;
>> +  g_free (command_line[4]);
>> +  command_line[4] = NULL;
>>
>>     if (plugin_debug)
>>     {
>> -      g_free (command_line[2]);
>> -      command_line[2] = NULL;
>> -      g_free (command_line[3]);
>> -      command_line[3] = NULL;
>> -      g_free (command_line[4]);
>> -      command_line[4] = NULL;
>>         g_free (command_line[5]);
>>         command_line[5] = NULL;
>> +      g_free (command_line[6]);
>> +      command_line[6] = NULL;
>> +      g_free (command_line[7]);
>> +      command_line[7] = NULL;
>>     }
>>
>>     g_free(command_line);
>> @@ -2173,38 +2179,10 @@
>>       }
>>
>>     // Set appletviewer_executable.
>> -  Dl_info info;
>> -  int filename_size;
>> -  if (dladdr ((const void*) ITNP_New,&info) == 0)
>> -    {
>> -      PLUGIN_ERROR_TWO ("Failed to determine plugin shared object filename",
>> -                        dlerror ());
>> -      np_error = NPERR_GENERIC_ERROR;
>> -      goto cleanup_data_directory;
>> -    }
>> -  filename = (gchar*) malloc(sizeof(gchar)*1024);
>> -  filename_size = readlink(info.dli_fname, filename, 1023);
>> -  if (filename_size>= 0)
>> -  {
>> -      filename[filename_size] = '\0';
>> -  }
>> -
>> -  if (!filename)
>> -    {
>> -      PLUGIN_ERROR ("Failed to create plugin shared object filename.");
>> -      np_error = NPERR_OUT_OF_MEMORY_ERROR;
>> -      goto cleanup_data_directory;
>> -    }
>> -
>> -  if (filename_size<= 0)
>> -  {
>> -      free(filename);
>> -      filename = g_strdup(info.dli_fname);
>> -  }
>> -
>> -  appletviewer_executable = g_strdup_printf ("%s/../../bin/java",
>> -                                             dirname (filename));
>> -  PLUGIN_DEBUG(".so is located at: %s and the link points to: %s. Executing java from dir %s to run %s\n", info.dli_fname, filename, dirname (filename), appletviewer_executable);
>> +  filename = g_strdup(ICEDTEA_WEB_JRE);
>> +  appletviewer_executable = g_strdup_printf ("%s/bin/java",
>> +                                             filename);
>> +  PLUGIN_DEBUG("Executing java at %s\n", appletviewer_executable);
>>     if (!appletviewer_executable)
>>       {
>>         PLUGIN_ERROR ("Failed to create appletviewer executable name.");
>
>

Thanks for looking over the patch!

Cheers,
Omair



More information about the distro-pkg-dev mailing list