> On Feb 16, 2017, at 5:20 AM, Claes Redestad <claes.redes...@oracle.com> wrote: > >> >> In addition, if the main argument is specified but the version does not >> match, it will ignore the given argument. Should it be an error instead? >> We are the one who will generate a trace file and specify it in the jlink >> plugin option. It’s okay to ignore the default trace output if no plugin >> option is specified and I think no warning should be printed in this case. >> It’s just like this plugin is disabled. You may want to add a suboption to >> turn on verbose that will trace what is generated and what is ignored. > > I think a warning is reasonable in all cases: Using a different version of > jlink than the java.base you're linking will lose some optimizations and the > user would be none the wiser as to why, verbosity helps avoid surprises.
The plugin is enabled by default. With this change, I consider this plugin is "auto-enabled" when it’s creating the image of the version that this plugin supports (i.e. matching major.minor version). So if the —generate-jli-classes option is not specified, it might be confusing when I get this warning. I would prefer in this case no warning should be emitted and the plugin is not enabled. If the option is specified on the command-line, it should emit the warning. > > http://cr.openjdk.java.net/~redestad/8175026/jdk.02/ 322 if (!initialize(in)) { Maybe refactor line 175-190 in a new method and something like this: if (!checkVersion(getLinkedVersion(in))) : } Then follow with initialize(in) here. That’d make it explicit. One thing to handle is when exception is thrown when reading the trace file (default or mainArgument). Maybe that part can be done early in configure method and store the lines for later consumption. line 235-238: you may use orThrow in this case. Mandy
