RFR: Proposed jimage refresh for JDK9

[Remi Forax]

As Paul said,
it's a big patch ...

In ImageBuilder, Option should take the description as 3rd parameter 
instead of overriding description.
Once this is done, Option should not be abstract but takes the 
processing as an argument too
(as a lambda). i.e instead of

new Option(true, "--cmds") {
             @Override
              void process(ImageBuilder task, String opt, String arg) 
throws BadArgs {
                  task.options.cmds = splitPath(arg, File.pathSeparator);
              }
             @Override
              String description() { return "Location of native commands"; }
          }
}

it's better to write:
new Option(true, "--cmds", "Location of native commands",
     (ImageBuilder task, String opt, String arg) -> {
       task.options.cmds = splitPath(arg, File.pathSeparator);
     })

the code is ligther, it uses delegation instead of inheritance and 
remove a dozen of inner classes on disk.

In ModuleArchive, close() should continue to close the opened input 
streams even if one throw an exception,
@Override
public void close() throws IOException {
       IOException e = null;
       for(InputStream stream : opened) {
            try {
              stream.close();
           }catch(IOException ex) {
              if (e == null) {
                e = ex;
              } else {
                e.addSuppressed(ex);
              }
           }
       }
       if (e != null) {
         throw e;
       }
}

In BasicImageWriter, getString can be simplified to:
public String getString(int offset) {
     UTF8String utf8 = strings.get(offset);
     if(utf8 != null) {
          return utf8.toString();
     }
     return null;
}

also in generatePerfectHash, you don't need to create a Stream to do a 
forEach on it:
   input.stream().forEach(
you can write
   input.forEach(

ImageReader.LinkNode, instead of 
@SuppressWarnings("LeakingThisInConstructor"),
the constructor should be private and you should add a static factory 
method that
does parent.addChild(linkNode) after calling the constructor.

in JrtFileAttributeView.get() that takes a Class, you forget to create 
the JrtFileAttributeViews
with options as 3 parameters (as you do in get that takes a String).

in JrtPath, instead of
   boolean isSame = Arrays.equals(this.getResolvedPath(),
                               ((JrtPath)other).getResolvedPath());
   return isSame? isSame : jrtfs.isSameFile(this, (JrtPath)other);
the usual code is:
   JrtPath path = (JrtPath) other;
   return Arrays.equals(this.getResolvedPath(), path.getResolvedPath()) ||
              jrtfs.isSameFile(this, path);

and I fully agree with the review of Paul :)

Rémi

On 05/15/2015 05:45 PM, Jim Laskey (Oracle) wrote:
> http://cr.openjdk.java.net/~jlaskey/jdk9-jimage/top/ <http://cr.openjdk.java.net/~jlaskey/jdk9-jimage/top/>
> http://cr.openjdk.java.net/~jlaskey/jdk9-jimage/hotspot/ <http://cr.openjdk.java.net/~jlaskey/jdk9-jimage/hotspot/>
> http://cr.openjdk.java.net/~jlaskey/jdk9-jimage/jdk/ <http://cr.openjdk.java.net/~jlaskey/jdk9-jimage/jdk/>
> http://cr.openjdk.java.net/~jlaskey/jdk9-jimage/langtools/ <http://cr.openjdk.java.net/~jlaskey/jdk9-jimage/langtools/>
>
> There are some issues with regard to the hotspot changes, but I’m working with Coleen and John R. to resolve those.
>
> Cheers,
>
> — Jim
>