Reviewer needed - fix for Re: Strange behaviour during javaws -about
Dr Andrew John Hughes
ahughes at redhat.com
Thu Feb 24 16:33:17 PST 2011
On 14:14 Thu 24 Feb , Omair Majid wrote:
> On 02/24/2011 01:44 PM, Dr Andrew John Hughes wrote:
> > On 12:52 Thu 24 Feb , Omair Majid wrote:
> >> Hi,
> >>
> >> I am attaching a patch that makes net.sourceforge.jnlp.runtime.Boot use
> >> the location of about.jnlp as determined at build time. There are a few
> >> other things that I will be adding to build.properties.
> >>
> >> The rest of this email assumes this patch is applied before applying
> >> your patch, and where there are conflicts (such as the codebase of
> >> about.jnlp, this patch takes precedence).
> >>
> >
> > snip...
>
> >>>> @@ -218,8 +223,16 @@
> >>>> cl = ClassLoader.getSystemClassLoader();
> >>>> }
> >>>> try {
> >>>> - return
> >>>> cl.getResource("net/sourceforge/jnlp/resources/about.jnlp").toString();
> >>>> + //exctracts full path to about.jnlp
> >>>
> >>> Typo; extracts
> >>>
> >>>> + String s =
> >>>> cl.getResource("net/sourceforge/jnlp/runtime/Boot.class").toString();
> >>>> + s=s.substring(0,s.indexOf("!"));
> >>>> + s=s.substring(s.indexOf(":")+1);
> >>>> + s=s.substring(s.indexOf(":")+1);
> >>>> + s="file://"+s.replace("netx.jar","about.jnlp");
> >>>> + System.out.println(s);
> >>>> + return s;
> >>>
> >>> What does this do?
> >>>
> >>
> >> If I understand this properly, it uses the installed location of
> >> netx.jar to compute the location of about.jnlp. That is, if netx.jar is
> >> at /usr/local/share/icedtea-web/netx.jar, then it finds about.jnlp to be
> >> located at /usr/local/share/icedtea-web/about.jnlp.
> >>
> >
> > It's the use of ':' and '!' that confuses me. Exactly what does cl.getResource
> > return here?
> >
>
> It returns a jar url. Jar urls look like this:
>
> jar:file:/home/omajid/code/icedtea-web-image/share/icedtea-web/netx.jar!/net/sourceforge/jnlp/runtime/Boot.class
>
> The ! separates the url of the jar file itself from the path of the file
> stored inside a jar.
>
Ok makes more sense now. Still think the System.out should go though or
at least be switch to output to System.err only when debugging is on.
> >> The attached patch determines the paths at build time and puts them in a
> >> build.properties file.
> >>
> >
> > I'd prefer that approach.
> >
> >> Cheers,
> >> Omair
> >>
> >> [1]
> >> http://download.oracle.com/javase/tutorial/uiswing/concurrency/dispatch.html
> >
> >> diff -r d7fee305bd4f Makefile.am
> >> --- a/Makefile.am Wed Feb 23 13:37:10 2011 -0500
> >> +++ b/Makefile.am Thu Feb 24 12:06:16 2011 -0500
> >> @@ -242,7 +242,11 @@
> >> netx-source-files.txt:
> >> find $(NETX_SRCDIR) -name '*.java' | sort> $@
> >>
> >> -stamps/netx.stamp: netx-source-files.txt stamps/bootstrap-directory.stamp
> >> +build.properties: $(NETX_SRCDIR)/net/sourceforge/jnlp/build.properties.in
> >> + sed "s#[@]ABOUT_JNLP_LOCATION[@]#$(DESTDIR)$(datadir)/icedtea-web/about.jnlp#"< $< > $@
> >
> > Is there a reason we aren't doing this via configure?
> >
>
> Yes, as indicated in the autoconf manual [1], configure will replace
> @datadir@ with {datarootdir}, not the actual path.
>
Ugh ok. You still need to fix the DESTDIR issue.
> > $(DESTDIR) will not be the final install location so should be in here.
> > It's a staging location.
> >
> >> +
> >> +stamps/netx.stamp: netx-source-files.txt stamps/bootstrap-directory.stamp \
> >> + build.properties
> >> mkdir -p $(NETX_DIR)
> >> $(BOOT_DIR)/bin/javac $(IT_JAVACFLAGS) \
> >> -d $(NETX_DIR) \
> >> @@ -255,6 +259,7 @@
> >> ${INSTALL_DATA} -D $${files} \
> >> $(NETX_DIR)/net/sourceforge/jnlp/resources/$${files}; \
> >> done)
> >> + cp -pPR build.properties $(NETX_DIR)/net/sourceforge/jnlp/
> >
> > Any reason not to just use cp -a?
> >
>
> No. None that I can think of.
>
> >> mkdir -p stamps
> >> touch $@
> >>
> >> @@ -274,6 +279,7 @@
> >>
> >> clean-netx:
> >> rm -rf $(NETX_DIR)
> >> + rm -f build.properties
> >
> > configure would do this for you.
> >
>
> Yes, except as I said above, it wont work with file paths :(
>
> >> rm -f stamps/netx-dist.stamp
> >> rm -f netx-source-files.txt
> >> rm -f stamps/netx.stamp
> >> diff -r d7fee305bd4f netx/net/sourceforge/jnlp/build.properties.in
> >> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> >> +++ b/netx/net/sourceforge/jnlp/build.properties.in Thu Feb 24 12:06:16 2011 -0500
> >> @@ -0,0 +1,1 @@
> >> +about.jnlp.location=@ABOUT_JNLP_LOCATION@
> >> diff -r d7fee305bd4f netx/net/sourceforge/jnlp/resources/about.jnlp
> >> --- a/netx/net/sourceforge/jnlp/resources/about.jnlp Wed Feb 23 13:37:10 2011 -0500
> >> +++ b/netx/net/sourceforge/jnlp/resources/about.jnlp Thu Feb 24 12:06:16 2011 -0500
> >> @@ -1,5 +1,5 @@
> >> <?xml version="1.0" encoding="utf-8"?>
> >> -<jnlp spec="1.0" href="about.jnlp" codebase="http://icedtea.classpath.org/netx/">
> >> +<jnlp spec="1.0" href="about.jnlp" codebase=".">
> >> <information>
> >> <title>About window for NetX</title>
> >> <vendor>NetX</vendor>
> >> diff -r d7fee305bd4f netx/net/sourceforge/jnlp/runtime/Boot.java
> >> --- a/netx/net/sourceforge/jnlp/runtime/Boot.java Wed Feb 23 13:37:10 2011 -0500
> >> +++ b/netx/net/sourceforge/jnlp/runtime/Boot.java Thu Feb 24 12:06:16 2011 -0500
> >> @@ -19,6 +19,7 @@
> >> import static net.sourceforge.jnlp.runtime.Translator.R;
> >>
> >> import java.io.File;
> >> +import java.io.InputStream;
> >> import java.io.IOException;
> >> import java.net.MalformedURLException;
> >> import java.net.URL;
> >> @@ -27,6 +28,7 @@
> >> import java.util.ArrayList;
> >> import java.util.Arrays;
> >> import java.util.List;
> >> +import java.util.Properties;
> >>
> >> import net.sourceforge.jnlp.AppletDesc;
> >> import net.sourceforge.jnlp.ApplicationDesc;
> >> @@ -213,15 +215,31 @@
> >> * does not exist.
> >> */
> >> private static String getAboutFile() {
> >> + String location = null;
> >> ClassLoader cl = Boot.class.getClassLoader();
> >> if (cl == null) {
> >> cl = ClassLoader.getSystemClassLoader();
> >> }
> >> + InputStream in = cl.getResourceAsStream("net/sourceforge/jnlp/build.properties");
> >> try {
> >> - return cl.getResource("net/sourceforge/jnlp/resources/about.jnlp").toString();
> >> - } catch (Exception e) {
> >> - return null;
> >> + Properties buildProperties = new Properties();
> >> + buildProperties.load(in);
> >> + location = buildProperties.getProperty("about.jnlp.location");
> >> + } catch (IOException e) {
> >> + // ignore and return null later on
> >> + if (JNLPRuntime.isDebug()) {
> >> + e.printStackTrace();
> >> + }
> >> + } finally {
> >> + if (in != null) {
> >> + try {
> >> + in.close();
> >> + } catch (IOException e) {
> >> + e.printStackTrace();
> >> + }
> >> + }
> >> }
> >> + return location;
> >> }
> >>
> >> /**
> >
> > Code changes look fine.
>
> Thanks for looking over the path. Unfortunately the changes are not
> sufficient. We will still need to either sign about.jar or somehow fix
> it so it works without all permissions.
>
> Jiri, what's your take?
>
> Cheers,
> Omair
>
> [1]
> http://www.gnu.org/software/hello/manual/autoconf/Installation-Directory-Variables.html
--
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: F5862A37 (https://keys.indymedia.org/)
Fingerprint = EA30 D855 D50F 90CD F54D 0698 0713 C3ED F586 2A37
More information about the distro-pkg-dev
mailing list