[rfc][icedtea-web] integrated setupable JVM to plugin

[Adam Domurad]

On 04/02/2013 07:20 AM, Jiri Vanek wrote:
 > This  is last part of setup-able jvm. Now integrated to plugin by
 > usage of IcedTeaParseProperties.
 > Also adding fallback (same as is now in plugin) to launchers. To use
 > defualt JRE if custom one is invalid.
 >
 >
 > J.

 > diff -r 39e2d8f2ba60 launcher/itweb-settings.in
 > --- a/launcher/itweb-settings.in    Tue Apr 02 09:45:25 2013 +0200
 > +++ b/launcher/itweb-settings.in    Tue Apr 02 12:52:22 2013 +0200
 > @@ -7,13 +7,18 @@
 >  BINARY_LOCATION=@ITWEB_SETTINGS_BIN_LOCATION@
 >  PROGRAM_NAME=itweb-settings
 >
 > -CUSTOM_JRE_REGEX="^deployment.jre.dir *= *"
 > +PROPERTY_NAME=deployment.jre.dir
 > +CUSTOM_JRE_REGEX="^$PROPERTY_NAME *= *"
 >  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
 > +  if [ -d  "$CUSTOM_JRE" -a -f "$CUSTOM_JRE/bin/java" ] ; then
 > +    JAVA=$CUSTOM_JRE/bin/java
 > +  else
 > +    echo "your custom JRE $CUSTOM_JRE read from deployment.properties
 > under key $PROPERTY_NAME as $CUSTOM_JRE is not valid. Using default
 > ($JAVA) in attempt to start. Please fix this"

[nit] I'd start with a capital & end with a period

 > +  fi
 >  fi;
 >
 >  ${JAVA} ${LAUNCHER_BOOTCLASSPATH} ${LAUNCHER_FLAGS} \
 > diff -r 39e2d8f2ba60 launcher/javaws.in
 > --- a/launcher/javaws.in    Tue Apr 02 09:45:25 2013 +0200
 > +++ b/launcher/javaws.in    Tue Apr 02 12:52:22 2013 +0200
 > @@ -1,4 +1,4 @@
 > -#!/bin/bash
 > +#!/bin/sh
 >
 >  JAVA=@JAVA@
 >  LAUNCHER_BOOTCLASSPATH=@LAUNCHER_BOOTCLASSPATH@
 > @@ -9,14 +9,19 @@
 >  PROGRAM_NAME=javaws
 >  CP=@JRE@/lib/rt.jar
 >
 > -CUSTOM_JRE_REGEX="^deployment.jre.dir *= *"
 > +PROPERTY_NAME=deployment.jre.dir
 > +CUSTOM_JRE_REGEX="^$PROPERTY_NAME *= *"
 >  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
 > -  CP=$CUSTOM_JRE/lib/rt.jar
 > +  if [ -d  "$CUSTOM_JRE" -a -f "$CUSTOM_JRE/bin/java" -a -f
 > "$CUSTOM_JRE/lib/rt.jar" ] ; then
 > +    JAVA=$CUSTOM_JRE/bin/java
 > +    CP=$CUSTOM_JRE/lib/rt.jar
 > +  else
 > +    echo "your custom JRE $CUSTOM_JRE read from deployment.properties
 > under key $PROPERTY_NAME as $CUSTOM_JRE is not valid. Using default
 > ($JAVA, $CP) in attempt to start. Please fix this"

[nit] I'd start with a capital & end with a period

 > +  fi
 >  fi;
 >
 >  JAVA_ARGS=( )
 > diff -r 39e2d8f2ba60 plugin/icedteanp/IcedTeaNPPlugin.cc
 > --- a/plugin/icedteanp/IcedTeaNPPlugin.cc    Tue Apr 02 09:45:25 2013
 > +0200
 > +++ b/plugin/icedteanp/IcedTeaNPPlugin.cc    Tue Apr 02 12:52:22 2013
 > +0200
 > @@ -48,6 +48,9 @@
 >  #include <sys/types.h>
 >  #include <unistd.h>
 >
 > +//IcedTea-plugin includes
 > +#include "IcedTeaPluginUtils.h"
 > +#include "IcedTeaParseProperties.h"
 >  // Liveconnect extension
 >  #include "IcedTeaScriptablePluginObject.h"
 >  #include "IcedTeaNPPlugin.h"
 > @@ -74,6 +77,7 @@
 >  #include <nsServiceManagerUtils.h>
 >  #endif
 >
 > +
 >  // Error reporting macros.
 >  #define PLUGIN_ERROR(message)                                       \
 >    g_printerr ("%s:%d: thread %p: Error: %s\n", __FILE__, __LINE__,  \
 > @@ -287,13 +291,29 @@
 >  #endif
 >
 >  static std::string get_plugin_executable(){
 > -      return std::string (appletviewer_default_executable);
 > -
 > +      std::string custom_jre = "";

[nit] = "" is redundant and may cause string allocation

 > +      bool custom_jre_defined = find_custom_jre(custom_jre);
 > +      if (custom_jre_defined) {
 > +            if
 > (IcedTeaPluginUtilities::file_exists(custom_jre+"/bin/java")){
 > +                  return custom_jre+"/bin/java";
 > +            } else {
 > +                  PLUGIN_DEBUG("Your custom jre (/bin/java check) %s
 > is not valid. Please fix %s in your %s. In attempt to run using
 > default one. \n", custom_jre.c_str(), custom_jre_key.c_str(),
 > default_file_ITW_deploy_props_name.c_str());

I strongly think this should be logged even if they aren't debugging the
plugin.

 > +            }
 > +      }
 > +      return std::string (appletviewer_default_executable);

[nit] you can drop the std::string call here.

 >  }
 >
 >  static std::string get_plugin_rt_jar(){
 > -      return std::string (appletviewer_default_rtjar);
 > -
 > +      std::string custom_jre = "";

[nit] = "" is redundant and may cause string allocation

 > +      bool custom_jre_defined = find_custom_jre(custom_jre);
 > +      if (custom_jre_defined) {
 > +            if
 > (IcedTeaPluginUtilities::file_exists(custom_jre+"/lib/rt.jar")){
 > +                  return custom_jre+"/lib/rt.jar";
 > +            } else {
 > +                  PLUGIN_DEBUG("Your custom jre (/lib/rt.jar check)
 > %s is not valid. Please fix %s in your %s. In attempt to run using
 > default one. \n", custom_jre.c_str(), custom_jre_key.c_str(),
 > default_file_ITW_deploy_props_name.c_str());

I strongly think this should be logged even if they aren't debugging the
plugin.

 > +            }
 > +      }
 > +      return std::string (appletviewer_default_rtjar);

[nit] you can drop the std::string call here.
 >
 >  }
 >
 >

Otherwise looks fine to me.
Thanks so much for doing this!
-Adam