Looks good! Thanks, /Staffan
> On 2 nov. 2015, at 19:54, Carsten Varming <[email protected]> wrote: > > Dear Staffan, > > I updated the webrev at http://cr.openjdk.java.net/~cvarming/jvmtiGen.02/ > <http://cr.openjdk.java.net/~cvarming/jvmtiGen.02/> > > I kept the verbose flag to avoid printing the fallback warnings. > > Carsten > > On Mon, Nov 2, 2015 at 8:34 AM, Carsten Varming <[email protected] > <mailto:[email protected]>> wrote: > Dear Staffan, > > Stacktraces: I can see your point. I'll add them back unconditionally. > > Warnings: The OpenJDK build gets a little noisy when you start printing > warnings. The fallback on failed includes in hotspot/src/share/vm/trace/*.xml > cause a lot of printing; see below. That is why I introduced the verbose flag > in the first place. This is not a big deal as it pollutes only the build log, > but I would still prefer the program to be silent on normal execution. Please > let me know if you still prefer to discard the verbose flag and print all > warnings. > > I'll update the webrev after your reply. > > Thank you, > Carsten > > Noisy warnings: > > jvmtiGen warning: Include operation failed, reverting to fallback. Resource > error reading file as XML > (href='../../../closed/share/vm/trace/traceeventtypes.xml'). Reason: > /Users/cvarming/workspace/jdk9/hotspot/src/closed/share/vm/trace/traceeventtypes.xml > (No such file or directory) > jvmtiGen warning: Include operation failed, reverting to fallback. Resource > error reading file as XML > (href='../../../closed/share/vm/trace/traceevents.xml'). Reason: > /Users/cvarming/workspace/jdk9/hotspot/src/closed/share/vm/trace/traceevents.xml > (No such file or directory) > jvmtiGen warning: Include operation failed, reverting to fallback. Resource > error reading file as XML > (href='../../../closed/share/vm/trace/traceeventtypes.xml'). Reason: > /Users/cvarming/workspace/jdk9/hotspot/src/closed/share/vm/trace/traceeventtypes.xml > (No such file or directory) > jvmtiGen warning: Include operation failed, reverting to fallback. Resource > error reading file as XML > (href='../../../closed/share/vm/trace/traceevents.xml'). Reason: > /Users/cvarming/workspace/jdk9/hotspot/src/closed/share/vm/trace/traceevents.xml > (No such file or directory) > jvmtiGen warning: Include operation failed, reverting to fallback. Resource > error reading file as XML > (href='../../../closed/share/vm/trace/traceeventtypes.xml'). Reason: > /Users/cvarming/workspace/jdk9/hotspot/src/closed/share/vm/trace/traceeventtypes.xml > (No such file or directory) > jvmtiGen warning: Include operation failed, reverting to fallback. Resource > error reading file as XML > (href='../../../closed/share/vm/trace/traceevents.xml'). Reason: > /Users/cvarming/workspace/jdk9/hotspot/src/closed/share/vm/trace/traceevents.xml > (No such file or directory) > > > > On Mon, Nov 2, 2015 at 2:37 AM, Staffan Larsen <[email protected] > <mailto:[email protected]>> wrote: > Hi Carsten, > > Thanks for spending the time on this - this version looks a lot cleaner. > > I would prefer to always print the exception stack trace. The reason is that > if something goes wrong, it will likely happen on a build server where it > isn’t simple to find and insert the extra ‘-verbose’ flag. Better to log as > much information as possible in case of failure so it’s easier to find the > root cause. > > That leaves warnings from the parser as the only usage of -verbose. Are there > any warnings presently? If not, I would remove the -verbose flag altogether > and always print the warnings as well. > > Thanks, > /Staffan > >> On 1 nov. 2015, at 20:48, Carsten Varming <[email protected] >> <mailto:[email protected]>> wrote: >> >> Dear Serguei and Steffan, >> >> Sorry about the late reply. The office was a little busy Friday. >> >> Steffan's suggestion is to remove all the exception handling code sound >> reasonable. I am not a fan of the main method throwing checked exceptions, >> so I wrapped all the exception handling into a tiny handler that simply >> prints the exception and exits with a non-zero value. I never cared much for >> the stacktrace, so I put printing of the stacktrace under the verbose flag. >> >> Speaking of cleaning up, there is code in there to use Apache xalan instead >> of the standard xslt processor due to an ancient bug fixed in Java 1.5. I >> removed that as well. >> >> There was also a comment about the field "document" being package globally >> accessible due to: "ref'd by the tree-adapter". I have no idea what that >> means, but changing document to a local variable in main didn't break the >> build, so I guess that comment is out-of-date. Please let me know if I am >> missing something here. >> >> There was also a use of java.io.PrintStream. That class suppresses >> IOExceptions: "Unlike other output streams, a PrintStream never throws an >> IOException", so I removed the PrintStream as I want to get a failure when >> something goes wrong. >> >> I have put a new webrev at http://cr.openjdk.java.net/~cvarming/jvmtiGen.02/ >> <http://cr.openjdk.java.net/~cvarming/jvmtiGen.02/> >> >> Carsten >> > > >
