[icedtea-web][rfc] Extract native code caching from JNLPClassLoader into a small class

[Adam Domurad]

On 05/29/2013 06:21 AM, Jiri Vanek wrote:
> On 05/23/2013 09:19 PM, Adam Domurad wrote:
>> On 03/12/2013 10:26 AM, Jiri Vanek wrote:
>>> On 03/05/2013 10:20 PM, Adam Domurad wrote:
>>>> This is an incremental part of the effort to reduce the 
>>>> responsibilities of JNLPClassLoader.
>>>>
>>>> 2013-03-05 Adam Domurad <adomurad at redhat.com>
>>>>
>>>> * netx/net/sourceforge/jnlp/cache/NativeLibraryStorage.java: New,
>>>> stores and searches for native library files that are loaded from 
>>>> jars.
>>>> * netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java: Move code
>>>> that handled native jar caching to NativeLibraryStorage.
>>>>
>>>> Happy hacking,
>>>> -Adam
>>>
>>> Is there any more reason for this refactoring then much nicer, more 
>>> readable, and testable
>>> jnlpclasslaoder?
>>> Anyway I'm fan of this kind of refactoring. And I'm for this to be 
>>> done.
>>>
>>> - there must be unittests for this chnage
>>> - i would like to see even reproducer for this
>>> - it should go to 1.3 to after some time in head.
>>
>> Probably a little late now, but 1.4 sure, unless you think 1.3 is 
>> still a good idea.
>
> I'm hesitating with 1.4 now. But probably yes.  But you owe me your 
> head:)
>>
>>>
>>> I have not check if there is something more then pure refactoring, 
>>> but if there isn't and tsts
>>> will be added, then this will be approved.
>>
>> It is a refactoring only.
>>
>>>
>>> J.
>>
>> I have created some unit tests, hopefully it is enough to push with ?
>
> nope. Some moreover important changes needed.
>
> Especially ExecUtils.execAndLog have no reason to live. Please use 
> processWrapper. It is designed for this and have logging correctly 
> adapted.
>
> Also I'm against such a huge usage  of ecxec. Java is not Python.
>
> Especiallythe touch and mkdir  is nothing but pure laziness :) Please 
> use java calls.

Agreed

>
> For jar -cf ... well.. Java have api to work with its own jars.. 
> please follow this api. Do not fork processes if possible. But ..well. 
> this api can be trap :) So choose wisely.
>
> I would recommend to do the refactoring of DummyJNLPFileWithJar into 
> separate changset which you can proceed immediately, but I do not 
> insists.

I have spliced this and pushed.

>
> In all cases, thanx for check and refactoring! It is deeply appreciated.
>
> J.

See new version, I have removed all execs from both the new test, and 
JNLPClassLoaderTest.

New changelog:
2013-XX-XX  Adam Domurad  <adomurad at redhat.com>

     * netx/net/sourceforge/jnlp/util/StreamUtils.java
     (copyStream): New, copies input stream to output stream
     * 
tests/netx/unit/net/sourceforge/jnlp/cache/NativeLibraryStorageTest.java:
     New, tests lookup of native libraries from folders and jars.
     * tests/test-extensions/net/sourceforge/jnlp/util/FileTestUtils.java:
     New, contains utilities for testing open file descriptors, creating 
temporary
     directories, and creating jars.
     * 
tests/netx/unit/net/sourceforge/jnlp/runtime/JNLPClassLoaderTest.java:
     Replace jar creation methods with ones from FileTestUtils.

Happy hacking,
-Adam

-------------- next part --------------
A non-text attachment was scrubbed...
Name: native-lib-test-try2-no-execs.patch
Type: text/x-patch
Size: 23845 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20130530/fda175f2/native-lib-test-try2-no-execs.patch