[icedtea-web] RFE: Fix for broken JRE dir install
Dr Andrew John Hughes
ahughes at redhat.com
Fri Jan 21 05:35:40 PST 2011
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.
> 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