<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><div class="">Just send another diff. I’ve already created an internal Graal PR for this.</div><br class=""><div class="">
<span class="Apple-style-span" style="border-collapse: separate; color: rgb(0, 0, 0); font-family: Helvetica; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-border-horizontal-spacing: 0px; -webkit-border-vertical-spacing: 0px; -webkit-text-decorations-in-effect: none; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px;  ">igor<br class=""><br class=""><br class=""></span>

</div>
<div><br class=""><blockquote type="cite" class=""><div class="">On Feb 4, 2019, at 2:22 PM, Andrew Luo <<a href="mailto:andrewluotechnologies@outlook.com" class="">andrewluotechnologies@outlook.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div class="WordSection1" style="page: WordSection1; caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;"><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: "Times New Roman", serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class="">Makes sense.<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: "Times New Roman", serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class=""><o:p class=""> </o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: "Times New Roman", serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class="">Ah, I forgot to add a Files.exists(vswhere) check and just return null if it doesn’t exist… But besides that, I suppose we can just try/catch the exceptions anyways so we can try other versions as well in case finding the VS2017+ linker fails for whatever reason.  I didn’t see any other similar error handling logic in the code, so I wasn’t sure (and didn’t want to eat the exception in case someone wanted to figure out why VS2017 isn’t being chosen), but it appears in other classes we’re just printing the stack trace to stderr…<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: "Times New Roman", serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class=""><o:p class=""> </o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: "Times New Roman", serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class="">Right, -latest probably should be passed… It doesn’t make a difference now, but when VS2019 comes out it will matter.<span class="Apple-converted-space"> </span></span><span style="font-size: 11pt; font-family: Wingdings; color: rgb(31, 73, 125);" class="">J</span><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class=""><o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: "Times New Roman", serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class=""><o:p class=""> </o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: "Times New Roman", serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class="">I’ll make these changes and send out another diff (or perhaps just a pull request on Github).<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: "Times New Roman", serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class=""><o:p class=""> </o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: "Times New Roman", serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class="">Thanks,<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: "Times New Roman", serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class=""><o:p class=""> </o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: "Times New Roman", serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class="">-Andrew<o:p class=""></o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: "Times New Roman", serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class=""><o:p class=""> </o:p></span></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: "Times New Roman", serif;" class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif; color: rgb(31, 73, 125);" class=""><o:p class=""> </o:p></span></div><div class=""><div style="border-style: solid none none; border-top-width: 1pt; border-top-color: rgb(225, 225, 225); padding: 3pt 0in 0in;" class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: "Times New Roman", serif;" class=""><b class=""><span style="font-size: 11pt; font-family: Calibri, sans-serif;" class="">From:</span></b><span style="font-size: 11pt; font-family: Calibri, sans-serif;" class=""><span class="Apple-converted-space"> </span>Igor Veresov <<a href="mailto:igor.veresov@oracle.com" class="">igor.veresov@oracle.com</a>><span class="Apple-converted-space"> </span><br class=""><b class="">Sent:</b><span class="Apple-converted-space"> </span>Monday, February 4, 2019 12:54 PM<br class=""><b class="">To:</b><span class="Apple-converted-space"> </span>Andrew Luo <<a href="mailto:andrewluotechnologies@outlook.com" class="">andrewluotechnologies@outlook.com</a>><br class=""><b class="">Cc:</b><span class="Apple-converted-space"> </span>Vladimir Kozlov <<a href="mailto:vladimir.kozlov@oracle.com" class="">vladimir.kozlov@oracle.com</a>>; <a href="mailto:hotspot-compiler-dev@openjdk.java.net" class="">hotspot-compiler-dev@openjdk.java.net</a><br class=""><b class="">Subject:</b><span class="Apple-converted-space"> </span>Re: [PATCH] Enhance jaotc to automatically find VS2017+ linker<o:p class=""></o:p></span></div></div></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: "Times New Roman", serif;" class=""><o:p class=""> </o:p></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: "Times New Roman", serif;" class="">Perhaps, we should catch all the exceptions that are thrown by getVC141AndNewerLinker() and try other methods of getting to a linker if it fails?<o:p class=""></o:p></div></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: "Times New Roman", serif;" class=""><o:p class=""> </o:p></div></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: "Times New Roman", serif;" class="">Another issue raised by Bob is that perhaps we should be passing “-latest” to vswhere for the case when multiple VS version are installed. What do you think?<o:p class=""></o:p></div></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: "Times New Roman", serif;" class=""><o:p class=""> </o:p></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: "Times New Roman", serif;" class=""><span class="apple-style-span"><span style="font-family: Helvetica, sans-serif;" class="">igor</span></span><span style="font-family: Helvetica, sans-serif;" class=""><br class=""><br class=""><br class=""><br class=""></span><o:p class=""></o:p></div></div><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: "Times New Roman", serif;" class=""><br class=""><br class=""><o:p class=""></o:p></div><blockquote style="margin-top: 5pt; margin-bottom: 5pt;" class=""><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: "Times New Roman", serif;" class="">On Feb 4, 2019, at 12:00 PM, Vladimir Kozlov <<a href="mailto:vladimir.kozlov@oracle.com" style="color: purple; text-decoration: underline;" class="">vladimir.kozlov@oracle.com</a>> wrote:<o:p class=""></o:p></div></div><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: "Times New Roman", serif;" class=""><o:p class=""> </o:p></div><div class=""><div class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: "Times New Roman", serif;" class="">Hi Andrew,<br class=""><br class="">I looked on changes nire and I have question about throwing next and following errors in getVC141AndNewerLinker():<br class=""><br class="">+    private static Path getVC141AndNewerLinker() throws Exception {<br class="">+        String programFilesX86 = System.getenv("ProgramFiles(x86)");<br class="">+        if (programFilesX86 == null) {<br class="">+            throw new InternalError("Could not read the ProgramFiles(x86) environment variable");<br class=""><br class="">Method getVC141AndNewerLinker() is called before any other paths are checked. If this variable is not set or other checks in getVC141AndNewerLinker() failed, caller getWindowsLinkPath() exits before following code is executed and other paths are checked.<br class=""><br class="">I don't think it is correct if old VS is installed instead of VS2017+. What dio you think. Can you verify that it works in all cases?<br class=""><br class="">Thanks,<br class="">Vladimir<br class=""><br class="">On 1/29/19 2:33 PM, Vladimir Kozlov wrote:<br class=""><br class=""><o:p class=""></o:p></div><blockquote style="margin-top: 5pt; margin-bottom: 5pt;" class=""><div style="margin: 0in 0in 0.0001pt; font-size: 12pt; font-family: "Times New Roman", serif;" class="">Looks good.<br class="">Thanks,<br class="">Vladimir<br class="">On 1/25/19 3:56 PM, Andrew Luo wrote:<br class=""><br class=""><o:p class=""></o:p></div><blockquote style="margin-top: 5pt; margin-bottom: 5pt;" class=""><p class="MsoNormal" style="margin: 0in 0in 12pt; font-size: 12pt; font-family: "Times New Roman", serif;">Minor public -> private visibility fix.  Just noticed right after I sent it out…<br class=""><br class="">Thanks,<br class=""><br class="">-Andrew<br class=""><br class="">*From:* hotspot-compiler-dev <<a href="mailto:hotspot-compiler-dev-bounces@openjdk.java.net" style="color: purple; text-decoration: underline;" class="">hotspot-compiler-dev-bounces@openjdk.java.net</a>> *On Behalf Of *Andrew Luo<br class="">*Sent:* Friday, January 25, 2019 3:55 PM<br class="">*To:*<span class="Apple-converted-space"> </span><a href="mailto:hotspot-compiler-dev@openjdk.java.net" style="color: purple; text-decoration: underline;" class="">hotspot-compiler-dev@openjdk.java.net</a><br class="">*Subject:* [PATCH] Enhance jaotc to automatically find VS2017+ linker<br class=""><br class="">See attached patch.  Any feedback is welcome.<br class=""><br class="">Tested on a system with only VS2017 installed, just ran jaotc with a simple class file, and got the expected .dll output with no errors…<br class=""><br class="">Thanks,<br class=""><br class="">-Andrew</p></blockquote></blockquote></div></div></blockquote></div></div></div></blockquote></div><br class=""></body></html>