>>>> 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.
It is, although not in a stream. My inspection of usages led me to an unused method, findPackages, that I will remove (I did so and all tests passed). > >> 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
