Dear Staffan, I updated the webrev at 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]> 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]> > 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]> 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/ >> >> Carsten >> >> >> >> >
