[icedtea-web] RFE: Fix for broken JRE dir install
Deepak Bhole
dbhole at redhat.com
Fri Jan 21 07:13:27 PST 2011
* Dr Andrew John Hughes <ahughes at redhat.com> [2011-01-21 08:35]:
> On 21:53 Thu 20 Jan , Deepak Bhole wrote:
> > * Dr Andrew John Hughes <ahughes at redhat.com> [2011-01-20 20:32]:
> > > On 20:10 Thu 20 Jan , Deepak Bhole wrote:
> > > > * Dr Andrew John Hughes <ahughes at redhat.com> [2011-01-20 19:46]:
> > >
> > > snip...
> > >
> >
> > Thanks for the review! I'm attaching a new patch that addresses all the
> > issues discussed.
> >
> > ChangeLog:
> > * Makefile.am
> > ($(NETX_DIR)/launcher/%.o): Build javaws without the
> > "-J-Djava.icedtea-web.bin=..." arg.
> > (install-exec-local): Use new JRE_DIR_PREFIX variables rather than
> > hardcoded '/jre' in pathname. Fix mechanism used to determine if links
> > should be created in jre/bin.
> > (install-data-local): Use new JRE_DIR_PREFIX variables rather than
> > hardcoded '/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.
> >
> > I've also tweaked the link creation logic slightly as the previous logic was
> > inadequate to handle jre dir installations correctly.
> >
> > I'll commit tomorrow if there are no further comments.
> >
>
> This bit is confusing:
>
> > - ${INSTALL_PROGRAM} $(NETX_DIR)/launcher/javaws $(DESTDIR)$(bindir)
> > + ${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
>
> Why are we still copying? I thought we agreed this didn't need changing?
>
> As I read this, it installs to jre/bin and then tries to also copy over the top from
> $(bindir) where nothing has been installed? If you're working with a jre tree, there
> won't be a jre/bin so the first line will install to $(DESTDIR)$(bindir) as before.
> If it's a JDK tree, it will install to jre/bin then try to copy from $(bindir) to
> jre/bin and fail.
>
> The existing logic seems fine (install to bindir, add a link in jre/bin if it exists).
> Why are you trying to change this and breaking it in the process?
>
> We already went through several revisions to get the current version. Please leave it
> as it is unless there is a very good reason to change it.
>
> I approve the JRE_DIR_PREFIX additions and the java.home logic for 1.0 ONLY.
> Please remove these other changes and post a new patch.
>
The previous logic was:
"Install to JDK|JRE bin/; check if jre/bin exists; link there if so"
This is incorrect because the _file_ should be in jre/bin and it must
because of the way installations work with packages. 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.
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"
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
More information about the distro-pkg-dev
mailing list