RFR: JDK-8062849 -- Optimize EnumMap.equals

2015-07-28 Thread Steve Drach
Please review the following simple fix for the issue reported in https://bugs.openjdk.java.net/browse/JDK-8062849 https://bugs.openjdk.java.net/browse/JDK-8062849. - # HG changeset patch # User sdrach # Date 143193 25200 # Fri Jul 24 15:33:13 2015 -0700 # Node ID

FYI - fix for JDK-8062849 -- Optimize EnumMap.equals

2015-07-27 Thread Steve Drach
Hi, I’ve fixed the issue reported in https://bugs.openjdk.java.net/browse/JDK-8062849 https://bugs.openjdk.java.net/browse/JDK-8062849. It’s been reviewed by Brent Christian and Paul Sandoz. Both the changeset and webrev.zip are attached. Steve

Re: FYI - fix for JDK-8062849 -- Optimize EnumMap.equals

2015-07-27 Thread Steve Drach
Well this is strange. The attachments seem to have been discarded by some part of the mail system. I will provide them to anyone who requests them. Sorry for that. On Jul 27, 2015, at 9:38 AM, Steve Drach steve.dr...@oracle.com wrote: Hi, I’ve fixed the issue reported in https

RFR: JDK-8114832 it.next on ArrayList throws wrong type of Exception after remove(-1)

2015-07-24 Thread Steve Drach
Hello, Please review the fix for JDK-8114832. I did what was suggested in the comments in the bug report, moving the increment of modcount to the “right” place. https://bugs.openjdk.java.net/browse/JDK-8114832 https://bugs.openjdk.java.net/browse/JDK-8114832

RFR: JDK-8066013 (prefs) Unused variable in src/java.prefs/share/classes/java/util/prefs/MacOSXPreferences.java

2015-07-22 Thread Steve Drach
Hello, Please review the fix for JDK-8066013, removing two unused variables from MacOSXPreferences.java. https://bugs.openjdk.java.net/browse/JDK-8066013 https://bugs.openjdk.java.net/browse/JDK-8066013 http://slc01hfj.us.oracle.com/webrevs/JDK-8066013/webrev/index.html

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2015-10-26 Thread Steve Drach
Hi, We’ve published another webrev for review. Issue: https://bugs.openjdk.java.net/browse/JDK-8132734 JEP 238: https://bugs.openjdk.java.net/browse/JDK-8047305 Webrev: http://cr.openjdk.java.net/~psandoz/multiversion-jar/jar-webrev/ This one addresses the issues regarding CodeSigners,

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2015-11-09 Thread Steve Drach
It’s nice to see these issues get resolved while I was sleeping ;-) >> On 08/11/2015 20:38, Paul Sandoz wrote: >>> : >>> The name is derived from the requirement that if the fragment is present a >>> call to getRuntimeVersionedEntry is required (or the equivalent of), thus >>> the URL is a

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2015-11-09 Thread Steve Drach
>>> As to whether this is implementation vs. JAR URL spec then I assume it >>> needs to be spec so that libraries can create URLs that will use runtime >>> versioning when access the JAR. >>> >> >> Yeah, i don’t think we can avoid it. > > Ok. I guess I should put the information in

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2015-11-11 Thread Steve Drach
Hi, Please review the new webrev that addresses the issues raised by Sherman and Alan in the last iteration. In particular: - fixed the race condition in isMultiRelease() and another one with the variables ‘version’ and ‘configured’ - changed the fragment for JarURLConnection runtime

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2015-11-03 Thread Steve Drach
I'm fine to be told "the pros and cons were considered, and this > is the best for the > supported use scenario":-) Yes, that is the case. > In that case, it might deserve some wording in the spec notes to > prepare the developer the possible unexpected. > > Thanks, > She

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2015-11-06 Thread Steve Drach
ntries added to it, it should be resigned. So, if folks do the right thing, your scenario shouldn’t occur, although I don’t think there is a way to guarantee that. I will point this out to the vulnerability team. Again, thanks for your comments. Steve > > -Sherman > >

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2015-11-06 Thread Steve Drach
d the > corresponding entry for that > root name is signed. But we return a linked versioned-entry, and it's not > signed. This might not > be an issue at all. Please confirm this when doing security review. > > -Sherman > > On 11/6/15 9:23 AM, Steve Drach wrot

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2015-11-05 Thread Steve Drach
called one or more times after construction and before > the first operation that obtains an entry, > after which the jar file configuration’s is considered immutable, and > subsequent calls to this method will result in an ISE. > > Paul. > >> On 3 Nov 2015, at 18:11, Stev

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2015-11-04 Thread Steve Drach
nsidered immutable, and > subsequent calls to this method will result in an ISE. Okay. > > Paul. > >> On 3 Nov 2015, at 18:11, Steve Drach <steve.dr...@oracle.com> wrote: >> >> Please review the latest webrev that addresses the issue raised in Sherman’s >> c

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2015-11-03 Thread Steve Drach
he old getEntry() method? Yes, for general entry retrieval as well as for loading classes. > > Thanks > Max > >> On Nov 4, 2015, at 1:11 AM, Steve Drach <steve.dr...@oracle.com> wrote: >> >> Webrev: http://cr.openjdk.java.net/~psandoz/multiversion-jar/jar-webrev/ >> >

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2015-11-06 Thread Steve Drach
n around the > block a few times :-) IMO, baring any major issues, it’s time to push and we > can clean up any ancillary issues with later pushes. > > Paul. > >> On 5 Nov 2015, at 18:10, Steve Drach <steve.dr...@oracle.com> wrote: >> >> Hi, >>

RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2015-10-14 Thread Steve Drach
Hi, Let’s try again, this time there are tests. Please review the following webrev that adds support for multi-release jars as specified in JEP-238. Issue: https://bugs.openjdk.java.net/browse/JDK-8132734 JEP 238: https://bugs.openjdk.java.net/browse/JDK-8047305 Webrev:

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2015-10-16 Thread Steve Drach
> So why do you want to put this field into the super class JarEntry, not the > JarFileEntry, don't see any > benefit of doing that. I changed it. I put the versionedEntry in JarFileEntry instead of JarEntry. Not too difficult although I had to add a package private method to support

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2015-10-14 Thread Steve Drach
>> The current test directory contains binary jar files. In fact in all the >> test directories, there are 52 binary .jar files. > I know but we need to work to remove those. I figured that might be the response, but thought it was worth the try ;-) > > >> I added three more. I thought

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2015-10-14 Thread Steve Drach
The current test directory contains binary jar files. In fact in all the test directories, there are 52 binary .jar files. >>> I know but we need to work to remove those. >> >> I figured that might be the response, but thought it was worth the try ;-) >> > > A reasonable way forward

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2015-10-14 Thread Steve Drach
>> Let’s try again, this time there are tests. Please review the following >> webrev that adds support for multi-release jars as specified in JEP-238. > Good to have tests this time :-) > > I see several JAR files in the webrev, shouldn't the tests create these so > that we don't have to check

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2015-10-14 Thread Steve Drach
> Any reason the JarEntry.get/setSize() are the only ZipEntry methods get > overridden? It didn’t seem necessary. The root entries are the “public interface”, we’re just providing aliased entry contents. > > -Sherman > > On 10/14/2015 09:07 AM, Steve Drach wrote: >&

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2015-11-17 Thread Steve Drach
Hi Alan, Thanks for looking at this. >> Please review the new webrev that addresses the issues raised by Sherman and >> Alan in the last iteration. In particular: >> - fixed the race condition in isMultiRelease() and another one with the >> variables ‘version’ and ‘configured’ >> - changed

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2015-09-29 Thread Steve Drach
>> Please review the following webrev that adds support for multi-release jars >> as specified in JEP-238. >> >> Issue: https://bugs.openjdk.java.net/browse/JDK-8132734 >> >> JEP 238: https://bugs.openjdk.java.net/browse/JDK-8047305 >>

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2015-09-30 Thread Steve Drach
Hi Max, > Can you describe if there is any effect on signed jars? Including: > > 1. Will jarsigner be able to sign such a jar? The jarsigner from 1.8.0_51 can sign the jar. The jarsigner from jdk9/dev can not, giving me the error jarsigner: unable to sign jar: javax.net.ssl.SSLException:

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2015-09-30 Thread Steve Drach
>>> The jarsigner from jdk9/dev can not, giving me the error >>> >>> jarsigner: unable to sign jar: javax.net.ssl.SSLException: >>> java.lang.RuntimeException: Unexpected error: >>> java.security.InvalidAlgorithmParameterException: the trustAnchors >>> parameter must be non-empty >>> >>> I’m

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2015-09-30 Thread Steve Drach
Hi Max, Here are updated answers to your questions, after I received some help (thank you Sean Mullan). > Can you describe if there is any effect on signed jars? Including: > > 1. Will jarsigner be able to sign such a jar? The jarsigner from 1.8.0_51 can sign the jar. The jarsigner from

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2015-10-01 Thread Steve Drach
>>> - JDK 8 jar signer does not work with a JDK 9 created keystore >>> - JDK 8 signed jar with JDK 8 created keystore is not the same size as JDK >>> 9 signed jar with JDK 9 keystore >>> - JDK 8 signed jar with JDK 8 created keystore is not the same size as JDK >>> 9 signed jar with the same JDK

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2015-10-01 Thread Steve Drach
Please review the following webrev that adds support for multi-release jars as specified in JEP-238. Issue: https://bugs.openjdk.java.net/browse/JDK-8132734

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2015-10-01 Thread Steve Drach
>> I think I’m getting distracted here, not focusing on getting tests created. >> Is it okay to move on? > > Please move on. If you think anything is strange, just send me all related > files. > > One last request about the SSLException, can you still generate the full > debug output for jar

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2015-10-02 Thread Steve Drach
>> To elaborate a bit, the implementation is quite straight forward. >> ZipFileSystem builds an Inode tree on open. That tree is implemented as a >> Map. For regular zip/jar files both the key and the value are >> the same Inode. For multi-release jar files the value is the

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2015-10-02 Thread Steve Drach
>> Okay. On the the ZipFileSystem then it would be good to get a summary on >> what you are proposing. > > Details of any JavacFileManager/ZipFileSystem support are still being hashed > out. They are mentioned in the JEP and we can update once those details are > finalized. To elaborate a

RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2015-09-29 Thread Steve Drach
Hi, Please review the following webrev that adds support for multi-release jars as specified in JEP-238. Issue: https://bugs.openjdk.java.net/browse/JDK-8132734 JEP 238: https://bugs.openjdk.java.net/browse/JDK-8047305

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2015-10-05 Thread Steve Drach
Hi Alan, > It would be good to add @since to Attributes#MULTI_RELEASE before this goes > in. None of the other attributes have this. Are you suggesting I do this? /** * {@code Name} object for {@code Multi-Release} * manifest attribute that indicates this is a multi-release JAR file. * *

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2015-11-17 Thread Steve Drach
Hi Sherman, Thanks for looking at this. Comments in-line below. > On Nov 17, 2015, at 9:49 AM, Xueming Shen <xueming.s...@oracle.com> wrote: > > On 11/11/15 8:44 AM, Steve Drach wrote: >> Hi, >> >> Please review the new webrev that addresses the issu

Re: RFR: 8066272: pack200 must support Multi-Release Jars

2015-11-19 Thread Steve Drach
I’m not an official reviewer so here are my unofficial comments: 1. I see no obvious flaws 2. The name MultiVersion should be changed to MultiRelease to correspond with the name change in the JEP. 3. While for this test it’s okay to have versions 7, 8, and 9, in practice we won’t process

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2015-11-18 Thread Steve Drach
Hi, Please review the latest iteration of the webrev for runtime support of multi-release jar files. Issue: https://bugs.openjdk.java.net/browse/JDK-8132734 JEP 238: https://bugs.openjdk.java.net/browse/JDK-8047305 Webrev: http://cr.openjdk.java.net/~psandoz/multiversion-jar/jar-webrev/ I

RFR: 8153652 Update javap to be multi-release jar aware

2016-06-13 Thread Steve Drach
Hi, Please review the following changeset that simply supplies the help information for the already existing javap command line option, -multi-release. webrev: http://cr.openjdk.java.net/~sdrach/8153652/webrev.00/ issue:

RFR: 81536534 Update jdeps to be multi-release jar aware

2016-06-13 Thread Steve Drach
Hi, Please review the following changeset to make jdeps multi-release jar aware. webrev: http://cr.openjdk.java.net/~sdrach/8153654/webrev.00/index.html issue: https://bugs.openjdk.java.net/browse/JDK-8153654

Re: RFR: 81536534 Update jdeps to be multi-release jar aware

2016-06-14 Thread Steve Drach
Hi Mandy, >> Please review the following changeset to make jdeps multi-release jar aware. >> >> webrev: http://cr.openjdk.java.net/~sdrach/8153654/webrev.00/index.html >> >> issue: https://bugs.openjdk.java.net/browse/JDK-8153654

Re: RFR: 8153652 Update javap to be multi-release jar aware

2016-06-14 Thread Steve Drach
>> Please review the following changeset that simply supplies the help >> information for the already existing javap command line option, >> -multi-release. >> >> webrev: http://cr.openjdk.java.net/~sdrach/8153652/webrev.00/ >> >> issue:

RFR: 8114827: JDK 9 multi-release enabled jar tool

2016-06-01 Thread Steve Drach
Hi, Please review the following changeset that makes it easier to create multi-release jar files with jar tool. webrev: http://cr.openjdk.java.net/~sdrach/8114827/webrev.01/ issue: https://bugs.openjdk.java.net/browse/JDK-8114827 The changeset is the implementation of a new command line

Re: RFR: 8150680 JarFile.Release enum needs reconsideration with respect to it's values

2016-06-16 Thread Steve Drach
implementation, etc. > > Cheers, > > -Joe > > On 6/15/2016 3:49 PM, Steve Drach wrote: >> I’ve updated the webrev to address the issue of the constructor accepting >> values like Version.parse(“7.1”) >> >> http://cr.openjdk.java.net/~sdrach/8150680/webrev.01/

Re: RFR: 8150680 JarFile.Release enum needs reconsideration with respect to it's values

2016-06-21 Thread Steve Drach
d nothing in that enum specific except for the documentation > and BASE_VERSION. Wouldn't it better to create a top-level Release enum that > can be used to identify anything in the JDK with release semantics -- apart > from Jar files? > > Cheers, > Paul > > On Tue, Jun

Re: RFR: 8150680 JarFile.Release enum needs reconsideration with respect to it's values

2016-06-22 Thread Steve Drach
> 144 private boolean notVersioned; // legacy constructor called > > Do you need this? Unfortunately yes. It’s used in entries() and stream(). If it’s set, they have the JDK 8 semantics. If not set, entries/stream only see the appropriate versioned entries. This will go away

Re: RFR: 8150680 JarFile.Release enum needs reconsideration with respect to it's values

2016-06-20 Thread Steve Drach
did not see either class loaded. I also tried the tests with a jar file on the class path and did see both classes loaded. > > cheers, > Rémi > > - Mail original - >> De: "Steve Drach" <steve.dr...@oracle.com> >> À: "Joseph D. Darcy&q

Re: RFR: 8150680 JarFile.Release enum needs reconsideration with respect to it's values

2016-06-15 Thread Steve Drach
I’ve updated the webrev to address the issue of the constructor accepting values like Version.parse(“7.1”) http://cr.openjdk.java.net/~sdrach/8150680/webrev.01/ <http://cr.openjdk.java.net/~sdrach/8150680/webrev.01/> > On Jun 15, 2016, at 8:56 AM, Steve Drach <steve.dr...@oracl

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2016-01-13 Thread Steve Drach
Hi, CCC had some suggestions to improve the code that was previously approved during review. As a consequence, I had to make significant changes to the API, and I believe the code need further review. Please review the latest iteration of the webrev for runtime support of multi-release jar

RFR: 8155770 Correct URLClassLoader API documentation to explicitly say jar-scheme URL's are accepted

2016-06-24 Thread Steve Drach
Hi, Please review this simple change to the URLClassLoader specification. issue: https://bugs.openjdk.java.net/browse/JDK-8155770 webrev: http://cr.openjdk.java.net/~sdrach/8155770/webrev/

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2016-02-09 Thread Steve Drach
Hi, Yet another webrev, http://cr.openjdk.java.net/~sdrach/8132734/webrev.06/ , with a change to JarEntryIterator to fix a problem discovered by performance tests — calling hasNext() twice as often as needed. I also removed the @since 9

Re: RFR JDK-8149769: Null pointer exception in ZipFileSystemProvider

2016-02-12 Thread Steve Drach
>> Please review this simple fix to ZipFileSystemProvider. The issue is >> JDK-8149769 . I didn’t do >> a webrev but instead provide the following patch. >> >> > This looks okay. Can one of the existing tests be updated to cover this case?

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2016-01-28 Thread Steve Drach
s Release.valueOf(version)? That might be confusing. Also, having it consistent helps with testing. > > sherman > > On 01/27/2016 03:37 PM, Steve Drach wrote: >>> I'm still wondering about the phrase "root entry" as it continues to give >>> the impression (to

Re: RFR: 8147607: Remove test library dependency on sun.security.tools.jarsigner.Main

2016-01-28 Thread Steve Drach
>>> Shouldn't you also include the FileOuputStream in try-with-resources? >> >> It is. Do you need to refresh your browers? > > If you read the code quickly, I missed it too, maybe it would be better to > put the opening brace on a separate line, i.e.: > > try (ZipFile in = new

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2016-01-28 Thread Steve Drach
by consistency, or in this case inconsistency. > I'm confused, what did I miss? I think you are focussing on a minor implementation detail. Internally versions are small integer values, but externally, publicly, they are values of the Release Enum. > > sherman > > &

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2016-01-29 Thread Steve Drach
>> I’ve released a new webrev, >> http://cr.openjdk.java.net/~sdrach/8132734/webrev.04/index.html >> that >> addresses the above issue. >> > Thank you, the javadoc is much clearer and readable now. > > The first mention of

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2016-01-28 Thread Steve Drach
of having a dedicated "JarFile.runtimeVersioned"? Based on >> its only usages at ln#356 and #381, it appears, shouldn't getVersion() simply >> returns Release.valueOf(version)? >> >> sherman >> >> On 01/27/2016 03:37 PM, Steve Drach wrote:

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2016-01-27 Thread Steve Drach
> I'm still wondering about the phrase "root entry" as it continues to give the > impression (to me anyway) that it's a resource in the root directory. I think > "root" works in the JEP because it deals with simple resources like A.class > and B.class that are in the root directory but it's

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2016-02-01 Thread Steve Drach
confusing and we should just return all entries without regard to versioning. Then create the two new methods for specific versioned entries. > On Feb 1, 2016, at 12:18 PM, Steve Drach <steve.dr...@oracle.com> wrote: > >>>>> Alan’s point is that traversing usin

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2016-02-01 Thread Steve Drach
> On Jan 30, 2016, at 12:00 AM, Alan Bateman wrote: > > > On 29/01/2016 17:39, Paul Sandoz wrote: >> : >> Alan’s point is that traversing using entries()/stream() always returns the >> versioned entries (if any) rather than all entries, thus in a sense filters. >> >>

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2016-02-01 Thread Steve Drach
Alan’s point is that traversing using entries()/stream() always returns the versioned entries (if any) rather than all entries, thus in a sense filters. My assumption was the traversal should by default be consistent with a calls to getEntry, thus:

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2016-02-03 Thread Steve Drach
Thanks for the comments Alan. Responses in-line. >> I created a new webrev, >> http://cr.openjdk.java.net/~sdrach/8132734/webrev.05/ >> , that implements >> what I outlined above. In particular I enhanced the JarEntryIterator class >>

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2016-02-02 Thread Steve Drach
rev.05/>, that implements what I outlined above. In particular I enhanced the JarEntryIterator class and I added additional commentary to the entries() and stream() methods. I also added a new test, MultiReleaseJarIterators, to test entries() and stream(). > >> On Feb 1, 2016, a

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2016-01-29 Thread Steve Drach
>>> One other thing that I've been wondering about is the stream() and >>> entries() methods. Has there been any discussion about these doing filter? >> >> Not that I’m aware of. >> >>> Maybe it is too expensive and best left to the user of the API? Part of the >>> context for asking is

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2016-01-21 Thread Steve Drach
Thank you for the review Alan. See comments in line below. > Overall I think the API looks much better. With the advantage of being much simpler too. > For Release then I have to admit that I dislike _9 and wonder if other > options were considered? javax.lang.model.SourceVersion uses the

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2016-01-21 Thread Steve Drach
>> I suspected this is a bike shed candidate. I think Release._9 is nicer and >> it conveys the same information in a less cluttered way than >> Release.RELEASE_9. > Yes a bike shed, I'm just saying that Release._9 looks odd/inconsistent when > we have SourceVersion.RELEASE_9 elsewhere. Maybe

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2016-01-22 Thread Steve Drach
Hi Alan, et. al., I’ve released a new webrev that addresses all the issues you raised. http://cr.openjdk.java.net/~sdrach/8132734/webrev.03/index.html Specifically: > For Release then I have to admit that I dislike _9 and

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2016-01-20 Thread Steve Drach
Hi, This is a repeat of the RFR I sent last Wed (Jan 13). CCC had some suggestions to improve the code that was previously approved during review. As a consequence, I had to make significant changes to the API, and I believe the code need further review. Please review the latest iteration

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2016-01-26 Thread Steve Drach
> On Jan 25, 2016, at 8:54 AM, Alan Bateman wrote: Somehow I missed this, sorry for the delayed response. >> >> Changed to BASE, i.e. Release.BASE >> > This looks better. Release.BASE is probably okay although it still feels like > Release.UNVERSIONED, esp. when it

RFR JDK-8149769: Null pointer exception in ZipFileSystemProvider

2016-02-12 Thread Steve Drach
Hi, Please review this simple fix to ZipFileSystemProvider. The issue is JDK-8149769 . I didn’t do a webrev but instead provide the following patch. Thanks Steve diff -r 2d6c2c75f338

Re: RFR JDK-8149769: Null pointer exception in ZipFileSystemProvider

2016-02-12 Thread Steve Drach
request and will close the bug report with appropriate comment. > > -Sherman > > On 2/12/16 1:11 PM, Steve Drach wrote: >> Hi, >> >> Please review this simple fix to ZipFileSystemProvider. The issue is >> JDK-8149769 <https://bugs.openjdk.java.net/browse/J

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2016-02-14 Thread Steve Drach
> "ze". Not a big issue for sure, but a kind of "regression". Defining two > iterator classes as suggested > above will also workaround this issue, as the "notVersioned" one will work > just as expected, no > regression. > > -Sherman > > On 2/

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2016-02-15 Thread Steve Drach
Thank you Alan. I’ll address the issues you bring up before integration. > On Feb 15, 2016, at 4:30 AM, Alan Bateman <alan.bate...@oracle.com> wrote: > > > On 10/02/2016 01:04, Steve Drach wrote: >> Hi, >> >> Yet another webrev, http://cr.openjdk.java.net/

Re: RFR - 8132734: java.util.jar.* changes to support multi-release jar files

2016-02-18 Thread Steve Drach
>> Thank you Alan. I’ll address the issues you bring up before integration. > Thanks. Are you planning to update the webrev too as it would be nice to see > the final javadoc? http://cr.openjdk.java.net/~sdrach/8132734/webrev.07/index.html

Request for comments -- resource reification vs. mrjar scheme for runtime versioning of multi-release jar files

2016-04-12 Thread Steve Drach
We’ve identified two possible ways to address issue JDK-8151542. One way is to append a #runtime fragment to the input URL in URLClassPath to convey to URLJarFile that we want to have the JarFile opened with the parameter Release.RUNTIME, so

Re: RFR: 8153213: Jar manifest attribute "Multi-Release" accepts any value

2016-04-08 Thread Steve Drach
>> I’ve put up a new webrev that addresses the issue of having spaces before >> (and after) the value “true” in the Multi-Release attribute. >> > > Is some or all of that really necessary? since the we can specify domain of > values. I think it is. The spec states that one can have an

Re: RFR: 8153213: Jar manifest attribute "Multi-Release" accepts any value

2016-04-08 Thread Steve Drach
I’ve put up a new webrev that addresses the issue of having spaces before (and after) the value “true” in the Multi-Release attribute. >>> >>> Is some or all of that really necessary? since the we can specify domain of >>> values. >> >> I think it is. The spec states that one

Re: RFR: 8153213: Jar manifest attribute "Multi-Release" accepts any value

2016-04-08 Thread Steve Drach
Okay, I’ll prepare a new webrev. I think all we need to check for after “true” is \r or \n. If the manifest just ends without at least one of those, it’s not a legal manifest. > On Apr 8, 2016, at 11:25 AM, Claes Redestad <claes.redes...@oracle.com> wrote: > > On 04/08/2016

Re: RFR: 8153213: Jar manifest attribute "Multi-Release" accepts any value

2016-04-08 Thread Steve Drach
ue” followed by either ‘\r’ or ‘\n’ is the only acceptable value. > On Apr 8, 2016, at 11:34 AM, Steve Drach <steve.dr...@oracle.com> wrote: > > Okay, I’ll prepare a new webrev. I think all we need to check for after > “true” is \r or \n. If the manifest just ends without

RFR: 8153213: Jar manifest attribute "Multi-Release" accepts any value

2016-04-07 Thread Steve Drach
I’ve put up a new webrev that addresses the issue of having spaces before (and after) the value “true” in the Multi-Release attribute. > Hi, > Please review this simple fix to require that the jar manifest Multi-Release > attribute has a value of “true" in order to be effective, i.e. to assert

RFR: 8153213: Jar manifest attribute "Multi-Release" accepts any value

2016-04-11 Thread Steve Drach
Hi, I’ve updated the following patch, incorporating code by Claes Redestad to handle some corner cases while processing the attribute value. Note that we’ve limited the location of the value part of the attribute to accommodate startup performance requirements. It’s not without precedent as

Re: RFR: 8152733: Avoid creating Manifest when checking for Multi-Release attribute

2016-03-25 Thread Steve Drach
Hi, A minor nit on the comment > + * Since there are no repeated substrings in our search strings, > + * the good character shifts can be replaced with a comparison. Probably should be “good suffix shifts”. Steve

RFR 8150679: closed/javax/crypto/CryptoPermission/CallerIdentification.sh fails after fix for JDK-8132734

2016-03-02 Thread Steve Drach
Please review the following fix for JDK-8150679 webrev: http://cr.openjdk.java.net/~sdrach/8150679/webrev/ issue: https://bugs.openjdk.java.net/browse/JDK-8150679 The test was modified to

Re: RFR 8150679: closed/javax/crypto/CryptoPermission/CallerIdentification.sh fails after fix for JDK-8132734

2016-03-03 Thread Steve Drach
> On Mar 3, 2016, at 3:26 AM, Paul Sandoz <paul.san...@oracle.com> wrote: > > >> On 2 Mar 2016, at 20:12, Steve Drach <steve.dr...@oracle.com> wrote: >> >> Please review the following fix for JDK-8150679 >> >> webrev: http://cr

RFR: 8151339 Adding fragment to JAR URLs breaks ant

2016-03-07 Thread Steve Drach
Hi, Please review the following changeset. We’d like to get this into build 109, which means by noon today. This is essentially a temporary fix, but it’s been tested and Lucene has been built against it. We will follow up with a more comprehensive fix by build 110. webrev:

Re: 8151339 Adding fragment to JAR URLs breaks ant

2016-03-07 Thread Steve Drach
ve > > Uwe > > - > Uwe Schindler > uschind...@apache.org > ASF Member, Apache Lucene PMC / Committer > Bremen, Germany > http://lucene.apache.org/ > >> -Original Message- >> From: core-libs-dev [mailto:core-libs-dev-boun...@openjdk.java.net

RFR: 8153213: Jar manifest attribute "Multi-Release" accepts any value

2016-04-01 Thread Steve Drach
Hi, Please review this simple fix to require that the jar manifest Multi-Release attribute has a value of “true" in order to be effective, i.e. to assert the jar is a multi-release jar. issue: https://bugs.openjdk.java.net/browse/JDK-8153213

Re: RFR: 8151542: URL resources for multi-release jar files have a #runtime fragment appended to them

2016-04-29 Thread Steve Drach
>> I’ve updated the webrev to change all instances of the word “reified” to >> “real” as in getRealName(). >> >> Issue: https://bugs.openjdk.java.net/browse/JDK-8151542 >> >> >> Webrev: http://cr.openjdk.java.net/~sdrach/8151542/webrev.01/ >>

RFR: 8151542: URL resources for multi-release jar files have a #runtime fragment appended to them

2016-04-27 Thread Steve Drach
Issue: https://bugs.openjdk.java.net/browse/JDK-8151542 Webrev: http://cr.openjdk.java.net/~sdrach/8151542/webrev/ This changeset causes the URL returned from a

Re: RFR: 8151542: URL resources for multi-release jar files have a #runtime fragment appended to them

2016-04-29 Thread Steve Drach
> On Apr 29, 2016, at 4:47 PM, Xueming Shen <xueming.s...@oracle.com> wrote: > > On 4/29/16 3:30 PM, Steve Drach wrote: >> I updated the webrev with changes that Alan suggested >> >> still needs to be fixed to compare the URL protocol without regar

Re: RFR: 8151542: URL resources for multi-release jar files have a #runtime fragment appended to them

2016-04-29 Thread Steve Drach
Hopefully the last one ;-) This webrev removes the lowercase of protocol, and incorporates better (in my mind) seperation of choices for choosing the loader, similar to what Paul suggested. Everything else remains the same. Only URLClassPath changed from previous webrev.

RFR: 8151542: URL resources for multi-release jar files have a #runtime fragment appended to them

2016-04-28 Thread Steve Drach
I’ve updated the webrev to change all instances of the word “reified” to “real” as in getRealName(). Issue: https://bugs.openjdk.java.net/browse/JDK-8151542 Webrev: http://cr.openjdk.java.net/~sdrach/8151542/webrev.01/

Re: RFR: 8151542: URL resources for multi-release jar files have a #runtime fragment appended to them

2016-04-28 Thread Steve Drach
Keeping with the path precedent, is it acceptable to change “getReifiedName” to “getRealName”? >>> >>> Diction Note: Reified X means X wasn't real (in some sense) until now. As >>> in non-reified types, which are not real at runtime because the static >>> compiler discarded them. >>> >>

Re: RFR: 8151542: URL resources for multi-release jar files have a #runtime fragment appended to them

2016-04-28 Thread Steve Drach
>> Issue: https://bugs.openjdk.java.net/browse/JDK-8151542 >> >> >> Webrev: http://cr.openjdk.java.net/~sdrach/8151542/webrev/ >> >> >> This changeset causes the URL returned from a

Re: RFR: 8151542: URL resources for multi-release jar files have a #runtime fragment appended to them

2016-04-29 Thread Steve Drach
I put a new webrev out with the change suggested by Paul, using file.indexOf(“!/“) instead of file.endsWith(“!/“). http://cr.openjdk.java.net/~sdrach/8151542/webrev.02/index.html > So you are planning to eventually change the

Re: RFR: 8151542: URL resources for multi-release jar files have a #runtime fragment appended to them

2016-04-29 Thread Steve Drach
>> I just didn’t see it as any higher than P4. What would you like it to be? >> >> > We've stumbled on an issue where the spec and implementation are in conflict. > The right thing to do it to fix it while we have a handle on it. I bumped the priority to P2.

Re: RFR: 8151542: URL resources for multi-release jar files have a #runtime fragment appended to them

2016-04-29 Thread Steve Drach
>> I put a new webrev out with the change suggested by Paul, using >> file.indexOf(“!/“) instead of file.endsWith(“!/“). >> >> http://cr.openjdk.java.net/~sdrach/8151542/webrev.02/index.html >> This >> still needs to be fixed

Re: RFR: 8151542: URL resources for multi-release jar files have a #runtime fragment appended to them

2016-04-29 Thread Steve Drach
I updated the webrev with changes that Alan suggested still needs to be fixed to compare the URL protocol without regard to case And Sherman suggested Shouldn't the realname just be the "super.getName()” ? The webrev is at

Re: RFR: 8151542: URL resources for multi-release jar files have a #runtime fragment appended to them

2016-05-02 Thread Steve Drach
Another webrev: http://cr.openjdk.java.net/~sdrach/8151542/webrev.05/index.html Only URLClassPath has changed. I put a comment on url.getProtocol indicating the URL assures it’s lower case and I changed the long lines into

Re: RFR: JDK-8151914 java/util/jar/JarFile/MultiReleaseJar* tests do not declare module dependences

2016-05-02 Thread Steve Drach
Looks fine to me, although I am not an official reviewer. Thanks for doing this. > On May 2, 2016, at 1:03 PM, Alexandre (Shura) Iline > wrote: > > Hi, > > Can you please take a look on: > http://cr.openjdk.java.net/~shurailine/8151914/webrev.01/ > ? > > Thank

Re: Multi-Release JAR runtime support

2016-04-19 Thread Steve Drach
Hi Herve, I checked the jar file created from your code and, as others have suspected, the manifest does not contain the “Multi-Release” attribute. I added the attribute with emacs and tried it out: $ java -classpath multirelease/target/multirelease-0.8-SNAPSHOT.jar base.Base 9-ea+113 FROM

  1   2   >