RFR 8020802: Need an ability to create jar files that are invariant to the pack200 packing/unpacking
Kumar Srinivasan
kumar.x.srinivasan at oracle.com
Tue Oct 22 15:32:59 UTC 2013
Hi Alex,
> thanks for suggestion, dt.jar seems to be a fine test subject.
> Fixed @test field and the .jar used in the test.
I am sorry! I misled you yesterday, it should simply be @test
> New webrev is here: http://cr.openjdk.java.net/~kizune/8020802/webrev.03
Here are some nits, mostly conventions.
Main.java
Suggestion breaking this line:
204 String tmpbase = (fname == null) ? "tmpjar" : fname.substring(fname.indexOf(File.separatorChar) + 1);
This makes it clearer
Ref:
http://www.oracle.com/technetwork/java/javase/documentation/codeconventions-136091.html#248
String tmpbase = (fname == null)
?"tmpjar"
:fname.substring(fname.indexOf(File.separatorChar) + 1);
Can tmpbase be made final ?
TestNormal.java
I don't like the way this line is broken up, break after the Paren,
usually it is a comma.
or use a static final for Line.separator, that might make it shorter.
66 Process proc = Runtime.getRuntime()
67 .exec(java_home + File.separator + "bin" + File.separator + cmd);
There are other coding convention issues wrt. if/else and while, please
check.
http://www.oracle.com/technetwork/java/javase/documentation/codeconventions-142311.html#449
Thanks
Kumar
>
> /Alex
>
> On 10/22/13 24:33, Kumar Srinivasan wrote:
>> Alex,
>>
>> The @test should be just TestNormal not TestNormal.java
>>
>> if you need an alternate/small jar you can use one in $JDK/lib/
>> ex: jconsole.jar or dt.jar
>>
>> Kumar
>>
>>
>> On 10/21/2013 7:41 AM, Alexander Zuev wrote:
>>> Alan,
>>>
>>> thanks for a review, see my comments inline.
>>>
>>> On 10/20/13 23:15, Alan Bateman wrote:
>>>> On 19/10/2013 16:14, Kumar Srinivasan wrote:
>>>>> Hi Alex,
>>>>>
>>>>> This looks good.
>>>>>
>>>>> Hi Sherman, Alan,
>>>>>
>>>>> Could one of you review this, please
>>>>>
>>>> The "-n" options seems okay but I wonder if there has any thought
>>>> given to having an option on jarsigner to normalize it before signing?
>>> That was requirement for PM - looks like they have more usages for
>>> this option than just signing a jar file.
>>>> On the patch itself then I think it needs to be cleaned up before
>>>> pushing it. From a quick look:
>>>>
>>>> - It's using a raw type for the packer properties, should be
>>>> Map<String, String>
>>> Ok, will fix that.
>>>> - Is any reason not to use try-with-resources?
>>> Because not all of the resources used are inherited from
>>> AutoCloseable - you can't use File
>>> in try-with-resources block so to use it i will have to rewrite code
>>> a bit. Looks like it's a little bit too late
>>> considering i will need to test it on all the platforms.
>>>> - I assume the catching of Exception in createTemporaryFile should
>>>> be IOException|SecurityException
>>> Yes, will change that.
>>>> - Is the @test tag in the test right?
>>> You mean that @summary does not match the issue summary and does not
>>> include bug ID? Can be fixed too.
>>>> - The test appears to extract jfr.jar but I don't think we can
>>>> assume this exists
>>> I'm trying to find a .jar file that is in the jdk distribution,
>>> contains class files and of a reasonable size - attempt with
>>> tools.jar caused test to timeout on Windows machine even with
>>> 20minutes timeout - and i do not want to make a test
>>> with the timeout greater than even 10 minutes - much less one of
>>> 20+minutes.
>>> Any suggestions? I'm putting the jfr.jar so far but can change it
>>> later - test stabilization can be done after the Oct 24th.
>>>> - Some of the jar tests use sun.tools.jar.Main and this might be
>>>> useful here
>>> I'm trying to check the command-line option and want it to go over
>>> the full path of command-line option parsing - just to be sure.
>>> Do not think it's a big deal.
>>>> One other thing, Mathias's mail to jdk8-dev [1] asks for changes
>>>> that require translation to be in the master no longer than 10/24.
>>>> Does the update to jar.properties count?
>>> Yes, i've been pinged by the documentation team already so i'm
>>> trying to push this stuff as soon as possible.
>>> Of course without giving up on the quality of the code.
>>>
>>> The new webrev can be found at
>>> http://cr.openjdk.java.net/~kizune/8020802/webrev.02
>>>
>>> /Alex
>>>>
>>>> -Alan
>>>>
>>>> [1]
>>>> http://mail.openjdk.java.net/pipermail/jdk8-dev/2013-October/003443.html
>>>
>>
>
More information about the core-libs-dev
mailing list