Mandy, On 14 Mar 2016, at 20:37, Mandy Chung <mandy.ch...@oracle.com> wrote:
> >> On Mar 11, 2016, at 1:39 AM, Alan Bateman <alan.bate...@oracle.com> wrote: >> >> >> I've refreshed the webrevs here: >> http://cr.openjdk.java.net/~alanb/8142968/2 > > > I have reviewed the jmod tool and some comments: > > 299 private boolean printModuleDescriptor(InputStream in) > > jmod -p option prints the output in different sections. > java -listmods:<MODULE> prints the module descriptor closer to > module-info.java declaration. Also jmod -p does not do any > sorting and names are unordered. > > It would be better for both options to use similar format. I think > closer to how it is declared in module-info.java would be preferred. > The optional attributes will follow it - the existing format is fine. Good idea. I updated the output as close as possible, where applicable, to -listmods:<MODULE>. > It’d help if the package names and uses are printed in alphabetical order. > > 584 } catch (ZipException x) { > 585 // Skip. Do nothing. No packages will be added > > When ZipException is thrown? Should it be handled in the same way as > IOException? I do remember adding this explicit catch. I’m reluctant to remove it until I can find my notes, as to why it was added. I’ll have to get back to you on this. > 603 .filter(pkg -> pkg.length() > 0) // module-info > > I think jmod should detect if there is any unnamed package and output an > error since unnamed package is not allowed in named module. Currently any > classes in unnamed package are include in the jmod file. Classes in the unnamed package are now disallowed. > findPackages should filter module-info.class explicitly. > > 396 Path tempTarget = target.resolveSibling(target.getFileName() + > ".tmp”); > > When any error occurs, foo.mod.tmp is left behind. Fixed. > jmods.properties - some unused messages. > > err.cp.must.be.specified:--class-path must be specified > err.dir.not.empty=not empty: {0} > err.invalid.arg.for.option=invalid argument for option: {0} > err.option.after.class=option must be specified before classes: {0} Removed. Changeset with the above updates: http://hg.openjdk.java.net/jigsaw/jake/jdk/rev/7e5d2398a250 -Chris.
