Reviewer needed - fix for Re: Strange behaviour during javaws -about
Omair Majid
omajid at redhat.com
Thu Feb 24 11:14:29 PST 2011
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.
>> 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.
> $(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
More information about the distro-pkg-dev
mailing list