> 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