[icedtea-web] RFE: Fix for broken JRE dir install
Deepak Bhole
dbhole at redhat.com
Thu Jan 20 17:10:17 PST 2011
* Dr Andrew John Hughes <ahughes at redhat.com> [2011-01-20 19:46]:
> On 19:37 Thu 20 Jan , Deepak Bhole wrote:
> > * Dr Andrew John Hughes <ahughes at redhat.com> [2011-01-20 19:22]:
> > > On 18:56 Thu 20 Jan , Deepak Bhole wrote:
> > > > Hi,
> > > >
> > > > Currently, installation to a jre directory is broken in both HEAD and
> > > > 1.0. Installing to j2re-image for example will end up creating
> > > > jre-image/jre/lib which is incorrect (files should go into
> > > > j2re-image/lib in this case).
> > > >
> > > > Attached patch fixes it so that install to both, jre and jdk works and
> > > > as does uninstall. It also makes javaws and itweb-setings actual files
> > > > (similar to 'java' and other binaries which are copied to JDK_HOME/bin
> > > > and JDK_HOME/jre/bin rather than being symlinked) and makes sure that
> > > > javaws can be called from either location and that it works.
> > > >
> > > > ChangeLog:
> > > > 2011-01-20 Deepak Bhole <dbhole at redhat.com>
> > > >
> > > > * Makefile.am
> > > > (install-exec-local): Use new JRE_DIR_PREFIX variables rather than
> > > > hardcoded '/jre' in pathname. Copy javaws and itweb-settings rather than
> > > > symlinking.
> > > > (install-data-local): Use new JRE_DIR_PREFIX variables rather than
> > > > jardcoded '/jre' in pathname.
> > > > (uninstall-local): Same. Also, remove all parent dirs of $(htmldir) until
> > > > a non-empty one is encountered.
> > > > * configure.ac: Set JRE_DIR_PREFIX to jre/ if prefix is a JDK dir and set
> > > > it to empty if prefix apears to be a JRE dir.
> > > > * netx/net/sourceforge/jnlp/Launcher.java (launchExternal): Revert back to
> > > > using java.home when selecting javaws binary to execute for fork.
> > > >
> > > >
> > > > Note: This should be backported to 1.0 after 9397074c2c39 is. Please see this
> > > > for rationale:
> > > > http://mail.openjdk.java.net/pipermail/distro-pkg-dev/2011-January/011806.html
> > > >
> > > > I have tested this considerably to make sure nothing is broken, but if
> > > > anyone feels the scope of this patch is too much for 1.0 this close to
> > > > release (I was hoping to tag today but looks like it will have to be
> > > > tomorrow now), I can post a separate path for 1.0 that only addresses
> > > > the issues mentioned in the thread above.
> > > >
> > > > Cheers,
> > > > Deepak
> > >
> > > Ok, number of points:
> > >
> > > * Why do we need to have duplicate copies? What is wrong with the symlink?
> > > We discussed this previously and no objections were raised at the time.
> >
> > I have no objections to symlink. I was just trying to mimic what the
> > default upstream openjdk install does with its binaries. I am fine with
> > symlinking too.
> >
>
> Ok, then don't change it :-)
>
Heh, okay :)
> > > * Why the change to the htmldir deletion? How does rm -rf fail? I don't
> > > think this long option to rmdir should be used as I doubt it is standard.
> >
> > We create a JDK_HOME/share/... dir. By default there is no share/ dir
> > in JDK_HOME. I think an uninstall should attempt to remove _everything_
> > it creates, and hence the rmdir -p which attempts to remove the dir and
> > its parents all the way to the top (until it encounters a non-empty
> > dir).
> >
>
> I suppose we can accomodate this, though it again seems to be something
> just to handle installing in a JDK tree. /usr/share would normally exist
> and be non-empty.
>
Yep, I understand. Once that is working, this may be removed.
> > > * Using java.home breaks installation outside a JDK tree. See previous
> > > discussion. That's why icedtea.web.bin was introduced.
> >
> > I was unable to reproduce such breakage after this patch.
> >
> > http://download.oracle.com/javase/tutorial/essential/environment/sysprop.html
> > states that java.home is the jre install dir (and this is true even if
> > javaws is invoked from JDK_HOME/bin rather than JDK_HOME/jre/bin).
> > Assuming jre/bin contains the actual file, java.home/bin/javaws will always
> > exist and work regardless of where javaws was called from.
> >
>
> I don't follow. If the user specifies --prefix=/usr, your patch will be looking
> in completely the wrong place. You shouldn't let the user specify the install
> directory and then just completely ignore it and assume something else.
>
> I gather that /usr installation is broken in the code at present too,
> but I don't see why we want to create even more breakage.
>
Fair enough. However in it's current incarnation, the JDK is relocatable
just by copying the tree. Until we can support /usr installation, we
need to install in the JDK tree, and while we are installation there, I
would like icedtea-web to not be the reason why relocatability is
broken...
Cheers,
Deepak
> > > * Typo: whether not weather.
> > >
> >
> > Doh! Thanks.
> >
> > Cheers,
> > Deepak
> >
> > > > diff -r 06940cdcfef8 Makefile.am
> > > > --- a/Makefile.am Thu Jan 20 11:06:41 2011 -0500
> > > > +++ b/Makefile.am Thu Jan 20 18:47:48 2011 -0500
> > > > @@ -103,37 +103,37 @@
> > > > 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_DATA} $(NETX_DIR)/lib/classes.jar $(DESTDIR)$(prefix)$(JRE_DIR_PREFIX)/lib/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 \
> > > > + if [ -e $(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 ; \
> > > > + cp -a $(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)$(prefix)$(JRE_DIR_PREFIX)/lib
> > > > ${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 \
> > > > + if [ -e $(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 ; \
> > > > + cp -a $(DESTDIR)$(bindir)/itweb-settings $(DESTDIR)$(prefix)$(JRE_DIR_PREFIX)/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)$(prefix)$(JRE_DIR_PREFIX)/lib
> > > > if ENABLE_DOCS
> > > > ${mkinstalldirs} $(DESTDIR)$(htmldir)
> > > > (cd ${abs_top_builddir}/docs/netx; \
> > > > @@ -151,23 +151,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 \
> > > > + if [ -e $(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 \
> > > > + if [ -e $(DESTDIR)$(prefix)/jre/bin/itweb-settings ] ; then \
> > > > rm -f $(DESTDIR)$(prefix)/jre/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
> > > >
> > > > @@ -350,7 +352,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 06940cdcfef8 configure.ac
> > > > --- a/configure.ac Thu Jan 20 11:06:41 2011 -0500
> > > > +++ b/configure.ac Thu Jan 20 18:47:48 2011 -0500
> > > > @@ -80,4 +80,12 @@
> > > > IT_CHECK_FOR_CLASS(SUN_APPLET_APPLETIMAGEREF, [sun.applet.AppletImageRef])
> > > > IT_CHECK_FOR_APPLETVIEWERPANEL_HOLE
> > > >
> > > > +# Set JRE prefix based on weather 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 06940cdcfef8 netx/net/sourceforge/jnlp/Launcher.java
> > > > --- a/netx/net/sourceforge/jnlp/Launcher.java Thu Jan 20 11:06:41 2011 -0500
> > > > +++ b/netx/net/sourceforge/jnlp/Launcher.java Thu Jan 20 18:47:48 2011 -0500
> > > > @@ -327,7 +327,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