> On Oct 24, 2016, at 1:39 PM, Steve Drach <steve.dr...@oracle.com> wrote:
> 
>>> There is a new webrev at 
>>> http://cr.openjdk.java.net/~sdrach/8164805/webrev.01/ 
>> 
>> sun/tools/jar/Main.java
>> 
>> Thanks for refactoring and adding the findConcealedPackages method.  What I 
>> actually meant was to move out this line:
>>   concealedPackages = findConcealedPackages(rd);
>> 
>> to probably before calling addExtendedModuleAttributes(moduleInfos) above 
>> line 342 and 1101.
> 
> I tried that and decided it was non-optimal because we’d have to construct 
> the ModuleDescriptor from the modInfos twice in succession.  Let's compromise 
> here, okay?

OK.  It wasn’t clear to me that you considered that.  It’s fine to leave it for 
a future clean up.

> 
>> 
>> 2014                 .filter(p -> !p.equals("”))
> 
> 
>> 
>> For a modular JAR, there should be no unnamed package.  I think the jar tool 
>> should fail if it detects an unnamed package.  Your test does not have any 
>> unnamed package - how did you find this?
> 
> the modularJar/Basic test found a bug.  Then when I was fixing the bug in 
> toPackageNames I noticed it could return unnamed packages (“”).  And in fact 
> there are some, a few, classes that aren’t in a package.  Jar tool shouldn’t 
> fail with unnamed packages.

I meant for modular JAR.

> They could even exist in a modular multi-release jar when the module-info 
> class is only in a versioned directory.  

module-info.class should be filtered before calling toPackageName.

> I guess it should fail if a class in an unnamed package is in a module, 
> although I’m not even sure about that.


Mandy

Reply via email to