[icedtea-web] RFE: Fix for broken JRE dir install

Deepak Bhole dbhole at redhat.com
Fri Jan 21 14:40:39 PST 2011 

* Dr Andrew John Hughes <ahughes at redhat.com> [2011-01-21 15:46]:
> On 10:13 Fri 21 Jan     , Deepak Bhole wrote:
> > * Dr Andrew John Hughes <ahughes at redhat.com> [2011-01-21 08:35]:
> > > 
> 
> [I'm going to reply to http://mail.openjdk.java.net/pipermail/distro-pkg-dev/2011-January/011835.html
> here too, as we have two concurrent discussions going on here and it's
> about time we did a join() ;-)]
> 
> By 'break everything else', I meant that you break installation where the
> root is not java.home.  As I've said previously, the user is going to specify
> something like --prefix=/blah and you're going to blatantly ignore that and
> use something else.
> 

Ah, fair enough. I agree, this change should definitely not go in HEAD
as it is very reliant on the prefix being a JDK/JRE dir.

> I don't see why the JDK needs to be relocatable.  If you move installed system
> files and directories around, it's likely that things will break.  If I move
> /lib/libc.so.6 into /var/tmp, should I be surprised that something goes wrong?
> I don't see why were bending over backwards to allow for this.
>

Well it is not bending over as much as trying to avoid a 'here is an
add-on component, but it breaks relocatability, something the JDK has
supported for ages' scenario.
 
> I guess we're talking about flexibility from two different angles here.  From
> a FOSS perspective, users generally expect to be able to choose where an application
> is installed using the standard autotools machinery.  Your patch would break this.
> From the 'application living in its own little directory hierarchy' idea favoured
> by many proprietary applications, or ones with such heritage like the JDK, flexibility
> is "I can unpack it anywhere, but I can't break apart this homogenous blob of stuff".
> 
> I would have thought that distros generally were strong proponents of the former approach,
> while you seem to be advocating the latter.  Maybe you need to explain more clearly what
> you need and why, because you seem to be doing something more convoluted than just wanting
> to work with the jre-image as opposed to j2sdk-image.
> 
> > 
> > The previous logic was:
> > 
> > "Install to JDK|JRE bin/; check if jre/bin exists; link there if so"
> > 
> 
> No it says install in the $(bindir) specified by the user as expected.
> It then supports the unusual case of jre/bin existing (wouldn't work in /usr
> for example), and adds a link into it.
> 
> > This is incorrect because the _file_ should be in jre/bin 
> 
> Why should it? What if it doesn't exist?
> 
> > and it must
> > because of the way installations work with packages.
> 
> Please explain.  
> 

The RPM build mechanism works as follows (excluding the minor details):
Build IcedTea
Pushd to j2sdk-image
Break off jre/ + misc. stuff into a java-1.6.0-openjdk package
Take the bin/ (i.e. j2sdk-image/bin), lib/ etc. and put it into
java-1.6.0-openjdk-devel package, etc.

Now, consider a scenario where the user has only java-1.6.0-openjdk
installed. Their system contains
/usr/lib/jvm/java-1.6.0-openjdk-1.6.0.0.x86_64/ which has:

$ /usr/lib/jvm/java-1.6.0-openjdk-1.6.0.0.x86_64/
jre

And that is it.

If I were to install an icedtea-web package now, it needs to go into
jre/* and work fully from there only. If jre/bin/javaws were to be a
link, I would be forced to install ../../bin/javaws (i.e. create a
/usr/lib/jvm/java-1.6.0-openjdk-1.6.0.0.x86_64/bin and put it there)
which is not a good thing for it to be doing since that dir should
really be created by the -devel package. 

This brings up an interesting point though.. what happens in the case
where the user installs -devel later .. there wont be a symlink in
JDK_HOME/bin ... I guess with rpm we will have to bite the bullet and
put the executables in jre/bin only.

> >RPM for example builds the whole tree, and divides it into subpackages. In such a case,
> > if RPM were to split off JRE and install it, JRE/bin will have a dangling symlink.
> 
> Why is this relevant to IcedTea-Web?  This is a separate package which should depend
> on the openjdk package already being installed.
> 
> If you want the behaviour you're asking for, why not just using ${java.home}/jre as your
> prefix?
> 

It is not RPM that is the issue. The issue is that icedtea-web only
needs the jre to run. To that end, it should be installable in a jre
directory and work. The fact that we put things in JDK_HOME/bin is a
secondary concern. The way things are now, JDK_HOME/bin is a primary
concern and jre/bin (where links are made) is secondary.

> > 
> > The new logic 
> > 
> > "Install to JDK/JRE bin; check if there is a JRE/bin; if yes, move the
> > actual file to it, and link from JDK/bin; else leave it alone"
> > 
> 
> That might be what you meant, but it's not what your patch says:
> 
>  +	${INSTALL_PROGRAM} $(NETX_DIR)/launcher/javaws $(DESTDIR)$(prefix)$(JRE_DIR_PREFIX)/bin
>   	if [ -d $(DESTDIR)$(prefix)/jre/bin ] ; then \
>  +	  cp -a $(DESTDIR)$(bindir)/javaws $(DESTDIR)$(prefix)/jre/bin ; \
>  +	  ln -sf $(DESTDIR)$(prefix)/jre/bin/javaws $(DESTDIR)$(bindir)/javaws ; \
> >  	fi
> 
> If jre/bin exists and JRE_DIR_PREFIX is thus jre/, this does:
> 
> 1. Installs javaws in $(prefix)/jre/bin
> 2. Tries to _copy_ $(prefix)/bin/javaws (which doesn't exist) into $(prefix)/jre/bin where it was already installed to
> 3. Links $(prefix)/jre/bin/javaws to $(prefix)/bin/javaws 
>
> I think what you actually intended was just 1 & 3.
> 
>  +	${INSTALL_PROGRAM} $(NETX_DIR)/launcher/javaws $(DESTDIR)$(prefix)$(JRE_DIR_PREFIX)/bin
>   	if [ -d $(DESTDIR)$(prefix)/jre/bin ] ; then \
>  +	  ln -sf $(DESTDIR)$(prefix)/jre/bin/javaws $(DESTDIR)$(bindir)/javaws ; \
>   	fi
> 
> If jre/bin doesn't exist, the symlink will not be created and the binary will have already been installed in $(bindir)
> as $(JRE_DIR_PREFIX) is empty, giving $(prefix)/bin which is equal to $(bindir) by default.
> 

Doh! You are right, that is a bug in my patch. Good catch! I agree with
your correction -- it is what I meant to do (with one minor change --
the target would be ../jre/bin/.. i.e. it would be relative instead of
absolute, which is another thing I missed).

Cheers,
Deepak

> > This guarantees that the actual file will be in jre/bin, and should the
> > prefix have been a JDK tree, a link will be created from JDK_BIN/bin to
> > jre/bin/<file>.
> > 
> > Deepak
> > 
> > > > Thanks,
> > > > Deepak
> > > 
> > > > 
> > > > diff -r 43212217e9c0 Makefile.am
> > > > --- a/Makefile.am	Wed Dec 15 10:17:51 2010 -0500
> > > > +++ b/Makefile.am	Thu Jan 20 21:51:05 2011 -0500
> > > > @@ -102,37 +102,29 @@
> > > >   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)$(prefix)$(JRE_DIR_PREFIX)/lib/$(INSTALL_ARCH_DIR)
> > > >  if ENABLE_PLUGIN
> > > > -	${INSTALL_PROGRAM} $(PLUGIN_DIR)/IcedTeaPlugin.so $(DESTDIR)$(prefix)/jre/lib/$(INSTALL_ARCH_DIR)/
> > > > +	${INSTALL_PROGRAM} $(PLUGIN_DIR)/IcedTeaPlugin.so $(DESTDIR)$(prefix)$(JRE_DIR_PREFIX)/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_DATA} $(abs_top_builddir)/liveconnect/lib/classes.jar $(DESTDIR)$(prefix)$(JRE_DIR_PREFIX)/lib/plugin.jar
> > > >  endif
> > > > -	${INSTALL_DATA} $(NETX_DIR)/lib/classes.jar $(DESTDIR)$(prefix)/jre/lib/netx.jar
> > > > -	${INSTALL_PROGRAM} $(NETX_DIR)/launcher/javaws $(DESTDIR)$(bindir)
> > > > +	${INSTALL_DATA} $(NETX_DIR)/lib/classes.jar $(DESTDIR)$(prefix)$(JRE_DIR_PREFIX)/lib/netx.jar
> > > > +	${INSTALL_PROGRAM} $(NETX_DIR)/launcher/javaws $(DESTDIR)$(prefix)$(JRE_DIR_PREFIX)/bin
> > > >  	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 ; \
> > > > +	  cp -a $(DESTDIR)$(bindir)/javaws $(DESTDIR)$(prefix)/jre/bin ; \
> > > > +	  ln -sf $(DESTDIR)$(prefix)/jre/bin/javaws $(DESTDIR)$(bindir)/javaws ; \
> > > >  	fi
> > > > -	${INSTALL_DATA} extra-lib/about.jar $(DESTDIR)$(prefix)/jre/lib
> > > > -	${INSTALL_PROGRAM} $(NETX_DIR)/launcher/controlpanel/itweb-settings $(DESTDIR)$(bindir)
> > > > +	${INSTALL_DATA} extra-lib/about.jar $(DESTDIR)$(prefix)$(JRE_DIR_PREFIX)/lib
> > > > +	${INSTALL_PROGRAM} $(NETX_DIR)/launcher/controlpanel/itweb-settings $(DESTDIR)$(prefix)$(JRE_DIR_PREFIX)/bin
> > > >  	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 ; \
> > > > +	  cp -a $(DESTDIR)$(bindir)/itweb-settings $(DESTDIR)$(prefix)/jre/bin ; \
> > > > +	  ln -sf $(DESTDIR)$(prefix)/jre/bin/itweb-settings $(DESTDIR)$(bindir)/itweb-settings ; \
> > > >  	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)$(prefix)$(JRE_DIR_PREFIX)/lib
> > > >  if ENABLE_DOCS
> > > >  	${mkinstalldirs} $(DESTDIR)$(htmldir)
> > > >  	(cd ${abs_top_builddir}/docs/netx; \
> > > > @@ -150,23 +142,25 @@
> > > >  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)$(prefix)$(JRE_DIR_PREFIX)/lib/$(INSTALL_ARCH_DIR)/IcedTeaPlugin.so
> > > > +	rm -f $(DESTDIR)$(prefix)$(JRE_DIR_PREFIX)/lib/plugin.jar
> > > > +	rm -f $(DESTDIR)$(prefix)$(JRE_DIR_PREFIX)/lib/netx.jar
> > > > +	rm -f $(DESTDIR)$(prefix)$(JRE_DIR_PREFIX)/lib/about.jnlp
> > > > +	rm -f $(DESTDIR)$(prefix)$(JRE_DIR_PREFIX)/lib/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 ; \
> > > > +	rm -f $(DESTDIR)$(prefix)$(JRE_DIR_PREFIX)/bin/javaws
> > > > +	if [ -L $(DESTDIR)$(prefix)/bin/javaws ] ; then \
> > > > +	  rm -f $(DESTDIR)$(prefix)/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 ; \
> > > > +	rm -f $(DESTDIR)$(prefix)$(JRE_DIR_PREFIX)/bin/itweb-settings
> > > > +	if [ -L $(DESTDIR)$(prefix)/bin/itweb-settings ] ; then \
> > > > +	  rm -f $(DESTDIR)$(prefix)/bin/itweb-settings ; \
> > > >  	fi
> > > > -	rm -rf $(DESTDIR)$(htmldir)
> > > > +	rm -rf $(DESTDIR)$(htmldir)/*
> > > > +	if [ -d $(DESTDIR)$(htmldir) ] ; then \
> > > > +	  rmdir -p --ignore-fail-on-non-empty $(DESTDIR)$(htmldir) ; \
> > > > +	fi
> > > >  
> > > >  # Plugin
> > > >  
> > > > @@ -349,7 +343,7 @@
> > > >  $(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",  }' \
> > > > +	  -DJAVA_ARGS='{ "-J-ms8m", "net.sourceforge.jnlp.runtime.Boot",  }' \
> > > >  	  -DPROGNAME='"javaws"' -c -o $@ $<
> > > >  
> > > >  $(NETX_DIR)/launcher/controlpanel/%.o: $(LAUNCHER_SRCDIR)/%.c
> > > > diff -r 43212217e9c0 configure.ac
> > > > --- a/configure.ac	Wed Dec 15 10:17:51 2010 -0500
> > > > +++ b/configure.ac	Thu Jan 20 21:51:05 2011 -0500
> > > > @@ -78,4 +78,12 @@
> > > >  IT_CHECK_FOR_CLASS(SUN_APPLET_APPLETIMAGEREF, [sun.applet.AppletImageRef])
> > > >  IT_CHECK_FOR_APPLETVIEWERPANEL_HOLE
> > > >  
> > > > +# Set JRE prefix based on whether to-level prefix is a JDK dir or a JRE dir
> > > > +if test -d ${prefix}/jre ; then 
> > > > +    JRE_DIR_PREFIX="/jre" ; 
> > > > +else
> > > > +    JRE_DIR_PREFIX="" ;     
> > > > +fi ;
> > > > +AC_SUBST([JRE_DIR_PREFIX])
> > > > +
> > > >  AC_OUTPUT
> > > > diff -r 43212217e9c0 netx/net/sourceforge/jnlp/Launcher.java
> > > > --- a/netx/net/sourceforge/jnlp/Launcher.java	Wed Dec 15 10:17:51 2010 -0500
> > > > +++ b/netx/net/sourceforge/jnlp/Launcher.java	Thu Jan 20 21:51:05 2011 -0500
> > > > @@ -330,7 +330,12 @@
> > > >              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.home") +
> > > > +                                      File.separatorChar +
> > > > +                                      "bin" +
> > > > +                                      File.separatorChar +
> > > > +                                      "javaws";
> > > > +
> > > >              commands.add(pathToWebstartBinary);
> > > >              // use -Jargument format to pass arguments to the JVM through the launcher
> > > >              for (String arg : vmArgs) {
> > > 
> > > 
> > > -- 
> > > Andrew :)
> > > 
> > > Free Java Software Engineer
> > > Red Hat, Inc. (http://www.redhat.com)
> > > 
> > > Support Free Java!
> > > Contribute to GNU Classpath and IcedTea
> > > http://www.gnu.org/software/classpath
> > > http://icedtea.classpath.org
> > > PGP Key: 94EFD9D8 (http://subkeys.pgp.net)
> > > Fingerprint = F8EF F1EA 401E 2E60 15FA  7927 142C 2591 94EF D9D8
> 
> -- 
> Andrew :)
> 
> Free Java Software Engineer
> Red Hat, Inc. (http://www.redhat.com)
> 
> Support Free Java!
> Contribute to GNU Classpath and IcedTea
> http://www.gnu.org/software/classpath
> http://icedtea.classpath.org
> PGP Key: 94EFD9D8 (http://subkeys.pgp.net)
> Fingerprint = F8EF F1EA 401E 2E60 15FA  7927 142C 2591 94EF D9D8

More information about the distro-pkg-dev mailing list